aws-amplify / amplify-android

The fastest and easiest way to use AWS from your Android app.
https://docs.amplify.aws/lib/q/platform/android/
Apache License 2.0
250 stars 117 forks source link

clientMetadata can't be passed into AWSCognitoAuthPlugin::signIn #2013

Closed wangf1978 closed 2 years ago

wangf1978 commented 2 years ago

Before opening, please confirm:

Language and Async Model

Java

Amplify Categories

Authentication

Gradle script dependencies

```groovy // Put output below this line // Amplify core dependency def amplify_version = "1.37.3" implementation "com.amplifyframework:core:$amplify_version" implementation "com.amplifyframework:aws-auth-cognito:$amplify_version" implementation "com.amplifyframework:aws-storage-s3:$amplify_version" implementation "com.amplifyframework:aws-api:$amplify_version" implementation "com.amplifyframework:aws-datastore:$amplify_version" ```

Environment information

``` # Put output below this line ------------------------------------------------------------ Gradle 7.2 ------------------------------------------------------------ Build time: 2021-08-17 09:59:03 UTC Revision: a773786b58bb28710e3dc96c4d1a7063628952ad Kotlin: 1.5.21 Groovy: 3.0.8 Ant: Apache Ant(TM) version 1.10.9 compiled on September 27 2020 JVM: 11.0.12 (JetBrains s.r.o 11.0.12+7-b1504.28-7817840) OS: Windows 10 10.0 amd64 ```

Please include any relevant guides or documentation you're referencing

No response

Describe the bug

Now metadata is passed into the parameter 'validationData' in this function, but the parameter 'clientMetadata' is always using a fixed value Collections.emptyMap() If so, when signing in successfully with username/password, in the Post authentication Lambda trigger of AWS Cognito,

image

clientMetadata in this trigger parameters can't have the specified key/value pairs from android client side, but it can be set by amplify-js and amplify-ios.

After checking the current amplify-android implementation, it is found that no clientMetadata is passed into awsMobileClient.signIn , please see aws-auth-cognito/src/main/java/com/amplifyframework/auth/cognito/AWSCognitoAuthPlugin.java#L425, and then in AWS Coginito lambda trigger, no chance to pass clientMetadata from client side

here is the code clip from amplify-android sdk image

amplify-js sdk image

amplify-ios sdk image

Reproduction steps (if applicable)

  1. Call Amplify.Auth.signIn with clientMetadata, for example, sessionId, email, language
  2. Check the Pre authentication Lambda trigger of AWS Cognito, the clientMetadata information passed from android client side can be got from validationData attribute in the request, please see the details here
  3. Check the Post authentication Lambda trigger of AWS Cognito, the clientMetadata information passed from android client side can't be got in "clientMetadata" attribute (no validationData attribute) in the request, please see the details here

Code Snippet

// Put your code below this line.
private void signInNextStepSignIn(@NonNull String user, @NonNull String password, @NonNull String session) {
    Map<String, String> metadata = new HashMap<>();
    AWSCognitoAuthSignInOptions options;

    metadata.put(SESSION_ID, session);
    metadata.put("email", user);
    metadata.put("langcode", "en");
    options = AWSCognitoAuthSignInOptions
            .builder()
            .metadata(metadata)
            .build();
    Amplify.Auth.signIn(user, password, options,
            result -> {
                AuthUser authUser = Amplify.Auth.getCurrentUser();
                if (authUser != null && result.isSignInComplete()) {
                    Log.i(TAG, "signIn success");
                    handleNewUserSignIn(authUser);
                    signInNextStepGetUserInfo();
                } else {
                    Log.e(TAG, "signIn failed: " + result.getNextStep());
                    onSignInError(result);
                }
            },
            (e) -> {
                Log.e(TAG, "signIn failed: ", e);
                onSignInError(e);
            });
}

Log output

``` // Put your logs below this line ```

amplifyconfiguration.json

No response

GraphQL Schema

```graphql // Put your schema below this line ```

Additional information and screenshots

No response

wangf1978 commented 2 years ago

Dear amplify-android team,

We have a product which will be deployed at the end of this month, the web/ios platform worked well, but now it is blocked by this issue in android platform. In a short word, is it possible to provide a way with amplify-android to bring some information into AWS Coginito Post confirmation Lambda trigger? Many thanks.

tylerjroach commented 2 years ago

@wangf1978 Thank you for such a well structured and detailed report! I'm still digging into this, but it does appear to be a bug. I'll follow up with you when I have finished researching.

wangf1978 commented 2 years ago

@tylerjroach I have the same concern with you in code commit comment, if validationData is nil, it will cause another issue, the pre authentication lambda trigger can't have validationData, according to our test, although amplify-ios passed the nil value, but finally it seems it fills validationData with clientMetadata in somewhere, we didn't dig into this case in amplify-ios. In amplify-js, there is a global clientMetadata passed from amplfy.auth.configure, metadata passed in Amplify Auth signIn will work as valididationData, but the global clientMetadata will work as clientMetdata. for this part, I am also a little confused about the implementation and lambada trigger, signIn is related with pre/post authentication lambda trigger, but these 2 lambda trigger need 2 different metadata from clientSide.

tylerjroach commented 2 years ago

@wangf1978 We let the clientMetadata to be passed in validationData to prevent breaking implementations. We have also added the clientMetadata to properly be passed in signIn, so the data should be accessible as clientMetadata in both the pre and post triggers. The fix will be live in the next release.

github-actions[bot] commented 2 years ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

wangf1978 commented 2 years ago

@tylerjroach thanks for your clarification, I can't wait for your new release:)