Skip to content

Instantly share code, notes, and snippets.

@lbalmaceda
Last active July 5, 2017 13:25
Show Gist options
  • Save lbalmaceda/0f1aa82760da554cdb360dcd569aa4cc to your computer and use it in GitHub Desktop.
Save lbalmaceda/0f1aa82760da554cdb360dcd569aa4cc to your computer and use it in GitHub Desktop.
Credentials Manager Report

CredentialsManager

Recently we introduced a helper class available here to handle Auth0 Credentials. Usage is simple: Once you create a new instance by passing an AuthenticationAPIClient (to renew credentials) and a Storage (to do persistance), you can call any of this 2 public methods:

  • saveCredentials: To synchronously save a pair of credentials in the storage. Will throw if the access_token or id_token is null, or if the expires_at date is missing.
  • getCredentials: To asynchronously get a pair of credentials from the storage.
    • If no credentials were stored before the methods fails.
    • If stored credentials have expired (expires_at > now) and no refresh_token was saved the method fails.
    • If the call to renew the credentials using the refresh_token fails, the method fails.
    • In any other case, a valid pair of credentials is returned. Either the existing and non-expired ones or a fresh new pair.

As a side note, if the credentials were expired AND refreshed successfully they are NOT updated in the storage. It's up to the user to call again saveCredentials and persist the new value.

Use cases

  1. Check "logged in" status: When the main screen of the app is first shown and the developer needs to check whether a user has already logged in or not. With the current implementation it will be something like:

    • Call getCredentials
    • If the method succeeds the user has "previously logged in". After that the tokens are discarded as the dev doesn't need them now and also doesn't know if the method had just refreshed the tokens or if they were already valid.
    • On the other side, if the method fails the dev must check if the exception is of type Auth0Exception (or whatever the Authentication client throws) which will mean that it was a network error (either timeout or server rejected the refresh_token) to discard that from a non-existent or expired token.
  2. Store a "logged in" status: User logs in by using Auth0's Authentication API and persists the credentials in the storage.

    • Calls saveCredentials to save the credentials.
  3. Authenticate against an API: User needs to send the token in a header to his API.

    • He calls getCredentials expecting either a valid/fresh pair of credentials or an error.
    • An error would mean that either the credentials have expired or they couldn't be renewed. If the call fails, he can decide wether to retry if it was a network problem or force the user to log in again.
  4. Remove the "logged in" status (aka log out): User logs out and the developer needs to ensure that the next time he asks if a user was logged in, the answer is NOPE. With the current implementation it's not possible, as the saveCredentials method will null-check the access_token and refresh_token.

Proposed changes

(1) Is the user logged in?

It's discussable but (1) can be handled by the developer locally (outside our SDK scope). He must save a boolean/flag to indicate the "logged in" status. If he also stores the expires_at date he's covered, in other case he's not and he may get false positives if he tries to use expired credentials.

Another option is that we provide a method hasCredentials or hasValidCredentials that without performing a network request (this is the WIN part) it can tell the developer with 99% success rate that a valid pair of credentials is available in the storage. How will the logic be??

  • User calls a sync method hasCredentials expecting a boolean.
  • If credentials exists and they haven't expired (expires_at < now) it will return true.
  • If expired credentials exists but a refresh_token is available, it will return true. As this works locally/offline it's not possible to determine if a refresh_token was revoked, but it's a very rare case as it's done via dashboard or Management API. This is the 1%.
  • On any other case, return false.

(4) Clear the credentials

This is definitely missing. We must provide a way for the developer to clear the existing credentials. A simple method that sets to null or removes the existing entries from the storage. Proposed name is clearCredentials.

@aaguiarz
Copy link

aaguiarz commented Jul 4, 2017

I like the suggested changes and I think hasCredentials is a better name.

We need to explain in the docs that before making an API call they should always call getCredentials, to make sure the token is refreshed if needed.

@cocojoe
Copy link

cocojoe commented Jul 5, 2017

So we can agree (4) clearCredentials

This proposed boolean is basically a convenience wrapper around getCredentials (without the potential final API call to renew so it can be sync) . The question is, do we really need to do this for the user? It is easy to add functionality to an API but not easy to remove. In regards to naming I think 'hasCredentials' is better, rather not add guarantees of 'valid' to the name.

I do not have strong feelings on this and in general I would lean to the side of helping the user, so this is fine.

Also if we are adding a clearCredentials method, let's use it to auto clear credentials that have expired as part of our general logic.

I've added to Trello.

@lbalmaceda
Copy link
Author

  • About the hasCredentials boolean: the only way the user has to know if a valid credential is present is to call the getCredentials and check if it doesn't fail with AuthException. I think that we can avoid that ugly block of 4/6 lines of code with this wrapper. Also like I said above, if the idea is just to check for authentication but not use the credentials right away, we can also avoid the network call. This last thing wouldn't matter that much if our manager stores the refreshed credentials when the network request succeeds as it would "cache" the credentials for the next call, but we decided not to do that in a first place.
    So if you don't agree we should have it, can we keep this for 1 or 2 months on hold and see if the users need it?
  • About the clearCredentials: by auto-clear you mean our getCredentials method should call it whenever it fails to validate the token existence/expiration?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment