Repository navigation
cgi-bin: Port Web Interface OAuth login to device authorization grant (#1232) - #1741
Open
abubakarsabir924-cell wants to merge 1 commit into
Open
abubakarsabir924-cell wants to merge 1 commit into
abubakarsabir924-cell wants to merge 1 commit into
Conversation
Replace the single-session authorization-code (PKCE) flow in
do_login()/finish_login() with the device authorization grant flow,
following the pattern already used by ippeveprinter's OAuth support
in libcups.
Each browser session tracks its own device grant in a CUPS_DEVGRANT
cookie (the device code, polling interval, etc. as JSON). A request
to "/?LOGIN=Login":
- with no CUPS_DEVGRANT cookie: requests a new device grant
(cupsOAuthGetDeviceGrant()) and shows the user code and
verification URL via a new oauth-login.tmpl template, with a
meta-refresh timed to the server's requested polling interval.
- with a pending CUPS_DEVGRANT cookie: makes a single, non-blocking
cupsOAuthGetTokens() attempt. On success, stores the access token
in CUPS_BEARER and clears the grant cookie. While still pending
(authorization_pending/slow_down), the same page is shown again.
On a hard failure, the grant is discarded so the next visit
requests a fresh one.
A new grant is only requested on a POST (the existing Login button,
which is protected by the CSRF token), not on the GET requests used
for polling, so a stray GET can't silently start a new authorization
request against the OAuth server.
do_logout() now also clears the CUPS_DEVGRANT cookie and calls
cupsOAuthClearTokens() to clear the server-side token store, in
addition to clearing CUPS_BEARER as before.
Notes for reviewers:
- The old code's CUPS_REFERER/CUPS_REFERRER-based return-to-referrer
redirect is dropped; the mismatched env var/cookie names meant it
never actually worked, and login now always redirects to "/".
- cupsOAuthGetDeviceGrant() looks up the client ID under
CUPS_OAUTH_REDIRECT_URI; a server previously configured only for
the authorization-code Web Interface flow may need its client ID
re-registered under that redirect URI.
- Not tested against a live OAuth provider: a local environment TLS
trust issue (unrelated to this change) prevented HTTPS connections
to an IdP during development. The device-grant call pattern mirrors
tools/cups-oauth.c's existing, working usage; manual verification
against a live IdP before merging is recommended.
Contributor
Author
|
@michaelrsweet Sir, Whenever you get a chance, I'd appreciate your review on this. Let me know if you'd like any changes to the approach. |
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
1 participant
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
Implements #1232 by replacing the Web Interface's single-session OAuth authorization-code (PKCE) flow with the device authorization grant flow, following the direction from @michaelrsweet and the pattern already used by ippeveprinter's OAuth support in libcups.
Changes
Notes for reviewers
Testing