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

Openid Connect implementation #262

Merged
merged 5 commits into from
Nov 8, 2023
Merged

Openid Connect implementation #262

merged 5 commits into from
Nov 8, 2023

Conversation

xerbalind
Copy link
Member

@xerbalind xerbalind commented Oct 26, 2023

It follows Authorization Code Flow using Oauth2.
The id_token is not yet signed as it should be, described in the specification. IMPLEMENTED
UserInfo does not yet contain the sub claim (see the specification).
The id field could be replaced with sub at /current_user or another endpoint could be added specific for OpenId Connect.

Support for scopes might be handy.

With this pull request gitea is happy 😄 (gitea does not even check the id_token signature)

@xerbalind xerbalind force-pushed the openidconnect branch 2 times, most recently from 86c9926 to e684bf4 Compare October 26, 2023 16:23
Copy link
Collaborator

@rien rien left a comment

Choose a reason for hiding this comment

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

If you want to implement OpenID with JWT completely, you might want to check out the https://github.com/Keats/jsonwebtoken crate, it seems to be the most popular crate for creating and veryfing signed JWT tokens.

I'll take a deeper look into this later, but I think we might want to add another check to see if the id_token is implemented correctly.

src/controllers/oauth_controller.rs Outdated Show resolved Hide resolved
@xerbalind xerbalind requested a review from rien October 31, 2023 16:57
@xerbalind xerbalind requested a review from NuttyShrimp November 6, 2023 23:08
Copy link
Collaborator

@rien rien left a comment

Choose a reason for hiding this comment

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

Looking good!

The OAuth test code is cleaner now as well, thanks.

@xerbalind xerbalind merged commit 68e900b into main Nov 8, 2023
3 checks passed
@xerbalind xerbalind deleted the openidconnect branch November 8, 2023 22:15
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