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

oauth2/introspect: make endpoint rfc7662 compatible #289

Closed
waynerobinson opened this issue Oct 7, 2016 · 7 comments
Closed

oauth2/introspect: make endpoint rfc7662 compatible #289

waynerobinson opened this issue Oct 7, 2016 · 7 comments
Labels
breaking change Changes behavior in a breaking manner. bug Something is not working.
Milestone

Comments

@waynerobinson
Copy link

/oauth2/introspect endpoint requires a valid client's client_credentials token to validate other access_tokens.

There seems to be a check in the endpoint that the client's sub must equal the access token's aud.

Also don't seem to be able to use basic digest authentication of a client's key/secret during these requests, only a valid client token.

I thought the point of this endpoint was to be able to just pass the access token as the Bearer and have it validate and return itself.

@aeneasr
Copy link
Member

aeneasr commented Oct 7, 2016

Also don't seem to be able to use basic digest authentication of a client's key/secret during these requests, only a valid client token.

RFC6749 is not normative on which method is supported, it just states that some sort of authentication must be done:

To prevent token scanning attacks, the endpoint MUST also require
some form of authorization to access this endpoint, such as client
authentication as described in OAuth 2.0 [RFC6749] or a separate
OAuth 2.0 access token such as the bearer token described in OAuth
2.0 Bearer Token Usage [RFC6750]. The methods of managing and
validating these authentication credentials are out of scope of this
specification.

We decided to use access token only to reduce the probability of significant CPU drain when validating tokens. I am open to support both though. Nevertheless, this should be documented - ping #287

There seems to be a check in the endpoint that the client's sub must equal the access token's aud.

This is interesting, I remember that this check was required to prevent token substitution attacks, but it does not seem to be in the current RFC any more, so it will be removed with an upcoming patch.

@aeneasr aeneasr added the bug Something is not working. label Oct 7, 2016
@aeneasr aeneasr added this to the 0.6.0 milestone Oct 7, 2016
@aeneasr
Copy link
Member

aeneasr commented Oct 7, 2016

It appears that the token substitution part has been replaced in favor of:

If the introspection call is properly authorized but the token is not
active, does not exist on this server, or the protected resource is
not allowed to introspect this particular token
, then the
authorization server MUST return an introspection response with the
"active" field set to "false".

@aeneasr aeneasr changed the title oauth2: /oauth2/introspect can't authenticate with it's own token oauth2/introspect: make endpoint rfc7662 compatible Oct 7, 2016
@aeneasr
Copy link
Member

aeneasr commented Oct 7, 2016

In fact, we could actually remove the /warden/valid endpoint and use the warden for policy-based checks only. This would further strengthen the separation of concerns and help people better understand what the warden is doing. What do you think?

@aeneasr aeneasr added the breaking change Changes behavior in a breaking manner. label Oct 9, 2016
aeneasr pushed a commit that referenced this issue Oct 9, 2016
* warden: make it clear that ladon.Request.Subject is not required or break bc and remove it - closes #270
aeneasr pushed a commit that referenced this issue Oct 9, 2016
* warden: make it clear that ladon.Request.Subject is not required or break bc and remove it - closes #270
@waynerobinson
Copy link
Author

So has this been changed to allow the token to be able to used to authenticate itself (and return at_ext values)?

This is quite a common operation for us so we'd like it to be as fast as possible.

Currently, we have to get a client_credentials token (with any scope, including blank) to be able to be able to validate the access_token.

@aeneasr
Copy link
Member

aeneasr commented Oct 10, 2016

So has this been changed to allow the token to be able to used to authenticate itself (and return at_ext values)?

The subject == token.Audience check was removed, apart from that nothing changed, as at_ext was already returned by introspection and RFC7662 requires a separate token:

To prevent token scanning attacks, the endpoint MUST also require
some form of authorization to access this endpoint, such as client
authentication as described in OAuth 2.0 [RFC6749] or a separate
OAuth 2.0 access token
such as the bearer token described in OAuth
2.0 Bearer Token Usage [RFC6750]. The methods of managing and
validating these authentication credentials are out of scope of this
specification.


This is quite a common operation for us so we'd like it to be as fast as possible.

This will depend on the latency introduced by the store, the operations in hydra take only a few nanoseconds.

@waynerobinson
Copy link
Author

No problem.

So any token will be allowed to validate others?

On Monday, 10 October 2016, Aeneas notifications@github.com wrote:

So has this been changed to allow the token to be able to used to
authenticate itself (and return at_ext values)?

The subject == token.Audience check was removed, apart from that nothing
changed, as at_ext was already returned by introspection and RFC7662
requires a separate token:

To prevent token scanning attacks, the endpoint MUST also require
some form of authorization to access this endpoint, such as client
authentication as described in OAuth 2.0 [RFC6749] or a
separate OAuth 2.0 access token such as the bearer token described in
OAuth
2.0 Bearer Token Usage [RFC6750]. The methods of managing and
validating these authentication credentials are out of scope of this
specification.

This is quite a common operation for us so we'd like it to be as fast as
possible.

This will depend on the latency introduced by the store, the operations in
hydra take only a few nanoseconds.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#289 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAG4pm1tRh1TjcaSQj3Uu4cNAlqz51AUks5qyerqgaJpZM4KQrC0
.

@aeneasr
Copy link
Member

aeneasr commented Oct 10, 2016

yes :)

aeneasr pushed a commit that referenced this issue Oct 16, 2016
* warden: make it clear that ladon.Request.Subject is not required or break bc and remove it - closes #270
aeneasr pushed a commit that referenced this issue Oct 17, 2016
* connections: remove connections API - closes #265
* oauth2: token revocation endpoint - closes #233
* vendor: update to fosite 0.5.0
@aeneasr aeneasr closed this as completed Oct 24, 2016
aeneasr pushed a commit that referenced this issue Oct 25, 2016
* oauth2: scopes should be separated by %20 and not +, to ensure javascript compatibility - closes #277
* oauth2/introspect: make endpoint rfc7662 compatible - closes #289
* warden: make it clear that ladon.Request.Subject is not required or break bc and remove it - closes #270
* travis: execute gox build only when new commit is a new tag - closes #285
* docs: improve introduction (#267)
* core: (health) monitoring endpoint - closes #216
* oauth2/introspect: make endpoint rfc7662 compatible - closes #289
* connections: remove connections API - closes #265
* oauth2: token revocation endpoint - closes #233
* vendor: update to fosite 0.5.0
* core: add sql support #292
* connections: remove connections API - closes #265
* all: coverage report is missing covered lines of nested packages - closes #296
* cmd: prettify the `hydra token user` output - closes #281
* travis: make it possible for travis-ci to build forked repos - closes #295
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Changes behavior in a breaking manner. bug Something is not working.
Projects
None yet
Development

No branches or pull requests

2 participants