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

feat!: Issue JWT for session pre-authentication #1927

Merged
merged 1 commit into from
Oct 28, 2024

Conversation

MoritzWeber0
Copy link
Member

@MoritzWeber0 MoritzWeber0 commented Oct 25, 2024

Instead of the non-structured session token, issue a nested JWT containing session.id, user.id, user.name, user.email and user.role. More claims will be added in the future.

During session connection, the backend issues a signed JWT token. The private key is auto-generated in the backend (if it doesn't exist) and can be exchanged via new CLI endpoints.

The JWT is validated automatically for all requests to HTTP-based sessions. The JWT can be read from the ccm_session_token cookie and can be trusted by sessions. It may be used to extract user or session information in the sessions itself.

The validate_token endpoint doesn't require an active database session anymore, reducing network traffic and improving the response times. This effectively makes sessions slightly faster.

One of the best changes is for our developers: It's finally possible to reach sessions from the live-refresh environment again :)

BREAKING CHANGE: Users with active sessions have to reconnect to their sessions after the update has been rolled out. We recommend to install the update when there are no active sessions.

Copy link

github-actions bot commented Oct 25, 2024

API Changelog 4.9.1.dev2+gcd986def

This comment has been minimized.

@MoritzWeber0 MoritzWeber0 force-pushed the pre-authentication-jwt branch from 0cc22d9 to 3f102c9 Compare October 25, 2024 15:47

This comment has been minimized.

@MoritzWeber0 MoritzWeber0 force-pushed the pre-authentication-jwt branch from 3f102c9 to f61f8be Compare October 25, 2024 16:33

This comment has been minimized.

@MoritzWeber0 MoritzWeber0 force-pushed the pre-authentication-jwt branch from f61f8be to 4517e50 Compare October 25, 2024 20:20
Copy link

codecov bot commented Oct 25, 2024

Codecov Report

Attention: Patch coverage is 84.15842% with 16 lines in your changes missing coverage. Please review.

Project coverage is 84.80%. Comparing base (5cb85b9) to head (99fe4b6).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
backend/capellacollab/sessions/auth.py 72.50% 9 Missing and 2 partials ⚠️
backend/capellacollab/cli/keys.py 84.00% 2 Missing and 2 partials ⚠️
backend/capellacollab/sessions/routes.py 92.85% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1927      +/-   ##
==========================================
+ Coverage   84.73%   84.80%   +0.06%     
==========================================
  Files         208      211       +3     
  Lines        6785     6875      +90     
  Branches      466      472       +6     
==========================================
+ Hits         5749     5830      +81     
- Misses        874      879       +5     
- Partials      162      166       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This comment has been minimized.

@MoritzWeber0 MoritzWeber0 marked this pull request as ready for review October 25, 2024 20:44
@MoritzWeber0 MoritzWeber0 force-pushed the pre-authentication-jwt branch from 4517e50 to 18231ce Compare October 25, 2024 20:44

This comment has been minimized.

@MoritzWeber0 MoritzWeber0 force-pushed the pre-authentication-jwt branch from 18231ce to df53de3 Compare October 28, 2024 07:45

This comment has been minimized.

@MoritzWeber0 MoritzWeber0 requested a review from zusorio October 28, 2024 08:26
@MoritzWeber0 MoritzWeber0 force-pushed the pre-authentication-jwt branch from df53de3 to 7b43e38 Compare October 28, 2024 11:32

This comment has been minimized.

@MoritzWeber0 MoritzWeber0 force-pushed the pre-authentication-jwt branch from 7b43e38 to 3007a67 Compare October 28, 2024 13:13

This comment has been minimized.

Copy link
Member

@zusorio zusorio left a comment

Choose a reason for hiding this comment

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

LGTM

Instead of the non-structured session token, issue a JWT containing
`session_id`, `user_id`, `user_name` and `user_role`. More claims
will be added in the future.

During session connection, the backend issues a signed JWT token.
The private key is auto-generated in the backend (if it doesn't exist)
and can be exchanged via new CLI endpoints.

The JWT is validated automatically for all requests to HTTP-based
sessions. The JWT can be read from the `ccm_session_token` cookie
and can be trusted by sessions. It may be used to extract user or
session information in the sessions.

The validate_token endpoint doesn't require an active database session
anymore, reducing network traffic and improving the response times. This
effectively makes sessions faster and improved stability.

BREAKING CHANGE: Users with active sessions have to reconnect to their
sessions after the update has been rolled out. We recommend to install
the update when there are no active sessions.
@MoritzWeber0 MoritzWeber0 force-pushed the pre-authentication-jwt branch from 3007a67 to 99fe4b6 Compare October 28, 2024 14:03
Copy link

sonarcloud bot commented Oct 28, 2024

Copy link

This report was generated by comparing cd986de with 5cb85b9.
If you would like to check difference, please check here.

success

ArtifactName: reg

✨✨ That's perfect, there is no visual difference! ✨✨

item count
pass 350
change 0
new 0
delete 0

@MoritzWeber0 MoritzWeber0 merged commit 69562e6 into main Oct 28, 2024
29 of 30 checks passed
@MoritzWeber0 MoritzWeber0 deleted the pre-authentication-jwt branch October 28, 2024 14:08
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.

2 participants