[TIMOB-24142] iOS: Custom Ti.Facebook entitlements-file overrides CLI-generated entitlements
GitHub Issue | n/a |
---|---|
Type | Bug |
Priority | High |
Status | Closed |
Resolution | Fixed |
Resolution Date | 2016-12-19T13:19:56.000+0000 |
Affected Version/s | Release 6.0.0, Release 5.5.1 |
Fix Version/s | Release 6.0.1 |
Components | iOS, Tooling |
Labels | aps, entitlements, facebook, keychain-access, team-id |
Reporter | Hans Knöchel |
Assignee | Chris Barber |
Created | 2016-11-12T10:05:30.000+0000 |
Updated | 2016-12-19T13:19:59.000+0000 |
*Workaround*: Copy the contents of the generated entitlements file in
build/iphone
to your own file and add thekeychain-access-groups
key to it afterwards. Then place that file in the root of your project as suggested [here](https://github.com/appcelerator-modules/ti.facebook/blob/3feee7d77ed9eef20936d173292c9e21089c1361/apidoc/Facebook.yml#L17-L34).Ok, so here is my proposal: PR (titanium_mobile/master): https://github.com/appcelerator/titanium_mobile/pull/8603 PR (ti.facebook/master): https://github.com/appcelerator-modules/ti.facebook/pull/75 It basically checks if we use the ti.facebook iOS module and if so, it injects the
keychain-access-groups
key. This key will already be used for device-builds, but different to our push-notifications, we already need the capability in the Simulator when using Ti.Facebook (because Facebook uses the keychain to store private data, probably the access-token). But as we don't have theappPrefix
from the provisioning profile in the Simulator, we cannot grab it from there. -So I added a check for the<team-id/>
that we already use for watchOS-enabled apps. If it's there -> great, use it. If not, throw an error because Ti.Facebook won't work.- I updated the PR to use the$(AppIdentifierPrefix)
flag instead that will take care of the Simulator keychain. Also, I'm wondering why we couldn't use this for device-builds as well - can the team-id from the provisioning be different from what Xcode uses when processing the flag? I don't think so. But anyway, both will work and the above PR has been tested for the following use-cases: - Ti.Facebook not included -> Skip - Ti.Facebook included, but not the iOS one -> Skip - Ti.Facebook included, run on device -> Proper entitlements generated inbuild/iphone
- Ti.Facebook included, run on simulator -> Proper entitlements generated inbuild/iphone
- Ti.Facebook included, push used, run on device -> Proper entitlements generated inbuild/iphone
- Ti.Facebook included, push used, run on simulator -> Proper entitlements generated inbuild/iphone
Code-style to be discussed, [~cbarber] is the man to check! Backport coming as soon as the master-PR is accepted. *EDIT*: And if we accept the PR, we can also remove the manual notes from the Ti.Facebook docs and delelopers will be happy because they don't need to curate multiple entitlements files. Test-case:After giving the titanium_mobile PR much consideration, I think that we should NOT accept it. It tightly couples an optional module into the already overly complicated iOS build system. Instead, I propose we add a new
"build.ios.writeEntitlements"
hook to the iOS build in the_embedCapabilitiesAndWriteEntitlementsPlist()
function. This function is called for both the Titanium app's entitlements file and each iOS Extension's entitlements file, so we would need to pass in a flag to indicate which context so that it can create the correct hook name"build.ios.writeEntitlements"
or"build.ios.writeExtentionEntitlements"
. It was easier for me to just code it then the tell you how to do it. :) PR: https://github.com/appcelerator/titanium_mobile/pull/8606 Then in the Facebook module, add ahooks/facebook.js
that hooks into the"build.ios.writeEntitlements"
event and adds the missingkeychain-access-groups
. It would look like:Much cleaner integration. I should note that this code has not been tested, so give it a quick test.
[~cbarber] Will this be ready for 6.0.1? It needs to be well-tested against many use-cases. Thx!
[~hansknoechel] Well, I wasn't planning on allocating any more time to this ticket. I agree, it needs to be well-tested.
The problem that I'm having with this is that it needs both module and SDK-updates as well as TIMOB-24041 resolved. So either use the other solution for 6.0.1 and come up with the refactored one in 6.1.0 or finish both tickets in this sprint. More worried about the available resources for this issue, that's why I came up with my workaround.
Well, separate out the module hook into a new MOD ticket. That should take 5 minutes. Then let's get the PR for this ticket tested and merged. That should take an hour. Then lets fix TIMOB-24041, which should take about 2 hours. Then I've already written the hook for the Facebook module, just need to drop that in and test it, then merge and issue a PR for the updated iOS Facebook module. Maybe 2 hours of work. Easy peasy.
Created MOD-2313 for adding the CLI-hook.
I confirmed the new hooks are fired and the entitlements can be manipulated. The new hooks are
build.ios.writeEntitlements
andbuild.ios.writeExtensionEntitlements
. TiSDK master PR: https://github.com/appcelerator/titanium_mobile/pull/8606 TiSDK 6_0_X PR: https://github.com/appcelerator/titanium_mobile/pull/8622The PR works very well, tested together with MOD-2313 and the hook is called correctly, the capabilities are updated as expected.
[~cbarber], [~hansknoechel], I am seeing some unexpected behavior when building with a custom entitlements file in
/platform/ios
. On first/clean builds, the custom file looks to be copied to the build folder. On subsequent builds, the entitlements file in the build folder is overwritten with the generated file. I would expect the same file in the build folder on each build, without any modifications to the project. Is this accurate? Tested with SDK 6.0.1.v20161130023500 and Facebook Module 5.2.9The issue has been narrowed down to the Module hook, so moving the conversation to that ticket: MOD-2313
[~ewieber] So, here's the deal. Custom entitlements files MUST be named
Entitlements.plist
and it must go in the root of your Titanium app. This custom entitlements file is read in, processed, and written tobuild/iphone/<app name>.entitlements
. However, this is not what you did. You put a<app name>.entitlements
file in theplatform/ios
directory. Your custom entitlements file will be copied over the generated<app name>.entitlements
. When a subsequent build occurs, it will generate thebuild/iphone/<app name>.entitlements
file again. However, this time when it copies theplatform/ios/<app name>.entitlements
file, it will find the entry from the previous build in the manifest and check the timestamps and hash of theplatform/ios/<app name>.entitlements
against what was copied in the previous build. Since they are the same, theplatform/ios/<app name>.entitlements
file is NOT copied. The problem is the build has written a newly generated<app name>.entitlements
and the entry in the previous build manifest is now stale. Probably the easiest thing to do is delete the entry from the previous build manifest when the generated<app name>.entitlements
is created. When the iOS build goes to copyplatform/ios/<app name>.entitlements
, there will be no entry and it will force copy the custom entitlements file. The fix should be done in the_embedCapabilitiesAndWriteEntitlementsPlist()
function, after thethis.unmarkBuildDirFile(dest);
call. We need to do something like this amazing and untested code:Note that we have to be careful since the
_embedCapabilitiesAndWriteEntitlementsPlist()
function is also called for entitlements files from extensions and watch apps, hence the relative path handling.[~cbarber], what you explained makes sense and maybe I need to better clarify the entitlements files we are talking about. There are custom entitlements.plist files and custom .entitlements files at work. We recommend to use the latter in TIMOB-23897 and is what uncovered this issue. Anyway, can we expect a PR for your fix or would you like to break it off into its own ticket?
[~ewieber] I'll just reopen this ticket since it's a "bug" with the fix for this ticket.
TiSDK master PR: https://github.com/appcelerator/titanium_mobile/pull/8653 TiSDK 6_0_X PR: https://github.com/appcelerator/titanium_mobile/pull/8654 To test:
Create the
platform/ios
directoryBuild an iOS app for simulator
ti build -p ios --build-only
Copy the
build/iphone/<app name>.entitlements
toplatform/ios/<app name>.entitlements
Modify the
platform/ios/<app name>.entitlements
by adding<key>foo</key><string>bar</string>
Clean the project
ti clean
Build the iOS app for simulator
ti build -p ios --build-only
cat
build/iphone/<app name>.entitlements
and it should contain thefoo
keyBuild the app again
ti build -p ios --build-only
cat
build/iphone/<app name>.entitlements
and it should contain thefoo
keyTested with both
platform/ios
andplatform/iphone
, the custom one is used after this change, so the patch is working.Verified fixed, using: MacOS 10.12 (16A323) Ti SDK 6.0.1.v20161202124626 Appc NPM 4.2.8 Appc CLI 6.0.0 Alloy 1.9.4 Xcode 8.1 (8B62) The correct entitlements are present after building with a custom entitlements.plist,
keychain-access-groups
is populated and with the exception of an issue where the key has multiple values (MOD-2313) the file looks correct.