AppAuth-iOS icon indicating copy to clipboard operation
AppAuth-iOS copied to clipboard

How to handle concurrent performAction calls?

Open LeoSnek opened this issue 1 year ago • 17 comments

We have a project which can fire off multiple API calls concurrently. Each request calls performAction and in some cases this results in the same refresh token being used more than once and sometimes this is rejected by the server.

Is there a standard way to handle this issue or could the library be updated to handle this internally perhaps?

I have tried using semaphores and DispatchQueue but this doesn't work properly as the calls are async.

LeoSnek avatar Jul 26 '22 14:07 LeoSnek

If you're referring to the access token returned in the completion for that method, it is intended that this token be reused unless it is expired. When one of your requests is rejected by the server, what is the error?

petea avatar Jul 26 '22 15:07 petea

That the refresh token has already been consumed.

LeoSnek avatar Jul 26 '22 15:07 LeoSnek

You should be using an access token to authorize an API request, not the refresh token. The same access token will be returned by performAction until it has expired, at which point the refresh token will be used to request a new access token which will then be returned.

Can you provide more detail about the identity provider you're using and which API you're making requests to?

petea avatar Jul 26 '22 20:07 petea

Would you mind re-reading my original post please because it doesn’t seem like you have understood the issue.

LeoSnek avatar Jul 26 '22 21:07 LeoSnek

You're correct, I don't fully understand your initial explanation of the problem, and I'm asking questions to try and clarify.

petea avatar Jul 27 '22 22:07 petea

My issue isn’t really to do with access tokens, just refresh tokens. When we’re performing multiple API calls each calls ‘performAction’ to get the access token. However if the access token has expired each request will try and refresh the token with the same refresh token, that’s where the problem lies.

We can cache our requests to queue them but it seems like there could be a much smarter way that when a refresh call is in the pipeline the callbacks from other calls to ‘performAction’ could be queued internally.

However, I’m more asking if there’s another viable solution which doesn’t rely on holding an array of closures.

DispatchQueue sync doesn’t work because the request is async. Semaphores don’t seem to work I think because AppAuth gets held up on the main thread.

LeoSnek avatar Jul 28 '22 08:07 LeoSnek

I see, so you're occasionally encountering an error during the access token refresh request when calling performAction repeatedly. It's possible that, with your OAuth provider, the refresh token itself is being refreshed along with the access token and that in some scenarios a previously queued request isn't using the new refresh token when it should be.

Can you tell me what OAuth provider you're using?

petea avatar Jul 28 '22 15:07 petea

I’m sorry but you still are not following what I am saying. I don’t know how to explain it any better. Not sure on the OAuth provider, I would have to take a look, but that side of things is working as intended, the refresh token is being rejected once it has been consumed.

LeoSnek avatar Jul 28 '22 15:07 LeoSnek

Your OAuth provider would be the one occasionally rejecting the refresh token, correct?

petea avatar Jul 28 '22 17:07 petea

Of course yep. Why?

LeoSnek avatar Jul 29 '22 14:07 LeoSnek

Can you tell me who that OAuth provider is?

petea avatar Aug 01 '22 19:08 petea

I’m not at work this week so unable to look. What difference does it make though? The issue would happen with any provider.

LeoSnek avatar Aug 01 '22 19:08 LeoSnek

Hello,

I don't know which kind of error you have regarding that, but in our case, sometimes we have :

"Session doesn't have required client".

I think this is because we're using the "performAction" process in same times in some conditions, and it's failing to refresh / use the correct AuthState object. I don't really know what is hapenning, but here it's not related to the refresh token which is available for 1 year. So I guess when the access token is expired and we have two calls at the exact same time, there is a concurrency issue.

AntonyARHS avatar Sep 02 '22 05:09 AntonyARHS

Actually on our side we have this concurrency issue when we configure the "Revoke Refresh Token" + "Refresh Token Max Reuse" to 0. So the refresh is supposed to be renew at each call along the access token. But in case of concurrency calls... it seems the lib doesn't handle it.

AntonyARHS avatar Sep 07 '22 09:09 AntonyARHS

We are experiencing the same issue as well; Occasionally, two or more concurrent requests are being performed and they will try to refresh the access token because it was expired. One of them succeeds, and the rest will get an exception that the refreshToken is no longer valid.

The IdP is IdentityServer (v4 I think).

fpaaske avatar Jul 27 '23 09:07 fpaaske

In the end our solution was to store an array of closures and to only fire off the performAction for the first one. Once that has returned we loop through all the closures in the array and fire off the callbacks with the same response. That’s been in production for months without issue.

LeoSnek avatar Jul 27 '23 09:07 LeoSnek

So I was able to implement it using the actor class that's using thread-safe access. Something like this:

actor OAuthManager {
  
    private var authState: OIDAuthState? {
        get {
            // get the value from the memory
 
            // ... put your code
   
            // get the value from the keychain

            // ... put your code
           
            return newAuthState
        }
        set {
            // set the value to the memory
            
            // ... put your code

            // set the value to the keychain
            
            // ... put your code
        }
    }

    func readCurrentState() throws -> OIDAuthState {
        return authState
    }

    func saveCurrentState(_ state: OIDAuthState?) async {
        authState = state
    }
}

shatodj avatar Jan 05 '24 12:01 shatodj