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

fix: update refresh token TTL to 30 days #1585

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

nnnkkk7
Copy link
Contributor

@nnnkkk7 nnnkkk7 commented Mar 13, 2025

fix #1579

  • Changed default refresh token TTL from 1 hour to 30 days
  • Added explicit WithRefreshTokenTTL() in service initialization

In the generateToken function, there was logic to set a default 30-day TTL but override it with the options value if present:

refreshTokenTTL := 30 * day
if s.opts.refreshTokenTTL > 0 {
    refreshTokenTTL = s.opts.refreshTokenTTL
}

However, the defaultOptions had the refresh token TTL set to just 1 hour:

var defaultOptions = options{
    refreshTokenTTL: time.Hour,
    logger:          zap.NewNop(),
}

And the service initialization did not explicitly specify the WithRefreshTokenTTL option, so the default 1-hour value was used.

The condition if s.opts.refreshTokenTTL > 0 was always true, causing the default 30-day setting to always be overridden with the 1-hour value.

@@ -825,6 +825,7 @@ func (s *server) createAuthService(
}
serviceOptions := []authapi.Option{
authapi.WithLogger(logger),
authapi.WithRefreshTokenTTL(30 * 24 * time.Hour),
Copy link
Member

Choose a reason for hiding this comment

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

Instead of passing a fixed value here, how about implementing it as a config value being passed using flags?
This way, the user can set what is best for him when hosting the project.

E.g.

refreshTokenTTL: cmd.Flag("refresh-token-ttl", "refresh token ttl").Default("168h").Duration() // 7 days

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. Thank you!
Is it ok ? feat: add refresh token TTL config

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.

chore: investigate why the console is getting a 1-hour refresh token instead of 1 month
2 participants