Repository navigation
Fix cupsOAuthGetUserId cache key, lifetime, and status checks - #190
Open
anabobillo3 wants to merge 1 commit into
Open
anabobillo3 wants to merge 1 commit into
anabobillo3 wants to merge 1 commit into
Conversation
michaelrsweet
requested changes
Oct 22, 2026
michaelrsweet
left a comment
Member
There was a problem hiding this comment.
Needs work, and I really don't want to add more (separate) tests of the caching code.
Will provide more feedback as time allows (dealing with a rather high volume of reports these days with no end in sight...)
- oauth_make_path: hash the whole access token for "urn:token:" URIs instead of the text before the first '/' (and at most 1023 bytes), so different tokens no longer share a cached user ID. - cupsOAuthGetUserId: save an expiration time with the cached user ID (the access token's expiration time from cupsOAuthSaveTokens, otherwise five minutes) and get it again from the userinfo endpoint once it expires; only use a 200 OK response containing a "sub" claim; cache the userinfo claims as JSON since cupsJWTExportString no longer exports an unsigned JWT; close the userinfo connection.
anabobillo3
force-pushed
the
oauth-userid-cache
branch
from
October 22, 2026 14:13
1a1347d to
9c39d05
Compare
Contributor
Author
|
Thanks for taking a look. I've removed the separate cache test and the Makefile change, so the PR now only touches |
This branch has not been deployed
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
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.
cupsOAuthGetUserIdkeeps the user ID for an access token in the OAuth store, keyed by the token'surn:token:URI. Refiled from GHSA-84cm-m3c6-56gp as a regular report, as asked there.What it does now (master 5e60665):
oauth_make_pathhandles aurn:token:URI the way it handles an IPPS URI: it copies the text from offset 7 (en:and the token) into a 1024-byte buffer, cuts it at the first/and hashes the rest. So the key covers only the token up to its first/, and at most its first 1020 characters; two access tokens that agree up to there share one cached user ID.cupsOAuthSaveTokensstores for an access token has no expiration time, andcupsOAuthGetUserIdreturns a cached value without checking one, so it is still returned after the access token'saccess_expireshas passed.cupsOAuthClearTokensclears the user ID stored for the resource URI, not the one stored for the access token.userinfo_endpointresponse is used whenever its JSON has noerrormember, whatever the HTTP status, and without asubclaim. The result is then not cached, sincecupsJWTExportStringreturnsNULLfor the unsigned JWT andoauth_save_valuetakesNULLas a delete. The connection to the endpoint is not closed.What this changes:
oauth_make_pathhashes the wholeurn:token:URI.oauth_save_user_id, used bycupsOAuthSaveTokensandcupsOAuthGetUserId, stores the user ID followed by an expiration time, as the access token is stored: the access token's expiration time fromcupsOAuthSaveTokens, otherwise five minutes (_CUPS_OAUTH_USERID_TIMEOUT).cupsOAuthGetUserIduses a cached user ID only before that time and only with asubclaim; otherwise it removes it and asks the userinfo endpoint again. A user ID cached by an earlier version has no expiration time, so it is fetched again once.subclaim is used. Its claims are cached as JSON (cupsJSONExportString) and made into a JWT again withcupsJWTNewwhen loaded. The connection is closed.cupsOAuthGetUserIddocumentation says how long the user ID is cached.Testing: checked offline with a private store:
cupsOAuthGetUserIdreturns the cached user ID for its own access token, but no longer for a different token with the same text before its/, for a 1100-byte token that differs only in its last character, or once the access token has expired (without theoauth.cchanges it returns the cached ID in those three cases).make testresults are unchanged. As asked in the review, the PR adds no separate test of the caching code.Assisted-by: Claude Code:claude-opus-5-5