Authentication problems / is the SDK invalidating my saved access and refresh tokens?

[horacioj]

Hi,

I think I'm doing the authentication workflow in the right way, but too ofter I get 'refresh token has expired'. It is supposed to last for two months, so it can't be that, because the problem happens within the same day (within hours).

This is what I do: The first time (just once), I get my first access and refresh tokens pair. This is working fine.

Then, in the following hours (and luckily days and so on) I get (and save) new access and refresh tokens doing this:

var config = new BoxConfig(_settings.ClientId, _settings.ClientSecret, new Uri(RedirectUrl));
var sessionInfo = new OAuthSession(_settings.AccessToken, _settings.RefreshToken,
                                                       AccessTokenExpiresSeconds, "bearer");
_client = new BoxClient(config, sessionInfo);

var newSessionInfo = await client.Auth.RefreshAccessTokenAsync(client.Auth.Session.AccessToken);
// and I update _settings.AccessToken and _settings.RefreshToken 
// with the values found in newSessionInfo 

Why then, I'm getting the 'refresh token has expired'? Digging into the SDK code, I see this in \Box.V2\Managers\BoxResourceManager.cs:

            switch (response.Status)
            {
                // Refresh the access token if the status is "Unauthorized" (HTTP Status Code 401: Unauthorized)
                // This will only be attempted once as refresh tokens are single use
                case ResponseStatus.Unauthorized:
                    response = await RetryExpiredTokenRequest<T>(request).ConfigureAwait(false);
                    break;

...
...
...
        protected async Task<IBoxResponse<T>> RetryExpiredTokenRequest<T>(IBoxRequest request)
            where T : class
        {
            OAuthSession newSession = await _auth.RefreshAccessTokenAsync(request.Authorization).ConfigureAwait(false);
            AddAuthorization(request, newSession.AccessToken);
            return await _service.ToResponseAsync<T>(request).ConfigureAwait(false);
        }

Therefore, could it be, you are refreshing the access and refresh tokens when you identify an expired access token? If so, you are getting a new access and refresh tokens pair, but I never see nor get them; and my saved tokens are now invalid. Am I right? If so, how to workaround this? If I'm not right, why do you think I'm losing the authentication?

Thanks, Horacio.-

[alitaleg]

Hi Horacio,

You are correct that BoxResourceManager detects an expired token and refreshes it. In your example, you are probably overwriting refresh tokens.

Have a look at RefreshAccessTockenAsync in AuthRepository.cs. The method makes a call to refresh the token and then replaces "Session" with a new OAuthSession object. Your new auth session is the stored as _client.Auth.Session.RefreshToken.

You should not need to store the auth token or refresh token separately within your app.

Regards, Ali

[horacioj]

Hi Ali, Thanks for your fast reply.

Have a look at RefreshAccessTockenAsync in AuthRepository.cs

This is what I'm calling when I do "var newSessionInfo = await client.Auth.RefreshAccessTokenAsync(client.Auth.Session.AccessToken);"

Do you mean, I should not call it because it happens automatically as needed? And then I just update my saved AccessToken RefreshToken if I found they were updated? And I know if they have changed comparing my saved values with the current ones in client.Auth.Session.AccessToken and client.Auth.Session.RefreshToken?

You should not need to store the auth token or refresh token separately within your app.

Sorry. I don't understand. They seem to change on every request. If I don't save them, my next API call would fail, right?

Many thanks, Horacio.-

[alitaleg]

Yes, you should not call RefreshAccessTokenAsync because it happens automatically if needed.

The current auth and refresh tokens are always stored in _client.Auth.Session and will be used when making a network call.

[horacioj]

Many thanks, Ali. That should be what was happening to me in my current tests. I'll remove the RefreshAccessTokenAsync() call. Now my problem is when to get and save the new values (in case they have changed), because my development is not an app, but a kind of API called by external code. Saving them on every API (my API) call would be very inefficient. There could be many calls in a row. And leaving it to happen ones on my main class destroyer is not safe, because it may never happen or whatever can happen and the in-memory values could be lost. With Google Drive, Dropbox and others also using OAuth, this is not a problem at all. Box is requiring to keep changing and updating the tokens too often. Or I'm missing something.

[horacioj]

I'm lost... I removed the call to , but I'm still getting the "{"error":"invalid_grant","error_description":"Refresh token has expired"}". Which is the right workflow for my application, one I get the first access and refresh tokens? What and when to call to authenticate? The access token last for 1h and the refresh token for 20 days. right? Why then I get "Refresh token has expired" in the same day? I'm doing something wrong, but I don't know what. Thanks.

[brianytang]

Hi Horacio,

Just read the thread, and hopefully I can provide some clarity.

I see you're creating a BoxClient and providing the auth session yourself. This leads me to believe that you are performing the OAuth workflow through some other means and retrieving the Access Token/Refresh Tokens from there. This should not be a problem and you are correct that the Access Token lasts 1 hour, and Refresh tokens last 60 days (ie. 2 months).

Using this newly created BoxClient within the hour should return proper responses and objects. I think where it may get confusing, is after that hour is up. If you continue to use this same BoxClient after the expiration period, on the first request, the SDK will detect that the Access Token has expired and attempt to refresh the session using the Refresh Token. If successful, it will maintain the new access token and refresh tokens in the AuthSession. This same process will happen after the new Access Tokens are expired.

The flow I described above should work for your purposes as a service, assuming that once the service is started, it is never stopped/restarted. If you restart the service, you will need to re create a new auth session for the Client to use as the session is not persisted. The simplest way to accomplish this is to persist the AccessToken/RefreshToken of the Auth Session when the app/service is closed. Then when the service is started again, you can recreate that same Auth Session with the persisted Access Token/Refresh Token. After the first request is made through the BoxClient, the SDK should detect if the Access Token is expired. If it is, it will attempt to refresh the tokens as mentioned before.

Hopefully that clarifies things. If you are using the exact flow as described above, and you are still coming across token expiration issues -- please let us know.

Will wait for your response before closing this issue.

[horacioj]

Hello,

I see you're creating a BoxClient and providing the auth session yourself.

Not really. Or at least, it was not my intention. Before going forward: This was already implemented and work fine for the Box API v1. It is also already implemented and working fine for DropBox, Google Drive, Amazon S3, Windows Azure, OneDrive, etc.

The thing is, mine is not a mobile nor windows service or application. It is something running within web sites. In fact, many web sites, and on each one of these web sites it will be its own 'box application'. It is not a centralized app. Each website needs to create their own app, and API keys in their own Box accounts to connect their web sites with their own Box.

A web site's admin should be able to make the (painful) authorization process just one time, and then the connection/authorization should be kept forever, or until it is explicitly deleted. My piece of software is something running behind scenes. If the authorization is lost, they cannot simply re-do it. Regular web site's users cannot do that.

On the regular operation of 'my' software, the authorization is assumed to be already in place. I.e. already configured since days, months or years ago.

This is what happens just one time, when the connection (link) to Box is created (what a web site's admin does from a UI only available to them):

var config = new BoxConfig(clientId, clientSecret, new Uri(redirectUrl));
var client = new BoxClient(config);
var sessionInfo = GetAuthentication(client, authorizationCode).Result;

I save the sessionInfo.AccessToken and sessionInfo.RefreshToken. I.e. the first ones that were given to me. This will allow me to authenticate future API requests until forever (*).

Am I right?

(*) In theory RefreshToken will be good for two months. (side note: Two months are too little time. if in the web site where the link to Box was created, nobody does anything that implies the indirect renewal of the RefreshToken. they will have to start over, but this is a different problem - which any of the other services have, BTW - )

That's why going forward I do (behind scenes, triggered by some regular user's action that implies accessing something on Box):

var sessionInfo = new OAuthSession(_settings.AccessToken, _settings.RefreshToken, AccessTokenExpiresSeconds, "bearer");
_client = new BoxClient(config, sessionInfo); 

Because I don't want the authorization to happen again, it was already done, and the provided (saved) "_settings.RefreshToken" should be OK (if it was created no more than 2 months ago).

Notice the original BoxClient() instance died long time ago. Futhermore, this new BoxClient() will live maybe just for a few minutes, and it is even possible that other BoxClient() are created at the same time or so by others (or even the same) web site's users. Also, I don't know when each ones of those possible BoxClient() instances will die. What I was trying to do is to always keep in my '_settings' (stored ultimately in a database) the current, latest, valid Access and Refresh tokens, and maybe this is impossible with the current SDK, or too hard (besides the fact that it seems to be insane that I have to keep track and keep saving everchanging tokens all the time).

Could this be this is the problem? maybe your SDK was not built with this use case in mind? Maybe it assumes you create a single BoxClient() which will live basically forever, and will take care of the RefreshToken renewal by itself? And if the authorization is lost, it is not a big deal becasue the user can simply re-do it?

Maybe, among other changes, "private async Task ExchangeRefreshToken(string refreshToken)" should be made public to allow the tokens nightmare to be handled from outside the SDK?

Anyway, here http://developers.box.com/docs/ it says "A refresh token retrieved in the final leg of OAuth 2. In most cases these are valid for 60 days, or until used." Therefore: I have to save always persist (save) last refresh_token or the authentication will be lost. Am I right? There is no way I can keep track of the lastest refresh_token, which can changes all the time (not every hours or 60 days. All the time. This does not what others are doing. Why can't I save a refresh_token and use it many times to get the even more frequent changing access_token? Why a new refresh_token invalidates a previous one (if it is still within the 60 days limit)?

Please let me know how can I make this work.

Thanks, Horacio.-

[horacioj]

Any ideas?

[brianytang]

Hi Haracio,

I think I'm understanding the issue you're describing -- and due to the distributed nature of your environment, this may be a complex issue to solve.

From your description it appears you have separate web applications that are all independently trying to access information on Box. The mental picture that I'm seeing is that each time these web applications need to make a request to Box, they create a new BoxClient, query the database for the current Access Tokens/Refresh Tokens, then proceed to make the call. Because these requests are happening in parrallel, simply returning the persisted tokens as they are requested will not work once the initial access tokens expire as there is nothing keeping the tokens in sync.

For clarity, if app A and app B both attempt to make a request:

• A and B both retrieve the current tokens to create their BoxClient
• They will both receive the same tokens Access: 123 and Refresh: 456
• A and B will both work using those provided access tokens for the next hour
• After the first hour, A makes a request and the SDK sees that the access token is expired
• A's tokens are refreshed to Access: 321 and Refresh: 654
• The next request for B will not work as now both it's Access tokens and Refresh tokens are invalid

This is the exact issue we were hoping to overcome by baking the refresh logic into our SDK. Unfortunately, because these are distributed systems, the solution we've included will likely not fit your needs.

For your particular case, I believe there may be 2 possible solutions 1) Adding another layer on top of the database call that retrieves the Tokens. This layer will need to be responsible for retrieving current access tokens, as well as refreshing expired tokens and persisting the new tokens. I'd imagine this logic would be somewhat similar to what we have built into the SDK where we exchange expired access tokens for new access tokens. Additionally, a change will need to be made in the SDK to disable automatic refresh. 2) Create a centralized service that uses the SDK to make requests to Box. This forces all requests to go through a centralized place and would make token management a bit simpler

