refactor: reject JWT expiration value at startup if less than 1 sec - #1172
refactor: reject JWT expiration value at startup if less than 1 sec #1172ShradhaGupta31 wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1172 +/- ##
==========================================
- Coverage 50.16% 50.16% -0.01%
==========================================
Files 147 147
Lines 13574 13586 +12
==========================================
+ Hits 6810 6815 +5
- Misses 6172 6178 +6
- Partials 592 593 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR prevents a misconfigured auth.jwtExpiration / auth.redirectionJWTExpiration (zero or negative durations) from allowing the server to start and then immediately issuing already-expired JWTs, which effectively denies access to legitimate users.
Changes:
- Adds startup-time config validation for JWT expiration durations.
- Introduces sentinel errors for invalid JWT expiration settings.
- Adds unit tests covering zero/negative durations and valid defaults.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
config/config.go |
Adds Config.validate() and calls it from NewConfig() to fail fast on non-positive JWT expirations. |
config/config_test.go |
Adds tests ensuring validation rejects zero/negative expirations and accepts defaults. |
Suppressed comments (3)
config/config_test.go:133
- The test asserts via substring matching on the error string. Since validate() returns a sentinel error, use require.ErrorIs so the test remains stable if the message wording changes.
err := cfg.validate()
require.Error(t, err)
assert.Contains(t, err.Error(), "auth.jwtExpiration")
}
config/config_test.go:144
- The test asserts via substring matching on the error string. Since validate() returns a sentinel error, use require.ErrorIs so the test remains stable if the message wording changes.
err := cfg.validate()
require.Error(t, err)
assert.Contains(t, err.Error(), "auth.redirectionJWTExpiration")
}
config/config_test.go:155
- The test asserts via substring matching on the error string. Since validate() returns a sentinel error, use require.ErrorIs so the test remains stable if the message wording changes.
err := cfg.validate()
require.Error(t, err)
assert.Contains(t, err.Error(), "auth.redirectionJWTExpiration")
}
b8d9299 to
e990ba4
Compare
sudhir-intc
left a comment
There was a problem hiding this comment.
LGTM, check if wee need to keep a minimum value of atleast 1m
e990ba4 to
f6ab15d
Compare
|
@ShradhaGupta31 The current validation rejects anything under 1 minute, but the commit says it’s rejecting zero/negative expirations. Please change it to
|
f6ab15d to
365da3f
Compare
ff50cb2 to
df465df
Compare
@madhavilosetty-intel - Updated title & config.go as well. |
- Modified config.go to validate jwtExpiration while console startup - Reject non-positive values of jwtExpiration Signed-off-by: ShradhaGupta31 <shradha.gupta@intel.com>
df465df to
8526529
Compare
sudhir-intc
left a comment
There was a problem hiding this comment.
LGTM.
Please ensure the CI passes before the merge
Changes Done:
Description:
config.ymlacceptsjwtExpiration: 0swhich is a syntactically valid zero duration that the YAML parser accepts without error. At runtime, every issued JWT hasexp = time.Now(), so all tokens expire at the moment of issuance and every subsequent API call is rejected.Before fix : Server starts silently with jwtExpiration 0s, login returns an already-expired token:
After fix : Server would fail to start if jwtExpiration is set to 0s