Skip to content

feat: add jwt refresh interval variable#1952

Merged
kmendell merged 1 commit intomainfrom
feat/jwt-refrsh
Mar 6, 2026
Merged

feat: add jwt refresh interval variable#1952
kmendell merged 1 commit intomainfrom
feat/jwt-refrsh

Conversation

@kmendell
Copy link
Copy Markdown
Member

@kmendell kmendell commented Mar 5, 2026

What This PR Implements

Related issue

Related Issue

Fixes # #1950

Changes Made

Testing Done

  • Development environment started: ./scripts/development/dev.sh start
  • Frontend verified at http://localhost:3000
  • Backend verified at http://localhost:3552
  • Manual testing completed (describe):
  • No linting errors (e.g., just lint all)
  • Backend tests pass: just test backend

Checklist

  • This PR is not opened from my fork’s main branch

AI Tool Used (if applicable)

AI Tool:
Assistance Level:
What AI helped with:
I reviewed and edited all AI-generated output:
I ran all required tests and manually verified changes:

Additional Context

Disclaimer Greptiles Reviews use AI, make sure to check over its work.

To better help train Greptile on our codebase, if the comment is useful and valid Like the comment, if its not helpful or invalid Dislike

Greptile Summary

This PR adds JWT refresh token expiry duration configurability via the JWT_REFRESH_EXPIRY environment variable, replacing the previously hardcoded 7 * 24 * time.Hour value.

Key Implementation Details:

  • Adds JWTRefreshExpiry time.Duration to Config struct with default:"168h" tag
  • Extends setFieldValueInternal with robust time.Duration handling: validates parsed values, rejects non-positive durations, logs warnings with the invalid supplied value, and falls back to the field's own struct-tag default
  • Uses a reusable applyDurationDefault closure to avoid duplication across error branches and to support future time.Duration config fields without hardcoding
  • Wires cfg.JWTRefreshExpiry into NewAuthService, cleanly replacing the hardcoded literal
  • Adds comprehensive test coverage: 6 subtests for default/custom/compound/invalid/zero/negative cases, plus a unit test verifying generic tag-based fallback
  • Documents the option in .env.example with clear guidance on accepted Go duration format

All edge cases are properly handled and tested. The fallback logic uses the field's own struct tag rather than any hardcoded value, making the implementation safe for future time.Duration config additions.

Confidence Score: 5/5

  • This PR is safe to merge. The change is well-bounded, backward-compatible, and all edge cases are covered by tests.
  • The implementation is clean and idiomatic. All six edge cases (default, custom, compound, invalid, zero, negative) are explicitly tested. The fallback logic uses the struct-tag default rather than any hardcoded value, making it safe for future time.Duration config additions. The rename to setFieldValueInternal is consistently applied across definition and both call sites. No regressions to existing field parsing (string, bool, uint32, int, custom types) are introduced.
  • No files require special attention.

Last reviewed commit: e29e95d

Copy link
Copy Markdown
Member Author

kmendell commented Mar 5, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@kmendell kmendell marked this pull request as ready for review March 5, 2026 01:08
@kmendell kmendell requested a review from a team March 5, 2026 01:08
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 5, 2026

🔍 Deadcode Analysis

Found 3 unreachable functions in the backend.

View details
pkg/libarcane/edge/transport.go:59:6: unreachable func: GetActiveTunnelTransport
pkg/utils/stdcopy/stdcopy.go:56:21: unreachable func: stdWriter.Write
pkg/utils/stdcopy/stdcopy.go:91:6: unreachable func: NewStdWriter

Only remove deadcode that you know is 100% no longer used.

Analysis from commit 16c1ca5

@getarcaneappbot
Copy link
Copy Markdown
Contributor

getarcaneappbot commented Mar 5, 2026

Container images for this PR have been built successfully!

  • Manager: ghcr.io/getarcaneapp/arcane:pr-1952
  • Agent: ghcr.io/getarcaneapp/arcane-headless:pr-1952

Built from commit e29e95d

Comment thread backend/internal/config/config.go
Comment thread backend/internal/config/config_test.go
@kmendell
Copy link
Copy Markdown
Member Author

kmendell commented Mar 5, 2026

@greptileai

Comment thread backend/internal/config/config.go Outdated
Comment thread backend/internal/config/config_test.go Outdated
@kmendell
Copy link
Copy Markdown
Member Author

kmendell commented Mar 5, 2026

@greptileai

Comment thread backend/internal/config/config.go Outdated
Comment thread backend/internal/config/config.go
@kmendell
Copy link
Copy Markdown
Member Author

kmendell commented Mar 5, 2026

@greptileai

Comment thread backend/internal/config/config.go Outdated
Comment thread backend/internal/config/config.go
@kmendell
Copy link
Copy Markdown
Member Author

kmendell commented Mar 5, 2026

@greptileai

@kmendell kmendell merged commit efec0c9 into main Mar 6, 2026
16 checks passed
@kmendell kmendell deleted the feat/jwt-refrsh branch March 6, 2026 00:38
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