I hope that helps.

[prexer]

Not sure if this response to an email will get through on github. Hopefully.

We added some capability to the refresh mechanism to give multi threaded environments a way to share the tokens. You still need to have a central keystore.

So given at1/rt1 when thread 1 refreshes the tokens it will get back at2/rt2 It should store the new one in the central key store before using at2. When a thread finds out that it's copy of at1 has expired, it should first check the keystore. If there is a new token pair, then it should pick them up and use them. If not, it can call the refresh and it will get the exact same at2/rt2 that was given to the other thread.

Once at2 gets used then rt1 will be on a very short fuse to expire.

Hope that helps. On Mar 7, 2014 5:22 PM, "brianytang" notifications@github.com wrote:

Hi Haracio,

I think I'm understanding the issue you're describing -- and due to the distributed nature of your environment, this may be a complex issue to solve.

From your description it appears you have separate web applications that are all independently trying to access information on Box. The mental picture that I'm seeing is that each time these web applications need to make a request to Box, they create a new BoxClient, query the database for the current Access Tokens/Refresh Tokens, then proceed to make the call. Because these requests are happening in parrallel, simply returning the persisted tokens as they are requested will not work once the initial access tokens expire as there is nothing keeping the tokens in sync.

For clarity, if app A and app B both attempt to make a request:

