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

Removes RevokeHandler from JWT Introspector #155

Merged
merged 2 commits into from
Apr 11, 2017
Merged

Removes RevokeHandler from JWT Introspector #155

merged 2 commits into from
Apr 11, 2017

Conversation

wezzle
Copy link
Contributor

@wezzle wezzle commented Apr 6, 2017

RevokeHandler has been removed because it conflicts with Stateless JWT
accesstokens and revocable hmac refresh tokens. The readme has been
updated to warn users about possible misconfiguration.

Closes 154

RevokeHandler has been removed because it conflicts with Stateless JWT
accesstokens and revocable hmac refresh tokens. The readme has been
updated to warn users about possible misconfiguration.
@coveralls
Copy link

coveralls commented Apr 6, 2017

Coverage Status

Coverage increased (+0.1%) to 72.565% when pulling dd97f1b on wezzle:closes-154 into 5e1c890 on ory:master.

README.md Outdated
@@ -335,6 +336,9 @@ Of course, fosite ships handlers for all OAuth2 and OpenID Connect flows.
[Authentication using the Implicit Flow](http://openid.net/specs/openid-connect-core-1_0.html#ImplicitFlowAuth),
[Authentication using the Hybrid Flow](http://openid.net/specs/openid-connect-core-1_0.html#HybridFlowAuth)

### JWT Introspection

Please note that when using the OAuth2StatelessJWTIntrospectionFactory access token revocation is not possible.

This section is missing documentation and we welcome any contributions in that direction.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line should be above the JWT introspection section, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean this line: "This section is missing documentation and we welcome any contributions in that direction." ? I thought it was a line belonging to the documentation chapter in general. If it was meant for the "Extensible handlers" then i'll move it above the "JWT Introspection".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, it was menat for the handler section :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected in new push

@coveralls
Copy link

coveralls commented Apr 9, 2017

Coverage Status

Coverage increased (+0.1%) to 72.565% when pulling 80810e6 on wezzle:closes-154 into 5e1c890 on ory:master.

@aeneasr
Copy link
Member

aeneasr commented Apr 11, 2017

Thank you so much for your continoous effort on these issues. One last thing, we have a DOC that you need to sign by doing git commit --amend -s

Signed-off-by: Wesley Bos <wesley@dutchfrontiers.com>
@wezzle
Copy link
Contributor Author

wezzle commented Apr 11, 2017

Your welcome. I've signed off on it. I did force push it over the original but as nobody is dependent on it i don't think that is a problem.

@coveralls
Copy link

coveralls commented Apr 11, 2017

Coverage Status

Coverage increased (+0.1%) to 72.565% when pulling 4e38efa on wezzle:closes-154 into 5e1c890 on ory:master.

@aeneasr
Copy link
Member

aeneasr commented Apr 11, 2017

Yup, that's the way to go :) Thanks!

@aeneasr aeneasr merged commit 344dbef into ory:master Apr 11, 2017
@wezzle wezzle deleted the closes-154 branch April 11, 2017 09:34
budougumi0617 added a commit to budougumi0617/fosite that referenced this pull request May 10, 2019
* Removes RevokeHandler from JWT Introspector

RevokeHandler has been removed because it conflicts with Stateless JWT
accesstokens and revocable hmac refresh tokens. The readme has been
updated to warn users about possible misconfiguration.

* Moves text back to correct section

Signed-off-by: Wesley Bos <wesley@dutchfrontiers.com>
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

Successfully merging this pull request may close these issues.

3 participants