Repository navigation
Conversation
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.
The browser (authorization code + PKCE) login hardcoded the redirect URI
http://localhost:8400/callbackand listened only on127.0.0.1:8400. That ruled out public client apps whose registered redirect URI has no path, such as Microsoft's own "Microsoft Graph Command Line Tools" app (14d82eec-204b-4c2f-b7e8-296a70dab67e, registeredhttp://localhost), because the identity platform ignores the port of a localhost redirect URI but matches the path exactly. It also failed when port 8400 was busy, and missed the callback when the browser resolvedlocalhostto::1.Behaviour changes:
--redirect-uri,TEAMS_CLI_REDIRECT_URIand the profile'sredirect_uriselect the redirect URI forauth login,auth consent-urlandauth doctor, with the same precedence as the client and tenant IDs. A delegated login saves the flag value to the profile with the IDs; the env value is used but not saved.127.0.0.1and::1forlocalhost) and accepts the callback on any path. A URI must behttpand point atlocalhostor a loopback IP literal; anything else is invalid input (exit 2) before a browser opens. A port held by another process is still an error, since that process could receive the authorization code.http://localhost/callbackon a free port instead ofhttp://localhost:8400/callback, so a busy port 8400 no longer blocks login. Registrations withhttp://localhost:8400/callbackkeep working because the port is ignored.Usage:
Review guide:
27057f8the configurable URI:resolve_redirect_uriandremember_registrationinconfig.rs,loopback_addrsandbind_loopbackinauth_code_pkce.rs.d8960cdthe free port:bind_loopbacknow returns the shared port,redirect_uri_with_portrewrites the URI, and the authorize URL is built after binding.Testing: unit tests cover URI validation, the shared free port across listeners, the busy-port error and the URI rewrite; CLI tests cover the flag, env, profile and default for
consent-urlanddoctor, and that a bad URI exits 2 for both the browser and device code flows. Which URIs the two apps accept was checked read-only against the authorize endpoint withprompt=none(a registered URI gets alogin_requiredredirect, an unregistered one gets an inline error): the OSO app acceptshttp://localhost/callbackon any port, the Microsoft app acceptshttp://localhoston any port, and neither accepts127.0.0.1. No live login was run from this branch; the first real sign-in is the remaining check.