• A and B both retrieve the current tokens to create their BoxClient
• They will both receive the same tokens Access: 123 and Refresh: 456
• A and B will both work using those provided access tokens for the next hour
• After the first hour, A makes a request and the SDK sees that the access token is expired
• A's tokens are refreshed to Access: 321 and Refresh: 654
• The next request for B will not work as now both it's Access tokens and Refresh tokens are invalid

This is the exact issue we were hoping to overcome by baking the refresh logic into our SDK. Unfortunately, because these are distributed systems, the solution we've included will likely not fit your needs.

For your particular case, I believe there may be 2 possible solutions 1) Adding another layer on top of the database call that retrieves the Tokens. This layer will need to be responsible for retrieving current access tokens, as well as refreshing expired tokens and persisting the new tokens. I'd imagine this logic would be somewhat similar to what we have built into the SDK where we exchange expired access tokens for new access tokens. Additionally, a change will need to be made in the SDK to disable automatic refresh. 2) Create a centralized service that uses the SDK to make requests to Box. This forces all requests to go through a centralized place and would make token management a bit simpler

I hope that helps.

Reply to this email directly or view it on GitHubhttps://github.com/box/box-windows-sdk-v2/issues/31#issuecomment-37084604 .

[horacioj]

Hi,

We added some capability to the refresh mechanism to give multi threaded environments a way to share the tokens.

