This repository was archived by the owner on Oct 27, 2018. It is now read-only.
AWSIdentityManager allow identity merging and fix hang #18
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
BruceBuckland
commented
Nov 8, 2016
BruceBuckland
commented
Required changes so that AWSSignInProviders can be created and installed. Allow any number of AWSSignInProviders to maintain sessions and have them restarted by interceptApplication didFinishLaunchinWithOptions. Without this fix, only Google and Facebook sessions are restarted. This behavior is accomplished by a configuration in Info.plist. If it is missing prior behaviour with hardcoded Facebook and Google providers is maintained. This allows Mobile Hub applications to run with no changes to Info.plist. The configuration relates class names (used to instantiate the singletons) with user friendly names (used for NSUserDefaultsKeys and for human readable provider names. The dictionary is located in AWS->IdentityManager->Default->SignInProviderClassDictionary and has a form like this: <key>SignInProviderKeyDictionary</key> <dict> <key>AWSCUPIdPSignInProvider</key><string>Cognito Pool</string> <key>AWSFacebookSignInProvider</key><string>Facebook</string> <key>AWSGoogleSignInProvider</key><string>Google</string> </dict> Additionally this fix: 1) Exposes currentSignInProvider, this is necessary so that apps can determine which provider is active and call it's methods to get access to properties needed for developer identities and Cognito (your) user pools (including the ability to get access to the pool). 2) Creates new AWSIdentityManager method providerKey to return the "Visible" name of an AWSSignInProvider so that the user may be prompted or notified of which provider is involved (for login errors, provider lists, etc.). 3) Expose completeLogin. Once the AWSSignInProvider completes the login, it must call completeLogin, however that method is private.
…lated to trying to start up when it's token is expired or something, because the crash is intermittent, but when it does crash. it keeps crashing until you clear the device settings in simulator. (Investigation of this is needed.) However the problem is bigger than that. If some AWSSignInProvider has a problem, then the app won't start. AWSSignInProviders are many and varied and hopefully more will be created, but we can be sure that going forward we will encounter problems with some one of them. These may even be temporary server side problems. And it is bad that a server side problem on one provider might cause an app to be unusable on ALL providers. For a deployed app, that would mean that until "Digits" (for example) fixed it's server that all of my customers that ever used "Digits" might have crashing apps that they could not fix. So to make AWSIdentityManager more forgiving of problems in AWSSignInProviders. As a fix to AWSGoogleSignInProvider crashes, this fix will reset NSUserDefaults for active sessions, and the keychain on the next App start, when the previous start did not complete AWSSignInProvider initialization. What the fix does is to set a NSUserDefaults flag (ProvidersOk) that get set to NO when starting and gets set to YES when the AWSSignInProviders have all completed initialization. If upon App start ProviderOk flag is still NO, then the previous app start failed before initializing providers. In which case AWSIdentitymanager drops all the provider keys and clears the keychain. That way, if a provider goes bad, the app will function after one crash as long as the user avoids the bad provider. (Also, the providers get a clean slate with no stored sessions, which fixed the AWSGoogleSignInProvider and may even fix other providers.) The cost is that the user must sign in again but that is better than the app crashing every time it starts.
Allow multiple identity merging and simultaneous authenticated identities
Fix AWSIdentityManager so that it tracks logins in a cache and allows
Cognito identity merging.
Controlled using flags set in Info.plist, this allows the developer to choose
how the identityId's are managed.
If there are no flags in the Info.plist behavior is like that of the
existing library (except for one fix, because currently AWSIdentityManager
if you log in to a second identity, gets an error, gets stuck, and has to
be restarted to get rid of it). This ensures Mobile Hub compatibility with
the helper.
The flags are stored in AWS->IdentityManager->Default and are as follows:
<key>IdentityManager</key>
<dict>
<key>Default</key>
<dict>
<key>Allow Identity Merging</key><true/>
<key>Allow Simultaneous Active Accounts</key><true/>
Legal values are
TRUE TRUE - allow both merging and simultaneous active accounts
FALSE TRUE - allow simultaneous stacked logins but don't merge
(never returns a merged logins dictionary)
FALSE FALSE - allow one login at a time, any new login
logs the previous identity out.
This fixes a current bug see below.
This setting is not legal
FALSE TRUE - Merging identities requires at least two simultaneous logins
Current AWSIdentityManager bug.
If you login twice, each login is independent, they do not merge,
and when you log out of one, you find yourself back logged in as the other
(different) identityId.
This feels strange to the user, and does not really work (I think
because the credentials are not associated with the new identity on
logout (only on login). At this point you get a whole bunch of logged
errors, and it does not recover gracefully from this condition.
The errors say "GetCredentialsForIdentity keeps failing. Clearing
identityId did not help. Please check your Amazon Cognito Identity
configuration."
But the problem is not that the configuration is bad, it is that
AWSIdentityManager mis-handles the logout so that the identityId remains
old, while the logins dictionary is new.
This fix no longer allows this condition regardless of settings.
If you are not allowing merging, then logging in one user is made to
log out the other first, invisibly and automatically.
Contributor
|
Hi @BruceBuckland , Thank you for submitting this PR. I will go through this PR and bring it up with the team and update with when we can merge this in. Currently, Mobile Hub doesn't support merging of identities. The Identity Manager only expects one sign-in provider to be signed it at any given time. Thanks, |
Contributor
Author
|
That is true, Identity Merging is not supported, however if a second login is done without a logout, that results in the configuration error. The configuration error happens BECAUSE the AWSIdentityManager does not support merging. With this pull request the error does not occur, and identity merging becomes a developer option. Alternatively a fix would be to force a logout or error it a second login occured. When the new mobile-hub-helper code gets pushed I will update this pull request so it is mergeable. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.