Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid grant error on first silent authorization after interactive sign in #2323

Open
sethdeckard opened this issue Sep 6, 2024 · 1 comment
Assignees
Labels

Comments

@sethdeckard
Copy link

sethdeckard commented Sep 6, 2024

I think the issue is the same one reported in #777, but I can't be sure because the steps mentioned to reproduce are not clear.

The error is the same:

Error Domain=MSALErrorDomain Code=-50002 "(null)" UserInfo={MSALErrorDescriptionKey=User interaction is required, MSALOAuthErrorKey=invalid_grant, MSALCorrelationIDKey=CAEECD18-31A1-4C0C-BD4A-33BDF2FD4E1C, NSUnderlyingError=0x300db63a0 {Error Domain=MSALErrorDomain Code=-50000 "(null)" UserInfo={MSALErrorDescriptionKey=AADB2C90128: The account associated with this grant no longer exists. Please reauthenticate and try again.
Correlation ID: 908d4992-71d8-4bda-bf03-80fd30bb0d96
Timestamp: 2024-09-06 13:45:41Z
, MSALOAuthErrorKey=invalid_grant, MSALCorrelationIDKey=CAEECD18-31A1-4C0C-BD4A-33BDF2FD4E1C, MSALInternalErrorCodeKey=-42004}}}

Steps to reproduce:

  1. Sign in with an account.
  2. Delete this account on the backend.
  3. Sign out of the account.
  4. Sign in and then sign up, creating a new account with the same email used in step 2.
  5. Call acquireTokenSilent.

Our sign out function is the same as the example app:

let thisAccount = try self.getAccountByPolicy(withAccounts: application.allAccounts(), policy: kSignupOrSigninPolicy)

if let accountToRemove = thisAccount {
    try application.remove(accountToRemove)
} else {
    self.updateLoggingText(text: "There is no account to signing out!")
}

The cache stored on the keychain gets corrupted or some kind of cleanup doesn't happen. Since keychain data survives app reinstall this happens over and over again even after reinstalling the app.

The problem eventually goes away on its own and I think that is what happen in #777, perhaps because the invalid data expires?

As a work-around, we've realized that you can remove all accounts right before sign-in to avoid this issue:

  1. Call allAccounts.
  2. Call removeAccount on each one.

Log file when issue occurs:
msal.log

@sethdeckard
Copy link
Author

sethdeckard commented Sep 10, 2024

It looks like this issue was caused by copying the behavior of the example/reference app. It's implemented in a way that allows MSAL's internal state to get out of sync with the app's state and when looking up the account for silent authorization, it uses the first one that matches the policy:

func getAccountByPolicy (withAccounts accounts: [MSALAccount], policy: String) throws -> MSALAccount? {
    
    for account in accounts {
        // This is a single account sample, so we only check the suffic part of the object id,
        // where object id is in the form of <object id>-<policy>.
        // For multi-account apps, the whole object id needs to be checked.
        if let homeAccountId = account.homeAccountId, let objectId = homeAccountId.objectId {
            if objectId.hasSuffix(policy.lowercased()) {
                return account
            }
        }
    }
    return nil
}

So if you have multiple accounts from some older account not getting cleaned up properly, you can run into this issue.

Instead the example app should persist the account identifier after sign in and use that for subsequent lookups via:

/**
 Returns account for the given account identifier (received from an account object returned in a previous acquireToken call)
 
 @param  error      The error that occured trying to get the accounts, if any, if you're
                    not interested in the specific error pass in nil.
 */
- (nullable MSALAccount *)accountForIdentifier:(nonnull NSString *)identifier
                                         error:(NSError * _Nullable __autoreleasing * _Nullable)error;

or better use currentAccount:

func currentAccount(with parameters: MSALParameters?) async throws -> (MSALAccount?, MSALAccount?)

The reference app also uses removeAccount rather than signOut which has an optional callback to keep state in sync?

Either way, I think the reference app should be updated so it doesn't act like a footgun.

@ameyapat ameyapat self-assigned this Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants