Skip to content

fix(auth): add JWT cookie configuration options#117

Merged
javi11 merged 2 commits intojavi11:mainfrom
MythodeaLoL:main-alt
Dec 11, 2025
Merged

fix(auth): add JWT cookie configuration options#117
javi11 merged 2 commits intojavi11:mainfrom
MythodeaLoL:main-alt

Conversation

@MythodeaLoL
Copy link
Copy Markdown
Contributor

Fix JWT Cookie Domain Configuration

JWT cookies were hardcoded to domain: "localhost" because setJWTCookie() and clearJWTCookie() reloaded configuration on every call instead of using the service's already-loaded config.

Changes

  • internal/auth/service.go

    • Store loaded config in Service struct, expose via GetConfig()
    • Change default CookieDomain from "localhost" to "" (browser uses current domain)
    • Pass JWTCookieDomain and SameSiteCookie to auth.Opts during initialization
    • Fallback to "localhost" for URL construction when domain is empty
  • internal/api/auth_handlers.go

    • Use s.authService.GetConfig() instead of auth.LoadConfigFromEnv()
    • Convert http.SameSite to Fiber cookie string format ("Strict"/"Lax"/"None")
    • Apply configured SameSite value instead of hardcoded "Lax"
    • Handle http.SameSiteDefaultMode explicitly

Addressed Review Comments

  • ✅ Added explicit handling for http.SameSiteDefaultMode in sameSiteToString() function to cover all http.SameSite values

Fix: #115

Copilot AI review requested due to automatic review settings December 6, 2025 23:05
Copy link
Copy Markdown
Contributor Author

@MythodeaLoL MythodeaLoL left a comment

Choose a reason for hiding this comment

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

Working on tests.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes JWT cookie configuration by removing hardcoded values and using the service's loaded configuration instead of reloading on every cookie operation.

  • Stores auth configuration in the Service struct and exposes it via GetConfig()
  • Changes default CookieDomain from "localhost" to "" to allow browsers to use the current domain
  • Converts http.SameSite values to Fiber cookie string format and applies configured SameSite values

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
internal/auth/service.go Stores config in Service struct, adds GetConfig() method, changes default CookieDomain to empty string, and passes JWTCookieDomain and SameSiteCookie to auth.Opts
internal/api/auth_handlers.go Uses authService.GetConfig() instead of reloading config, adds sameSiteToString() helper function, and applies configured cookie settings

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/api/auth_handlers.go Outdated
@javi11 javi11 merged commit b38c5df into javi11:main Dec 11, 2025
1 check passed
drondeseries referenced this pull request in drondeseries/altmount_old Apr 16, 2026
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.

New Setup - Authentication issue

3 participants