amplify-flutter icon indicating copy to clipboard operation
amplify-flutter copied to clipboard

[Request] Indicate current device on AuthDevice

Open dtodt opened this issue 1 year ago • 8 comments

Description

I'm implementing MFA SMS, and after confirm a user and try to remember the device, I get a DeviceNotTrackedException.

Console error
AmplifyAuthCognitoDart.rememberDevice (package:amplify_auth_cognito_dart/src/auth_plugin_impl.dart:938:7)
Exception message
{"message":"This device does not have an id, either it was never tracked or previously forgotten."}

Debugging the code I found out that the following code is returning null.

auth_plugin_impl.dart @ rememberDevice
Future<void> rememberDevice() async {
  final tokens = await stateMachine.getUserPoolTokens();
  final username = tokens.username;
  final deviceSecrets = await _deviceRepo.get(username); // << HERE
  final deviceKey = deviceSecrets?.deviceKey; // << HERE
  if (deviceSecrets == null || deviceKey == null) {
    throw const DeviceNotTrackedException();
  }
...

I'm not sure why, this should work normally.

[Update 1]

Context: We have as a business rule, to allow only one mobile device at a time, so on every new device, we forget the old ones.

Debugging the package, I found out that the local device secrets is created when I confirm sign in with the provided code. And when I run forgetDevice, the auth_plugin_impl.dart removes the local device secrets along with the remote ones, which causes the failure and exception at rememberDevice.

Is there any way to forget other devices only?

[Update 2]

After some search, I found out that, maybe during forgetDevice a fix could be done, so that when you are forgetting a device you doesn't accidentally forget the local device.

Possible fix
Future<void> forgetDevice([AuthDevice? device]) async {
  final tokens = await stateMachine.getUserPoolTokens();
  final username = tokens.username;
  final deviceSecrets = await _deviceRepo.get(username);
  final deviceKey = device?.id ?? deviceSecrets?.deviceKey;
  if (deviceKey == null) {
    throw const DeviceNotTrackedException();
  }
>>> current
  await _deviceRepo.remove(username);

>>> fix
  if (device == null || device.id == deviceSecrets?.deviceKey) {
    await _deviceRepo.remove(username);
  }
...

Categories

  • [X] Auth

Steps to Reproduce

  1. signIn
  2. confirmSignIn w/ code
  3. forgetDevice
  4. rememberDevice

Screenshots

AWS Console setup: aws_console_auth_config

Remember device method: remember_device_method

Amplify versions: amplify_version

[Update 2]

Forget device actual: Screenshot 2023-10-17 at 07 15 21

Forget device fixed: Screenshot 2023-10-17 at 07 23 01

Platforms

  • [X] iOS
  • [X] Android

Flutter Version

3.13.6

Amplify Flutter Version

1.4.1

dtodt avatar Oct 14 '23 00:10 dtodt

Hi @dtodt, could you provide a code sample of how you're using rememberDevice and forgetDevice so that we can get a clearer picture of how you're using it so we can reproduce it? Thank you.

khatruong2009 avatar Oct 19 '23 22:10 khatruong2009

Sure, I'll provide some snippets from my code tomorrow morning.

dtodt avatar Oct 19 '23 23:10 dtodt

@khatruong2009, I'm short in time to finish my next feature, so here's a reduced version of my datasource, check if you can evaluate with this. Otherwise I'll make a new project isolating the auth flow when I get some free time.

PS: I've replaced my custom exceptions for rethrows.

CognitoDatasource
class CognitoAuthDatasource {
  final AmplifyClass _amplify;

  const CognitoAuthDatasource(this._amplify);

  AuthCategory get _auth => _amplify.Auth;

  @override
  Future<void> forgetAllDevices() async {
    try {
      final devices = await _getCognito().fetchDevices();
      for (final device in devices) {
        await _getCognito().forgetDevice(device);
      }
    } on AmplifyException catch (error, stackTrace) {
      rethrow;
    } catch (error, stackTrace) {
      rethrow;
    }
  }

  @override
  Future<void> rememberDevice() async {
    try {
      await _getCognito().rememberDevice();
    } on AmplifyException catch (error, stackTrace) {
      rethrow;
    } catch (error, stackTrace) {
      rethrow;
    }
  }

  @override
  Future<bool> signIn(String password, String username) async {
    try {
      final result = await _getCognito().signIn(
        options: const SignInOptions(
          pluginOptions: CognitoSignInPluginOptions(
            authFlowType: AuthenticationFlowType.userSrpAuth,
          ),
        ),
        password: password,
        username: username,
      );
      //? when mfa active and device not remembered, isSignedIn is false
      return result.isSignedIn;
    } on ResourceNotFoundException {
      //! this error returns when a device is forgotten at another device
      // or at aws console.
      rethrow;
    } on AuthException catch (error, stackTrace) {
      rethrow;
    } on AmplifyException catch (error, stackTrace) {
      rethrow;
    } catch (error, stackTrace) {
      rethrow;
    }
  }

  @override
  Future<bool> confirmSignIn(String confirmation) async {
    try {
      final result = await _getCognito().confirmSignIn(
        confirmationValue: confirmation,
      );
      return result.isSignedIn;
    } on AuthNotAuthorizedException {
      rethrow;
    } on CodeMismatchException {
      rethrow;
    } on AmplifyException catch (error, stackTrace) {
      rethrow;
    } catch (error, stackTrace) {
      rethrow;
    }
  }

  @override
  Future<void> signOut(bool allDevices) async {
    try {
      await _getCognito().signOut(
        options: SignOutOptions(globalSignOut: allDevices),
      );
    } on AmplifyException catch (error, stackTrace) {
      rethrow;
    } catch (error, stackTrace) {
      rethrow;
    }
  }

  @override
  Future<void> updateMfa(bool enable) async {
    try {
      var state = MfaPreference.disabled;
      if (enable) {
        state = MfaPreference.enabled;
      }
      await _getCognito().updateMfaPreference(
        sms: state,
      );
    } on AmplifyException catch (error, stackTrace) {
      rethrow;
    } catch (error, stackTrace) {
      rethrow;
    }
  }

  AmplifyAuthCognito _getCognito() {
    return _auth.getPlugin(AmplifyAuthCognito.pluginKey);
  }
}

Thanks in advance.

dtodt avatar Oct 21 '23 12:10 dtodt

@khatruong2009, It seems that maybe my snippet was not enough, so I made a sample that reproduces the issue at the main branch and tries to solve it in the feat/check_issue_alternative branch.

Repository: https://github.com/dtodt/mfa_amplify

In this video the app logs in fine, but, the next time it tries to log in the code is asked, because the remember device failed (due to previous forget the local device). https://github.com/aws-amplify/amplify-flutter/assets/2112840/53cfac93-0e0d-4b76-886f-d1285da4d8cd

Console output:

signIn
confirmCode
rememberDevice
forgeting devices
forget succeed: true
remembering device
remember succeed: false

dtodt avatar Nov 01 '23 14:11 dtodt

Hello @dtodt - It sounds like you are requesting that the AuthDevice have a new boolean member added to indicate the current device. Please let me know if this is not the case.

This sounds like a reasonable request. This is something we will need to discuss internally to see if this is something we want to add on all platforms that we support. I will let you know when we have an update.

We will track this as a feature request. I am going to update the issue title / description to reflect that.

Jordan-Nelson avatar Nov 02 '23 18:11 Jordan-Nelson

@Jordan-Nelson it's part of it.

But the current code removes the local reference when you run forget device.

The reference that is created after confirm sign in succeed.

So I made some changes in the code, where you just remove the local reference if:

  • no device is sent in;
  • the local key is the same as the device sent in;

The code changes are in a PR referenced to this issue.

The boolean at the device is not my main goal, but I figured that it could help to iterate over all registered devices.

🤔 One thing I could not manage to solve yet, that is, when a device remembered locally is forgotten at aws portal, I could not sign-in at this device unless I re-installed the app.

dtodt avatar Nov 02 '23 19:11 dtodt

But the current code removes the local reference when you run forget device.

Ah I see. That does appear to be a bug. I have opened https://github.com/aws-amplify/amplify-flutter/issues/4056 to track that.

I think we should treat this as two separate issues (the bug linked above, and the feature request). If we would like to open a PR for just the bug fix I think we could run our test suite against that and try to get that included relatively quickly. The feature request will require a bit more discussion as there could be alternate options to consider.

Jordan-Nelson avatar Nov 02 '23 19:11 Jordan-Nelson

@Jordan-Nelson sure, here they are:

https://github.com/aws-amplify/amplify-flutter/pull/4060

https://github.com/aws-amplify/amplify-flutter/pull/4061

dtodt avatar Nov 03 '23 11:11 dtodt