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

[RFC7662] Add introspect endpoint to introspect access & refresh token #3404

Merged
merged 10 commits into from
Mar 28, 2024

Conversation

supercairos
Copy link
Contributor

Overview

Implement RFC7662.
See https://datatracker.ietf.org/doc/html/rfc7662

What this PR does / why we need it

It implements the RFC7662.
A endpoint to determine the active state of an OAuth 2.0 token and to determine meta-information about this token.
OAuth 2.0 deployments can use this method to convey information about the authorization context of the token from the authorization server to the protected resource.

@supercairos
Copy link
Contributor Author

supercairos commented Mar 7, 2024

See issue: #3387 (for linking)

@supercairos
Copy link
Contributor Author

I've started the tests, but haven't finished. Would be happy to have a super quick review before doing all the tests @nabokihms 🙏

@supercairos supercairos force-pushed the rcaire/dex-introspect branch 2 times, most recently from 7fd9a91 to 12d37b8 Compare March 7, 2024 23:01
@supercairos supercairos changed the title [RFC7662] Add introspect endpoint to introspect access & refresh token. See issue #3387 [RFC7662] Add introspect endpoint to introspect access & refresh token Mar 7, 2024
@supercairos supercairos force-pushed the rcaire/dex-introspect branch 2 times, most recently from c87eca5 to 9a55445 Compare March 8, 2024 00:26
@supercairos
Copy link
Contributor Author

supercairos commented Mar 8, 2024

I think I've added the latests tests, feel free to review :)

Copy link
Member

@nabokihms nabokihms left a comment

Choose a reason for hiding this comment

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

Thanks, looks awesome. I have a couple of suggestions / concerns, but overall it looks good.

server/introspection.go Outdated Show resolved Hide resolved
server/server.go Outdated Show resolved Hide resolved
server/introspectionhandler.go Outdated Show resolved Hide resolved
server/introspection.go Outdated Show resolved Hide resolved
server/introspection.go Outdated Show resolved Hide resolved
server/introspectionhandler.go Outdated Show resolved Hide resolved
server/introspectionhandler.go Show resolved Hide resolved
@calderonth
Copy link

This looks good!
I think it's also missing the advertisement of the introspection_endpoint in the discovery structure and handler.

@supercairos
Copy link
Contributor Author

This looks good! I think it's also missing the advertisement of the introspection_endpoint in the discovery structure and handler.

There is actually no introspection_endpoint on the OpenID Connect discovery:
https://openid.net/specs/openid-connect-discovery-1_0.html

But I can definitely add this if you want :)

@supercairos
Copy link
Contributor Author

Ok, there is some in this RFC: https://datatracker.ietf.org/doc/html/rfc8414

I'll add it later today :)

@supercairos
Copy link
Contributor Author

This looks good! I think it's also missing the advertisement of the introspection_endpoint in the discovery structure and handler.

Done, I took the liberty of writing tests for the static handler function also :)

server/handlers.go Outdated Show resolved Hide resolved
Signed-off-by: Romain Caire <[email protected]>
server/handlers.go Outdated Show resolved Hide resolved
Copy link
Member

@nabokihms nabokihms left a comment

Choose a reason for hiding this comment

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

@supercairos, thank you, it looks good!
@calderonth, also, thank you for pointing out the discovery.

@nabokihms nabokihms merged commit 8755308 into dexidp:master Mar 28, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants