aws-amplify / aws-sdk-ios

AWS SDK for iOS. For more information, see our web site:
https://aws-amplify.github.io/docs
Other
1.68k stars 877 forks source link

AWSCognitoIdentityProvider is not clearing IdentityId when clearKeychain() is invoked. When a new user signs in after the previous one signed out, the previous IdentityId is used. #1014

Closed ShadyBoukhary closed 5 years ago

ShadyBoukhary commented 6 years ago

The IdentityId is not cleared even when clearKeyChain() is called upon signout. I checked the actual method implementation and I found out that it only clears the IdentityId property, but not the one in keyChain. I tried to edit the SDK myself to delete the IdentityId from KeyChain when clearKeyChain() is called. This successfully deletes the IdentityId. When the user signs in again, the old IdentityId is gone as expected. When a new user signs in, printing the identityId straight after signing in shows the correct identityId. The problem is, when an AWS Lambda API is invoked from the app, the identityId is automatically retrieved again in order to be used for the API, but it is the old IdentityId that belonged to the previous user. So there seems to be 2 versions of the IdentityId. The correct one is retrieved when I manually call credentialsProvider.identityId, and the old, wrong one is returned when it is automatically retrieved from the API call.

To help us solve your problem better, please answer the following list of questions.

Create a simple app where you sign in using AWSCognito and CredentialsProvider and the getSession() method. Then create a sign out functionality. Try signing in for the first time and print the IdentityId using credentailsProvider.identityId. Then sign out and call clearKeyChain() to clear the identityId. Implement an API call to AWS Lambda using their downloaded SDK. Sign in using a different account. The API will use the old IdentityId instead of the new one.

rohandubal commented 6 years ago

Hello @ShadyBoukhary

Sorry that you are having trouble using the SDK. The IdentityId once assigned to a user is always the same and does not change. So, if another user signs in, even if we use the old identity id with the new token, it should return a different identity id from the service.

One possible thing is that the token is stored/ cached in the client too and is not cleared. So if the same token is available and old id is cached, it can result in this. Are you using Cognito Credentials Provider?

Could you provide the code snippet of how you are initializing your configuration/ credentials provider and service configuration.

Thanks, Rohan

ShadyBoukhary commented 6 years ago

Hello @rohandubal, Yes, I am using Cognito Credentials Provider. Please note that I am clearing the token and the identity Id from KeyChain once the user signs out. The only time i get the old Identity Id is when I invoke an API, which automatically tries to retrieve the Identity Id. However, If I call credentialsProvider.IdentityId manually, It returns the correct one. Here is the code for my initialization:

let serviceConfiguration = AWSServiceConfiguration(region: region, credentialsProvider: nil);
let userPoolConfiguration = AWSCognitoIdentityUserPoolConfiguration(clientId: clientId, clientSecret: clientSecret, poolId: userPoolId)
AWSCognitoIdentityUserPool.register(with: serviceConfiguration, userPoolConfiguration: userPoolConfiguration, forKey: "UserPool")
self.userPool = AWSCognitoIdentityUserPool(forKey: "UserPool")

As you can see, the credentials provider is initially null. When the user signs in, I create a subclass of the credentials provider and store the token manually there. That is to fix an issue where the API SDK could not find the token. I think it is because signing in with user.getSession() for some reason does not sync with the Credentials Provider automatically.

class CustomIdentityProvider: NSObject, AWSIdentityProviderManager{
    var tokens : NSDictionary?
    init(tokens: NSDictionary) {
        self.tokens = tokens
    }
    @objc func logins() -> AWSTask<NSDictionary> {
        return AWSTask(result: tokens)
    }
}

When the user signs in, here's how I store the token manually:

let logins = NSDictionary(dictionary: [COGNITO_KEY: AWSCognitoRepository.awsCognitoRepository.getToken()])
 let customProviderManager = CustomIdentityProvider(tokens: logins)
 print(logins)
 self.credentialsProvider = AWSCognitoCredentialsProvider(
            regionType: AWSRegionType.USEast2,
            identityPoolId: IDENTITY_POOL_ID,
            identityProviderManager: customProviderManager)
 let serviceConfiguration = AWSServiceConfiguration(region: AWSRegionType.USEast2,  
            credentialsProvider: self.credentialsProvider)
  AWSServiceManager.default().defaultServiceConfiguration = serviceConfiguration

Then when configuring the AWS API Client:

init(configuration: AWSServiceConfiguration) {
        super.init()

        self.configuration = configuration.copy() as! AWSServiceConfiguration
        var URLString: String = BASE_URL
        if URLString.hasSuffix("/") {
            URLString = URLString.substring(to: URLString.index(before: URLString.endIndex))
        }
        self.configuration.endpoint = AWSEndpoint(region: configuration.regionType, service: .APIGateway, url: URL(string: URLString))
        let signer: AWSSignatureV4Signer = AWSSignatureV4Signer(credentialsProvider: configuration.credentialsProvider, endpoint: self.configuration.endpoint)
        if let endpoint = self.configuration.endpoint {
            self.configuration.baseURL = endpoint.url
        }
        self.configuration.requestInterceptors = [AWSNetworkingRequestInterceptor(), signer]
    }

