-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #18651 from desktop/enable-path-based-accounts
Support multiple accounts for remote urls that differ in path
- Loading branch information
Showing
9 changed files
with
173 additions
and
176 deletions.
There are no files selected for viewing
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,107 +1,40 @@ | ||
| import { parseRemote } from './remote-parsing' | ||
| import { getKeyForEndpoint } from './auth' | ||
| import { TokenStore } from './stores/token-store' | ||
|
|
||
| const tryParseURLHostname = (url: string) => { | ||
| try { | ||
| return new URL(url).hostname | ||
| } catch { | ||
| return undefined | ||
| } | ||
| } | ||
|
|
||
| /** Get the hostname to use for the given remote. */ | ||
| export function getGenericHostname(remoteURL: string): string { | ||
| const parsed = parseRemote(remoteURL) | ||
| if (parsed) { | ||
| if (parsed.protocol === 'https') { | ||
| return tryParseURLHostname(remoteURL) ?? parsed.hostname | ||
| } | ||
|
|
||
| return parsed.hostname | ||
| } | ||
|
|
||
| return tryParseURLHostname(remoteURL) ?? remoteURL | ||
| } | ||
|
|
||
| export const genericGitAuthUsernameKeyPrefix = 'genericGitAuth/username/' | ||
|
|
||
| function getKeyForUsername(hostname: string): string { | ||
| return `${genericGitAuthUsernameKeyPrefix}${hostname}` | ||
| function getKeyForUsername(endpoint: string): string { | ||
| return `${genericGitAuthUsernameKeyPrefix}${endpoint}` | ||
| } | ||
|
|
||
| /** Get the username for the host. */ | ||
| export function getGenericUsername(hostname: string): string | null { | ||
| const key = getKeyForUsername(hostname) | ||
| export function getGenericUsername(endpoint: string): string | null { | ||
| const key = getKeyForUsername(endpoint) | ||
| return localStorage.getItem(key) | ||
| } | ||
|
|
||
| /** Set the username for the host. */ | ||
| export function setGenericUsername(hostname: string, username: string) { | ||
| const key = getKeyForUsername(hostname) | ||
| export function setGenericUsername(endpoint: string, username: string) { | ||
| const key = getKeyForUsername(endpoint) | ||
| return localStorage.setItem(key, username) | ||
| } | ||
|
|
||
| /** Set the password for the username and host. */ | ||
| export function setGenericPassword( | ||
| hostname: string, | ||
| endpoint: string, | ||
| username: string, | ||
| password: string | ||
| ): Promise<void> { | ||
| const key = getKeyForEndpoint(hostname) | ||
| const key = getKeyForEndpoint(endpoint) | ||
| return TokenStore.setItem(key, username, password) | ||
| } | ||
|
|
||
| /** Delete a generic credential */ | ||
| export function deleteGenericCredential(hostname: string, username: string) { | ||
| localStorage.removeItem(getKeyForUsername(hostname)) | ||
| return TokenStore.deleteItem(getKeyForEndpoint(hostname), username) | ||
| } | ||
|
|
||
| /** | ||
| * Migrate generic git credentials from the old format which could include | ||
| * a path to the new format which only includes the hostname. | ||
| */ | ||
| export async function removePathFromGenericGitAuthCreds() { | ||
| try { | ||
| for (const key of Object.keys(localStorage)) { | ||
| if (key.startsWith(genericGitAuthUsernameKeyPrefix)) { | ||
| const oldHostname = key.substring( | ||
| genericGitAuthUsernameKeyPrefix.length | ||
| ) | ||
| const slashIx = oldHostname.indexOf('/') | ||
| if (slashIx === -1) { | ||
| continue | ||
| } | ||
|
|
||
| const newHostname = oldHostname.substring(0, slashIx) | ||
| log.info(`Migrating credentials ${oldHostname} → ${newHostname}`) | ||
|
|
||
| // Don't overwrite existing credentials | ||
| if (getGenericUsername(newHostname)) { | ||
| continue | ||
| } | ||
|
|
||
| const username = getGenericUsername(oldHostname) | ||
| /** Get the password for the given username and host. */ | ||
| export const getGenericPassword = (endpoint: string, username: string) => | ||
| TokenStore.getItem(getKeyForEndpoint(endpoint), username) | ||
|
|
||
| if (!username) { | ||
| continue | ||
| } | ||
|
|
||
| const password = await TokenStore.getItem( | ||
| getKeyForEndpoint(oldHostname), | ||
| username | ||
| ) | ||
|
|
||
| if (password) { | ||
| setGenericUsername(newHostname, username) | ||
| setGenericPassword(newHostname, username, password) | ||
|
|
||
| deleteGenericCredential(oldHostname, username) | ||
| } | ||
| } | ||
| } | ||
| } catch { | ||
| log.error('Failed to remove path from generic git credentials') | ||
| } | ||
| /** Delete a generic credential */ | ||
| export function deleteGenericCredential(endpoint: string, username: string) { | ||
| localStorage.removeItem(getKeyForUsername(endpoint)) | ||
| return TokenStore.deleteItem(getKeyForEndpoint(endpoint), username) | ||
| } |
This file contains 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
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,8 @@ | ||
| import { getKeyForEndpoint } from '../auth' | ||
| import { | ||
| getSSHKeyPassphrase, | ||
| keepSSHKeyPassphraseToStore, | ||
| } from '../ssh/ssh-key-passphrase' | ||
| import { AccountsStore, TokenStore } from '../stores' | ||
| import { AccountsStore } from '../stores' | ||
| import { | ||
| ITrampolineCommand, | ||
| TrampolineCommandHandler, | ||
|
|
@@ -17,7 +16,7 @@ import { | |
| import { removePendingSSHSecretToStore } from '../ssh/ssh-secret-storage' | ||
| import { getHTMLURL } from '../api' | ||
| import { | ||
| getGenericHostname, | ||
| getGenericPassword, | ||
| getGenericUsername, | ||
| setGenericPassword, | ||
| setGenericUsername, | ||
|
|
@@ -30,6 +29,7 @@ import { | |
| setMostRecentGenericGitCredential, | ||
| } from './trampoline-environment' | ||
| import { IGitAccount } from '../../models/git-account' | ||
| import memoizeOne from 'memoize-one' | ||
|
|
||
| async function handleSSHHostAuthenticity( | ||
| prompt: string | ||
|
|
@@ -177,98 +177,135 @@ const handleAskPassUserPassword = async ( | |
| ) => { | ||
| const info = (msg: string) => log.info(`askPassHandler: ${msg}`) | ||
| const debug = (msg: string) => log.debug(`askPassHandler: ${msg}`) | ||
| const warn = (msg: string) => log.warn(`askPassHandler: ${msg}`) | ||
|
|
||
| const { trampolineToken } = command | ||
| const hostname = getGenericHostname(remoteUrl) | ||
| const account = await findAccount(trampolineToken, accountsStore, hostname) | ||
| const parsedUrl = new URL(remoteUrl) | ||
| const endpoint = urlWithoutCredentials(remoteUrl) | ||
| const account = await findAccount(trampolineToken, accountsStore, remoteUrl) | ||
|
|
||
| if (!account) { | ||
| if (getHasRejectedCredentialsForEndpoint(trampolineToken, hostname)) { | ||
| debug(`not requesting credentials for ${hostname}`) | ||
| return undefined | ||
| } | ||
|
|
||
| if (getIsBackgroundTaskEnvironment(trampolineToken)) { | ||
| debug('background task environment, skipping prompt') | ||
| return undefined | ||
| } | ||
| if (account) { | ||
| const accountKind = account instanceof Account ? 'account' : 'generic' | ||
| debug(`${accountKind} ${kind.toLowerCase()} for ${remoteUrl} found`) | ||
| return kind === 'Username' ? account.login : account.token | ||
| } | ||
|
|
||
| info(`no account found for ${hostname}`) | ||
| if (getHasRejectedCredentialsForEndpoint(trampolineToken, endpoint)) { | ||
| debug(`not requesting credentials for ${remoteUrl}`) | ||
| return undefined | ||
| } | ||
|
|
||
| if (hostname === 'github.com') { | ||
| // We don't want to show a generic auth prompt for GitHub.com and we | ||
| // don't have a good way to turn the sign in flow into a promise. More | ||
| // specifically we can create a promise that resolves when the GH sign in | ||
| // flow completes but we don't have a way to have the promise reject if | ||
| // the user cancels. | ||
| return undefined | ||
| } | ||
| if (getIsBackgroundTaskEnvironment(trampolineToken)) { | ||
| debug('background task environment, skipping prompt') | ||
| return undefined | ||
| } | ||
|
|
||
| const { username, password } = | ||
| await trampolineUIHelper.promptForGenericGitAuthentication(hostname) | ||
| info(`no account found for ${remoteUrl}`) | ||
|
|
||
| if (username.length > 0 && password.length > 0) { | ||
| setGenericUsername(hostname, username) | ||
| await setGenericPassword(hostname, username, password) | ||
| if (parsedUrl.hostname === 'github.com') { | ||
| // We don't want to show a generic auth prompt for GitHub.com and we | ||
| // don't have a good way to turn the sign in flow into a promise. More | ||
| // specifically we can create a promise that resolves when the GH sign in | ||
| // flow completes but we don't have a way to have the promise reject if | ||
| // the user cancels. | ||
| return undefined | ||
| } | ||
|
|
||
| info(`acquired generic credentials for ${hostname}`) | ||
| const { username, password } = | ||
| await trampolineUIHelper.promptForGenericGitAuthentication( | ||
| remoteUrl, | ||
| parsedUrl.username === '' ? undefined : parsedUrl.username | ||
| ) | ||
|
|
||
| return kind === 'Username' ? username : password | ||
| } else { | ||
| info('user cancelled generic git authentication') | ||
| setHasRejectedCredentialsForEndpoint(trampolineToken, hostname) | ||
| } | ||
| if (!username || !password) { | ||
| info('user cancelled generic git authentication') | ||
| setHasRejectedCredentialsForEndpoint(trampolineToken, endpoint) | ||
|
|
||
| return undefined | ||
| } else { | ||
| const accountKind = account instanceof Account ? 'account' : 'generic' | ||
| if (kind === 'Username') { | ||
| debug(`${accountKind} username for ${hostname} found`) | ||
| return account.login | ||
| } else if (kind === 'Password') { | ||
| const token = | ||
| account instanceof Account && account.token.length > 0 | ||
| ? account.token | ||
| : await TokenStore.getItem( | ||
| getKeyForEndpoint(account.endpoint), | ||
| account.login | ||
| ) | ||
|
|
||
| if (token) { | ||
| debug(`${accountKind} password for ${hostname} found`) | ||
| } else { | ||
| // We have a username but no password, that warrants a warning | ||
| warn(`${accountKind} password for ${hostname} missing`) | ||
| } | ||
|
|
||
| return token ?? undefined | ||
| } | ||
| } | ||
|
|
||
| return undefined | ||
| // Git will ordinarily prompt us twice, first for the username and then | ||
| // for the password. For the second prompt the url will contain the | ||
| // username. For example: | ||
| // Prompt 1: Username for 'https://example.com': | ||
| // < user enters username > | ||
| // Prompt 2: Password for 'https://[email protected]': | ||
| // | ||
| // So when we get a prompt that doesn't include the username we know that | ||
| // it's the first prompt. This matters because users can include the | ||
| // username in the remote url in which case Git won't even prompt us for | ||
| // the username. For example: | ||
| // https://[email protected]/org/repo/_git/repo | ||
| // | ||
| // If we're getting prompted for password directly with the username we | ||
| // don't want to store the username association, only the password. | ||
| if (parsedUrl.username === '') { | ||
| setGenericUsername(endpoint, username) | ||
| } | ||
|
|
||
| await setGenericPassword(endpoint, username, password) | ||
|
|
||
| info(`acquired generic credentials for ${remoteUrl}`) | ||
|
|
||
| return kind === 'Username' ? username : password | ||
| } | ||
|
|
||
| /** | ||
| * When we're asked for credentials we're typically first asked for the username | ||
| * immediately followed by the password. We memoize the getGenericPassword call | ||
| * such that we only call it once per endpoint/login pair. Since we include the | ||
| * trampoline token in the invalidation key we'll only call it once per | ||
| * trampoline session. | ||
| */ | ||
| const memoizedGetGenericPassword = memoizeOne( | ||
| (_trampolineToken: string, endpoint: string, login: string) => | ||
| getGenericPassword(endpoint, login) | ||
| ) | ||
|
|
||
| async function findAccount( | ||
| trampolineToken: string, | ||
| accountsStore: AccountsStore, | ||
| hostname: string | ||
| remoteUrl: string | ||
| ): Promise<IGitAccount | undefined> { | ||
| const accounts = await accountsStore.getAll() | ||
| const parsedUrl = new URL(remoteUrl) | ||
| const endpoint = urlWithoutCredentials(remoteUrl) | ||
| const account = accounts.find( | ||
| a => new URL(getHTMLURL(a.endpoint)).hostname === hostname | ||
| a => new URL(getHTMLURL(a.endpoint)).origin === parsedUrl.origin | ||
| ) | ||
|
|
||
| if (account) { | ||
| return account | ||
| } | ||
|
|
||
| const login = getGenericUsername(hostname) | ||
| const login = | ||
| parsedUrl.username === '' | ||
| ? getGenericUsername(endpoint) | ||
| : parsedUrl.username | ||
|
|
||
| if (hostname && login) { | ||
| setMostRecentGenericGitCredential(trampolineToken, hostname, login) | ||
| return { login, endpoint: hostname } | ||
| if (!login) { | ||
| return undefined | ||
| } | ||
|
|
||
| return undefined | ||
| const token = await memoizedGetGenericPassword( | ||
| trampolineToken, | ||
| endpoint, | ||
| login | ||
| ) | ||
|
|
||
| if (!token) { | ||
| // We have a username but no password, that warrants a warning | ||
| log.warn(`askPassHandler: generic password for ${remoteUrl} missing`) | ||
| return undefined | ||
| } | ||
|
|
||
| setMostRecentGenericGitCredential(trampolineToken, endpoint, login) | ||
|
|
||
| return { login, endpoint, token } | ||
| } | ||
|
|
||
| function urlWithoutCredentials(remoteUrl: string): string { | ||
| const url = new URL(remoteUrl) | ||
| url.username = '' | ||
| url.password = '' | ||
| return url.toString() | ||
| } | ||
Oops, something went wrong.