Ask about SSO only where the answer would change anything - #1217
Draft
ericmj wants to merge 2 commits into
Draft
Conversation
An organization that requires SSO expires its members' access on a clock it sets, and when that lapses the token grant stops carrying the organization's scope and says which ones it dropped for that reason. The first thing the client can do with that is ask. Asking once for the batch is the part that takes work. Every private organization a resolution can need is named in the project's own dependencies, since a published package's dependencies come from the public repository or from its own organization, so the needed set is known before any fetch. Intersecting it with what the server flagged is what makes the question "acme requires SSO authentication. Authenticate now?" instead of a 403 halfway through, and it is why a member of ten SSO organizations who depends on two is asked about two. Saying yes gets a URL bound to this session, so the session and its refresh token survive; the refresh afterwards is what picks up the scopes, since the access token that lacks them has not expired and would otherwise be used for another half hour. Saying no says what it costs and continues. The flag is stored with the token rather than kept for the run. It is learned at refresh, and a later run inside the access token's 30 minutes does not refresh, so keeping it in memory would mean the second mix deps.get 403s with nothing to explain it. CI is untouched: it authenticates as the organization, which is never governed and never flagged.
Adversarial review of the prompt found four ways it fires or fails badly. It was not gated on whether the resolution uses the stored token at all. An organization authenticated with its own key, or a build running with HEX_REPOS_KEY, fetches without ever touching it, so the prompt asked about an organization that was already working. check_and_refresh_auth one line up had that gate; this now uses the same one. Offline and HEX_API_KEY both say so instead of asking. Offline, the flow would go to the network for a URL it cannot use. With HEX_API_KEY set, the request authenticates as the key rather than as the session that needs authorizing, so the URL would come back bound to the wrong thing and the flag would never clear. A verification_uri that is not an http URL threw out of open_browser and took the resolution with it, after the user had already answered yes, and a "message" that was an object rather than a string raised on interpolation. Opening a browser is a convenience on top of a printed URL, so it no longer ends anything. The URL is in the prompt rather than beside it. Mix.Shell.Quiet drops info output and keeps prompts, so under --quiet the flow asked people to finish in a browser without telling them where.
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.
Prompts once, at the terminal, when an organization the project depends on needs its SSO session renewed.
mix deps.getalready knows every repository it needs before it fetches anything, because a published package's dependencies can only come from the public repository or the package's own organization. That set is intersected with the organizations the last token refresh flagged, so a member of ten SSO organizations who depends on two is asked about two:On yes it requests a re-authorization URI, prints it, and waits. Completing it in a browser renews the existing session rather than replacing it, so the refresh token survives and nobody re-runs device auth. On no it warns and carries on without those packages.
It asks only where the answer would change something. A repository authenticated with an organization key never touches the stored token, so nothing about it is worth asking. Offline says so instead of prompting, and
HEX_API_KEYsays it authenticates as itself.Vendors hexpm/hex_core#213, which has to merge first.