Skip to content

Set cookies for OAuth flows when SendJWTHeader is enabled #241

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

paskal
Copy link
Collaborator

@paskal paskal commented Mar 24, 2025

This fixes umputun/remark42#1877 where OAuth authentication fails when the send-jwt-header option is enabled. The problem occurred because:

  1. When SendJWTHeader is enabled, the auth service only sends the JWT as a header
    without setting cookies
  2. During OAuth flows, the authentication involves redirects between the app and the provider
  3. HTTP headers don't persist through redirects, so the authentication state was lost

The solution:

  • Modified the jwt.go token Set method to always set cookies during OAuth handshake phases (when claims.Handshake != nil), even when SendJWTHeader is enabled
  • For normal authentication (non-handshake), maintain the original behavior where SendJWTHeader=true will only set headers
  • This ensures the OAuth flow works properly while maintaining the correct behavior for API requests

@paskal paskal requested a review from umputun as a code owner March 24, 2025 22:44
@paskal paskal marked this pull request as draft March 24, 2025 22:55
@paskal paskal force-pushed the paskal/fix_send_jwt_header branch from 28bd825 to 3905782 Compare March 24, 2025 22:57
@coveralls
Copy link

coveralls commented Mar 24, 2025

Pull Request Test Coverage Report for Build 14726384563

Details

  • 9 of 9 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 83.608%

Totals Coverage Status
Change from base Build 14460341874: 0.03%
Covered Lines: 2693
Relevant Lines: 3221

💛 - Coveralls

@paskal paskal marked this pull request as ready for review March 24, 2025 23:01
@paskal paskal marked this pull request as draft April 28, 2025 06:02
@paskal paskal force-pushed the paskal/fix_send_jwt_header branch 3 times, most recently from d800887 to 16536d5 Compare April 29, 2025 23:36
This fixes umputun/remark42#1877 where OAuth
 authentication
fails when the send-jwt-header option is enabled. The problem occurred
 because:

1. When SendJWTHeader is enabled, the auth service only sends the JWT
as a header
   without setting cookies
2. During OAuth flows, the authentication involves redirects between
the app and the
   provider
3. HTTP headers don't persist through redirects, so the authentication
 state was lost

The solution:
- Modified the jwt.go token Set method to always set cookies during
OAuth handshake
  phases (when claims.Handshake != nil), even when SendJWTHeader is
enabled
- For normal authentication (non-handshake), maintain the original
behavior where
  SendJWTHeader=true will only set headers
- This ensures the OAuth flow works properly while maintaining the
correct behavior
  for API requests
@paskal paskal force-pushed the paskal/fix_send_jwt_header branch from 16536d5 to 1e9c879 Compare April 29, 2025 23:47
@paskal
Copy link
Collaborator Author

paskal commented Apr 29, 2025

This diff doesn't work for Remark42 as-is because of reasons described in umputun/remark42#1877 (comment), but it would work oAuth for users of the library. It just seems that no one ever set this flag to hit the buggy behaviour with oAuth.

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.

SendJWTHeader leads to {"error":"failed to get token"}
2 participants