Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to verify secret hash for client in v2.13.1 too but working fine for iOS #889

Closed
usmanrana07 opened this issue Apr 9, 2019 · 5 comments · Fixed by #1144
Closed
Assignees
Labels
bug Something isn't working cognito Issues with the AWS Android SDK for Cognito pending-community-response Issue is pending response from the issue requestor

Comments

@usmanrana07
Copy link

usmanrana07 commented Apr 9, 2019

I'm implementing CustomAuthChallenge'. This issue occurs when:

  • I try to pass user name for getting user session which is lets say 'abc_xyz'
  • Then it comes in 'getAuthenticationDetails' method of the callback where i set username and auth type.
  • Then 'authenticateChallenge' method is called in which 'continuation' object is received which has a username in its params and that username is not the same that i passed but is userId '12121' of that user.
  • I set ANSWER in 'authenticateChallenge' and let it continueTask
  • 'onFailure' is called with the error 'Unable to verify secret hash for client'

I checked the sdk and i saw that userId is treated as 'usernameInternal' . If i use userId '12121' in replacement of username 'abc_xyz' instead then it works fine. But in iOS it is working with username and we don't want to disclose userId so using username is mandatory. How can i resolve this issue?

@desokroshan desokroshan added cognito Issues with the AWS Android SDK for Cognito question General question labels Apr 9, 2019
@usmanrana07
Copy link
Author

when is this expected to be resolved?

@charandas
Copy link

@usmanrana07 what's interesting is that secretHash can be calculated with userId for getAuthenticationDetails, it allows you to override (not that you need this capability):

           override fun getAuthenticationDetails(authContinuation: AuthenticationContinuation, username: String) {
                val authParameters = mutableMapOf<String, String>()
                authParameters[CognitoServiceConstants.AUTH_PARAM_USERNAME] = username
                val hash = CognitoSecretHash.getSecretHash(
                    authParams?.userId,
                    CognitoUserPoolHelper.CLIENT_ID,
                    CognitoUserPoolHelper.CLIENT_SECRET
                )

                val authenticationDetails = AuthenticationDetails(authParams?.userId, authParameters, null)
                authenticationDetails.authenticationType = CognitoServiceConstants.AUTH_TYPE_INIT_CUSTOM_AUTH
                authenticationDetails.customChallenge = CognitoServiceConstants.CHLG_TYPE_CUSTOM_CHALLENGE
                authenticationDetails.setAuthenticationParameter(CognitoServiceConstants.AUTH_PARAM_SECRET_HASH, hash)
                authenticationDetails.setAuthenticationParameter(CognitoServiceConstants.CHLG_RESP_SECRET_HASH, hash)
                authContinuation.setAuthenticationDetails(authenticationDetails)
                authContinuation.continueTask()
            }

The same cannot be said of challengeContinuation (now the overriding capability could get you past this issue 😉, but there is no such capability). With challengeContinuation it would read it out of the user. The user would set it up using userId. I have noticed that the backend does not want this.

    public CognitoUser getUser(String userId) {

        if (userId == null) {
            return getUser();
        }

        if (userId.isEmpty()) {
            return getUser();
        }

        return new CognitoUser(this, userId, clientId, clientSecret,
                CognitoSecretHash.getSecretHash(userId, clientId, clientSecret),
                client, context);
    }

The only silver lining is a user one sets up with userPool.getUser() instead of getUser(userId). The latter sets secretHash to null, and so ends up benefiting from this null check in updateInternalName.

    private void updateInternalUsername(Map<String, String> challengeParameters) {
        if (usernameInternal == null) {
            if (challengeParameters != null && challengeParameters
                    .containsKey(CognitoServiceConstants.CHLG_PARAM_USERNAME)) {
                usernameInternal = challengeParameters
                        .get(CognitoServiceConstants.CHLG_PARAM_USERNAME);
                deviceKey = CognitoDeviceHelper.getDeviceKey(usernameInternal, pool.getUserPoolId(),
                        context);
                if (secretHash == null) {
                    secretHash = CognitoSecretHash.getSecretHash(usernameInternal, clientId,
                            clientSecret);
                }
            }
        }
    }

However, I run into other issues with this getUser(), such as not being able to call updateUserAttributes successfully after authenticating. Errors like User-Id is null come back.

Here @kvasukib loosely recommends using a public (as opposed to confidential client) for custom auth. I am kinda concerned (security-wise) about this for this mobile app I am working out.

@charandas
Copy link

My settings are like so. I wonder if people are not using username attributes like phone number, if this ends up working for them. I am using the phone number as one.

Screen Shot 2019-05-15 at 3 11 17 PM

@desokroshan desokroshan added bug Something isn't working and removed question General question labels Jun 6, 2019
@mutablealligator
Copy link
Contributor

We are adding this issue to our backlog. We will post an update here when we get to it.

@desokroshan
Copy link
Contributor

A fix has been released in the version 2.15.2 of the SDK. Please feel free to upgrade and verify.

@palpatim palpatim added pending-community-response Issue is pending response from the issue requestor and removed pending-release Code has been merged but pending release labels Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cognito Issues with the AWS Android SDK for Cognito pending-community-response Issue is pending response from the issue requestor
Projects
None yet
6 participants