Access token auth #91
Conversation
lib/fhir_client/client.rb
Outdated
| # authorize_path -- absolute path of authorization endpoint | ||
| # token_path -- absolute path of token endpoint | ||
| # auto_configure -- whether or not to configure the oauth endpoints from the servers capability statement | ||
| # Addtional options can be passed in as supported by the OAuth2::AcessToken class |
There was a problem hiding this comment.
Spelling: OAuth2::AcessToken should be OAuth2::AccessToken
|
Very minor detail, but filename has a spelling error: |
|
Perhaps this is obvious, but I'm going to ask anyway... This Thoughts on what you did hear (confidential client) with what I've done here (public client): https://github.com/mitre/scorecard_app/blob/master/app.rb#L131 |
|
For the most part yes, only for confidential clients as recommended by the
smart best practices.
http://docs.smarthealthit.org/authorization/best-practices/
The primary use case I have is the need for the client to act on a users
behalf while the user is no longer present, periodically checking for
updates to the patients records. There is also the possibility to get
refresh_token for the online_access scope as well, though I'm not sure how
applicable that would be for this client.
As far as what this is doing compared to what is in the scorecard app I
think it comes down to the use case that is required for different apps.
If there isn't a need for a confidential client or more specifically
accessing data with out a user present then I would do what you did in the
scorecard app. When there is a need for offline_access scopes I would do
what this PR is doing. I can certainly do what this PR is adding outside
the context of the client. I could use the refresh_token to get a new
access_token and then configure the client with the new access_token via
the set_bearer_token method. I thought that is was better suited to be
included in the client for others to possibly make use of and to
encapsulate that logic in a single location.
…On Tue, Jun 5, 2018 at 9:09 AM, Jason Walonoski ***@***.***> wrote:
Perhaps this is obvious, but I'm going to ask anyway... This
set_auth_from_token method only works for confidential clients (i.e. they
can protect a client secret), right? The more usual SMART on FHIR
interactions are public clients (i.e. they cannot protect a client secret).
Thoughts on what you did hear (confidential client) with what I've done
here (public client): https://github.com/mitre/
scorecard_app/blob/master/app.rb#L131
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#91 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABFJvCkIABDSFEQAXyDxxYqnoql-bHDks5t5oLugaJpZM4UXQhe>
.
|
|
Sorry for taking so long to look at this @rdingwell! One issue that might come up is that the 'expires_at' field is optional in OAuth, I believe, and it isn't guaranteed to be correct, so you may not know that the current token is expired or revoked until you try the token. See: https://www.quora.com/Is-it-possible-to-know-when-an-OAuth2-token-expires That is something I only recently learned. Expires_at is intended to be a hint... clients need to be able to handle the case of token revocation anyhow, so in practice it is easier just to handle token expiration and token revocation in the same way (look for a 401, and then try the refresh token). I realize that we have code in the plan_executor that works in this same way, which needs to get fixed. I'm also wondering if we should pull out the OAuth client from FHIR client, as it currently is very incomplete, and the implementation is scary (it replaces the REST client that is being used, even though the API is a bit different). It seems like we should either completely embrace OAuth and provide all the helper functions necessary to do all OAuth-related flows (such as swapping codes for access tokens). Or we should remove the OAuth dependency and have users be responsible for any OAuth handshake, and then provide them with the ability to inject whatever is necessary into authorized requests (e.g. adding headers). As you know, OAuth is a beast, so fully supporting all OAuth would realistically mean we would just expose the OAuth gem that this is using anyhow. Or we could scope down to SMART on FHIR, but then we'd want to rename the functions to make it clear that we are only supporting the SMART profile. Furthermore, if we extend the API to support all of OAuth, then we probably should extend it further to support all the OpenID Connect related flows as well (which is an optional part of SMART). What do you think @jawalonoski @rdingwell ? I'm inclined to scope down what we are doing in the client in favor of having less surface area to try to maintain. |
Part of the SMART on FHIR specification is the support for clients that operate on behave of a user when the user does not have an active session. This is accomplished through the use of the SMART offline_access scope which provides the client application with a refresh_token that can be used to obtain a new access_token. This PR enables the ability of the fhir_client to be configured with a refresh_token that can then be exchanged for an access_token.