fix(auth): handle missing or empty signUpOptions (#627)

* handle missing or empty signUpOptions

* add android unit test

* fix iOS tests

* add integration tests for auth.signUp

* remove unused imports, formatting

* update auth category after NS changes

* update unit test for NS changes

* address PR comments

* remove extra parens

* Apply SignUpRequest changes from code review

packages/amplify_auth_cognito/android/src/main/kotlin/com/amazonaws/amplify/amplify_auth_cognito/types/FlutterSignUpRequest.kt

@@ -17,34 +17,36 @@
 
 package com.amazonaws.amplify.amplify_auth_cognito.types
 
-import androidx.annotation.NonNull
 import com.amazonaws.amplify.amplify_auth_cognito.utils.createAuthUserAttribute
 import com.amazonaws.amplify.amplify_core.exception.ExceptionMessages
 import com.amazonaws.amplify.amplify_core.exception.InvalidRequestException
-import com.amplifyframework.AmplifyException
 import com.amplifyframework.auth.AuthUserAttribute
 import com.amplifyframework.auth.cognito.options.AWSCognitoAuthSignUpOptions
 
 data class FlutterSignUpRequest(val map: HashMap<String, *>) {
     val username: String = map["username"] as String
     val password: String = map["password"] as String
-    val options: AWSCognitoAuthSignUpOptions = formatOptions(map["options"] as HashMap<String, *>)
+    val options: AWSCognitoAuthSignUpOptions = formatOptions(map["options"] as HashMap<String, *>?)
 
-    private fun formatOptions(@NonNull rawOptions: HashMap<String, *>): AWSCognitoAuthSignUpOptions {
-        var options =  AWSCognitoAuthSignUpOptions.builder();
-        var authUserAttributes: MutableList<AuthUserAttribute> = mutableListOf();
-        var validationData = rawOptions["validationData"] as? MutableMap<String, String>;
+    private fun formatOptions(rawOptions: HashMap<String, *>?): AWSCognitoAuthSignUpOptions {
+        val optionsBuilder =  AWSCognitoAuthSignUpOptions.builder()
+        if (rawOptions != null) {
+            val attributeData = rawOptions["userAttributes"] as? MutableMap<String, String>
+            val validationData = rawOptions["validationData"] as? MutableMap<String, String>
 
-        (rawOptions["userAttributes"] as HashMap<String, String>).forEach { (key, value) ->
-            var attribute = createAuthUserAttribute(key, value);
-            authUserAttributes.add(attribute);
-        }
-        options.userAttributes(authUserAttributes);
+            if (attributeData is MutableMap<String, String>) {
+                val authUserAttributes: List<AuthUserAttribute> = attributeData.map { (key, value) ->
+                    createAuthUserAttribute(key, value)
+                }
+                optionsBuilder.userAttributes(authUserAttributes)
+            }
+
+            if (validationData is MutableMap<String, String>) {
+                optionsBuilder.validationData(validationData)
+            }
 
-        if (validationData is MutableMap<String, String>) {
-            options.validationData(validationData)
         }
-        return options.build();
+        return optionsBuilder.build()
     }
 
     companion object {
@@ -53,12 +55,6 @@ data class FlutterSignUpRequest(val map: HashMap<String, *>) {
             if (req == null) {
                 throw InvalidRequestException(validationErrorMessage, ExceptionMessages.missingAttribute.format( "request map" ))
             }
-            if (req.get("options") == null) {
-                throw AmplifyException(validationErrorMessage, ExceptionMessages.missingAttribute.format( "options map" ))
-            }
-            if (!(req?.get("options") as HashMap<String, String>).containsKey("userAttributes")) {
-                throw InvalidRequestException(validationErrorMessage, ExceptionMessages.missingAttribute.format( "userAttributes" ))
-            }
             if (!req.containsKey("password")) {
                 throw InvalidRequestException(validationErrorMessage, ExceptionMessages.missingAttribute.format( "password" ))
             }

packages/amplify_auth_cognito/android/src/test/kotlin/com/amazonaws/amplify/amplify_auth_cognito/AmplifyAuthCognitoPluginTest.kt

@@ -123,6 +123,39 @@ class AmplifyAuthCognitoPluginTest {
         verify(mockResult, times(1)).success(res);
     }
 
+    @Test
+    fun signUpNoOptions_returnsSuccess() {
+        // Arrange
+        doAnswer { invocation: InvocationOnMock ->
+            plugin.prepareSignUpResult(mockResult, mockSignUpResult)
+            null as Void?
+        }.`when`(mockAuth).signUp(anyString(), anyString(), any(AuthSignUpOptions::class.java), ArgumentMatchers.any<Consumer<AuthSignUpResult>>(), ArgumentMatchers.any<Consumer<AuthException>>())
+
+        val data: HashMap<*, *> = hashMapOf(
+                "username" to "[email protected]",
+                "password" to "testPassword"
+        )
+        val arguments = hashMapOf("data" to data)
+        val call = MethodCall("signUp", arguments)
+        val res = mapOf(
+                "isSignUpComplete" to false,
+                "nextStep" to mapOf(
+                        "signUpStep" to "CONFIRM_SIGN_UP_STEP",
+                        "codeDeliveryDetails" to mapOf(
+                                "destination" to "[email protected]",
+                                "deliveryMedium" to AuthCodeDeliveryDetails.DeliveryMedium.EMAIL.name,
+                                "attributeName" to "email"
+                        )
+                )
+        )
+
+        // Act
+        plugin.onMethodCall(call, mockResult)
+
+        // Assert
+        verify(mockResult, times(1)).success(res);
+    }
+
     @Test
     fun confirmSignUpWithoutOptions_returnsSuccess() {
         // Arrange

packages/amplify_auth_cognito/example/integration_test/main_test.dart

@@ -20,6 +20,7 @@ import 'package:amplify_auth_cognito/amplify_auth_cognito.dart';
 import 'package:amplify_auth_cognito_example/amplifyconfiguration.dart';
 
 import 'sign_in_sign_out_test.dart' as sign_in_sign_out_tests;
+import 'sign_up_test.dart' as sign_up_tests;
 
 void main() async {
   IntegrationTestWidgetsFlutterBinding.ensureInitialized();
@@ -32,5 +33,6 @@ void main() async {
     });
 
     sign_in_sign_out_tests.main();
+    sign_up_tests.main();
   });
 }

packages/amplify_auth_cognito/example/integration_test/sign_in_sign_out_test.dart

@@ -17,41 +17,29 @@ import 'package:integration_test/integration_test.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:amplify_flutter/amplify.dart';
 import 'package:amplify_auth_cognito/amplify_auth_cognito.dart';
-import 'package:uuid/uuid.dart';
 
-import 'package:amplify_auth_cognito_example/amplifyconfiguration.dart';
-
-final uuid = Uuid();
+import 'utils/mock_data.dart';
+import 'utils/setup_utils.dart';
 
 void main() {
   IntegrationTestWidgetsFlutterBinding.ensureInitialized();
 
-  final username = 'TEMP_USER-${uuid.v4()}';
-  final password = uuid.v4();
+  final username = generateUsername();
+  final password = generatePassword();
 
   group('signIn and signOut', () {
     setUpAll(() async {
-      if (!Amplify.isConfigured) {
-        final authPlugin = AmplifyAuthCognito();
-        await Amplify.addPlugins([authPlugin]);
-        await Amplify.configure(amplifyconfig);
-      }
+      await configureAuth();
 
       await Amplify.Auth.signUp(
           username: username,
           password: password,
           options: CognitoSignUpOptions(userAttributes: {
-            'email': 'test-amplify-flutter-${uuid.v4()}@test${uuid.v4()}.com',
-            'phone_number': '+15555551234'
+            'email': generateEmail(),
+            'phone_number': mockPhoneNumber
           }));
 
-      // ensure no user is currently signed in
-      try {
-        await Amplify.Auth.signOut();
-        // ignore: unused_catch_clause
-      } on AuthException catch (e) {
-        // Ignore a signOut error because we only care when someone signed in.
-      }
+      await signOutUser();
     });
 
     testWidgets('should signIn a user', (WidgetTester tester) async {

packages/amplify_auth_cognito/example/integration_test/sign_up_test.dart

@@ -0,0 +1,64 @@
+/*
+ * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file is distributed
+ * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+import 'package:integration_test/integration_test.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:amplify_flutter/amplify.dart';
+import 'package:amplify_auth_cognito/amplify_auth_cognito.dart';
+
+import 'utils/mock_data.dart';
+import 'utils/setup_utils.dart';
+
+void main() {
+  IntegrationTestWidgetsFlutterBinding.ensureInitialized();
+
+  group('signUp', () {
+    setUpAll(() async {
+      await configureAuth();
+      await signOutUser();
+    });
+
+    testWidgets('should signUp a user with valid parameters',
+        (WidgetTester tester) async {
+      final username = generateUsername();
+      final password = generatePassword();
+
+      var res = await Amplify.Auth.signUp(
+          username: username,
+          password: password,
+          options: CognitoSignUpOptions(userAttributes: {
+            'email': generateEmail(),
+            'phone_number': mockPhoneNumber
+          }));
+      // should be uncommented when https://github.com/aws-amplify/amplify-flutter/issues/581 is closed
+      // currently this just confirms there is no error thrown
+      // expect(res.isSignUpComplete, true);
+    });
+
+    testWidgets(
+        'should throw an InvalidParameterException without required attributes',
+        (WidgetTester tester) async {
+      final username = generateUsername();
+      final password = generatePassword();
+      try {
+        await Amplify.Auth.signUp(username: username, password: password);
+      } catch (e) {
+        expect(e, TypeMatcher<InvalidParameterException>());
+        return;
+      }
+      throw Exception('Expected InvalidParameterException');
+    });
+  });
+}

packages/amplify_auth_cognito/example/integration_test/utils/mock_data.dart

@@ -0,0 +1,12 @@
+import 'package:uuid/uuid.dart';
+
+final uuid = Uuid();
+
+final String mockPhoneNumber = '+15555551234';
+
+String generateEmail() =>
+    'test-amplify-flutter-${uuid.v4()}@test${uuid.v4()}.com';
+
+String generatePassword() => uuid.v4();
+
+String generateUsername() => 'TEMP_USER-${uuid.v4()}';

packages/amplify_auth_cognito/example/integration_test/utils/setup_utils.dart

@@ -0,0 +1,21 @@
+import 'package:amplify_auth_cognito/amplify_auth_cognito.dart';
+import 'package:amplify_auth_cognito_example/amplifyconfiguration.dart';
+import 'package:amplify_flutter/amplify.dart';
+
+Future<void> configureAuth() async {
+  if (!Amplify.isConfigured) {
+    final authPlugin = AmplifyAuthCognito();
+    await Amplify.addPlugins([authPlugin]);
+    await Amplify.configure(amplifyconfig);
+  }
+}
+
+// ensure no user is currently signed in
+Future<void> signOutUser() async {
+  try {
+    await Amplify.Auth.signOut();
+    // ignore: unused_catch_clause
+  } on AuthException catch (e) {
+    // Ignore a signOut error because we only care when someone signed in.
+  }
+}

packages/amplify_auth_cognito/example/ios/unit_tests/amplify_auth_cognito_tests.swift

@@ -234,24 +234,59 @@ class amplify_auth_cognito_tests: XCTestCase {
         })
     }
 
+    func test_signUpSuccessNoOptions() {
+
+        class SignUpMock: AuthCognitoBridge {
+            override func onSignUp(flutterResult: @escaping FlutterResult, request: FlutterSignUpRequest){
+                let signUpRes = Result<AuthSignUpResult,AuthError>.success(
+                    AuthSignUpResult(AuthSignUpStep.confirmUser(AuthCodeDeliveryDetails(destination: DeliveryDestination.email(_email)), ["foo": "bar"])))
+                let signUpData = FlutterSignUpResult(res: signUpRes)
+                flutterResult(signUpData)
+            }
+        }
+
+        plugin = SwiftAuthCognito.init(cognito: SignUpMock())
+
+        _attributes = ["email" : _email]
+        _data = [
+            "username": _username,
+            "password": _password,
+        ]
+        _args = ["data": _data]
+        let call = FlutterMethodCall(methodName: "signUp", arguments: _args)
+        plugin.handle(call, result: {(result)->Void in
+            if let res = result as? FlutterSignUpResult {
+                XCTAssertEqual( false, res.isSignUpComplete )
+                XCTAssertEqual( "CONFIRM_SIGN_UP_STEP", res.signUpStep)
+                let codeDeliveryJson = ((res.toJSON()["nextStep"] as! [String: Any])["codeDeliveryDetails"] as! [String: String])
+                let additionalInfoJson = ((res.toJSON()["nextStep"] as! [String: Any])["additionalInfo"] as! [String: String])
+                XCTAssertEqual(_email, codeDeliveryJson["destination"]!)
+                XCTAssertEqual("bar", additionalInfoJson["foo"]!)
+
+            } else {
+                XCTFail()
+            }
+        })
+    }
+
     func test_signUpValidation() {
         let rawOptions: Dictionary<String, Any> = ["foo": "bar"]
         var rawData: NSMutableDictionary = ["options":rawOptions]
 
         // Throws with no password
         XCTAssertThrowsError(try FlutterSignUpRequest.validate(dict: rawData))
 
-        // Throws with no options
+        // Does not thow an error with valid parameters
         rawData = ["password": _password]
-        XCTAssertThrowsError(try FlutterSignUpRequest.validate(dict: rawData))
+        XCTAssertNoThrow(try FlutterSignUpRequest.validate(dict: rawData))
     }
 
     func test_signUpFormatAttributes() {
         let rawAttributes: Dictionary<String, Any> = ["email": _email, "customAttribute": "female"]
         let rawOptions: Dictionary<String, Any> = ["userAttributes": rawAttributes]
         let rawData: NSMutableDictionary = ["options":rawOptions, "username": _username, "password": _password]
         let request = FlutterSignUpRequest(dict: rawData);
-        XCTAssertEqual(2, request.userAttributes.count)
+        XCTAssertEqual(2, request.options?.userAttributes?.count)
     }
 
     func test_signUpError() {

packages/amplify_auth_cognito/ios/Classes/AuthCognitoBridge.swift

@@ -24,9 +24,8 @@ class AuthCognitoBridge {
 
 
     func onSignUp(flutterResult: @escaping FlutterResult, request: FlutterSignUpRequest) {
-        let options = AuthSignUpRequest.Options(userAttributes: request.userAttributes)
 
-        _ = Amplify.Auth.signUp(username: request.username, password:request.password, options: options) { response in
+        _ = Amplify.Auth.signUp(username: request.username, password:request.password, options: request.options) { response in
             switch response {
             case .success:
                 let signUpData = FlutterSignUpResult(res: response)