Remove hardcoded 14-day token lifetime from auth cookie handling#3732
Open
dauglyon wants to merge 1 commit into
Open
Remove hardcoded 14-day token lifetime from auth cookie handling#3732dauglyon wants to merge 1 commit into
dauglyon wants to merge 1 commit into
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.
|
Member
|
I am not at all qualified to review this... |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.