Users randomly getting ExpiredTokenExceptions and 403s on API calls

invzdev commented 4 years ago

Describe the bug We have a React Native app that uses Amplify via manual configuration. Auth is done via a Cognito User and Identity Pool and our APIs are currently just Lambda functions that the app users are permissioned to be able to call without any API Gateway in between.

We're sporadically seeing a lot of 403 ExpiredTokenExceptions from our users whenever their app tries to call any of our APIs. It seems like things are working fine, then requests will all just start failing. Users are able to fix the issue by force closing and reopening the app. That means that credential refreshing is still something that works in theory, but seems to randomly stop working without a forced restart. This is happening across both iOS and Android, but seems to happen more often in Android.

To Reproduce We don't have a reliable means of reproduction. It seems to just happen randomly after the app has been in the background for a while and lots of other apps have been used. It doesn't take a long period of inactivity either (I've seen it after around an hour). It doesn't happen every time though and really seems to be random.

Expected behavior I would expect Amplify to refresh user credentials without needing the app to be force-closed and restarted.

Code Snippet

import { API, Auth } from 'aws-amplify'

/**
 * Fetches the JWT token of the current session. If the session is no longer 
 * valid, it dispatches and action to treat the user as if they were logged out.
 */
const getJWT = async (): Promise<string> => {
  try {
    const session = await Auth.currentSession()
    if (!session.isValid()) {
      throw new Error('Session is no longer valid')
    }
    return session.getIdToken().getJwtToken()
  } catch (err) {
    bugsnag.leaveBreadcrumb(
      'Expired session detected and redirecting to signin'
    )
    store.dispatch(logout())
    defer(() => Alert.alert('Your session has expired. Please log back in.'))
    throw err
  }
}

const invokeLambda = async (fnName: string, reqBody: object) => {
  try {  
    const idToken = await getJWT()
    const response = await API.post(fnName, '', { ...reqBody, idToken })
    if (response.errorType) {
      const err = new Error(`${fnName}: ${response.errorMessage}`)
      bugsnag.notify(err, report => {
        report.addMetadata('LambdaAPI', 'apiName', fnName)
        report.addMetadata('LambdaAPI', 'errorType', response.errorType)
      })
      throw err
    }
    return response
  } catch (err) {
    bugsnag.notify(err, report => {
      report.addMetadata('LambdaAPI', 'apiName', fnName)
      report.addMetadata('LambdaAPI', 'errorType', 'ControlPlaneError')
    })
    throw err
  }
}

That invokeLambda function is failing at the call to API.post with an HTTP 403 response of ExpiredTokenException: The security token included in the request is expired.

Screenshots If applicable, add screenshots to help explain your problem.

N/A

What is Configured? If applicable, please provide what is configured for Amplify CLI: N/A. We use a manual Amplify configuration.

If applicable, please provide your manual configuration example:

Amplify.configure({
Auth: {
  identityPoolId: env.AWS_IDENTITY_POOL_ID,
  region: env.AWS_REGION,
  userPoolId: env.AWS_USER_POOL_ID,
  userPoolWebClientId: env.AWS_CLIENT_ID,
  mandatorySignIn: false
},
API: {
  endpoints: [
    {
      name: 'OurLambdaFnName',
      endpoint: 'https://lambda.us-east-2.amazonaws.com/2015-03-31/functions/OurLambdaFnName/invocations ',
      service: 'lambda',
      region: 'us-east-2'
    },
    // And other endpoints like this...
  ]
},
Storage: {
  AWSS3: {
    bucket: env.AWS_S3_BUCKET
  }
},
storage: RNKeychainAmplifyStorage
})

In case it's relevant, this is our implementation of our custom AmplifyStorage adapter using react-native-keychain:

/* eslint-disable @typescript-eslint/no-floating-promises */ // We do this because Amplify forces us to call async APIs in synchronous functions
import * as Keychain from 'react-native-keychain'
import { fromPairs, isString } from 'lodash'

/**
 * Key prefix for values stored by Amplify.
 */
const memoryKeyPrefix = '@MyAppAmplifyStorage'

/**
 * A key to metadata information about the keys that Amplify stores.
 */
const metaKey = '@MyAppMetaKey'

/**
 * An in-memory reference to all key names stored by Amplify.
 */
const rnKeychainItems = new Set<string>()

/**
 * An in-memory map of Amplify keys to their values.
 */
let dataMemory: Record<string, unknown> = {}

/**
 * Helper function to update the persistent list of item keys used by this
 * Storage implementation.
 */
const updateStorageKeys = async () => {
  return Keychain.setInternetCredentials(
    metaKey,
    metaKey,
    JSON.stringify([...rnKeychainItems])
  )
}

/**
 * An Amplify compatible Storage layer that uses react-native-keychain for
 * persistence. Amplify seems to force synchronous APIs on us and
 * react-native-keychain doesn't support list operations or storage of types
 * other than strings. The combination of these limitaitons forces us to do some
 * strange things as workarounds.
 *
 * We use this custom storage layer for the following reasons:
 *   - Until a somewhat recent update, the stock Storage adapter was broken on 
 *     the latest versions of React Native.
 *   - The stock Storage adapter is *not* encrypted at rest. This is a security
 *     concern for us.
 */
export class RNKeychainAmplifyStorage {
  static syncPromise: Promise<unknown> | null = null

  static getItem(key: string) {
    return Object.prototype.hasOwnProperty.call(dataMemory, key)
      ? dataMemory[key]
      : undefined
  }

  static setItem(key: string, value: unknown) {
    const typeInfo = isString(value) ? 'string' : 'json'
    const serializedValue =
      typeInfo === 'string' ? (value as string) : JSON.stringify(value)
    rnKeychainItems.add(key)
    dataMemory[key] = value

    Keychain.setInternetCredentials(
      memoryKeyPrefix + key,
      typeInfo,
      serializedValue
    )
    updateStorageKeys()

    return dataMemory[key]
  }

  static removeItem(key: string) {
    if (rnKeychainItems.has(key)) {
      rnKeychainItems.delete(key)
      updateStorageKeys()
    }
    Keychain.resetInternetCredentials(memoryKeyPrefix + key)
    return delete dataMemory[key]
  }

  static clear() {
    dataMemory = {}
    rnKeychainItems.forEach(key => {
      Keychain.resetInternetCredentials(memoryKeyPrefix + key)
    })
    rnKeychainItems.clear()
    updateStorageKeys()
    return dataMemory
  }

  static async sync() {
    if (!RNKeychainAmplifyStorage.syncPromise) {
      const keyItems = await Keychain.getInternetCredentials(metaKey)
      if (keyItems) {
        const items: string[] = JSON.parse(keyItems.password)
        items.forEach(_ => rnKeychainItems.add(_))
      }
      const items = await Promise.all(
        [...rnKeychainItems].map(async key => {
          const data = await Keychain.getInternetCredentials(
            memoryKeyPrefix + key
          )
          if (!data) {
            return []
          }
          const typeInfo = data.username
          const serializedValue = data.password
          const value =
            typeInfo === 'string'
              ? serializedValue
              : JSON.parse(serializedValue)
          return [key, value]
        })
      )
      dataMemory = fromPairs(items.filter(_ => _.length > 0))
      RNKeychainAmplifyStorage.syncPromise = Promise.resolve(dataMemory)
    }
  }
}

* If applicable, provide more configuration data, for example for Amazon Cognito, run `aws cognito-idp describe-user-pool --user-pool-id us-west-2_xxxxxx` (Be sure to remove any sensitive data)

``` { "UserPool": { "Id": "us-east-2_xxxxxx", "Name": "MyAppUsers", "Policies": { "PasswordPolicy": { "MinimumLength": 8, "RequireUppercase": true, "RequireLowercase": true, "RequireNumbers": true, "RequireSymbols": false, "TemporaryPasswordValidityDays": 14 } }, "LambdaConfig": { "PostConfirmation": "arn:aws:lambda:us-east-2::function:AuthTriggers-PostConfirmation" }, "LastModifiedDate": "2020-06-12T16:07:53.951000-07:00", "CreationDate": "2018-04-03T09:08:08.208000-07:00", "SchemaAttributes": [ { "Name": "sub", "AttributeDataType": "String", "DeveloperOnlyAttribute": false, "Mutable": false, "Required": true, "StringAttributeConstraints": { "MinLength": "1", "MaxLength": "2048" } }, { "Name": "name", "AttributeDataType": "String", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": true, "StringAttributeConstraints": { "MinLength": "0", "MaxLength": "2048" } }, { "Name": "given_name", "AttributeDataType": "String", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": false, "StringAttributeConstraints": { "MinLength": "0", "MaxLength": "2048" } }, { "Name": "family_name", "AttributeDataType": "String", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": false, "StringAttributeConstraints": { "MinLength": "0", "MaxLength": "2048" } }, { "Name": "middle_name", "AttributeDataType": "String", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": false, "StringAttributeConstraints": { "MinLength": "0", "MaxLength": "2048" } }, { "Name": "nickname", "AttributeDataType": "String", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": false, "StringAttributeConstraints": { "MinLength": "0", "MaxLength": "2048" } }, { "Name": "preferred_username", "AttributeDataType": "String", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": false, "StringAttributeConstraints": { "MinLength": "0", "MaxLength": "2048" } }, { "Name": "profile", "AttributeDataType": "String", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": false, "StringAttributeConstraints": { "MinLength": "0", "MaxLength": "2048" } }, { "Name": "picture", "AttributeDataType": "String", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": false, "StringAttributeConstraints": { "MinLength": "0", "MaxLength": "2048" } }, { "Name": "website", "AttributeDataType": "String", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": false, "StringAttributeConstraints": { "MinLength": "0", "MaxLength": "2048" } }, { "Name": "email", "AttributeDataType": "String", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": true, "StringAttributeConstraints": { "MinLength": "0", "MaxLength": "2048" } }, { "Name": "email_verified", "AttributeDataType": "Boolean", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": false }, { "Name": "gender", "AttributeDataType": "String", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": false, "StringAttributeConstraints": { "MinLength": "0", "MaxLength": "2048" } }, { "Name": "birthdate", "AttributeDataType": "String", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": false, "StringAttributeConstraints": { "MinLength": "10", "MaxLength": "10" } }, { "Name": "zoneinfo", "AttributeDataType": "String", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": false, "StringAttributeConstraints": { "MinLength": "0", "MaxLength": "2048" } }, { "Name": "locale", "AttributeDataType": "String", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": false, "StringAttributeConstraints": { "MinLength": "0", "MaxLength": "2048" } }, { "Name": "phone_number", "AttributeDataType": "String", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": false, "StringAttributeConstraints": { "MinLength": "0", "MaxLength": "2048" } }, { "Name": "phone_number_verified", "AttributeDataType": "Boolean", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": false }, { "Name": "address", "AttributeDataType": "String", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": false, "StringAttributeConstraints": { "MinLength": "0", "MaxLength": "2048" } }, { "Name": "updated_at", "AttributeDataType": "Number", "DeveloperOnlyAttribute": false, "Mutable": true, "Required": false, "NumberAttributeConstraints": { "MinValue": "0" } } ], "AutoVerifiedAttributes": ["email"], "AliasAttributes": ["email"], "SmsVerificationMessage": "Your verification code is {####}. ", "EmailVerificationMessage": "Your verification code is {####}. ", "EmailVerificationSubject": "Your verification code", "VerificationMessageTemplate": { "SmsMessage": "Your verification code is {####}. ", "EmailMessage": "Your verification code is {####}. ", "EmailSubject": "Your verification code", "EmailMessageByLink": "Please click the link below to verify your email address and complete your registration. \n{##Verify Email##} ", "EmailSubjectByLink": "Welcome to myapp", "DefaultEmailOption": "CONFIRM_WITH_LINK" }, "SmsAuthenticationMessage": "Your authentication code is {####}. ", "MfaConfiguration": "OPTIONAL", "DeviceConfiguration": { "ChallengeRequiredOnNewDevice": false, "DeviceOnlyRememberedOnUserPrompt": false }, "EstimatedNumberOfUsers": "NumberRedacted", "EmailConfiguration": { "SourceArn": "arn:aws:ses:us-east-1::identity/info@myapp.domain", "ReplyToEmailAddress": "info@myapp.domain", "EmailSendingAccount": "DEVELOPER", "From": "myapp ", "ConfigurationSet": "CognitoEmailConfigSet" }, "SmsConfiguration": { "SnsCallerArn": "arn:aws:iam:::role/service-role/MyAppUsers-Cognito-SMS-Role", "ExternalId": "fd015998-8154-4ae7-9753-eae6892eca07" }, "UserPoolTags": { "Environment": "Prod", "Purpose": "Auth" }, "Domain": "myappdomain", "AdminCreateUserConfig": { "AllowAdminCreateUserOnly": false, "UnusedAccountValidityDays": 14, "InviteMessageTemplate": { "SMSMessage": "Your username is {username} and temporary password is {####}. ", "EmailMessage": "Your username is {username} and temporary password is {####}. ", "EmailSubject": "You've been invited to myapp!" } }, "Arn": "arn:aws:cognito-idp:us-east-2::userpool/us-east-2_xxxxxx", "AccountRecoverySetting": { "RecoveryMechanisms": [ { "Priority": 1, "Name": "verified_email" } ] } } } ```

Environment

``` npx envinfo --system --binaries --browsers --npmPackages --npmGlobalPackages System: OS: macOS 10.15.4 CPU: (8) x64 Intel(R) Core(TM) i7-7820HQ CPU @ 2.90GHz Memory: 220.00 MB / 16.00 GB Shell: 4.4.12 - /usr/local/bin/bash Binaries: Node: 12.2.0 - /usr/local/bin/node npm: 6.9.0 - /usr/local/bin/npm Watchman: 4.9.0 - /usr/local/bin/watchman Browsers: Chrome: 83.0.4103.116 Firefox: 77.0.1 Safari: 13.1 npmPackages: @babel/core: ^7.6.2 => 7.9.0 @babel/plugin-proposal-optional-chaining: ^7.6.0 => 7.9.0 @babel/runtime: ^7.6.2 => 7.9.2 @react-native-community/async-storage: ^1.6.2 => 1.9.0 @react-native-community/cli: ^4.10.1 => 4.10.1 @react-native-community/eslint-config: ^0.0.3 => 0.0.3 @react-native-community/netinfo: ^5.0.0 => 5.7.1 @types/jest: ^24.0.18 => 24.9.1 @types/lodash: ^4.14.147 => 4.14.150 @types/numeral: 0.0.26 => 0.0.26 @types/react: ^16.9.3 => 16.9.34 @types/react-native: ^0.60.15 => 0.60.31 @types/react-native-svg-charts: ^5.0.3 => 5.0.3 @types/react-redux: ^7.1.4 => 7.1.7 @types/react-test-renderer: 16.8.1 => 16.8.1 @types/redux-logger: ^3.0.7 => 3.0.7 @types/yup: ^0.26.24 => 0.26.37 @typescript-eslint/eslint-plugin: ^2.6.1 => 2.29.0 @typescript-eslint/parser: ^2.6.1 => 2.29.0 acorn: ^7.1.0 => 7.1.1 amazon-cognito-identity-js: ^3.2.0 => 3.3.3 aws-amplify: ^2.1.1 => 2.3.0 aws-amplify-react-native: ^2.2.3 => 2.3.3 babel-jest: ^24.9.0 => 24.9.0 babel-plugin-module-resolver: ^3.2.0 => 3.2.0 bugsnag-react-native: ^2.23.7 => 2.23.7 date-fns: ^2.9.0 => 2.12.0 date-fns-tz: ^1.0.10 => 1.0.10 eslint: ^6.5.1 => 6.8.0 eslint-config-prettier: ^6.5.0 => 6.10.1 eslint-plugin-import: ^2.18.2 => 2.20.2 eslint-plugin-prettier: 2.6.2 => 2.6.2 eslint-plugin-react-hooks: ^2.2.0 => 2.5.1 formik: ^2.0.6 => 2.1.4 fuse.js: ^5.2.3 => 5.2.3 iex-api: 0.0.3 => 0.0.3 jest: ^24.9.0 => 24.9.0 lodash: ^4.17.15 => 4.17.15 metro-react-native-babel-preset: ^0.58.0 => 0.58.0 numeral: ^2.0.6 => 2.0.6 polished: ^3.4.1 => 3.5.2 prettier: ^1.19.0 => 1.19.1 react: 16.11.0 => 16.11.0 react-devtools: ^4.7.0 => 4.7.0 react-native: 0.62.2 => 0.62.2 react-native-elements: ^1.2.6 => 1.2.7 react-native-gesture-handler: ^1.4.1 => 1.6.1 react-native-keychain: ^4.0.5 => 4.0.5 react-native-linear-gradient: ^2.5.6 => 2.5.6 react-native-purchases: ^3.2.0 => 3.2.0 react-native-reanimated: ^1.3.0 => 1.8.0 react-native-screens: ^1.0.0-alpha.23 => 1.0.0-alpha.23 react-native-scroll-into-view: ^1.0.3 => 1.0.3 react-native-splash-screen: ^3.2.0 => 3.2.0 react-native-svg: ^9.12.0 => 9.13.6 react-native-svg-charts: ^5.3.0 => 5.4.0 react-native-vector-icons: ^6.6.0 => 6.6.0 react-native-webview: ^7.4.3 => 7.6.0 react-navigation: ^4.0.10 => 4.3.7 react-navigation-stack: ^1.9.3 => 1.10.3 react-navigation-tabs: ^2.5.6 => 2.8.11 react-redux: ^7.1.1 => 7.2.0 react-test-renderer: 16.11.0 => 16.11.0 redux: ^4.0.4 => 4.0.5 redux-devtools-extension: ^2.13.8 => 2.13.8 redux-logger: ^3.0.6 => 3.0.6 redux-saga: ^1.1.1 => 1.1.3 typesafe-actions: ^4.4.2 => 4.4.2 typescript: ^3.7.5 => 3.8.3 victory-native: ^33.0.0 => 33.0.1 yup: ^0.27.0 => 0.27.0 npmGlobalPackages: @aws-amplify/cli: 4.20.0 @react-native-community/cli: 2.10.0 bugsnag-sourcemaps: 1.3.0 commander: 2.19.0 create-react-app: 3.4.1 create-react-native-app: 1.0.0 eslint: 4.10.0 exp: 57.2.1 expo-cli: 3.0.9 floki: 0.6.2 iex-api: 0.0.3 jest-cli: 21.2.1 json-2-csv: 3.4.2 mocha: 4.0.1 n: 2.1.12 ndjson-to-json: 1.0.0 npm: 6.9.0 prettier: 1.15.1 ts-jest: 24.0.2 ts-node: 8.10.1 tsc: 1.20150623.0 tslint: 5.11.0 typescript: 3.9.2 yamljs: 0.3.0 ```

Smartphone (please complete the following information):

Device: We've seen this issue across well over 100 distinct devices
OS: We've seen this across iOS12.1.4 through to iOS 13.5.1 and Android 6 through to Android 10.

Additional context This may not be particularly helpful since it's really just showing the axios flow, but this is the client-side stacktrace we get recorded from the failed API call:

Error Request failed with status code 403 
    node_modules/axios/lib/core/createError.js:16:18 anonymous
    node_modules/axios/lib/core/settle.js:17:11 anonymous
    node_modules/axios/lib/adapters/xhr.js:61:6 anonymous
    native call
    node_modules/event-target-shim/dist/event-target-shim.js:818:34 dispatchEvent
    node_modules/react-native/Libraries/Network/XMLHttpRequest.js:567:9 value
    node_modules/react-native/Libraries/Network/XMLHttpRequest.js:389:11 value
    native apply
    node_modules/react-native/Libraries/Network/XMLHttpRequest.js:502:8 anonymous
    native apply
    node_modules/react-native/Libraries/vendor/emitter/EventEmitter.js:189:32 value
    native apply
    node_modules/react-native/Libraries/BatchedBridge/MessageQueue.js:425:41 value
    node_modules/react-native/Libraries/BatchedBridge/MessageQueue.js:112:11 anonymous
    node_modules/react-native/Libraries/BatchedBridge/MessageQueue.js:373:8 value
    node_modules/react-native/Libraries/BatchedBridge/MessageQueue.js:111:9 value

elorzafe commented 4 years ago

@invzdev how you are checking the token on the lambda function?

invzdev commented 4 years ago

@elorzafe The 403 is coming from the Lambda control plane itself before it ever reaches my function. No lambda function invocation happens at all when we get the ExpiredTokenException. Not all of our Lambda functions do anything with the JWT token either. It's the IAM session credentials themselves that seem to be expired and not swapped out with the refreshed ones.

But to answer your question, our JWT token validation looks like this:

import jwt from 'jsonwebtoken'

const keyIdToPemMapping = {
 // Redacted
}

export interface CognitoIdToken {
  sub: string
  aud: string
  email_verified: boolean
  token_use: string
  auth_time: number
  iss: string
  'cognito:username': string
  exp: number
  iat: number
  email: string
}

export const verifyAndDecodeJWT = async (
  token: string
): Promise<CognitoIdToken> => {
  const jwk = JSON.parse(
    Buffer.from(token.split('.')[0], 'base64').toString('utf8')
  )
  const pem = keyIdToPemMapping[jwk.kid as keyof typeof keyIdToPemMapping]

  return new Promise((resolve, reject) => {
    jwt.verify(token, pem, (err, res) => {
      err ? reject(err) : resolve(res as CognitoIdToken)
    })
  })
}

invzdev commented 4 years ago

Is this related to these other two issues maybe?

https://github.com/aws-amplify/amplify-js/issues/6123 https://github.com/aws-amplify/amplify-js/issues/4850

invzdev commented 4 years ago

We are effectively calling and awaiting the result of Auth.currentSession() before every API call with this approach. I would expect that to be forcing a credential refresh if they're expired, but it seems like the API client under the hood isn't using updated creds.

invzdev commented 4 years ago

As a stopgap measure, how can we forcibly make the client used by the Amplify API.post construct use refreshed credentials?

invzdev commented 4 years ago

After further digging, it seems this was because the storage adapter expects the syncPromise value to be returned in the sync method, not just set. This wasn't explicitly stated anywhere, but the documentation example on the Amplify docs website at least added TypeScript types now, so that cleared some things up. Among other things, it seems the values being passed to setItem will always be strings, which wasn't an assumption I thought was safe to make earlier.

Changing my sync method implementation to the following fixed it:

  static sync() {
    if (!RNKeychainAmplifyStorage.syncPromise) {
      RNKeychainAmplifyStorage.syncPromise = (async () => {
        const keyItems = await Keychain.getInternetCredentials(metaKey)
        if (keyItems) {
          const items: string[] = JSON.parse(keyItems.password)
          items.forEach(_ => rnKeychainItems.add(_))
        }
        const items = await Promise.all(
          [...rnKeychainItems].map(async key => {
            const data = await Keychain.getInternetCredentials(
              memoryKeyPrefix + key
            )
            if (!data) {
              return []
            }
            const typeInfo = data.username
            const serializedValue = data.password
            const value =
              typeInfo === 'string'
                ? serializedValue
                : JSON.parse(serializedValue)
            return [key, value]
          })
        )
        dataMemory = fromPairs(items.filter(_ => _.length > 0))
        return dataMemory
      })()
    }
    return RNKeychainAmplifyStorage.syncPromise
  }

If anyone is stumbling on this github issue because they were looking for an implementation of a storage adapter that's encrypted at rest, here's some other useful info to note:

The implementation I have in this thread (after using the updated sync implementation) works perfectly fine for iOS. In some cases, it will have issues on Android. This is because react-native-keychain uses the iOS keychain for storage on iOS but RSA encryption on Android's keystore by default. You might run into IllegalBlockSizeException's in the Java layer because RSA is limited to 245 bytes and the values being stored may exceed that. You can fix that by explicitly setting the storage option on react-native-keychain to use AES encryption.

invzdev commented 4 years ago

Reopening because it turns out that didn't actually fix things. I even tried completely removing our custom storage adapter and we still see random 403s come up from some users. Over the last week, we've observed nearly 100 users across Android and iOS on our latest release run into this issue.

Can you please provide some advice on what we can do to force refresh the tokens used by the underlying clients on API.post?

invzdev commented 4 years ago

An agent from AWS Premium Support suggested that I could try forcing a credential refresh by following this process:

Get the current CognitoUser [1] from ‘Auth.currentAuthenticatedUser()’;
Get its current session with ‘cognitoUser.signInUserSession’;
Refresh its session with ‘cognitoUser.refreshSession()’ with the current session’s refresh token [2];
Run the API call.

I added a refreshSession implementation based on the above that looks like this:

import { throttle } from 'lodash'

const refreshSession = throttle(async () => {
    const user: CognitoUser = await Auth.currentAuthenticatedUser()
    const refreshToken = user.getSignInUserSession()?.getRefreshToken()
    if (!refreshToken) {
      throw new Error('Could not retrieve refresh token')
    }
    return new Promise((resolve, reject) => {
      user.refreshSession(refreshToken, (refreshErr, refreshSuccess) => {
        refreshErr ? reject(refreshErr) : resolve(refreshSuccess)
      })
    })
}, 60 * 1000, { leading: true, trailing: false })

When I saw a 403 error again, I did see this successfully make a call to cognito-idp.us-east-2.amazonaws.com with the refresh token and get new tokens back as a response in my network activity inspector. However, the retried API call still failed with a 403 even after the call to refreshSession completed successfully.

This is really frustrating, because it doesn't happen every time. Sometimes, the client credentials do refresh just fine. One thing I've noticed is that in those successful cases, there a lot of other API calls being made as well (typically four of them).

POST to https://cognito-idp.us-east-2.amazonaws.com/ with x-amz-target header set to AWSCognitoIdentityProviderService.InitiateAuth
POST to https://cognito-idp.us-east-2.amazonaws.com/ with x-amz-target header set to AWSCognitoIdentityProviderService.GetUser
POST to https://cognito-identity.us-east-2.amazonaws.com/ with x-amz-target set to AWSCognitoIdentityService.GetId
POST to https://cognito-identity.us-east-2.amazonaws.com/ with x-amz-target set to AWSCognitoIdentityService.GetCredentialsForIdentity

That last one seems to be the one that actually get updated AWS credentials back while the guidance from support only had that first call made.

At this point, I'm considering getting off Amplify all together.

invzdev commented 4 years ago

How can I force Amplify's Auth object to make an API call to AWSCognitoIdentityService.GetCredentialsForIdentity and update it's internal values? That seems like it'd address the problem.

invzdev commented 4 years ago

As for getting to a root cause here:

In cases when Amplify refreshes tokens on its own successfully, it does this without a 403 ever happening. I'm guessing it checks if the accessKey/secretKey/sessionToken credentials expiration time is before the current time somwhere? The issue here seems to be that's not triggering for some reason.

Out of desperation, I'm adding the following lines at the end of my refreshSession after the call to user.refreshSession succeeds:

    await Auth.currentUserPoolUser()
    await Auth.currentUserInfo()
    await Auth.currentUserCredentials()

After doing that, I noticed that Amplify is making an outbound call for AWSCognitoIdentityProviderService.GetUser and AWSCognitoIdentityService.GetCredentialsForIdentity. That may be enough to fix this, but I'm still waiting for a reproduction opportunity to verify this. One concern I have is that no call to AWSCognitoIdentityService.GetId was made. I haven't had a chance to dive deep enough through the Amplify and Cognito source yet to confirm/deny whether that'd be a problem.

amhinson commented 4 years ago

@invzdev It looks like you're using an older version of Amplify aws-amplify: ^2.1.1 => 2.3.0. Could you try with the latest version aws-amplify@latest? There have been some improvements made related to the issue you are seeing. Let me know if you're still seeing the errors after upgrading.

invzdev commented 4 years ago

It looks like 2.3.0 is the latest on the 2.x release line. I'm very reluctant to move onto the 3.x releases for two reasons:

There are backwards incompatible changes that currently have no migration plan defined. We use credentials to create AWS SDK client instances and the README currently says Migration plan on “How to migrate to using Amplify provided credentials” will follow in the coming weeks after GA launch..
The shift to AWS JS SDK v3 is concerning, since they currently have this to say regarding their production readiness: This project is in gamma. We want feedback from you, and may make breaking changes in future releases while the SDK is still in gamma.

I can appreciate that these changes are probably a net positive and that the Amplify team is moving things in the right direction, but I'm not comfortable shipping things that are explicitly marked as not production ready unless there's evidence suggesting that it'd actually improve the situation.

Unfortunately, I'm still waiting for a reproduction opportunity to be able to confirm/deny whether my credential refresh approach will work. Is there any documentation or commit you could point to that could help explain why the 3.x branch would help mitigate this issue?

sammartinez commented 4 years ago

It looks like 2.3.0 is the latest on the 2.x release line. I'm very reluctant to move onto the 3.x releases for two reasons:

There are backwards incompatible changes that currently have no migration plan defined. We use credentials to create AWS SDK client instances and the README currently says Migration plan on “How to migrate to using Amplify provided credentials” will follow in the coming weeks after GA launch..

The shift to AWS JS SDK v3 is concerning, since they currently have this to say regarding their production readiness: This project is in gamma. We want feedback from you, and may make breaking changes in future releases while the SDK is still in gamma.

I can appreciate that these changes are probably a net positive and that the Amplify team is moving things in the right direction, but I'm not comfortable shipping things that are explicitly marked as not production ready unless there's evidence suggesting that it'd actually improve the situation.

Unfortunately, I'm still waiting for a reproduction opportunity to be able to confirm/deny whether my credential refresh approach will work. Is there any documentation or commit you could point to that could help explain why the 3.x branch would help mitigate this issue?

@invzdev Thanks for the feedback, I agree that we do need to provide a migration plan I will double check on where its at and get it out to everyone as soon as its ready. As for your second callout, we, AWS Amplify, do have an agreement that the use cases the AWS SDK JavaScript team has provided to us are production ready for version 3 of AWS Amplify. As for the reproduction, this may be related to some of the Clock Drift work we solved recently. Please let us know if there is anything else you will need.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 4 years ago

This issue has been automatically closed because of inactivity. Please open a new issue if are still encountering problems.

joebernard commented 3 years ago

@invzdev Were you ever able to find a satisfactory resolution to this? I'm seeing the same intermittent 403 error when uploading to S3 with Storage.put on "aws-amplify": "3.3.27". Works 99% of the time but I'm still seeing many 403's.

hisham commented 3 years ago

@joebernard - for Storage.put, 403 errors can be caused by the client's clock being wrong, see https://github.com/aws-amplify/amplify-js/issues/6494#issuecomment-799739560

github-actions[bot] commented 2 years ago

This issue has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.

aws-amplify / amplify-js

Users randomly getting ExpiredTokenExceptions and 403s on API calls #6181