Skip to content

Commit

Permalink
Sign in callback invoked before all sign in operations (aws-amplify#1721
Browse files Browse the repository at this point in the history 
)

* AWSMobileClient callback was invoked way before other sign in operations were complete. This PR make sure that we invoke the userstate listener and sign in callback after all task are complete.

* signOut with callback was not invoking the callback if we do not pass any options. This change resolves that.
  • Loading branch information
@royjit
royjit authored Aug 13, 2019
1 parent abba0bf commit f39d113
Show file tree
Hide file tree
Showing 4 changed files with 209 additions and 56 deletions.
42 changes: 34 additions & 8 deletions AWSAuthSDK/Sources/AWSMobileClient/AWSMobileClient.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,9 @@ final public class AWSMobileClient: _AWSMobileClient {
}

internal func mobileClientStatusChanged(userState: UserState, additionalInfo: [String: String]) {
if (self.currentUserState == userState) {
return
}
self.currentUserState = userState
for listener in listeners {
listener.1(userState, additionalInfo)
Expand Down Expand Up @@ -459,13 +462,26 @@ final public class AWSMobileClient: _AWSMobileClient {
signInInfo[self.TokenKey] = session.accessToken!.tokenString
signInInfo[self.ProviderKey] = "OAuth"

self.performHostedUISuccessfulSignInTasks(disableFederation: hostedUIOptions.disableFederation, session: session, federationToken: federationToken!, federationProviderIdentifier: federationProviderIdentifier, signInInfo: &signInInfo)
completionHandler(.signedIn, nil)
if self.pendingGetTokensCompletion != nil {
self.tokenFetchLock.leave()
self.performHostedUISuccessfulSignInTasks(disableFederation: hostedUIOptions.disableFederation,
session: session,
federationToken: federationToken!,
federationProviderIdentifier: federationProviderIdentifier,
signInInfo: &signInInfo).continueWith { task in

if let error = task.error {
completionHandler(nil, error)
} else {
if self.pendingGetTokensCompletion != nil {
self.pendingGetTokensCompletion?(self.getTokensForCognitoAuthSession(session: session), nil)
self.pendingGetTokensCompletion = nil
self.tokenFetchLock.leave()
}
self.mobileClientStatusChanged(userState: .signedIn,
additionalInfo: signInInfo)
completionHandler(.signedIn, nil)
}
return nil
}
self.pendingGetTokensCompletion?(self.getTokensForCognitoAuthSession(session: session), nil)
self.pendingGetTokensCompletion = nil
}
}

Expand All @@ -477,8 +493,18 @@ final public class AWSMobileClient: _AWSMobileClient {
} else {
self.currentUser?.getSession().continueWith(block: { (task) -> Any? in
if let session = task.result {
self.performUserPoolSuccessfulSignInTasks(session: session)
completionHandler(.signedIn, nil)
self.performUserPoolSuccessfulSignInTasks(session: session).continueWith { task in
if let error = task.error {
completionHandler(nil, error)
} else {
let tokenString = session.idToken!.tokenString
self.mobileClientStatusChanged(userState: .signedIn,
additionalInfo: [self.ProviderKey: self.userPoolClient!.identityProviderName,
self.TokenKey: tokenString])
completionHandler(.signedIn, nil)
}
return nil
}
} else {
completionHandler(nil, task.error)
}
Expand Down
119 changes: 71 additions & 48 deletions AWSAuthSDK/Sources/AWSMobileClient/AWSMobileClientExtensions.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,21 +260,12 @@ extension AWSMobileClient {
completionHandler: @escaping ((UserState?, Error?) -> Void)) {

self.tokenFetchOperationQueue.addOperation {
var error: Error?

// At the end of operation if there is an error anywhere in the flow, we return it back to the developer; else return a successful signedIn state.
defer {
if error == nil {
completionHandler(UserState.signedIn, nil)
} else {
completionHandler(nil, error)
}
}


// If there is a userpools federation already active, we return an error.
// Developer cannot initiate a federatedSignIn when a UserPools sign in is active.
guard self.federationProvider != .userPools else {
error = AWSMobileClientError.federationProviderExists(message: "User is already signed in. Please sign out before calling this method.")
let error = AWSMobileClientError.federationProviderExists(message: "User is already signed in. Please sign out before calling this method.")
completionHandler(nil, error)
return
}

Expand All @@ -288,36 +279,50 @@ extension AWSMobileClient {
self.customRoleArnInternal = customRoleArn
_AWSMobileClient.sharedInstance().setCustomRoleArnInternal(customRoleArn, for: self)
}
self.performFederatedSignInTasks(provider: providerName, token: token)
// If any credentials operation is pending, we invoke the waiting block to resume with new credentials
if (self.pendingAWSCredentialsCompletion != nil) {
self.internalCredentialsProvider?.credentials().continueWith(block: { (task) -> Any? in
if let credentials = task.result {
self.pendingAWSCredentialsCompletion?(credentials, nil)
} else if let error = task.error {
self.pendingAWSCredentialsCompletion?(nil, error)
}
self.performFederatedSignInTasks(provider: providerName, token: token).continueWith { task in
// If any credentials operation is pending, we invoke the waiting block to resume with new credentials
if let credentials = task.result {
self.mobileClientStatusChanged(userState: .signedIn,
additionalInfo: [self.ProviderKey:providerName, self.TokenKey: token])
self.pendingAWSCredentialsCompletion?(credentials, nil)
completionHandler(UserState.signedIn, nil)
} else if let error = task.error {
self.pendingAWSCredentialsCompletion?(nil, error)
completionHandler(nil, error)
}
if self.pendingAWSCredentialsCompletion != nil {
self.credentialsFetchLock.leave()
self.pendingAWSCredentialsCompletion = nil
return nil
})
}
return nil
}

} else { // first time calling federatedSignIn
// If using developer authenticated identities, identityId is required.
// Check if identityId is specified by the developer, else return error.
if providerName == IdentityProvider.developer.rawValue {
if let devAuthenticatedIdentityId = federatedSignInOptions.cognitoIdentityId {
self.internalCredentialsProvider?.identityProvider.identityId = devAuthenticatedIdentityId
} else {
error = AWSMobileClientError.invalidParameter(message: "For using developer authenticated identities, you need to specify the `CognitoIdentityId` in `FederatedSignInOptions`.")
let error = AWSMobileClientError.invalidParameter(message: "For using developer authenticated identities, you need to specify the `CognitoIdentityId` in `FederatedSignInOptions`.")
completionHandler(nil, error)
return
}
}
if let customRoleArn = federatedSignInOptions.customRoleARN {
self.customRoleArnInternal = customRoleArn
_AWSMobileClient.sharedInstance().setCustomRoleArnInternal(customRoleArn, for: self)
}
self.performFederatedSignInTasks(provider: providerName, token: token)
self.performFederatedSignInTasks(provider: providerName, token: token).continueWith { task in
if let error = task.error {
completionHandler(nil, error)
return nil
}
self.mobileClientStatusChanged(userState: .signedIn,
additionalInfo: [self.ProviderKey:providerName, self.TokenKey: token])
completionHandler(UserState.signedIn, nil)
return nil
}
}
}
}
Expand Down Expand Up @@ -530,10 +535,22 @@ extension AWSMobileClient {
} else if let result = task.result {
self.internalCredentialsProvider?.clearCredentials()
self.federationProvider = .userPools
let signInResult = SignInResult(signInState: .signedIn)
self.userpoolOpsHelper.currentSignInHandlerCallback?(signInResult, nil)
self.userpoolOpsHelper.currentSignInHandlerCallback = nil
self.performUserPoolSuccessfulSignInTasks(session: result)
self.performUserPoolSuccessfulSignInTasks(session: result).continueWith { task in
if let error = task.error {
self.userpoolOpsHelper.currentSignInHandlerCallback?(nil, self.getMobileError(for: error))
self.userpoolOpsHelper.currentSignInHandlerCallback = nil

} else {
let signInResult = SignInResult(signInState: .signedIn)
let tokenString = result.idToken!.tokenString
self.mobileClientStatusChanged(userState: .signedIn,
additionalInfo: [self.ProviderKey: self.userPoolClient!.identityProviderName,
self.TokenKey: tokenString])
self.userpoolOpsHelper.currentSignInHandlerCallback?(signInResult, nil)
self.userpoolOpsHelper.currentSignInHandlerCallback = nil
}
return nil
}
}
return nil
}
Expand Down Expand Up @@ -579,7 +596,7 @@ extension AWSMobileClient {
return
}
signOut()

completionHandler(nil)
}

/// Signs out the current logged in user and clears the local keychain store.
Expand All @@ -600,41 +617,47 @@ extension AWSMobileClient {
self.mobileClientStatusChanged(userState: .signedOut, additionalInfo: [:])
}

internal func performUserPoolSuccessfulSignInTasks(session: AWSCognitoIdentityUserSession) {
internal func performUserPoolSuccessfulSignInTasks(session: AWSCognitoIdentityUserSession) -> AWSTask<AWSCredentials> {
let tokenString = session.idToken!.tokenString
self.developerNavigationController = nil
self.cachedLoginsMap = [self.userPoolClient!.identityProviderName: tokenString]
self.mobileClientStatusChanged(userState: .signedIn, additionalInfo: [ProviderKey:self.userPoolClient!.identityProviderName, TokenKey:tokenString])
self.saveLoginsMapInKeychain()
self.setLoginProviderMetadataAndSaveInKeychain(provider: .userPools)
self.internalCredentialsProvider?.clearCredentials()
self.internalCredentialsProvider?.credentials()
return postSignInKeychainAndCredentialsUpdate(provider: .userPools)
}

internal func performHostedUISuccessfulSignInTasks(disableFederation: Bool = false, session: AWSCognitoAuthUserSession, federationToken: String, federationProviderIdentifier: String? = nil, signInInfo: inout [String: String]) {
internal func performHostedUISuccessfulSignInTasks(disableFederation: Bool = false,
session: AWSCognitoAuthUserSession,
federationToken: String,
federationProviderIdentifier: String? = nil,
signInInfo: inout [String: String]) -> AWSTask<AWSCredentials> {
federationDisabled = disableFederation
if federationProviderIdentifier == nil {
self.cachedLoginsMap = [self.userPoolClient!.identityProviderName: federationToken]
} else {
self.cachedLoginsMap = [federationProviderIdentifier!: federationToken]
}
self.mobileClientStatusChanged(userState: .signedIn, additionalInfo: signInInfo)
self.saveLoginsMapInKeychain()
self.setLoginProviderMetadataAndSaveInKeychain(provider: .hostedUI)
self.internalCredentialsProvider?.clearCredentials()
self.internalCredentialsProvider?.credentials()
return postSignInKeychainAndCredentialsUpdate(provider: .hostedUI)
}

internal func performFederatedSignInTasks(provider: String, token: String) {
internal func performFederatedSignInTasks(provider: String, token: String) -> AWSTask<AWSCredentials> {
self.cachedLoginsMap = [provider:token]
self.mobileClientStatusChanged(userState: .signedIn, additionalInfo: [ProviderKey:provider, TokenKey: token])
self.federationProvider = .oidcFederation
self.saveLoginsMapInKeychain()
self.setLoginProviderMetadataAndSaveInKeychain(provider: .oidcFederation)
self.internalCredentialsProvider?.clearCredentials()
self.internalCredentialsProvider?.credentials()
return postSignInKeychainAndCredentialsUpdate(provider: .oidcFederation)
}

/// Post signin operations. Saves the login maps and provider metadata in keychain. Updates the credentials.
/// If credential update is successful, informs the listener with UserState.signedIn status.
///
/// - Parameters
/// - provider: provider to be updated in keychain.
func postSignInKeychainAndCredentialsUpdate(provider: FederationProvider) -> AWSTask<AWSCredentials> {
self.saveLoginsMapInKeychain()
self.setLoginProviderMetadataAndSaveInKeychain(provider: provider)
guard let credentialsProvider = self.internalCredentialsProvider else {
return AWSTask(error: AWSMobileClientError.internalError(message: "Credentials provider is nil."))
}
credentialsProvider.clearCredentials()
return credentialsProvider.credentials();
}

/// Confirm a sign in which requires additional validation via steps like SMS MFA.
///
Expand Down
Loading

0 comments on commit f39d113

Please sign in to comment.