Remove hardcoded 14-day token lifetime from auth cookie handling - #3732
Conversation
setCookie fell back to a hardcoded DEFAULT_TOKEN_LIFE of 14 days when no expiration was supplied. With the login token lifetime moving from two weeks to four weeks, that fallback would silently keep cookies at the old 14-day value. The production login and logout paths already pass an explicit expires sourced from the auth service, so they are unaffected. The only caller relying on the fallback was the localhost dev token-injection dialog. - Remove DEFAULT_TOKEN_LIFE and require an integer expires in setCookie (throws otherwise), so cookie expiry must always come from the auth service rather than a hardcoded client-side lifetime. - Dev token-injection dialog now fetches the token's real expiration via getTokenInfo and passes it through; invalid tokens are reported. - Update authSpec setCookie call to pass an explicit expires.
|
I am not at all qualified to review this... |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3732 +/- ##
===========================================
+ Coverage 25.92% 25.94% +0.01%
===========================================
Files 461 461
Lines 46697 46700 +3
===========================================
+ Hits 12106 12116 +10
+ Misses 34591 34584 -7
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
briehl
left a comment
briehl
left a comment
There was a problem hiding this comment.
Ok, lots of Narrative infrastructure updates later, this builds and tests fine.
Let's merge and try it on CI.
3 participants
Summary
KBase is extending the login token lifetime from two weeks to four weeks.
api/auth.jsdefinedDEFAULT_TOKEN_LIFE = 14 * 24 * 60 * 60 * 1000and used it as a fallback cookie expiration insetCookiewhenever noexpireswas supplied. After the change, that fallback would silently keep cookies pinned to the old 14-day value.Rather than bump the constant to 28 days, this removes it. Cookie expiry should always come from the auth service (the token's real expiration), never a hardcoded client-side lifetime.
Why this is safe
setAuthToken) and logout path (removeCookie) already pass an explicitexpires, so they are unaffected.Changes
kbase-extension/static/kbase/js/api/auth.js— removeDEFAULT_TOKEN_LIFE;setCookienow requires an integerexpiresand throws if one isn't provided. Docstring updated.kbase-extension/static/kbase/js/narrativeLogin.js— the dev token-injection dialog now looks up the pasted token's real expiration viagetTokenInfoand passes it tosetCookie; an invalid/unvalidatable token is reported instead of silently setting a bad cookie.test/unit/spec/api/authSpec.js— the directsetCookiecall (backup-cookie simulation) now passes an explicitexpires.Context
Companion to kbase/ui#259, which updates the user-facing "two weeks" copy. Reviewed narrative + kbase-ui (all plugins); this was the only hardcoded lifetime assumption in narrative. All other expiry logic already reads the server-provided value.