It may help. Where was this implemented? In the API, in the SDK (I don't see any recent update), or in a custom solution you've implemented?

You still need to have a central keystore.

This is my thought: all the hassle is to workaround Box's way to handle and expire tokens. Wouldn't be easier if simply Box changes that to be more like the others (dropbox, google drive, etc.) are doing? Even if I can manage to create this complex layer of software just to workaround Box's tokens handling, I think it would be very inefficient for me to keep comparing running vs. saved tokens, and updating saved (persisted) tokens basically on every request made to box. It will be also very easy for this to fail on a regular basis (i.e. for whatever reason the latest token is not persisted, and it is lost).

For your particular case, I believe there may be 2 possible solution

I think I'll remove Box support to my little application, and that's it. I've already spent literally countless of hours (including full days and nights) rewriting everything for the API v2 (software that was already working fine for the API v1).

Maybe if for API v3 this changes, I will revisit this matter. For now, the cost of working around the expired tokens, is too high for the benefit I could get. Neither the Box API nor the SDK is solving the complexity for me, and I cannot afford to do it myself. Furthermore, I think a good solution cannot be implemented in the current in the current state of the API & the SDK. I feel it would be inefficient, and very prone to fail (it just takes 1 missing token for the authentication to be lost, and for the kind of software this is, it is a big deal to re-do the authorization process every few days, or months.

[prexer]

Thanks for the input. Really appreciate your thoughts on the some other kind of token expiration, or maybe some other kind of authentication that would be less secure. If we did offer such a token, then we'd let customers know, and decide which level of security they were okay with.

-Peter

On Sat, Mar 8, 2014 at 8:41 AM, horacioj notifications@github.com wrote:

Hi,

We added some capability to the refresh mechanism to give multi threaded environments a way to share the tokens.

It may help. Where was this implemented? In the API, in the SDK (I don't see any recent update), or in a custom solution you've implemented?

You still need to have a central keystore.

This is my thought: all the hassle is to workaround Box's way to handle and expire tokens. Wouldn't be easier if simply Box changes that to be more like the others (dropbox, google drive, etc.) are doing? Even if I can manage to create this complex layer of software just to workaround Box's tokens handling, I think it would be very inefficient for me to keep comparing running vs. saved tokens, and updating saved (persisted) tokens basically on every request made to box. It will be also very easy for this to fail on a regular basis (i.e. for whatever reason the latest token is not persisted, and it is lost).

For your particular case, I believe there may be 2 possible solution

I think I'll remove Box support to my little application, and that's it. I've already spent literally countless of hours (including full days and nights) rewriting everything for the API v2 (software that was already working fine for the API v1).

Maybe if for API v3 this changes, I will revisit this matter. For now, the cost of working around the expired tokens, is too high for the benefit I could get. Neither the Box API nor the SDK is solving the complexity for me, and I cannot afford to do it myself. Furthermore, I think a good solution cannot be implemented in the current in the current state of the API & the SDK. I feel it would be inefficient, and very prone to fail (it just takes 1 missing token for the authentication to be lost, and for the kind of software this is, it is a big deal to re-do the authorization process every few days, or months.

Reply to this email directly or view it on GitHubhttps://github.com/box/box-windows-sdk-v2/issues/31#issuecomment-37102435 .

[ckapatch]

I agree with Horacio here, the SDK should not be responsible for managing the tokens. The client should be the one responsible for managing the tokens. When the token is refreshed in the SDK, the client has no idea this has been done and if the client goes down, the refresh token is lost requiring users to constantly go through the process of authenticating from the start.

Furthermore, there is no logic inside to prevent conditions like the following from happening in a multi-threaded environment (which has occurred more than once in my application):

Thread 1 sends a request to Box Thread 1 receives a 401 unauthorized Thread 2 sends a request to Box Thread 2 receives a 401 Unauthorized Thread 1 begins refreshing the token Thread 1 receives the new refresh token / access token pair Thread 1 sends a request to Box Thread 1 receives a 200 OK for his request The original refresh token is now destroyed since it is no longer valid Thread 2 sends a refresh token request (with the old token) Thread 2 receives an error because the token is expired.

I've actually removed the following section of code from my fork and explicitly send back an exception if the Access Token expires:

private async Task<IBoxResponse<T>> ExecuteRequest<T>(IBoxRequest request, bool queueRequest)
            where T : class
        {
            var response = queueRequest ?
                await _service.EnqueueAsync<T>(request).ConfigureAwait(false) :
                await _service.ToResponseAsync<T>(request).ConfigureAwait(false);

            switch (response.Status)
            {
                /// Removed To prevent automated token refreshing
                /*
                case ResponseStatus.Unauthorized:
                    response = await RetryExpiredTokenRequest<T>(request).ConfigureAwait(false);
                    break;
                */   
                // Continue to retry the request if the status is "Pending" (HTTP Status Code 202: Approved)
                // this will occur if a preview/thumbnail is not ready yet
                case ResponseStatus.Pending:
                    response = await ExecuteRequest<T>(request, queueRequest).ConfigureAwait(false);
                    break;
            }

            return response;
        }

[prexer]

What I'd recommend is that as soon as either thread hits a 401, that you go hit your keystore to see if some other thread has already taken the lock on the keystore to update it. If so, the thread can pause, retrieve the new keyset and move on. If not, then it can try to take the lock and do the refresh.

-Peter

On Thu, Apr 3, 2014 at 8:56 PM, Chad Kapatch notifications@github.comwrote:

I agree with Horacio here, the SDK should not be responsible for managing the tokens. The client should be the one responsible for managing the tokens. When the token is refreshed in the SDK, the client has no idea this has been done and if the client goes down, the refresh token is lost requiring users to constantly go through the process of authenticating from the start.

Furthermore, there is no logic inside to prevent conditions like the following from happening in a multi-threaded environment (which has occurred more than once in my application):

Thread 1 sends a request to Box Thread 1 receives a 401 unauthorized Thread 2 sends a request to Box Thread 2 receives a 401 Unauthorized Thread 1 begins refreshing the token Thread 1 receives the new refresh token / access token pair Thread 1 sends a request to Box Thread 1 receives a 200 OK for his request The original refresh token is now destroyed since it is no longer valid Thread 2 sends a refresh token request (with the old token) Thread 2 receives an error because the token is expired.

I've actually removed the following section of code from my fork and explicitly send back an exception if the Access Token expires:

private async Task<IBoxResponse> ExecuteRequest(IBoxRequest request, bool queueRequest) where T : class { var response = queueRequest ? await _service.EnqueueAsync(request).ConfigureAwait(false) : await _service.ToResponseAsync(request).ConfigureAwait(false);

        switch (response.Status)
        {
            /// Removed To prevent automated token refreshing
            /*                case ResponseStatus.Unauthorized:                    response = await RetryExpiredTokenRequest<T>(request).ConfigureAwait(false);                    break;                */
            // Continue to retry the request if the status is "Pending" (HTTP Status Code 202: Approved)
            // this will occur if a preview/thumbnail is not ready yet
            case ResponseStatus.Pending:
                response = await ExecuteRequest<T>(request, queueRequest).ConfigureAwait(false);
                break;
        }

        return response;
    }

Reply to this email directly or view it on GitHubhttps://github.com/box/box-windows-sdk-v2/issues/31#issuecomment-39529711 .

[ckapatch]

Hey Peter,

That's exactly what we'd like to do however the SDK automatically sends requests to update the access token on our behalf without us knowing about it nor locking it's own session to ensure it doesn't invalid the refresh token all on it's own. I'd prefer if the SDK simply notifies the client of an expired access token. This way the client can manage how to deal with it, especially in a distributed or multi-threaded system. If two separate threads both have a separate instance of the boxclient, and both send a request that result in a 401, the refresh token will invalidate. The SDK is not thread safe.

[brianytang]

Hi,

You are correct that the SDK should provide an override to allow the developers to perform token management manually. When we initially wrote the SDK, we tried to alleviate the pain of refreshing and maintaining tokens by baking that logic into our SDK. We will include that in a future update.

Per the scenario you described above, the SDK should perform those series of operations correctly. The locking occurs further down the call chain in the RefreshTokenAsync method where an access token is passed in as a parameter. The RefreshTokenAsync takes the access token token as a parameter to ensure multiple refreshes do not occur for the same access token.

Can you confirm that you are seeing these token contention issues in your multi threaded environment with the existing SDK token management logic?

[horacioj]

FYI, I could not find any way to make it work for my application (It was OK for API v1, though), and I had to remove BOX support from it. Commenting (or updating) code in the SDK that was (I think) refreshing the token didn't help. More sooner than later, I ended up with invalid tokens, and the application in an unusable state, because the users of this application cannot perform the re-authorization process.

[horacioj]

Besides, I still think the 'one time' usage for the refresh token is not the best solution. It is generating this kind of problems and processing overhead. I would not be less secure if if you fix this behavior. It is kind of funny that FTP is enabled by default. This is truly insecure. The WEB API, using persistent and re-utilizable refresh tokens (e.g. expiring in one year, like google drive does) would not be insecure, I think.

[brianytang]

Thanks for your input, Horacio. I'll be sure to forward your comments to the platform team and perhaps we can start that discussion on a different thread.

I'd like to keep this issues page relating specifically to issues experienced with the Windows SDK itself. That being said, for multi-threaded client applications, the SDK should support automatic token renewal when they are expired. If this is not the case, I would like to know so I can investigate.

For distributed systems, Box's current authentication will require manual management of the access/refresh tokens. We will be adding the ability to override token management in a future update

[ckapatch]

Hello brianytang, sorry didn't see the lock within the Token refresh call. So assuming the boxclient is a singleton you wouldn't have issues with tokens. As for our particular application, I didn't realize that the SDK was attempting to update the tokens itself and I have some robust logging around the token renewal calls. We would see logs like the following:

EXPIRED Access Token - Refresh Token: CSHuOnzP0ii40Rr0NZQokc3vNTxhNYRSWlBVADBlxXR1rvlsHRkcS1WH683SeuB3 AQUIRED Access Token vMJMlComfympBcrkSbdrkOkoh9moXbwD - Refresh Token: MuziDn4lDLRsu4WLVnZZ0rv6IU1m0JABuP1orsW2JopNY4hfetzur1VNhKt5O5gm EXPIRED Access Token - Refresh Token: MuziDn4lDLRsu4WLVnZZ0rv6IU1m0JABuP1orsW2JopNY4hfetzur1VNhKt5O5gm *** EXCEPTION WITH BOX TOKEN *** System.AggregateException: One or more errors occurred. ---> Box.V2.Exceptions.BoxException: invalid_grant: Refresh token has expired.

So we try to update the access token with the old refresh token. Nonetheless, it would obviously break when two separate threads used different instantiations of the boxclient and both received a 401 at the same time.

The issue here is that we had no idea that the SDK is updating refresh tokens without digging into the code to see it. This shouldn't be hidden. Take for example the Google Drive SDK which will allow the client to inject a datastore implementation that the SDK use on token changes. Something like the following (straight from the Google APIs):

    // Summary:
    //     Stores and manages data objects, where the key is a string and the value
    //     is an object.
    //     null keys are not allowed.
    public interface IDataStore
    {
        // Summary:
        //     Asynchronously clears all values in the data store.
        Task ClearAsync();
        //
        // Summary:
        //     Asynchronously deletes the given key. The type is provided here as well because
        //     the "real" saved key should contain type information as well, so the data
        //     store will be able to store the same key for different types.
        //
        // Parameters:
        //   key:
        //     The key to delete.
        //
        // Type parameters:
        //   T:
        //     The type to delete from the data store.
        Task DeleteAsync<T>(string key);
        //
        // Summary:
        //     Asynchronously returns the stored value for the given key or null if not
        //     found.
        //
        // Parameters:
        //   key:
        //     The key to retrieve its value.
        //
        // Type parameters:
        //   T:
        //     The type to retrieve from the data store.
        //
        // Returns:
        //     The stored object.
        Task<T> GetAsync<T>(string key);
        //
        // Summary:
        //     Asynchronously stores the given value for the given key (replacing any existing
        //     value).
        //
        // Parameters:
        //   key:
        //     The key.
        //
        //   value:
        //     The value to store.
        //
        // Type parameters:
        //   T:
        //     The type to store in the data store.
        Task StoreAsync<T>(string key, T value);
    }

If no implementation is given, then a default internal one can be used.

[prexer]

Sounds like a great idea. I think that was part of what we intended: to make the keystore a pluggable piece that you could replace. Some of our other SDKs maybe are doing this a little better. Would love your contributions to create the interface.

-Peter

On Fri, Apr 4, 2014 at 4:22 PM, Chad Kapatch notifications@github.comwrote:

Hello brianytang, sorry didn't see the lock within the Token refresh call. So assuming the boxclient is a singleton you wouldn't have issues with tokens. As for our particular application, I didn't realize that the SDK was attempting to update the tokens itself and I have some robust logging around the token renewal calls. We would see logs like the following:

EXPIRED Access Token - Refresh Token: CSHuOnzP0ii40Rr0NZQokc3vNTxhNYRSWlBVADBlxXR1rvlsHRkcS1WH683SeuB3 AQUIRED Access Token vMJMlComfympBcrkSbdrkOkoh9moXbwD - Refresh Token: MuziDn4lDLRsu4WLVnZZ0rv6IU1m0JABuP1orsW2JopNY4hfetzur1VNhKt5O5gm EXPIRED Access Token - Refresh Token: MuziDn4lDLRsu4WLVnZZ0rv6IU1m0JABuP1orsW2JopNY4hfetzur1VNhKt5O5gm *** EXCEPTION WITH BOX TOKEN *** System.AggregateException: One or more errors occurred. ---> Box.V2.Exceptions.BoxException: invalid_grant: Refresh token has expired.

So we try to update the access token with the old refresh token. Nonetheless, it would obviously break when two separate threads used different instantiations of the boxclient and both received a 401 at the same time.

The issue here is that we had no idea that the SDK is updating refresh tokens without digging into the code to see it. This shouldn't be hidden. Take for example the Google Drive SDK which will allow the client to inject a datastore implementation that the SDK use on token changes. Something like the following (straight from the Google APIs):

// Summary:
//     Stores and manages data objects, where the key is a string and the value
//     is an object.
//     null keys are not allowed.
public interface IDataStore
{
    // Summary:
    //     Asynchronously clears all values in the data store.
    Task ClearAsync();
    //
    // Summary:
    //     Asynchronously deletes the given key. The type is provided here as well because
    //     the "real" saved key should contain type information as well, so the data
    //     store will be able to store the same key for different types.
    //
    // Parameters:
    //   key:
    //     The key to delete.
    //
    // Type parameters:
    //   T:
    //     The type to delete from the data store.
    Task DeleteAsync<T>(string key);
    //
    // Summary:
    //     Asynchronously returns the stored value for the given key or null if not
    //     found.
    //
    // Parameters:
    //   key:
    //     The key to retrieve its value.
    //
    // Type parameters:
    //   T:
    //     The type to retrieve from the data store.
    //
    // Returns:
    //     The stored object.
    Task<T> GetAsync<T>(string key);
    //
    // Summary:
    //     Asynchronously stores the given value for the given key (replacing any existing
    //     value).
    //
    // Parameters:
    //   key:
    //     The key.
    //
    //   value:
    //     The value to store.
    //
    // Type parameters:
    //   T:
    //     The type to store in the data store.
    Task StoreAsync<T>(string key, T value);
}

If no implementation is given, then a default internal one can be used.

Reply to this email directly or view it on GitHubhttps://github.com/box/box-windows-sdk-v2/issues/31#issuecomment-39620890 .

[ckapatch]

Hey Peter, I'll have a look and see what I can do. I'll create a ticket for this issue.

[ptallett]

Looks like this has been fixed:

  client = new BoxClient(config);
  client.Auth.SessionAuthenticated += Auth_SessionAuthenticated;
  session = await client.Auth.AuthenticateAsync(token);

... private async void Auth_SessionAuthenticated(object sender, SessionAuthenticatedEventArgs e) { // The SDK may refresh the tokens, so we have to save after every authentication https://github.com/box/box-windows-sdk-v2/issues/31 await dispatcher.RunAsync(CoreDispatcherPriority.Normal, async () => { AuthRepository repo = (AuthRepository)sender; SaveTokens(repo.Session.AccessToken, repo.Session.RefreshToken); }); }

Cheers, Paul