Open Tolk-Haggard opened 9 years ago
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?
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
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.
@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?
@jruckle should take me 2 hours, assuming reasonable answers to the below
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.
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.