Repository navigation
feat: let desktop apps and CLIs sign in with a loopback redirect - #589
Closed
RamonGebben wants to merge 1 commit into
Closed
RamonGebben wants to merge 1 commit into
RamonGebben wants to merge 1 commit into
Conversation
The device authorize flow only redirected to devnull://, so only the phone app could sign in. A desktop app, a TUI or another CLI on the user's machine can now use a loopback redirect_uri (http://127.0.0.1:<port>/... or http://[::1]:<port>/...), as RFC 8252 recommends for native apps. The loopback address is parsed instead of prefix-matched, so http://127.0.0.1:80@evil.example can't send a code to another host. Other URL schemes for future apps stay a one-line config change. Either way the code only works with the key it was issued for and the PKCE verifier.
RamonGebben
marked this pull request as ready for review
October 19, 2026 11:44
Author
|
Closing because issue would be solved by #585 in a way better way. Making this obsolete. |
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.
Why
Signing in a device (
/mobile/authorize→POST /api/v1/devices) only redirected todevnull://, the iOS app's URL scheme. So phones were the only devices that could sign in. A TUI, another CLI or a desktop app can't register a custom URL scheme reliably, but it can listen on a local port. RFC 8252 (OAuth for native apps) recommends exactly that.This PR lets any native app on the user's machine sign in as a device, next to the phone app.
What changes
redirect_urimay now behttp://127.0.0.1:<port>/…orhttp://[::1]:<port>/…, besides thedevnull://prefix.http://127.0.0.1:toredirect_uri_prefixeswould be unsafe, becausestarts_withacceptshttp://127.0.0.1:80@evil.example/callback, whose real host isevil.example. The newApp\Rules\DeviceRedirectUriparses the URL instead. It requires schemehttp, host exactly127.0.0.1or[::1], a port, and no user, password or fragment.localhostis rejected, as RFC 8252 prefers the literal IP.config('mobile.redirect_uri_prefixes'). The rule is device-neutral, and both the authorize page and the Approve POST use it throughparameterRules().Out of scope: the
/mobile/...route and*MobileAuthorization*class names still say "mobile". Renaming them is a bigger, separate change.Tests
http://127.0.0.1:53682/callback,http://[::1]:53682/callback.http://127.0.0.1:80@evil.example/x,http://localhost:1234/x,https://127.0.0.1:1/x,http://127.0.0.1.evil.example/x, a loopback URL without a port, and a loopback URL with a fragment.redirect_urisends a 303 to it withcodeandstate.composer test:lint,test:typesandtest:type-coveragepass. Pest passes with CI's flags (2991 tests, 100% coverage), and so doesbun run test:unit.