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

Client Cannot Reauthenticate #2

Open
Tolk-Haggard opened this issue Jun 18, 2015 · 9 comments
Open

Client Cannot Reauthenticate #2

Tolk-Haggard opened this issue Jun 18, 2015 · 9 comments

Comments

@Tolk-Haggard
Copy link

The authentication service can provide a new bearer token but client authenticates in its constructor and re-authentication is not facilitated through any public API. This makes expected usage a bit unclear.

If the users are not expected to manually re-auth then client should attempt a re-auth upon 401/403 - currently no attempt at re-authentication is attempted when receiving a unauthorized or forbidden status code. This would probably be the most intuitive/convenient approach.

@rpierry
Copy link
Contributor

rpierry commented Jun 23, 2015

As there is limited to no utility from a non-authenticated client, the usage pattern in the case of failed auth would be something along the lines of clearing the stored auth info and redirecting to a login page, which would then use a new client when re-authing. Retries in general are problematic to bake into frameworks, especially for something like a 401, which isn't likely to change on subsequent requests (if your token has expired, it isn't coming back).

Does that make sense?

@Tolk-Haggard
Copy link
Author

No, I'm not following you there. Since this is an SDK I don't understand where a login page would come into the usage pattern, we should only be accessing the control API.

Let me try and rephrase this issue into what I would expect as a consumer of the SDK.

When a client is instantiated with a username and password, if the authentication service returns a 403, I agree there is nothing further to be done. However, if the client authenticates successfully and gets a bearer token, we can assume the username and password are good. If during a subsequent request we receive a 401 then it is likely that the token has expired or become invalidated. At that point, instead of returning an exception to the user the client could attempt to reauth, get a new bearer token and retry the request. If that reauth attempt receives a 403 then we can follow the same code path as if a 403 was received on initial authentication and throw an exception.

Further information about determining if a bearer token is expired can be found here (see the WWW-Authentication response header in section 3): https://tools.ietf.org/html/rfc6750

Basically as a consumer of this SDK:

Scenario: I create a client with an invalid username or password
Expected: Exception containing invalid username or password authentication error
Actual: Exception containing invalid username or password authentication error

Scenario: A valid client issues a request with an expired bearer token
Expected: Client uses username and password to get new bearer token and issues the request with the new token
Actual: Exception containing expired token authentication error

Scenario: A client with old credentials issues a request with an expired bearer token
Expected: Client attempts to reauthenticate with username and password and returns an exception containing invalid username or password authentication error
Actual: Exception containing expired token authentication error

@rpierry
Copy link
Contributor

rpierry commented Jun 23, 2015

Here is where the disconnect is. The SDK does not store the username and password. It is stateless in that respect (and in most respects). The intended pattern is not to hold onto Client instances for long periods of time - instead you new one up, use it, and let it go. If you make a call with a bearer token that is expired then you as the consumer of the SDK need to decide what to do with it. Maybe you take them to a login page to reauth, maybe you programmatically try to retry, etc. That stuff feels like app logic rather than something that should be baked into the SDK, especially since baking it in would require the SDK storing the username/password and also that the callers keep instances of the Client around.

@rpierry
Copy link
Contributor

rpierry commented Jun 23, 2015

@jruckle does the bearertoken not last that long? The pattern in the SDK is to authenticate with username and password once and then you can hang on to the bearer token and simply provide it again and again for subsequent requests, as long as the token is valid. This is what the mobile apps do. They even store the bearer token securely for up to 2 weeks (by design). Does that not cover what you are describing?

@rpierry
Copy link
Contributor

rpierry commented Jun 24, 2015

@jruckle should take me 2 hours, assuming reasonable answers to the below

  1. does the API return a consistent code to indicate when reauth should be attempted (is it just 401?)
  2. is there an easy way to test expiring a bearer token?

@Tolk-Haggard
Copy link
Author

If the differentiation is done by service (auth service versus all other services), you should be able to implement whatever behavior is desirable.

If the Auth service fails, you could throw an exception indicating invalid credentials. If any other service fails, you could throw an exception or attempt a re-auth (which would throw its own exception if the credentials were now invalid for some reason).

As far as the testing an expired bearer token, the simplest way to test that I can think of would be mocking out the v2 APIs so you can control the responses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants
@rpierry @Tolk-Haggard and others