bchavez / Coinbase

:moneybag: A .NET/C# implementation of the Coinbase API.
https://developers.coinbase.com/api/v2
MIT License
170 stars 92 forks source link

Automatic Refresh Tokens should have a callback to update Authentication. #43

Closed granthoff1107 closed 5 years ago

granthoff1107 commented 5 years ago

OAuth should have a callback to update refresh and access tokens.

If the AutoRefresh Tokens is set as part of the config e.g

public class OAuthConfig : Config
{
    public Func<OAuthResponse, Task> OnRefresh { get; set; }
    public bool AutoRefresh { get; set; }
    protected internal override void Configure(CoinbaseClient client)
    {
          client.Configure(settings => { 
                 UseOAuth(settings, client);
                 if(AutoRefresh)
                 {
                      settings.OnErrorAsync = TokenExpiredErrorHandler
                 }
          });
    }

     private void UseOAuth(ClientFlurlHttpSettings settings, CoinbaseClient client)
     {
           async Task ApplyAuthorization(HttpCall call)
           {
              call.FlurlRequest.WithOAuthBearerToken(this.AccessToken);

           }
         settings.BeforeCallAsync = ApplyAuthorization;
     }        

When the API Refreshes the access tokens,
If you don't updated the access tokens/refresh tokens in the HTTP Context, your next login will result in Revoked Token exception.

If the Renew was moved to the Configuration, a user would be able to configure their own call back e.g If the user is using Coinbase to Authenticate users

public async Task SetToken(RefreshResponse response)
{
    var auth = await HttpContext.AuthenticateAsync("Cookies");
    auth.Properties.StoreTokens(new List<AuthenticationToken>()
    {
        new AuthenticationToken()
        {
            Name = OpenIdConnectParameterNames.AccessToken,
            Value = response.AccessToken
       },
        new AuthenticationToken()
        {
            Name = OpenIdConnectParameterNames.RefreshToken,
            Value = response.RefreshToken
        }
    });

        await HttpContext.SignInAsync(auth.Principal, auth.Properties);
    }

Additionally then you could mask the clientId and Client secret by putting them in the Configuration

bchavez commented 5 years ago

Hey Grant,

Can you help me understand why you want the call back? Are you looking for a way to hook into the "refresh" event to do some kind of storage with the AccessToken and RefreshToken in your own database?

granthoff1107 commented 5 years ago

When a user uses OAuth Authentication, the Access Token and Refresh Token are stored in a cookie. So when the api Updates the Access/Refresh Tokens. The cookie will have invalid information. Any time refresh occurs the user should have access to the updated response to do additional work.

I've created a pull request with the minimal amount of changes to the api.

granthoff1107 commented 5 years ago

Maybe not in this version, but the OAuth Changes probably should move to the config. Imagine a user wants to setup the OAuth Client as a Scoped Dependency with DI. Instead of being able to just register a config.
They will have to register a factory to setup auto refresh.

bchavez commented 5 years ago

Closing. The issue should be fixed in latest beta-6 release

-Brian