This fixes the API issue with the token. When signing out, I call

user.globalSignOut().continueWith(block: { (task) -> Void in
            if task.error != nil {
                print("Sign Out Unsuccessful")
                print(task.error!)
            } else {
                self.setRememberMe(rememberMe: false)
                UserDefaults.standard.set(false, forKey: "authenticated")
                self.credentialsProvider?.clearCredentials()
                self.credentialsProvider?.invalidateCachedTemporaryCredentials()
                self.credentialsProvider?.clearKeychain()
                print(self.credentialsProvider?.identityId)

                print("Sign out successful")
            }
        })

This successfully clears the identity Id (it prints nil). When a new user signs in, printing it using the same method prints the correct Identity Id. But when the API Client tries to automatically retrieve it, It somehow retrieves the old one (I used the debugger to find that out).

Any ideas? Thank you

dwsolberg commented 6 years ago

I'm having what seems to be a related issue where AWSCognitoAuth returns old information:

Here are the steps:

  1. I use cognitoAuth.signOutLocallyAndClearLastKnownUser() to logout while offline.
  2. When back online, I call: cognitoAuth.getSession(viewController, completion: completion) Immediately after the page loads in SFSafariViewController, I get an error in the completion closure: 'AWSCognitoAuthClientErrorLoadingPageFailed'
  3. Upon restarting the app, 'cognitoAuth.getSession(completion)' returns the previously logged out user as though they were still logged in.

To test this out, I manually cleared everything in the app's keychain, but the previous session still revives itself after cognitoAuth.getSession(viewController, completion: completion) is called, even though it returns an error.

The behavior that seems expected is that if the user is signed out and cleared, calling cognitoAuth.getSession(viewController, completion: completion) should open a new authorization page with no knowledge of the prior user.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

ravishagnihotri45 commented 5 years ago

Facing issue that my keychain is not clearing the existing user data while calling Sign-out method and login using the other user

ravishagnihotri45 commented 5 years ago

I have fixed the existing issue by making following changes in pod file

-(void) addSecretHashDeviceKeyAndUsername:(NSMutableDictionary<NSString ,NSString> *) authParameters {

if(self.username == nil) {
   self.username = authParameters[@"USERNAME"];
}
else  //RA999 I have added this code to set always the new username which is entered by user responding the registered cell no he has entered, erlier we are getting the priviouly logged in user name even after we have logged out the user To Fix this i have added the code here
{
   if(authParameters[@"USERNAME"] != nil)
   {
        self.username = authParameters[@"USERNAME"];
   }

}
ShadyBoukhary commented 5 years ago

@ravishagnihotri45 what about the identity id? I had no problem with the user name just with the identity id.

rohandubal commented 5 years ago

Hello @ShadyBoukhary

To clear the identityId you will have to call the clearKeychain method on your self.credentialsProvider object.

Reference: https://github.com/aws-amplify/aws-sdk-ios/blob/master/AWSCore/Authentication/AWSCredentialsProvider.h#L252

As you can see in the reference, that method will clear the identityId and other credentials stored in the keychain.

Please let us know if this resolves your issue.

Thanks, Rohan

ekordin commented 5 years ago

@rohandubal , what has been changed? As in the description of the issue the function clearKeychain was actually called, but the issue is present... no?

ShadyBoukhary commented 5 years ago

@ekordin @rohanfubal clearKeychain is called in the original issue yet the problem still remains.

rohandubal commented 5 years ago

@ShadyBoukhary apologies for the delayed response. I read through the thread again and identified one thing which I previously missed.

In the configuration object which you are using and assigning to API Gateway client, it is likely keeping a copy of the object which has the old token. When you call globalSignOut, all the tokens are invalidated; BUT the issue is that the AWS Credentials are already retrieved and available to the API client which it uses for the next user.

What needs to be done is using a single AWSCredentialsProvider which is tied with the CognitoUserPools client. The custom implementation introduces a layer which can cause an impact in the behavior of AWSCredentialsProvider.

I would recommend you to use AWSMobileClient which does this management for you. If you need help setting up your custom provider correctly, I am happy to help as I can understand that migrating to a new library can be difficult.

Please let me know if you are still facing the issue and I can help you have the right setup for the use-case.

Best, Rohan

ShadyBoukhary commented 5 years ago

@rohandubal thanks for the response. The project has been put on hold due to unrelated reasons. Therefore, I no longer require a solution as I am no longer using this code.

I think you are right about the cause of the problem though.

palpatim commented 5 years ago

@ShadyBoukhary Thanks for the update. I'm closing this issue, but if you reopen the project and have more questions, please feel free to open a new issue.