-
Notifications
You must be signed in to change notification settings - Fork 7
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
Token Introspection #94
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few comments, and adding it to CHANGES
but looks good!
We should make sure this closes #33. |
Didn't we discuss moving token introspection to its own endpoint with its own rel value? If an implementation wants to use the same endpoint for both it would be possible with |
I don't recall this. If you look as what you yourself wrote in #33 we decided to use the same endpoint. |
@aaronpk @jamietanna Hopefully made the suggested changes. |
public/source/index.php
Outdated
@@ -698,12 +698,13 @@ | |||
</ul> | |||
<p>Note that the request to the endpoint will not contain any user-identifying information, so the resource server (e.g. Micropub endpoint) will need to know via out-of-band methods which token endpoint is in use.</p> | |||
|
|||
<p>The resource server SHOULD make a POST request to the token endpoint containing the Bearer token in the <code>token</code> parameter, which will generate a token verification response.</p> | |||
<p>The resource server SHOULD make a POST request to the token endpoint containing the Bearer token in the <code>token</code> parameter, which will generate a token verification response. The endpoint MUST also require a HTTP Authorization header with a separate OAuth 2.0 access token to allow the call to this endpoint. If the token does not contain sufficient privileges or is otherwise invalid for the request, the authorization server MUST respond with an HTTP 401 code.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I agree with this - it could be provided using HTTP Basic, not a full access token. I'd prefer saying it could be any method of HTTP Authorization
Rewrite to support separate endpoint using metadata proposal #43 once PR for that is merged. |
f2a1794
to
98b1854
Compare
Refreshed PR to accommodate metadata endpoint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of minor tweaks, otherwise LGTM!
This is a pretty big change to the existing behavior, but essentially it's doing it by adding a new introspection feature with a new introspection endpoint, and removing the old introspection feature at the token endpoint. I'm going to add a little warning to the section to refer to the previous behavior. |
As per the proposal at the 2021 Popup, this extends the token verification response to match the token introspection endpoint parameters, and extends the request to support the POST method used in that extension. It does not remove GET, but retains it as an alternative, but aligns the response to same. It also changes trhe response to an invalid token to a 200 in line with the spec.
Closes #33.