[Request] Indicate current device on AuthDevice

[dtodt]

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

• Auth

Steps to Reproduce

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

Screenshots

AWS Console setup:

Remember device method:

Amplify versions:

[Update 2]

Forget device actual:

Forget device fixed:

Platforms

• iOS
• Android

Flutter Version

3.13.6

Amplify Flutter Version

1.4.1

[khatruong2009]

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.

[dtodt]

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

[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]

@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

[Jordan-Nelson]

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.

[dtodt]

@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.

[Jordan-Nelson]

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 #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.

[dtodt]

@Jordan-Nelson sure, here they are:

#4060

#4061