diff --git a/app/src/lib/api.ts b/app/src/lib/api.ts index a5935eaba07..012f021e3c4 100644 --- a/app/src/lib/api.ts +++ b/app/src/lib/api.ts @@ -27,6 +27,7 @@ import { import { HttpStatusCode } from './http-status-code' import { CopilotError } from './copilot-error' import { BypassReasonType } from '../ui/secret-scanning/bypass-push-protection-dialog' +import { enableMultipleLoginAccounts } from './feature-flag' import { assertNever } from './fatal-error' const envEndpoint = process.env['DESKTOP_GITHUB_DOTCOM_API_ENDPOINT'] @@ -190,6 +191,7 @@ export interface IAPIRepository { readonly ssh_url: string readonly html_url: string readonly name: string + readonly login?: string readonly owner: IAPIIdentity readonly private: boolean | null // null if unknown readonly fork: boolean @@ -237,6 +239,7 @@ export interface IAPIRepositoryCloneInfo { /** Canonical clone URL of the repository. */ readonly url: string + readonly login?: string /** * Default branch of the repository, if any. This is usually either retrieved * from the API for GitHub repositories, or undefined for other repositories. @@ -284,6 +287,7 @@ export interface IBitbucketAPIRepository readonly parent?: IBitbucketAPIRepository readonly has_issues: boolean readonly updated_on: string + readonly login?: string readonly mainbranch: { readonly name: string } @@ -308,6 +312,7 @@ function toIAPIRepository(repo: IBitbucketAPIRepository): IAPIRepository { ssh_url: sshUrl, html_url: repo.links.html.href, name: repo.name, + login: repo.login, owner: toIAPIIdentity(repo.owner), private: repo.is_private, fork: false, @@ -1077,6 +1082,7 @@ function toIAPIEmailFromGitLab( export interface IGitLabAPIRepository { readonly id: number readonly name: string + readonly login: string readonly path: string readonly path_with_namespace: string readonly web_url: string @@ -1104,6 +1110,7 @@ function toIAPIRepositoryFromGitLab( ssh_url: repo.ssh_url_to_repo, html_url: repo.web_url, name: repo.path, + login: repo.login, owner: { id: repo.owner?.id ?? 0, login: ownerLogin, @@ -1392,12 +1399,17 @@ interface IAPIAliveWebSocket { readonly url: string } -type TokenInvalidatedCallback = (endpoint: string, token: string) => void +type TokenInvalidatedCallback = ( + endpoint: string, + token: string, + login?: string +) => void type TokenRefreshedCallback = ( endpoint: string, token: string, refreshToken: string, - expiresAt: number + expiresAt: number, + login?: string ) => void export interface IAPICreatePushProtectionBypassResponse { @@ -1428,9 +1440,13 @@ export class API { this.tokenRefreshedListeners.add(callback) } - protected static emitTokenInvalidated(endpoint: string, token: string) { + protected static emitTokenInvalidated( + endpoint: string, + token: string, + login?: string + ) { this.tokenInvalidatedListeners.forEach(callback => - callback(endpoint, token) + callback(endpoint, token, login) ) } @@ -1438,10 +1454,11 @@ export class API { endpoint: string, token: string, refreshToken: string, - expiresAt: number + expiresAt: number, + login?: string ) { this.tokenRefreshedListeners.forEach(callback => - callback(endpoint, token, refreshToken, expiresAt) + callback(endpoint, token, refreshToken, expiresAt, login) ) } @@ -1464,7 +1481,12 @@ export class API { ) case 'dotcom': case 'enterprise': - return new API(account.endpoint, account.token, account.copilotEndpoint) + return new API( + account.endpoint, + account.token, + account.copilotEndpoint, + account.login + ) default: assertNever(account.apiType, 'Unknown API type') } @@ -1472,6 +1494,7 @@ export class API { protected endpoint: string protected token: string + protected login?: string private copilotEndpoint?: string private refreshTokenPromise?: Promise @@ -1479,11 +1502,13 @@ export class API { public constructor( endpoint: string, token: string, - copilotEndpoint?: string + copilotEndpoint?: string, + login?: string ) { this.endpoint = endpoint this.token = token this.copilotEndpoint = copilotEndpoint + this.login = login } public getToken() { @@ -1648,7 +1673,8 @@ export class API { public async fetchRepositoryCloneInfo( owner: string, name: string, - protocol: GitProtocol | undefined + protocol: GitProtocol | undefined, + login?: string ): Promise { const response = await this.ghRequest('GET', `repos/${owner}/${name}`, { // Make sure we don't run into cache issues when fetching the repositories, @@ -1663,6 +1689,7 @@ export class API { const repo = await parsedResponse(response) return { url: protocol === 'ssh' ? repo.ssh_url : repo.clone_url, + login, defaultBranch: repo.default_branch, } } @@ -2474,7 +2501,8 @@ export class API { body?: Object customHeaders?: Object reloadCache?: boolean - } = {} + } = {}, + login?: string ): Promise { const expiration = this.getTokenExpiration() if (expiration !== null && expiration.getTime() < Date.now()) { @@ -2489,7 +2517,8 @@ export class API { path, options.body, { ...this.getExtraHeaders(), ...options.customHeaders }, - options.reloadCache + options.reloadCache, + login ) } @@ -2506,7 +2535,13 @@ export class API { reloadCache?: boolean } = {} ): Promise { - const response = await this.request(this.endpoint, method, path, options) + const response = await this.request( + this.endpoint, + method, + path, + options, + this.login + ) this.checkTokenInvalidated(response) @@ -2926,7 +2961,8 @@ export class BitbucketAPI extends API { this.endpoint, this.token, this.apiRefreshToken, - this.expiresAt.getTime() + this.expiresAt.getTime(), + this.login ) } catch (e) { log.warn('refreshOAuthTokenBitbucket failed', e) @@ -3282,7 +3318,8 @@ export class GitLabAPI extends API { this.endpoint, this.token, this.apiRefreshToken, - this.expiresAt.getTime() + this.expiresAt.getTime(), + this.login ) } catch (e) { log.warn('refreshOAuthTokenGitLab failed', e) @@ -3603,7 +3640,8 @@ export async function fetchUser( endpoint: string, token: string, refreshToken: string, - expiresAt: number + expiresAt: number, + login?: string ): Promise { let api: API if (endpoint === getBitbucketAPIEndpoint()) { @@ -3611,7 +3649,7 @@ export async function fetchUser( } else if (endpoint === getGitLabAPIEndpoint()) { api = GitLabAPI.get(token, refreshToken, expiresAt) } else { - api = new API(endpoint, token) + api = new API(endpoint, token, undefined, login) } try { const [user, emails, copilotInfo, features] = await Promise.all([ @@ -3757,9 +3795,49 @@ export function getGitLabAPIEndpoint(): string { /** Get the account for the endpoint. */ export function getAccountForEndpoint( accounts: ReadonlyArray, - endpoint: string + endpoint: string, + login?: string, + strict: boolean = false +): Account | null { + if (login !== undefined && login === '') { + // TODO: This is here temporarily for debugging, remove it when we're sure this isn't a possibility + log.error(`Empty string is not a valid login`) + } + + const result = accounts.find( + a => + a.endpoint === endpoint && + ((strict !== true && login === undefined) || a.login === login) + ) + + if (login !== undefined && result === undefined) { + // TODO: This is here temporarily for debugging, remove it when we're sure this isn't a possibility + log.warn(`Could not find an account to match ${login}@${endpoint}`) + } + + return result || null +} + +export function getAccountForEndpointToken( + accounts: ReadonlyArray, + endpoint: string, + token: string +): Account | null { + return ( + accounts.find( + a => + a.endpoint === endpoint && + (!enableMultipleLoginAccounts() || a.token === token) + ) || null + ) +} + +/** Get the account for the login. */ +export function getAccountForLogin( + accounts: ReadonlyArray, + login: string ): Account | null { - return accounts.find(a => a.endpoint === endpoint) || null + return accounts.find(a => a.login === login) || null } export function getOAuthAuthorizationURL( diff --git a/app/src/lib/auth.ts b/app/src/lib/auth.ts index 8da93338f88..51411cca433 100644 --- a/app/src/lib/auth.ts +++ b/app/src/lib/auth.ts @@ -1,8 +1,13 @@ import { Account } from '../models/account' +import { enableMultipleLoginAccounts } from './feature-flag' /** Get the auth key for the user. */ export function getKeyForAccount(account: Account): string { - return getKeyForEndpoint(account.endpoint) + if (enableMultipleLoginAccounts()) { + return getKeyForEndpointAndLogin(account.endpoint, account.login) + } else { + return getKeyForEndpoint(account.endpoint) + } } /** Get the auth key for the endpoint. */ @@ -11,3 +16,13 @@ export function getKeyForEndpoint(endpoint: string): string { return `${appName} - ${endpoint}` } + +/** Get the auth key for the endpoint. */ +export function getKeyForEndpointAndLogin( + endpoint: string, + login: string | undefined +): string { + const appName = __DEV__ ? 'GitHub Desktop Dev' : 'GitHub' + + return `${appName} - ${endpoint} - ${login}` +} diff --git a/app/src/lib/databases/repositories-database.ts b/app/src/lib/databases/repositories-database.ts index 15b00321dc1..56e18bb841d 100644 --- a/app/src/lib/databases/repositories-database.ts +++ b/app/src/lib/databases/repositories-database.ts @@ -34,6 +34,7 @@ export interface IDatabaseGitHubRepository { readonly isArchived?: boolean readonly permissions?: 'read' | 'write' | 'admin' | null + readonly login?: string } /** A record to track the protected branch information for a GitHub repository */ @@ -69,6 +70,7 @@ export interface IDatabaseRepository { * of Git and GitHub. */ readonly isTutorialRepository?: boolean + readonly login?: string } /** diff --git a/app/src/lib/desktop-fake-repository.ts b/app/src/lib/desktop-fake-repository.ts index 704de73ee28..f679f5c1dae 100644 --- a/app/src/lib/desktop-fake-repository.ts +++ b/app/src/lib/desktop-fake-repository.ts @@ -19,5 +19,6 @@ export const DesktopFakeRepository = new Repository( false, desktopUrl ), - true + true, + '' ) diff --git a/app/src/lib/feature-flag.ts b/app/src/lib/feature-flag.ts index 39889ea5368..2440739c11d 100644 --- a/app/src/lib/feature-flag.ts +++ b/app/src/lib/feature-flag.ts @@ -109,6 +109,7 @@ export const enableResizingToolbarButtons = () => true export const enableFilteredChangesList = () => true export const enableMultipleEnterpriseAccounts = () => true +export const enableMultipleLoginAccounts = () => true export const enableCommitMessageGeneration = (account: Account) => { return ( diff --git a/app/src/lib/find-account.ts b/app/src/lib/find-account.ts index 38717afbaae..5927100eb49 100644 --- a/app/src/lib/find-account.ts +++ b/app/src/lib/find-account.ts @@ -6,7 +6,8 @@ import { Account, isDotComAccount } from '../models/account' type RepositoryLookupFunc = ( account: Account, owner: string, - name: string + name: string, + login?: string ) => Promise /** @@ -38,7 +39,8 @@ async function canAccessRepositoryUsingAPI( export async function findAccountForRemoteURL( urlOrRepositoryAlias: string, accounts: ReadonlyArray, - canAccessRepository: RepositoryLookupFunc = canAccessRepositoryUsingAPI + canAccessRepository: RepositoryLookupFunc = canAccessRepositoryUsingAPI, + login?: string ): Promise { const allAccounts = [...accounts, Account.anonymous()] @@ -58,7 +60,23 @@ export async function findAccountForRemoteURL( allAccounts.find(a => { const htmlURL = getHTMLURL(a.endpoint) const parsedEndpoint = URL.parse(htmlURL) - return parsedURL.hostname === parsedEndpoint.hostname + if (login !== undefined && login === '') { + // TODO: This is here temporarily for debugging, remove it when we're sure this isn't a possibility + log.error(`Empty string is not a valid login`) + } + + const result = + parsedURL.hostname === parsedEndpoint.hostname && + (login === undefined || a.login === login) + + if (login !== undefined && result === undefined) { + // TODO: This is here temporarily for debugging, remove it when we're sure this isn't a possibility + log.warn( + `Could not find an account to match ${login}@${parsedURL.hostname}` + ) + } + + return result }) || null // If we find an account whose hostname matches the URL to be cloned, it's @@ -99,7 +117,7 @@ export async function findAccountForRemoteURL( } } - const canAccess = await canAccessRepository(account, owner, name) + const canAccess = await canAccessRepository(account, owner, name, login) if (canAccess) { return account } diff --git a/app/src/lib/generic-git-auth.ts b/app/src/lib/generic-git-auth.ts index 4eb5344f34f..e735d4d6d62 100644 --- a/app/src/lib/generic-git-auth.ts +++ b/app/src/lib/generic-git-auth.ts @@ -1,4 +1,5 @@ -import { getKeyForEndpoint } from './auth' +import { getKeyForEndpoint, getKeyForEndpointAndLogin } from './auth' +import { enableMultipleLoginAccounts } from './feature-flag' import { TokenStore } from './stores/token-store' export const genericGitAuthUsernameKeyPrefix = 'genericGitAuth/username/' @@ -25,7 +26,9 @@ export function setGenericPassword( username: string, password: string ): Promise { - const key = getKeyForEndpoint(endpoint) + const key = enableMultipleLoginAccounts() + ? getKeyForEndpointAndLogin(endpoint, username) + : getKeyForEndpoint(endpoint) return TokenStore.setItem(key, username, password) } @@ -40,10 +43,20 @@ export function setGenericCredential( /** Get the password for the given username and host. */ export const getGenericPassword = (endpoint: string, username: string) => - TokenStore.getItem(getKeyForEndpoint(endpoint), username) + TokenStore.getItem( + enableMultipleLoginAccounts() + ? getKeyForEndpointAndLogin(endpoint, username) + : getKeyForEndpoint(endpoint), + username + ) /** Delete a generic credential */ export function deleteGenericCredential(endpoint: string, username: string) { localStorage.removeItem(getKeyForUsername(endpoint)) - return TokenStore.deleteItem(getKeyForEndpoint(endpoint), username) + return TokenStore.deleteItem( + enableMultipleLoginAccounts() + ? getKeyForEndpointAndLogin(endpoint, username) + : getKeyForEndpoint(endpoint), + username + ) } diff --git a/app/src/lib/get-account-for-repository.ts b/app/src/lib/get-account-for-repository.ts index f761eefa9ad..8e08850a59e 100644 --- a/app/src/lib/get-account-for-repository.ts +++ b/app/src/lib/get-account-for-repository.ts @@ -5,12 +5,18 @@ import { getAccountForEndpoint } from './api' /** Get the authenticated account for the repository. */ export function getAccountForRepository( accounts: ReadonlyArray, - repository: Repository + repository: Repository, + strict: boolean = false ): Account | null { const gitHubRepository = repository.gitHubRepository if (!gitHubRepository) { return null } - return getAccountForEndpoint(accounts, gitHubRepository.endpoint) + return getAccountForEndpoint( + accounts, + gitHubRepository.endpoint, + gitHubRepository.login, + strict + ) } diff --git a/app/src/lib/git/clone.ts b/app/src/lib/git/clone.ts index 954f85cc078..6a8f1c6d361 100644 --- a/app/src/lib/git/clone.ts +++ b/app/src/lib/git/clone.ts @@ -28,6 +28,7 @@ export async function clone( url: string, path: string, options: CloneOptions, + login?: string, progressCallback?: (progress: ICloneProgress) => void ): Promise { const env = { @@ -35,6 +36,14 @@ export async function clone( GIT_CLONE_PROTECTION_ACTIVE: 'false', } + const remoteUrl = + login && login !== '' + ? url.replace( + /^((?:https|http|git+ssh|git|ssh|file)?:\/\/)/i, + '$1' + login + '@' + ) + : url + const defaultBranch = options.defaultBranch ?? (await getDefaultBranch()) const args = [ @@ -72,7 +81,7 @@ export async function clone( args.push('-b', options.branch) } - args.push('--', url, path) + args.push('--', remoteUrl, path) await git(args, __dirname, 'clone', opts) } diff --git a/app/src/lib/http.ts b/app/src/lib/http.ts index fbbafb0c0ee..e3a82be8624 100644 --- a/app/src/lib/http.ts +++ b/app/src/lib/http.ts @@ -120,7 +120,8 @@ export function request( path: string, jsonBody?: Object, customHeaders?: Object, - reloadCache: boolean = false + reloadCache: boolean = false, + login?: string ): Promise { const url = getAbsoluteUrl(endpoint, path) diff --git a/app/src/lib/infer-last-push-for-repository.ts b/app/src/lib/infer-last-push-for-repository.ts index a21b344e47d..eb61edd5c8a 100644 --- a/app/src/lib/infer-last-push-for-repository.ts +++ b/app/src/lib/infer-last-push-for-repository.ts @@ -31,7 +31,11 @@ export async function inferLastPushForRepository( const api = API.fromAccount(account) if (currentRemote !== null) { - const matchedRepository = matchGitHubRepository(accounts, currentRemote.url) + const matchedRepository = matchGitHubRepository( + accounts, + currentRemote.url, + account.login + ) if (matchedRepository !== null) { const { owner, name } = matchedRepository diff --git a/app/src/lib/parse-app-url.ts b/app/src/lib/parse-app-url.ts index cb59bdb8963..a9693b5a5c1 100644 --- a/app/src/lib/parse-app-url.ts +++ b/app/src/lib/parse-app-url.ts @@ -21,6 +21,8 @@ export interface IOpenRepositoryFromURLAction { /** the file to open after cloning the repository */ readonly filepath: string | null + + readonly login?: string } export interface IUnknownAction { @@ -99,6 +101,7 @@ export function parseAppURL(url: string): URLActionType { const pr = getQueryStringValue(query, 'pr') const branch = getQueryStringValue(query, 'branch') const filepath = getQueryStringValue(query, 'filepath') + const login = getQueryStringValue(query, 'username') if (pr != null) { if (!/^\d+$/.test(pr)) { @@ -121,6 +124,7 @@ export function parseAppURL(url: string): URLActionType { branch, pr, filepath, + login: login || undefined, } } diff --git a/app/src/lib/repository-matching.ts b/app/src/lib/repository-matching.ts index ca10921d0bd..bb08a88b983 100644 --- a/app/src/lib/repository-matching.ts +++ b/app/src/lib/repository-matching.ts @@ -25,21 +25,41 @@ export interface IMatchedGitHubRepository { /** The account matching the repository remote */ readonly account: Account + + readonly login?: string } /** Try to use the list of users and a remote URL to guess a GitHub repository. */ export function matchGitHubRepository( accounts: ReadonlyArray, - remote: string + remote: string, + login?: string ): IMatchedGitHubRepository | null { for (const account of accounts) { const htmlURL = getHTMLURL(account.endpoint) const { hostname } = URL.parse(htmlURL) const parsedRemote = parseRemote(remote) + if (login !== undefined && login === '') { + // TODO: This is here temporarily for debugging, remove it when we're sure this isn't a possibility + log.error(`Empty string is not a valid login`) + } if (parsedRemote !== null && hostname !== null) { - if (parsedRemote.hostname.toLowerCase() === hostname.toLowerCase()) { - return { name: parsedRemote.name, owner: parsedRemote.owner, account } + if ( + parsedRemote.hostname.toLowerCase() === hostname.toLowerCase() && + (login === undefined || account.login === login) + ) { + return { + name: parsedRemote.name, + owner: parsedRemote.owner, + account, + login, + } + } else { + if (login !== undefined) { + // TODO: This is here temporarily for debugging, remove it when we're sure this isn't a possibility + log.warn(`Could not find an account to match ${login}@${remote}`) + } } } } diff --git a/app/src/lib/stores/accounts-store.ts b/app/src/lib/stores/accounts-store.ts index c6ed7badfc4..f0eabe57fe5 100644 --- a/app/src/lib/stores/accounts-store.ts +++ b/app/src/lib/stores/accounts-store.ts @@ -6,6 +6,7 @@ import { fatalError } from '../fatal-error' import { TypedBaseStore } from './base-store' import { isGHE } from '../endpoint-capabilities' import { compare, compareDescending } from '../compare' +import { enableMultipleLoginAccounts } from '../feature-flag' // Ensure that GitHub.com accounts appear first followed by Enterprise // accounts, sorted by the order in which they were added. @@ -97,15 +98,24 @@ export class AccountsStore extends TypedBaseStore> { public async addAccount(account: Account): Promise { await this.loadingPromise - if (!(await this.storeAccountKey(account))) { - return null - } + const multipleLoginAccounts = enableMultipleLoginAccounts() + + this.storeAccountKey(account) const accountsByEndpoint = this.accounts.reduce( - (map, x) => map.set(x.endpoint, x), + (map, x) => + map.set( + multipleLoginAccounts ? x.endpoint + ':' + x.login : x.endpoint, + x + ), new Map() ) - accountsByEndpoint.set(account.endpoint, account) + accountsByEndpoint.set( + multipleLoginAccounts + ? account.endpoint + ':' + account.login + : account.endpoint, + account + ) this.accounts = sortAccounts([...accountsByEndpoint.values()]) @@ -203,7 +213,12 @@ export class AccountsStore extends TypedBaseStore> { } this.accounts = this.accounts.filter( - a => !(a.endpoint === account.endpoint && a.id === account.id) + a => + !( + a.endpoint === account.endpoint && + a.login === account.login && + a.id === account.id + ) ) this.save() @@ -301,6 +316,7 @@ async function updatedAccount(account: Account): Promise { account.endpoint, account.token, account.refreshToken, - account.tokenExpiresAt + account.tokenExpiresAt, + account.login ) } diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index 00584dde49b..d4adae34e26 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -98,10 +98,11 @@ import { import { API, deleteToken, - getAccountForEndpoint, getEndpointForRepository, IAPIComment, IAPICreatePushProtectionBypassResponse, + getAccountForEndpointToken, + getAccountForEndpoint, IAPIFullRepository, IAPIOrganization, IAPIRepoRuleset, @@ -787,7 +788,7 @@ export class AppStore extends TypedBaseStore { } private onTokenInvalidated = (endpoint: string, token: string) => { - const account = getAccountForEndpoint(this.accounts, endpoint) + const account = getAccountForEndpointToken(this.accounts, endpoint, token) if (account === null) { return @@ -817,7 +818,7 @@ export class AppStore extends TypedBaseStore { refreshToken: string, tokenExpiresAt: number ) => { - const account = getAccountForEndpoint(this.accounts, endpoint) + const account = getAccountForEndpointToken(this.accounts, endpoint, token) if (account === null) { return } @@ -1314,7 +1315,11 @@ export class AppStore extends TypedBaseStore { const branchName = findRemoteBranchName(tip, currentRemote, gitHubRepo) if (branchName !== null) { - const account = getAccountForEndpoint(this.accounts, gitHubRepo.endpoint) + const account = getAccountForEndpoint( + this.accounts, + gitHubRepo.endpoint, + gitHubRepo.login + ) if (account === null) { return @@ -2211,7 +2216,12 @@ export class AppStore extends TypedBaseStore { } public async _refreshIssues(repository: GitHubRepository) { - const user = getAccountForEndpoint(this.accounts, repository.endpoint) + const user = getAccountForEndpoint( + this.accounts, + repository.endpoint, + repository.login + ) + if (!user) { return } @@ -2232,7 +2242,11 @@ export class AppStore extends TypedBaseStore { } private refreshMentionables(repository: GitHubRepository) { - const account = getAccountForEndpoint(this.accounts, repository.endpoint) + const account = getAccountForEndpoint( + this.accounts, + repository.endpoint, + repository.login + ) if (!account) { return } @@ -2260,9 +2274,14 @@ export class AppStore extends TypedBaseStore { this.pullRequestCoordinator.stopPullRequestUpdater() } - public async fetchPullRequest(repoUrl: string, pr: string) { + public async fetchPullRequest(repoUrl: string, pr: string, login?: string) { const endpoint = getEndpointForRepository(repoUrl) - const account = getAccountForEndpoint(this.accounts, endpoint) + const remoteUrl = parseRemote(repoUrl) + + if (!remoteUrl) { + return null + } + const account = getAccountForEndpoint(this.accounts, endpoint, login) if (account) { const api = API.fromAccount(account) @@ -2799,7 +2818,8 @@ export class AppStore extends TypedBaseStore { this.repositories.find( r => r.constructor === selectedRepository.constructor && - r.id === selectedRepository.id + r.id === selectedRepository.id && + r.login === selectedRepository.login ) || null newSelectedRepository = r @@ -4525,6 +4545,7 @@ export class AppStore extends TypedBaseStore { } const { account, owner, name } = match + const { endpoint } = account const api = API.fromAccount(account) const apiRepo = await api.fetchRepository(owner, name) @@ -4534,10 +4555,12 @@ export class AppStore extends TypedBaseStore { // repository info. But if we didn't have a GitHub repository already or // the endpoint changed, the skeleton repository is better than nothing. if (endpoint !== repository.gitHubRepository?.endpoint) { - const ghRepo = await repoStore.upsertGitHubRepositoryFromMatch(match) + const ghRepo = await repoStore.upsertGitHubRepositoryFromMatch( + match, + repository.login + ) return repoStore.setGitHubRepository(repository, ghRepo) } - return repository } @@ -4546,10 +4569,16 @@ export class AppStore extends TypedBaseStore { await updateRemoteUrl(gitStore, repository.gitHubRepository, apiRepo) } - const ghRepo = await repoStore.upsertGitHubRepository(endpoint, apiRepo) + const ghRepo = await repoStore.upsertGitHubRepository( + endpoint, + apiRepo, + repository.login + ) + const freshRepo = await repoStore.setGitHubRepository(repository, ghRepo) await this.refreshBranchProtectionState(freshRepo) + return freshRepo } @@ -4581,7 +4610,8 @@ export class AppStore extends TypedBaseStore { const account = getAccountForEndpoint( this.accounts, - repository.gitHubRepository.endpoint + repository.gitHubRepository.endpoint, + repository.gitHubRepository.login ) if (account === null) { @@ -4609,7 +4639,7 @@ export class AppStore extends TypedBaseStore { const remote = gitStore.defaultRemote return remote !== null - ? matchGitHubRepository(this.accounts, remote.url) + ? matchGitHubRepository(this.accounts, remote.url, repository.login) : null } @@ -4641,6 +4671,18 @@ export class AppStore extends TypedBaseStore { await this._refreshRepository(repo) } + /** This shouldn't be called directly. See `Dispatcher`. */ + public async _updateRepositoryAccount( + repository: Repository, + account: Account | null + ): Promise { + const repo = await this.repositoriesStore.updateRepositoryAccount( + repository, + account + ) + await this._refreshRepository(repo) + } + public async _updateRepositoryEditorOverride( repository: Repository, customEditorOverride: EditorOverride | null @@ -5099,6 +5141,9 @@ export class AppStore extends TypedBaseStore { private async performPull(repository: Repository): Promise { return this.withPushPullFetch(repository, async () => { const gitStore = this.gitStoreCache.get(repository) + + await gitStore.loadRemotes() + const remote = gitStore.currentRemote if (!remote) { @@ -5291,12 +5336,18 @@ export class AppStore extends TypedBaseStore { public _clone( url: string, path: string, - options: { branch?: string; defaultBranch?: string } = {} + options: { branch?: string; defaultBranch?: string } = {}, + login?: string ): { promise: Promise repository: CloningRepository } { - const promise = this.cloningRepositoriesStore.clone(url, path, options) + const promise = this.cloningRepositoriesStore.clone( + url, + path, + options, + login + ) const repository = this.cloningRepositoriesStore.repositories.find( r => r.url === url && r.path === path )! @@ -6644,7 +6695,8 @@ export class AppStore extends TypedBaseStore { public async _addTutorialRepository( path: string, endpoint: string, - apiRepository: IAPIFullRepository + apiRepository: IAPIFullRepository, + login?: string ) { const type = await getRepositoryType(path) if (type.kind === 'regular') { @@ -6656,7 +6708,8 @@ export class AppStore extends TypedBaseStore { await this.repositoriesStore.addTutorialRepository( validatedPath, endpoint, - apiRepository + apiRepository, + login ) this.tutorialAssessor.onNewTutorialRepository() } else { @@ -6666,7 +6719,8 @@ export class AppStore extends TypedBaseStore { } public async _addRepositories( - paths: ReadonlyArray + paths: ReadonlyArray, + login?: string ): Promise> { const addedRepositories = new Array() const lfsRepositories = new Array() @@ -6679,9 +6733,13 @@ export class AppStore extends TypedBaseStore { }) if (repositoryType.kind === 'unsafe') { - const repository = await this.repositoriesStore.addRepository(path, { - missing: true, - }) + const repository = await this.repositoriesStore.addRepository( + path, + { + missing: true, + }, + login + ) addedRepositories.push(repository) continue @@ -6689,7 +6747,9 @@ export class AppStore extends TypedBaseStore { if (repositoryType.kind === 'regular') { const validatedPath = repositoryType.topLevelWorkingDirectory - log.info(`[AppStore] adding repository at ${validatedPath} to store`) + log.info( + `[AppStore] adding repository at ${validatedPath} @${login} to store` + ) const repositories = this.repositories const existing = matchExistingRepository(repositories, validatedPath) @@ -6702,7 +6762,9 @@ export class AppStore extends TypedBaseStore { } const addedRepo = await this.repositoriesStore.addRepository( - validatedPath + validatedPath, + undefined, + login ) // initialize the remotes for this new repository to ensure it can fetch @@ -6710,10 +6772,12 @@ export class AppStore extends TypedBaseStore { const gitStore = this.gitStoreCache.get(addedRepo) await gitStore.loadRemotes() + // This is adding the gitHubRepository to refreshedRepo const [refreshedRepo, usingLFS] = await Promise.all([ this.repositoryWithRefreshedGitHubRepository(addedRepo), this.isUsingLFS(addedRepo), ]) + addedRepositories.push(refreshedRepo) if (usingLFS) { @@ -6776,8 +6840,12 @@ export class AppStore extends TypedBaseStore { } } - public async _cloneAgain(url: string, path: string): Promise { - const { promise, repository } = this._clone(url, path) + public async _cloneAgain( + url: string, + path: string, + login?: string + ): Promise { + const { promise, repository } = this._clone(url, path, {}, login) await this._selectRepository(repository) const success = await promise if (!success) { @@ -7534,7 +7602,8 @@ export class AppStore extends TypedBaseStore { */ public async _convertRepositoryToFork( repository: RepositoryWithGitHubRepository, - fork: IAPIFullRepository + fork: IAPIFullRepository, + login?: string ): Promise { const gitStore = this.gitStoreCache.get(repository) const defaultRemoteName = gitStore.defaultRemote?.name @@ -7549,7 +7618,11 @@ export class AppStore extends TypedBaseStore { // update associated github repo return this.repositoriesStore.setGitHubRepository( repository, - await this.repositoriesStore.upsertGitHubRepository(endpoint, fork) + await this.repositoriesStore.upsertGitHubRepository( + endpoint, + fork, + repository.login + ) ) } } @@ -7589,7 +7662,12 @@ export class AppStore extends TypedBaseStore { } ) - await this._addTutorialRepository(path, account.endpoint, apiRepository) + await this._addTutorialRepository( + path, + account.endpoint, + apiRepository, + account.login + ) await this.statsStore.recordTutorialRepoCreated() } catch (err) { sendNonFatalException('tutorialRepoCreation', err) @@ -8830,7 +8908,7 @@ export class AppStore extends TypedBaseStore { const { endpoint, name, owner } = repository.gitHubRepository - const account = getAccountForEndpoint(this.accounts, endpoint) + const account = getAccountForEndpoint(this.accounts, endpoint, owner.login) if (account === null) { log.error( diff --git a/app/src/lib/stores/cloning-repositories-store.ts b/app/src/lib/stores/cloning-repositories-store.ts index 0e12579fd25..a7d00cf2147 100644 --- a/app/src/lib/stores/cloning-repositories-store.ts +++ b/app/src/lib/stores/cloning-repositories-store.ts @@ -20,9 +20,10 @@ export class CloningRepositoriesStore extends BaseStore { public async clone( url: string, path: string, - options: CloneOptions + options: CloneOptions, + login?: string ): Promise { - const repository = new CloningRepository(path, url) + const repository = new CloningRepository(path, url, login) this._repositories.push(repository) const title = `Cloning into ${path}` @@ -32,7 +33,7 @@ export class CloningRepositoriesStore extends BaseStore { let success = true try { - await cloneRepo(url, path, options, progress => { + await cloneRepo(url, path, options, login, progress => { this.stateByID.set(repository.id, progress) this.emitUpdate() }) @@ -45,6 +46,7 @@ export class CloningRepositoriesStore extends BaseStore { url, path, options, + login, } e = new ErrorWithMetadata(e, { retryAction, repository }) diff --git a/app/src/lib/stores/commit-status-store.ts b/app/src/lib/stores/commit-status-store.ts index 506e32f39cd..b1978bd728c 100644 --- a/app/src/lib/stores/commit-status-store.ts +++ b/app/src/lib/stores/commit-status-store.ts @@ -62,6 +62,8 @@ interface IRefStatusSubscription { /** If provided, we retrieve the actions workflow runs or the checks with this sub */ readonly branchName?: string + + readonly login?: string } /** @@ -273,10 +275,22 @@ export class CommitStatusStore { return } - const { endpoint, owner, name, ref } = subscription - const account = this.accounts.find(a => a.endpoint === endpoint) + const { endpoint, owner, name, ref, login } = subscription + + if (login !== undefined && login === '') { + // TODO: This is here temporarily for debugging, remove it when we're sure this isn't a possibility + log.error(`Empty string is not a valid login`) + } + + const account = this.accounts.find( + a => a.endpoint === endpoint && (login === undefined || a.login === login) + ) if (account === undefined) { + if (login !== undefined) { + // TODO: This is here temporarily for debugging, remove it when we're sure this isn't a possibility + log.warn(`Could not find an account to match ${login}@${endpoint}`) + } return } @@ -442,6 +456,7 @@ export class CommitStatusStore { ref, callbacks: new Set(), branchName, + login: repository.login, } this.subscriptions.set(key, subscription) @@ -496,9 +511,20 @@ export class CommitStatusStore { return checkRuns } - const { endpoint, owner, name } = subscription - const account = this.accounts.find(a => a.endpoint === endpoint) + const { endpoint, owner, name, login } = subscription + + if (login !== undefined && login === '') { + // TODO: This is here temporarily for debugging, remove it when we're sure this isn't a possibility + log.error(`Empty string is not a valid login`) + } + const account = this.accounts.find( + a => a.endpoint === endpoint && (login === undefined || a.login === login) + ) if (account === undefined) { + if (login !== undefined) { + // TODO: This is here temporarily for debugging, remove it when we're sure this isn't a possibility + log.warn(`Could not find an account to match ${login}@${endpoint}`) + } return checkRuns } @@ -523,10 +549,21 @@ export class CommitStatusStore { return checkRuns } - const { endpoint, owner, name } = subscription - const account = this.accounts.find(a => a.endpoint === endpoint) + const { endpoint, owner, name, login } = subscription + + if (login !== undefined && login === '') { + // TODO: This is here temporarily for debugging, remove it when we're sure this isn't a possibility + log.error(`Empty string is not a valid login`) + } + const account = this.accounts.find( + a => a.endpoint === endpoint && (login === undefined || a.login === login) + ) if (account === undefined) { + if (login !== undefined) { + // TODO: This is here temporarily for debugging, remove it when we're sure this isn't a possibility + log.warn(`Could not find an account to match ${login}@${endpoint}`) + } return checkRuns } @@ -540,7 +577,11 @@ export class CommitStatusStore { checkSuiteId: number ): Promise { const { owner, name } = repository - const account = getAccountForEndpoint(this.accounts, repository.endpoint) + const account = getAccountForEndpoint( + this.accounts, + repository.endpoint, + repository.login + ) if (account === null) { return false } @@ -554,7 +595,11 @@ export class CommitStatusStore { jobId: number ): Promise { const { owner, name } = repository - const account = getAccountForEndpoint(this.accounts, repository.endpoint) + const account = getAccountForEndpoint( + this.accounts, + repository.endpoint, + repository.login + ) if (account === null) { return false } @@ -568,7 +613,11 @@ export class CommitStatusStore { workflowRunId: number ): Promise { const { owner, name } = repository - const account = getAccountForEndpoint(this.accounts, repository.endpoint) + const account = getAccountForEndpoint( + this.accounts, + repository.endpoint, + repository.login + ) if (account === null) { return false } @@ -582,7 +631,11 @@ export class CommitStatusStore { checkSuiteId: number ): Promise { const { owner, name } = repository - const account = getAccountForEndpoint(this.accounts, repository.endpoint) + const account = getAccountForEndpoint( + this.accounts, + repository.endpoint, + repository.login + ) if (account === null) { return null } diff --git a/app/src/lib/stores/git-store.ts b/app/src/lib/stores/git-store.ts index bfdeb8bf50f..8de0f866684 100644 --- a/app/src/lib/stores/git-store.ts +++ b/app/src/lib/stores/git-store.ts @@ -1390,10 +1390,7 @@ export class GitStore extends BaseStore { parent.cloneURL ) - this._upstreamRemote = - (await this.performFailableOperation(() => - addRemote(this.repository, UpstreamRemoteName, url) - )) ?? null + await this.ensureUpstreamRemoteURL(url) } /** @@ -1406,6 +1403,7 @@ export class GitStore extends BaseStore { await this.performFailableOperation(async () => { try { await addRemote(this.repository, UpstreamRemoteName, remoteUrl) + this._upstreamRemote = { url: remoteUrl, name: UpstreamRemoteName } } catch (e) { if ( e instanceof DugiteError && @@ -1413,7 +1411,9 @@ export class GitStore extends BaseStore { ) { // update upstream remote if it already exists await setRemoteURL(this.repository, UpstreamRemoteName, remoteUrl) + this._upstreamRemote = { url: remoteUrl, name: UpstreamRemoteName } } else { + this._upstreamRemote = null throw e } } diff --git a/app/src/lib/stores/helpers/background-fetcher.ts b/app/src/lib/stores/helpers/background-fetcher.ts index 964b5a7868b..d3094d41305 100644 --- a/app/src/lib/stores/helpers/background-fetcher.ts +++ b/app/src/lib/stores/helpers/background-fetcher.ts @@ -121,7 +121,8 @@ export class BackgroundFetcher { ): Promise { const account = getAccountForEndpoint( await this.accountsStore.getAll(), - repository.endpoint + repository.endpoint, + repository.login ) let interval = DefaultFetchInterval diff --git a/app/src/lib/stores/helpers/create-tutorial-repository.ts b/app/src/lib/stores/helpers/create-tutorial-repository.ts index 6b82bad50a9..2781d356e2e 100644 --- a/app/src/lib/stores/helpers/create-tutorial-repository.ts +++ b/app/src/lib/stores/helpers/create-tutorial-repository.ts @@ -23,7 +23,7 @@ const InitialReadmeContents = `back to GitHub Desktop.${nl}` async function createAPIRepository(account: Account, name: string) { - const api = new API(account.endpoint, account.token) + const api = new API(account.endpoint, account.token, undefined, account.login) try { return await api.createRepository( diff --git a/app/src/lib/stores/pull-request-store.ts b/app/src/lib/stores/pull-request-store.ts index b83442a4569..f87a08f30f5 100644 --- a/app/src/lib/stores/pull-request-store.ts +++ b/app/src/lib/stores/pull-request-store.ts @@ -96,25 +96,32 @@ export class PullRequestStore { // after the timestamp we give it we will always get at least one issue // back. See `storePullRequests` for details on how that's handled. if (!lastUpdatedAt) { - return this.fetchAndStoreOpenPullRequests(api, repo) + return this.fetchAndStoreOpenPullRequests(api, repo, account.login) } else { - return this.fetchAndStoreUpdatedPullRequests(api, repo, lastUpdatedAt) + return this.fetchAndStoreUpdatedPullRequests( + api, + repo, + lastUpdatedAt, + account.login + ) } } private async fetchAndStoreOpenPullRequests( api: API, - repository: GitHubRepository + repository: GitHubRepository, + login?: string ) { const { name, owner } = getNameWithOwner(repository) const open = await api.fetchAllOpenPullRequests(owner, name) - await this.storePullRequestsAndEmitUpdate(open, repository) + await this.storePullRequestsAndEmitUpdate(open, repository, login) } private async fetchAndStoreUpdatedPullRequests( api: API, repository: GitHubRepository, - lastUpdatedAt: Date + lastUpdatedAt: Date, + login?: string ) { const { name, owner } = getNameWithOwner(repository) const updated = await api @@ -128,7 +135,11 @@ export class PullRequestStore { ) if (updated !== null) { - return await this.storePullRequestsAndEmitUpdate(updated, repository) + return await this.storePullRequestsAndEmitUpdate( + updated, + repository, + login + ) } else { // If we fail to load updated pull requests either because // there's too many updated PRs since the last time we @@ -146,7 +157,7 @@ export class PullRequestStore { // the number of merged PRs since the last time we fetched could // number in the thousands. await this.db.deleteAllPullRequestsInRepository(repository) - await this.fetchAndStoreOpenPullRequests(api, repository) + await this.fetchAndStoreOpenPullRequests(api, repository, login) } } @@ -216,9 +227,10 @@ export class PullRequestStore { */ private async storePullRequestsAndEmitUpdate( pullRequestsFromAPI: ReadonlyArray, - repository: GitHubRepository + repository: GitHubRepository, + login?: string ) { - if (await this.storePullRequests(pullRequestsFromAPI, repository)) { + if (await this.storePullRequests(pullRequestsFromAPI, repository, login)) { this.emitPullRequestsChanged(repository, await this.getAll(repository)) } } @@ -232,7 +244,8 @@ export class PullRequestStore { */ private async storePullRequests( pullRequestsFromAPI: ReadonlyArray, - repository: GitHubRepository + repository: GitHubRepository, + login?: string ) { if (pullRequestsFromAPI.length === 0) { return false @@ -262,7 +275,7 @@ export class PullRequestStore { // only thing we really care about to determine whether the // repository has already been inserted in the database is the clone // url since that's what the upsert method uses as its key. - cacheKey: (_, repo) => repo.clone_url, + cacheKey: (_, repo, login?) => repo.clone_url, }) for (const pr of pullRequestsFromAPI) { @@ -278,7 +291,7 @@ export class PullRequestStore { return fatalError('PR cannot have a null base repo') } - const baseGitHubRepo = await upsertRepo(endpoint, pr.base.repo) + const baseGitHubRepo = await upsertRepo(endpoint, pr.base.repo, login) if (pr.state === 'closed') { prsToDelete.push(getPullRequestKey(baseGitHubRepo, pr.number)) @@ -300,7 +313,7 @@ export class PullRequestStore { continue } - const headRepo = await upsertRepo(endpoint, pr.head.repo) + const headRepo = await upsertRepo(endpoint, pr.head.repo, login) prsToUpsert.push({ number: pr.number, diff --git a/app/src/lib/stores/repositories-store.ts b/app/src/lib/stores/repositories-store.ts index ed6af914e4d..f5361f2263e 100644 --- a/app/src/lib/stores/repositories-store.ts +++ b/app/src/lib/stores/repositories-store.ts @@ -29,6 +29,7 @@ import { clearTagsToPush } from './helpers/tags-to-push-storage' import { IMatchedGitHubRepository } from '../repository-matching' import { shallowEquals } from '../equality' import { EditorOverride } from '../../models/editor-override' +import { Account } from '../../models/account' type AddRepositoryOptions = { missing?: boolean @@ -71,13 +72,14 @@ export class RepositoriesStore extends TypedBaseStore< */ public async upsertGitHubRepositoryLight( endpoint: string, - apiRepository: IAPIRepository + apiRepository: IAPIRepository, + login?: string ) { return this.db.transaction( 'rw', this.db.gitHubRepositories, this.db.owners, - () => this._upsertGitHubRepository(endpoint, apiRepository, true) + () => this._upsertGitHubRepository(endpoint, apiRepository, true, login) ) } @@ -87,13 +89,14 @@ export class RepositoriesStore extends TypedBaseStore< */ public async upsertGitHubRepository( endpoint: string, - apiRepository: IAPIFullRepository + apiRepository: IAPIFullRepository, + login?: string ): Promise { return this.db.transaction( 'rw', this.db.gitHubRepositories, this.db.owners, - () => this._upsertGitHubRepository(endpoint, apiRepository, false) + () => this._upsertGitHubRepository(endpoint, apiRepository, false, login) ) } @@ -126,6 +129,7 @@ export class RepositoriesStore extends TypedBaseStore< const isBitbucket = repo.htmlURL && this.isBitbucketUrl(repo.htmlURL) const isGitLab = repo.htmlURL && this.isGitLabUrl(repo.htmlURL) + const login = repo.login const repoType = isBitbucket ? 'bitbucket' : isGitLab ? 'gitlab' : 'github' const ghRepo = new GitHubRepository( repo.name, @@ -138,7 +142,8 @@ export class RepositoriesStore extends TypedBaseStore< repo.issuesEnabled, repo.isArchived, repo.permissions, - parent + parent, + login ) // Dexie gets confused if we return a non-promise value (e.g. if this function @@ -177,7 +182,8 @@ export class RepositoriesStore extends TypedBaseStore< repo.defaultBranch, repo.workflowPreferences, repo.customEditorOverride, - repo.isTutorialRepository + repo.isTutorialRepository, + repo.login ) } @@ -224,7 +230,8 @@ export class RepositoriesStore extends TypedBaseStore< public async addTutorialRepository( path: string, endpoint: string, - apiRepo: IAPIFullRepository + apiRepo: IAPIFullRepository, + login?: string ) { await this.db.transaction( 'rw', @@ -232,7 +239,11 @@ export class RepositoriesStore extends TypedBaseStore< this.db.gitHubRepositories, this.db.owners, async () => { - const ghRepo = await this.upsertGitHubRepository(endpoint, apiRepo) + const ghRepo = await this.upsertGitHubRepository( + endpoint, + apiRepo, + login + ) const existingRepo = await this.db.repositories.get({ path }) return await this.db.repositories.put({ @@ -244,6 +255,7 @@ export class RepositoriesStore extends TypedBaseStore< missing: false, lastStashCheckDate: null, isTutorialRepository: true, + login, }) } ) @@ -258,7 +270,8 @@ export class RepositoriesStore extends TypedBaseStore< */ public async addRepository( path: string, - opts?: AddRepositoryOptions + opts?: AddRepositoryOptions, + login?: string ): Promise { const repository = await this.db.transaction( 'rw', @@ -279,6 +292,7 @@ export class RepositoriesStore extends TypedBaseStore< lastStashCheckDate: null, alias: null, defaultBranch: null, + login, } const id = await this.db.repositories.add(dbRepo) return this.toRepository({ id, ...dbRepo }) @@ -316,7 +330,8 @@ export class RepositoriesStore extends TypedBaseStore< repository.defaultBranch, repository.workflowPreferences, repository.customEditorOverride, - repository.isTutorialRepository + repository.isTutorialRepository, + repository.login ) } @@ -358,7 +373,36 @@ export class RepositoriesStore extends TypedBaseStore< defaultBranch, repository.workflowPreferences, repository.customEditorOverride, - repository.isTutorialRepository + repository.isTutorialRepository, + repository.login + ) + } + + /** + * Update the alias for the specified repository. + * + * @param repository The repository to update. + * @param defaultBranch The new default branch to use. + */ + public async updateRepositoryAccount( + repository: Repository, + account: Account | null + ): Promise { + await this.db.repositories.update(repository.id, { login: account?.login }) + + this.emitUpdatedRepositories() + + return new Repository( + repository.path, + repository.id, + repository.gitHubRepository, + repository.missing, + repository.alias, + repository.defaultBranch, + repository.workflowPreferences, + repository.customEditorOverride, + repository.isTutorialRepository, + account?.login ) } @@ -406,7 +450,8 @@ export class RepositoriesStore extends TypedBaseStore< repository.defaultBranch, repository.workflowPreferences, repository.customEditorOverride, - repository.isTutorialRepository + repository.isTutorialRepository, + repository.login ) } @@ -496,7 +541,8 @@ export class RepositoriesStore extends TypedBaseStore< } public async upsertGitHubRepositoryFromMatch( - match: IMatchedGitHubRepository + match: IMatchedGitHubRepository, + login?: string ) { return await this.db.transaction( 'rw', @@ -520,6 +566,7 @@ export class RepositoriesStore extends TypedBaseStore< lastPruneDate: null, name: match.name, ownerID: owner.id, + login: login, parentID: null, private: null, } @@ -554,7 +601,8 @@ export class RepositoriesStore extends TypedBaseStore< repo.defaultBranch, repo.workflowPreferences, repo.customEditorOverride, - repo.isTutorialRepository + repo.isTutorialRepository, + repo.login ) assertIsRepositoryWithGitHubRepository(updatedRepo) @@ -564,19 +612,24 @@ export class RepositoriesStore extends TypedBaseStore< private async _upsertGitHubRepository( endpoint: string, gitHubRepository: IAPIRepository | IAPIFullRepository, - ignoreParent = false + ignoreParent = false, + login?: string ): Promise { const parent = 'parent' in gitHubRepository && gitHubRepository.parent !== undefined ? await this._upsertGitHubRepository( endpoint, gitHubRepository.parent, - true + true, + login ) : await Promise.resolve(null) // Dexie gets confused if we return null - const { login, type } = gitHubRepository.owner - const owner = await this.putOwner(endpoint, login, type) + const owner = await this.putOwner( + endpoint, + gitHubRepository.owner.login, + gitHubRepository.owner.type + ) const existingRepo = await this.db.gitHubRepositories .where('[ownerID+name]') @@ -623,6 +676,7 @@ export class RepositoriesStore extends TypedBaseStore< ...(existingRepo?.id !== undefined && { id: existingRepo.id }), ownerID: owner.id, name: gitHubRepository.name, + login: login, private: gitHubRepository.private ?? existingRepo?.private ?? null, htmlURL: gitHubRepository.html_url, cloneURL: (gitHubRepository.clone_url || existingRepo?.cloneURL) ?? null, diff --git a/app/src/lib/stores/sign-in-store.ts b/app/src/lib/stores/sign-in-store.ts index df03a2282ec..0e19fb7c20d 100644 --- a/app/src/lib/stores/sign-in-store.ts +++ b/app/src/lib/stores/sign-in-store.ts @@ -28,6 +28,7 @@ import { IOAuthAction } from '../parse-app-url' import { shell } from '../app-shell' import noop from 'lodash/noop' import { AccountsStore } from './accounts-store' +import { enableMultipleLoginAccounts } from '../feature-flag' import { RepoType } from '../../models/github-repository' /** @@ -107,6 +108,7 @@ export interface IExistingAccountWarning extends ISignInState { */ export interface IEndpointEntryState extends ISignInState { readonly kind: SignInStep.EndpointEntry + readonly existingAccount?: Account readonly resultCallback: (result: SignInResult) => void } @@ -244,7 +246,7 @@ export class SignInStore extends TypedBaseStore { const existingAccount = this.accounts.find(a => a.apiType === 'dotcom') - if (existingAccount) { + if (existingAccount && !enableMultipleLoginAccounts()) { this.setState({ kind: SignInStep.ExistingAccountWarning, endpoint, @@ -546,7 +548,9 @@ export class SignInStore extends TypedBaseStore { const endpoint = getEnterpriseAPIURL(validUrl) - const existingAccount = this.accounts.find(x => x.endpoint === endpoint) + const existingAccount = currentState.existingAccount + ? currentState.existingAccount + : this.accounts.find(x => x.endpoint === endpoint) if (existingAccount) { this.setState({ diff --git a/app/src/lib/trampoline/find-account.ts b/app/src/lib/trampoline/find-account.ts index 151f1637914..10f6eef304a 100644 --- a/app/src/lib/trampoline/find-account.ts +++ b/app/src/lib/trampoline/find-account.ts @@ -19,13 +19,28 @@ const memoizedGetGenericPassword = memoizeOne( export async function findGitHubTrampolineAccount( accountsStore: AccountsStore, - remoteUrl: string + remoteUrl: string, + login?: string ): Promise { const accounts = await accountsStore.getAll() const parsedUrl = new URL(remoteUrl) - return accounts.find( - a => new URL(getHTMLURL(a.endpoint)).origin === parsedUrl.origin + if (login !== undefined && login === '') { + // TODO: This is here temporarily for debugging, remove it when we're sure this isn't a possibility + log.error(`Empty string is not a valid login`) + } + + const result = accounts.find( + a => + new URL(getHTMLURL(a.endpoint)).origin === parsedUrl.origin && + (login === undefined || a.login === login) ) + + if (result === undefined && login !== undefined) { + // TODO: This is here temporarily for debugging, remove it when we're sure this isn't a possibility + log.warn(`Could not find an account to match ${login}@${remoteUrl}`) + } + + return result } export async function findGenericTrampolineAccount( diff --git a/app/src/lib/trampoline/trampoline-credential-helper.ts b/app/src/lib/trampoline/trampoline-credential-helper.ts index b4fd1562700..a56d83e9e29 100644 --- a/app/src/lib/trampoline/trampoline-credential-helper.ts +++ b/app/src/lib/trampoline/trampoline-credential-helper.ts @@ -47,9 +47,13 @@ const error = (msg: string, e: any) => log.error(`credential-helper: ${msg}`, e) const credWithAccount = (c: Credential, a: IGitAccount | undefined) => a && new Map(c).set('username', a.login).set('password', a.token) -async function getGitHubCredential(cred: Credential, store: AccountsStore) { +async function getGitHubCredential( + cred: Credential, + store: AccountsStore, + login?: string +) { const endpoint = `${getCredentialUrl(cred)}` - const account = await findGitHubTrampolineAccount(store, endpoint) + const account = await findGitHubTrampolineAccount(store, endpoint, login) if (account) { info(`found GitHub credential for ${endpoint} in store`) } @@ -91,14 +95,19 @@ async function getExternalCredential(input: Credential, token: string) { } /** Implementation of the 'get' git credential helper command */ -async function getCredential(cred: Credential, store: Store, token: string) { - const ghCred = await getGitHubCredential(cred, store) +async function getCredential( + cred: Credential, + store: Store, + token: string, + login?: string +) { + const ghCred = await getGitHubCredential(cred, store, login) if (ghCred) { return ghCred } - const endpointKind = await getEndpointKind(cred, store) + const endpointKind = await getEndpointKind(cred, store, login) const accounts = await store.getAll() const endpoint = `${getCredentialUrl(cred)}` @@ -106,10 +115,23 @@ async function getCredential(cred: Credential, store: Store, token: string) { // If it appears as if the endpoint is a GitHub host and we don't have an // account for that endpoint then we should prompt the user to sign in. + if (login !== undefined && login === '') { + // TODO: This is here temporarily for debugging, remove it when we're sure this isn't a possibility + log.error(`Empty string is not a valid login`) + } + if ( endpointKind !== 'generic' && - !accounts.some(a => a.endpoint === apiEndpoint) + !accounts.some( + a => + a.endpoint === apiEndpoint && (login === undefined || a.login === login) + ) ) { + if (login !== undefined) { + // TODO: This is here temporarily for debugging, remove it when we're sure this isn't a possibility + log.warn(`Could not find an account to match ${login}@${apiEndpoint}`) + } + if (getIsBackgroundTaskEnvironment(token)) { debug('background task environment, skipping prompt') return undefined @@ -134,7 +156,11 @@ async function getCredential(cred: Credential, store: Store, token: string) { : getGenericCredential(cred, token) } -const getEndpointKind = async (cred: Credential, store: Store) => { +const getEndpointKind = async ( + cred: Credential, + store: Store, + login?: string +) => { const credentialUrl = getCredentialUrl(cred) const endpoint = `${credentialUrl}` @@ -164,7 +190,11 @@ const getEndpointKind = async (cred: Credential, store: Store) => { } } - const existingAccount = await findGitHubTrampolineAccount(store, endpoint) + const existingAccount = await findGitHubTrampolineAccount( + store, + endpoint, + login + ) if (existingAccount) { return isDotCom(existingAccount.endpoint) ? 'github.com' : 'enterprise' } @@ -179,8 +209,13 @@ const getEndpointKind = async (cred: Credential, store: Store) => { } /** Implementation of the 'store' git credential helper command */ -async function storeCredential(cred: Credential, store: Store, token: string) { - if ((await getEndpointKind(cred, store)) !== 'generic') { +async function storeCredential( + cred: Credential, + store: Store, + token: string, + login?: string +) { + if ((await getEndpointKind(cred, store, login)) !== 'generic') { return } @@ -199,8 +234,13 @@ const storeExternalCredential = (cred: Credential, token: string) => { } /** Implementation of the 'erase' git credential helper command */ -async function eraseCredential(cred: Credential, store: Store, token: string) { - if ((await getEndpointKind(cred, store)) !== 'generic') { +async function eraseCredential( + cred: Credential, + store: Store, + token: string, + login?: string +) { + if ((await getEndpointKind(cred, store, login)) !== 'generic') { return } @@ -227,6 +267,7 @@ export const createCredentialHelperTrampolineHandler: ( const { trampolineToken: token } = command const input = parseCredential(command.stdin) + const login = input.get('username') if (__DEV__) { debug( @@ -239,7 +280,7 @@ export const createCredentialHelperTrampolineHandler: ( try { if (firstParameter === 'get') { - const cred = await getCredential(input, store, token) + const cred = await getCredential(input, store, token, login) if (!cred) { const endpoint = `${getCredentialUrl(input)}` info(`could not find credential for ${endpoint}`) @@ -247,9 +288,9 @@ export const createCredentialHelperTrampolineHandler: ( } return cred ? formatCredential(cred) : undefined } else if (firstParameter === 'store') { - await storeCredential(input, store, token) + await storeCredential(input, store, token, login) } else if (firstParameter === 'erase') { - await eraseCredential(input, store, token) + await eraseCredential(input, store, token, login) } return undefined } catch (e) { diff --git a/app/src/models/account.ts b/app/src/models/account.ts index aeb77f6dc96..838ade60e0e 100644 --- a/app/src/models/account.ts +++ b/app/src/models/account.ts @@ -5,7 +5,7 @@ import { getHTMLURL, IAPIEmail, } from '../lib/api' - +import { enableMultipleLoginAccounts } from '../lib/feature-flag' /** * Returns a value indicating whether two account instances * can be considered equal. Equality is determined by comparing @@ -14,8 +14,15 @@ import { * while still maintaining the association between repositories * and a particular account. */ -export function accountEquals(x: Account, y: Account) { - return x.endpoint === y.endpoint && x.id === y.id +export function accountEquals(x: Account | null, y: Account | null) { + return ( + (x === null && y === null) || + (x !== null && + y !== null && + x.endpoint === y.endpoint && + x.id === y.id && + (enableMultipleLoginAccounts() || x.login === y.login)) + ) } export type AccountAPIType = 'dotcom' | 'enterprise' | 'bitbucket' | 'gitlab' diff --git a/app/src/models/cloning-repository.ts b/app/src/models/cloning-repository.ts index 89d256b0b23..26b4899137a 100644 --- a/app/src/models/cloning-repository.ts +++ b/app/src/models/cloning-repository.ts @@ -13,7 +13,8 @@ export class CloningRepository { public constructor( public readonly path: string, - public readonly url: string + public readonly url: string, + public readonly login?: string ) {} public get name(): string { @@ -26,6 +27,6 @@ export class CloningRepository { * Objects with the same hash are guaranteed to be structurally equal. */ public get hash(): string { - return `${this.id}+${this.path}+${this.url}` + return `${this.id}+${this.path}+${this.url}+${this.login}` } } diff --git a/app/src/models/github-repository.ts b/app/src/models/github-repository.ts index d9aa15ed392..442e56b388c 100644 --- a/app/src/models/github-repository.ts +++ b/app/src/models/github-repository.ts @@ -30,7 +30,8 @@ export class GitHubRepository { public readonly isArchived: boolean | null = null, /** The user's permissions for this github repository. `null` if unknown. */ public readonly permissions: GitHubRepositoryPermission = null, - public readonly parent: GitHubRepository | null = null + public readonly parent: GitHubRepository | null = null, + public readonly login?: string ) { this.hash = createEqualityHash( this.name, @@ -42,7 +43,8 @@ export class GitHubRepository { this.issuesEnabled, this.isArchived, this.permissions, - this.parent?.hash + this.parent?.hash, + this.login ) } diff --git a/app/src/models/repository.ts b/app/src/models/repository.ts index 3190cf59a0e..e45522259ee 100644 --- a/app/src/models/repository.ts +++ b/app/src/models/repository.ts @@ -69,7 +69,8 @@ export class Repository { * onboarding flow. Tutorial repositories trigger a tutorial user experience * which introduces new users to some core concepts of Git and GitHub. */ - public readonly isTutorialRepository: boolean = false + public readonly isTutorialRepository: boolean = false, + public readonly login?: string ) { this.mainWorkTree = { path } this.name = (gitHubRepository && gitHubRepository.name) || getBaseName(path) @@ -83,7 +84,8 @@ export class Repository { this.defaultBranch, getCustomOverrideHash(this.customEditorOverride), this.workflowPreferences.forkContributionTarget, - this.isTutorialRepository + this.isTutorialRepository, + this.login ) } diff --git a/app/src/models/retry-actions.ts b/app/src/models/retry-actions.ts index ca44814e963..aabceab7a0b 100644 --- a/app/src/models/retry-actions.ts +++ b/app/src/models/retry-actions.ts @@ -32,6 +32,7 @@ export type RetryAction = url: string path: string options: CloneOptions + login?: string } | { type: RetryActionType.Checkout diff --git a/app/src/ui/account-picker.tsx b/app/src/ui/account-picker.tsx index 1e2ced10a5e..a141eefffdf 100644 --- a/app/src/ui/account-picker.tsx +++ b/app/src/ui/account-picker.tsx @@ -15,7 +15,7 @@ import memoizeOne from 'memoize-one' interface IAccountPickerProps { readonly accounts: ReadonlyArray - readonly selectedAccount: Account + readonly selectedAccount: Account | null readonly onSelectedAccountChanged: (account: Account) => void /** @@ -65,7 +65,7 @@ export class AccountPicker extends React.Component< ( accounts: ReadonlyArray, selectedItemId: string | undefined, - selectedAccount: Account + selectedAccount: Account | null ) => this.getFilterListGroups(accounts) .flatMap(x => x.items) @@ -111,6 +111,16 @@ export class AccountPicker extends React.Component< private renderAccount = (item: IAccountListItem, matches: IMatches) => { const account = item.account + if (!account) { + return ( +
+
+
No account
+
+
+ ) + } + return (
this.setState({ selectedItemId: selectedItem?.id }) private getItemAriaLabel = (item: IAccountListItem) => - `@${item.account.login} ${item.account.friendlyEndpoint}` + item.account !== null + ? `@${item.account.login} ${item.account.friendlyEndpoint}` + : `@no-account` public render() { const account = this.props.selectedAccount @@ -148,9 +162,9 @@ export class AccountPicker extends React.Component< contentTitle="Choose an account" buttonContent={
- @{account.login} -{' '} + @{account?.login} -{' '} - {this.props.selectedAccount.friendlyEndpoint} + {this.props.selectedAccount?.friendlyEndpoint}
} diff --git a/app/src/ui/add-repository/add-existing-repository.tsx b/app/src/ui/add-repository/add-existing-repository.tsx index 6bfdb9887ce..18176514923 100644 --- a/app/src/ui/add-repository/add-existing-repository.tsx +++ b/app/src/ui/add-repository/add-existing-repository.tsx @@ -286,7 +286,7 @@ export class AddExistingRepository extends React.Component< return Path.resolve('/', untildify(path)) } - private addRepository = async () => { + private addRepository = async (login?: string) => { const { path } = this.state const isValidPath = await this.validatePath(path) @@ -299,7 +299,7 @@ export class AddExistingRepository extends React.Component< const { dispatcher } = this.props const resolvedPath = this.resolvedPath(path) - const repositories = await dispatcher.addRepositories([resolvedPath]) + const repositories = await dispatcher.addRepositories([resolvedPath], login) if (repositories.length > 0) { dispatcher.closeFoldout(FoldoutType.Repository) diff --git a/app/src/ui/add-repository/create-repository.tsx b/app/src/ui/add-repository/create-repository.tsx index 89ebf57a6e4..588a029e9cc 100644 --- a/app/src/ui/add-repository/create-repository.tsx +++ b/app/src/ui/add-repository/create-repository.tsx @@ -299,7 +299,7 @@ export class CreateRepository extends React.Component< return Path.join(currentPath, safeDirectoryName(this.state.name)) } - private createRepository = async () => { + private createRepository = async (login?: string) => { const fullPath = await this.resolveRepositoryRoot() if (fullPath === null) { @@ -337,7 +337,10 @@ export class CreateRepository extends React.Component< return this.props.dispatcher.postError(e) } - const repositories = await this.props.dispatcher.addRepositories([fullPath]) + const repositories = await this.props.dispatcher.addRepositories( + [fullPath], + login + ) if (repositories.length < 1) { return } diff --git a/app/src/ui/app.tsx b/app/src/ui/app.tsx index fe000e4b7a1..efdd45ad0e4 100644 --- a/app/src/ui/app.tsx +++ b/app/src/ui/app.tsx @@ -1629,7 +1629,8 @@ export class App extends React.Component { const state = this.props.repositoryStateManager.get(repository) const repositoryAccount = getAccountForRepository( this.state.accounts, - repository + repository, + true ) return ( @@ -1640,6 +1641,7 @@ export class App extends React.Component { dispatcher={this.props.dispatcher} repository={repository} repositoryAccount={repositoryAccount} + accounts={this.state.accounts} onDismissed={onPopupDismissedFn} /> ) diff --git a/app/src/ui/clone-repository/clone-github-repository.tsx b/app/src/ui/clone-repository/clone-github-repository.tsx index 9e58d5b77e6..00ee7f455b3 100644 --- a/app/src/ui/clone-repository/clone-github-repository.tsx +++ b/app/src/ui/clone-repository/clone-github-repository.tsx @@ -8,7 +8,10 @@ import { Button } from '../lib/button' import { IAPIRepository } from '../../lib/api' import { CloneableRepositoryFilterList } from './cloneable-repository-filter-list' import { ClickSource } from '../lib/list' -import { enableMultipleEnterpriseAccounts } from '../../lib/feature-flag' +import { + enableMultipleEnterpriseAccounts, + enableMultipleLoginAccounts, +} from '../../lib/feature-flag' import { AccountPicker } from '../account-picker' interface ICloneGithubRepositoryProps { @@ -101,7 +104,8 @@ export class CloneGithubRepository extends React.PureComponent - {enableMultipleEnterpriseAccounts() && + {(enableMultipleEnterpriseAccounts() || + enableMultipleLoginAccounts()) && this.props.accounts.length > 1 ? ( {this.renderAccountPicker()} ) : undefined} diff --git a/app/src/ui/clone-repository/clone-repository.tsx b/app/src/ui/clone-repository/clone-repository.tsx index 55a36869a11..6c9c3b9245f 100644 --- a/app/src/ui/clone-repository/clone-repository.tsx +++ b/app/src/ui/clone-repository/clone-repository.tsx @@ -208,24 +208,40 @@ export class CloneRepository extends React.Component< filterText: '', selectedItem: null, ...initialBaseTabState, + selectedAccount: + props.accounts + .filter(account => account.apiType === 'dotcom') + .at(0) || null, }, enterpriseTabState: { kind: 'enterprise', filterText: '', selectedItem: null, ...initialBaseTabState, + selectedAccount: + props.accounts + .filter(account => account.apiType === 'enterprise') + .at(0) || null, }, bitbucketTabState: { kind: 'bitbucket', filterText: '', selectedItem: null, ...initialBaseTabState, + selectedAccount: + props.accounts + .filter(account => account.apiType === 'bitbucket') + .at(0) || null, }, gitlabTabState: { kind: 'gitlab', filterText: '', selectedItem: null, ...initialBaseTabState, + selectedAccount: + props.accounts + .filter(account => account.apiType === 'gitlab') + .at(0) || null, }, urlTabState: { kind: 'url', @@ -440,12 +456,13 @@ export class CloneRepository extends React.Component< const tabState = this.getGitHubTabState(tab) const tabAccounts = this.getAccountsForTab(tab, this.props.accounts) - const selectedAccount = this.getAccountForTab(tab) - if (!selectedAccount) { + if (!tabState.selectedAccount) { return {this.renderSignIn(tab)} } else { - const accountState = this.props.apiRepositories.get(selectedAccount) + const accountState = this.props.apiRepositories.get( + tabState.selectedAccount + ) const repositories = accountState === undefined ? null : accountState.repositories const loading = accountState === undefined ? false : accountState.loading @@ -453,7 +470,7 @@ export class CloneRepository extends React.Component< return ( a.endpoint === tabState.selectedAccount?.endpoint - ) - : undefined) ?? tabAccounts.at(0) + const selectedAccount = tabState.selectedAccount return selectedAccount ?? null } @@ -823,26 +834,35 @@ export class CloneRepository extends React.Component< * if possible. */ private async resolveCloneInfo(): Promise { - const { url, lastParsedIdentifier } = this.getSelectedTabState() + const { url, lastParsedIdentifier, selectedAccount } = + this.getSelectedTabState() + const login = selectedAccount ? selectedAccount.login : undefined if (url.endsWith('.wiki.git')) { - return { url } + return { url, login } } - const account = await findAccountForRemoteURL(url, this.props.accounts) + const account = await findAccountForRemoteURL( + url, + this.props.accounts, + undefined, + login + ) if (lastParsedIdentifier !== null && account !== null) { const api = API.fromAccount(account) const { owner, name } = lastParsedIdentifier // Respect the user's preference if they provided an SSH URL const protocol = parseRemote(url)?.protocol - return api.fetchRepositoryCloneInfo(owner, name, protocol).catch(err => { - log.error(`Failed to look up repository clone info for '${url}'`, err) - return { url } - }) + return api + .fetchRepositoryCloneInfo(owner, name, protocol, login) + .catch(err => { + log.error(`Failed to look up repository clone info for '${url}'`, err) + return { url, login } + }) } - return { url } + return null } private onItemClicked = (repository: IAPIRepository, source: ClickSource) => { @@ -875,11 +895,11 @@ export class CloneRepository extends React.Component< return } - const { url, defaultBranch } = cloneInfo + const { url, defaultBranch, login } = cloneInfo this.props.dispatcher.closeFoldout(FoldoutType.Repository) try { - this.cloneImpl(url.trim(), path, defaultBranch) + this.cloneImpl(url.trim(), path, login, defaultBranch) } catch (e) { log.error(`CloneRepository: clone failed to complete to ${path}`, e) this.setState({ loading: false }) @@ -887,8 +907,13 @@ export class CloneRepository extends React.Component< } } - private cloneImpl(url: string, path: string, defaultBranch?: string) { - this.props.dispatcher.clone(url, path, { defaultBranch }) + private cloneImpl( + url: string, + path: string, + login?: string, + defaultBranch?: string + ) { + this.props.dispatcher.clone(url, path, { defaultBranch }, login) this.props.onDismissed() setDefaultDir(Path.resolve(path, '..')) diff --git a/app/src/ui/dispatcher/dispatcher.ts b/app/src/ui/dispatcher/dispatcher.ts index 22b6c58e9a6..87ae19c3005 100644 --- a/app/src/ui/dispatcher/dispatcher.ts +++ b/app/src/ui/dispatcher/dispatcher.ts @@ -8,6 +8,8 @@ import { IAPIRepoRuleset, getDotComAPIEndpoint, IAPICreatePushProtectionBypassResponse, + getAccountForEndpoint, + getEndpointForRepository, } from '../../lib/api' import { shell } from '../../lib/app-shell' import { @@ -35,6 +37,7 @@ import { getBranches, getRebaseSnapshot, getRepositoryType, + setConfigValue, } from '../../lib/git' import { isGitOnPath } from '../../lib/is-git-on-path' import { @@ -168,9 +171,10 @@ export class Dispatcher { * this will post an error to that affect. */ public addRepositories( - paths: ReadonlyArray + paths: ReadonlyArray, + login?: string ): Promise> { - return this.appStore._addRepositories(paths) + return this.appStore._addRepositories(paths, login) } /** @@ -187,9 +191,15 @@ export class Dispatcher { public addTutorialRepository( path: string, endpoint: string, - apiRepository: IAPIFullRepository + apiRepository: IAPIFullRepository, + login: string ) { - return this.appStore._addTutorialRepository(path, endpoint, apiRepository) + return this.appStore._addTutorialRepository( + path, + endpoint, + apiRepository, + login + ) } /** Resume an already started onboarding tutorial */ @@ -809,18 +819,24 @@ export class Dispatcher { * Clone a missing repository to the previous path, and update it's * state in the repository list if the clone completes without error. */ - public cloneAgain(url: string, path: string): Promise { - return this.appStore._cloneAgain(url, path) + public cloneAgain(url: string, path: string, login?: string): Promise { + return this.appStore._cloneAgain(url, path, login) } /** Clone the repository to the path. */ public async clone( url: string, path: string, - options?: { branch?: string; defaultBranch?: string } + options?: { branch?: string; defaultBranch?: string }, + login?: string ): Promise { return this.appStore._completeOpenInDesktop(async () => { - const { promise, repository } = this.appStore._clone(url, path, options) + const { promise, repository } = this.appStore._clone( + url, + path, + options, + login + ) await this.selectRepository(repository) const success = await promise // TODO: this exit condition is not great, bob @@ -828,13 +844,35 @@ export class Dispatcher { return null } - const addedRepositories = await this.addRepositories([path]) + const addedRepositories = await this.addRepositories([path], login) if (addedRepositories.length < 1) { return null } const addedRepository = addedRepositories[0] + + if (login) { + const state = this.appStore.getState() + const accounts = state.accounts + const endpoint = getEndpointForRepository(url) + const account = getAccountForEndpoint(accounts, endpoint, login) + + if (account) { + const verifiedEmails = account.emails + .filter(x => x.verified) + .map(x => x.email) + if (verifiedEmails && verifiedEmails.length > 0) { + await setConfigValue(addedRepository, 'user.name', account.name) + await setConfigValue( + addedRepository, + 'user.email', + verifiedEmails[0] + ) + } + } + } + await this.selectRepository(addedRepository) if (isRepositoryWithForkedGitHubRepository(addedRepository)) { @@ -864,6 +902,14 @@ export class Dispatcher { return this.appStore._updateRepositoryDefaultBranch(repository, branch) } + /** Changes the repository's account */ + public updateRepositoryAccount( + repository: Repository, + account: Account | null + ): Promise { + return this.appStore._updateRepositoryAccount(repository, account) + } + public updateRepositoryEditorOverride( repository: Repository, customEditorOverride: EditorOverride | null @@ -1859,12 +1905,12 @@ export class Dispatcher { } private async openRepositoryFromUrl(action: IOpenRepositoryFromURLAction) { - const { url, pr, branch, filepath } = action + const { url, pr, branch, filepath, login } = action let repository: Repository | null if (pr !== null) { - repository = await this.openPullRequestFromUrl(url, pr) + repository = await this.openPullRequestFromUrl(url, pr, login) } else if (branch !== null) { repository = await this.openBranchNameFromUrl(url, branch) } else { @@ -1918,9 +1964,10 @@ export class Dispatcher { private async openPullRequestFromUrl( url: string, - pr: string + pr: string, + login?: string ): Promise { - const pullRequest = await this.appStore.fetchPullRequest(url, pr) + const pullRequest = await this.appStore.fetchPullRequest(url, pr, login) if (pullRequest === null) { return null @@ -2204,7 +2251,12 @@ export class Dispatcher { return this.fetch(retryAction.repository, FetchType.UserInitiatedTask) case RetryActionType.Clone: - await this.clone(retryAction.url, retryAction.path, retryAction.options) + await this.clone( + retryAction.url, + retryAction.path, + retryAction.options, + retryAction.login + ) break case RetryActionType.Checkout: diff --git a/app/src/ui/lib/avatar.tsx b/app/src/ui/lib/avatar.tsx index 60bd9189bf8..35b74f7a38e 100644 --- a/app/src/ui/lib/avatar.tsx +++ b/app/src/ui/lib/avatar.tsx @@ -31,7 +31,7 @@ const avatarTokenCache = new ExpiringOperationCache< throw new Error('No account found for endpoint') } - const api = new API(endpoint, account.token) + const api = new API(endpoint, account.token, undefined, account.login) const token = await api.getAvatarToken() return forceUnwrap('Avatar token missing', token) @@ -73,7 +73,7 @@ const botAvatarCache = new ExpiringOperationCache< throw new Error('Email does not appear to be a bot email') } - const api = new API(endpoint, account.token) + const api = new API(endpoint, account.token, undefined, account.login) const apiUser = await api.fetchUser(login) if (!apiUser?.avatar_url) { @@ -232,6 +232,10 @@ function getAvatarUrlCandidates( const { email, avatarURL } = user const ep = user.endpoint ?? getDotComAPIEndpoint() + if (user.name === '' || user.name === 'No Account') { + return [] + } + // By leveraging the avatar url from the API (if we've got it) we can // load the avatar from one of the load balanced domains (avatars). We can't // do the same for GHES/GHAE however since the URLs returned by the API are diff --git a/app/src/ui/lib/test-ui-components/test-ui-components.ts b/app/src/ui/lib/test-ui-components/test-ui-components.ts index cba7e8fce53..127fc72e4e0 100644 --- a/app/src/ui/lib/test-ui-components/test-ui-components.ts +++ b/app/src/ui/lib/test-ui-components/test-ui-components.ts @@ -481,7 +481,8 @@ export function showTestUI( repository.gitHubRepository.issuesEnabled, repository.gitHubRepository.isArchived, repository.gitHubRepository.permissions, - repository.gitHubRepository // This ensures the repository has a parent even if it's not a fork for easier testing purposes + repository.gitHubRepository, // This ensures the repository has a parent even if it's not a fork for easier testing purposes + repository.gitHubRepository.login ), repository.missing ) diff --git a/app/src/ui/missing-repository.tsx b/app/src/ui/missing-repository.tsx index 49716efd935..af967380041 100644 --- a/app/src/ui/missing-repository.tsx +++ b/app/src/ui/missing-repository.tsx @@ -177,10 +177,13 @@ export class MissingRepository extends React.Component< return } + const login = gitHubRepository.login || undefined + try { await this.props.dispatcher.cloneAgain( cloneURL, - this.props.repository.path + this.props.repository.path, + login ) } catch (error) { this.props.dispatcher.postError(error) diff --git a/app/src/ui/preferences/accounts.tsx b/app/src/ui/preferences/accounts.tsx index fa0632842c4..76dd24f7338 100644 --- a/app/src/ui/preferences/accounts.tsx +++ b/app/src/ui/preferences/accounts.tsx @@ -1,5 +1,5 @@ import * as React from 'react' -import { Account } from '../../models/account' +import { Account, isDotComAccount } from '../../models/account' import { IAvatarUser } from '../../models/avatar' import { lookupPreferredEmail } from '../../lib/email' import { assertNever } from '../../lib/fatal-error' @@ -10,6 +10,7 @@ import { Avatar } from '../lib/avatar' import { CallToAction } from '../lib/call-to-action' import { enableMultipleEnterpriseAccounts, + enableMultipleLoginAccounts, enableBitbucketIntegration, enableGitLabIntegration, } from '../../lib/feature-flag' @@ -35,16 +36,15 @@ enum SignInType { export class Accounts extends React.Component { public render() { const { accounts } = this.props - const dotComAccount = accounts.find(a => a.apiType === 'dotcom') const bitbucketAccount = accounts.find(a => a.apiType === 'bitbucket') const gitlabAccount = accounts.find(a => a.apiType === 'gitlab') return (

GitHub.com

- {dotComAccount - ? this.renderAccount(dotComAccount, SignInType.DotCom) - : this.renderSignIn(SignInType.DotCom)} + {enableMultipleLoginAccounts() + ? this.renderMultipleDotComAccounts() + : this.renderSingleDotComAccount()}

GitHub Enterprise

{enableMultipleEnterpriseAccounts() @@ -72,6 +72,33 @@ export class Accounts extends React.Component { ) } + private renderSingleDotComAccount() { + const dotComAccount = this.props.accounts.find(isDotComAccount) + + return dotComAccount + ? this.renderAccount(dotComAccount, SignInType.DotCom) + : this.renderSignIn(SignInType.DotCom) + } + + private renderMultipleDotComAccounts() { + const dotComAccounts = this.props.accounts.filter(isDotComAccount) + + return ( + <> + {dotComAccounts.map(account => { + return this.renderAccount(account, SignInType.DotCom) + })} + {dotComAccounts.length === 0 ? ( + this.renderSignIn(SignInType.DotCom) + ) : ( + + )} + + ) + } + private renderSingleEnterpriseAccount() { const enterpriseAccount = this.props.accounts.find( a => a.apiType === 'enterprise' @@ -117,7 +144,10 @@ export class Accounts extends React.Component { type === SignInType.DotCom ? DialogPreferredFocusClassName : undefined return ( - +
diff --git a/app/src/ui/repositories-list/group-repositories.ts b/app/src/ui/repositories-list/group-repositories.ts index ecbd086ceab..95e8e42ed0b 100644 --- a/app/src/ui/repositories-list/group-repositories.ts +++ b/app/src/ui/repositories-list/group-repositories.ts @@ -22,6 +22,7 @@ export type RepositoryListGroup = | { kind: 'dotcom' owner: Owner + login: string } | { kind: 'enterprise' @@ -39,7 +40,7 @@ export const getGroupKey = (group: RepositoryListGroup) => { case 'recent': return `0:recent` case 'dotcom': - return `1:dotcom:${group.owner.login}` + return `1:dotcom:${group.owner.login}:${group.login}` case 'enterprise': return enableMultipleEnterpriseAccounts() ? `2:enterprise:${group.host}` @@ -71,7 +72,11 @@ const getGroupForRepository = (repo: Repositoryish): RepositoryListGroup => { return isGHE(repo.gitHubRepository.endpoint) || isGHES(repo.gitHubRepository.endpoint) ? { kind: 'enterprise', host: getHostForRepository(repo) } - : { kind: 'dotcom', owner: repo.gitHubRepository.owner } + : { + kind: 'dotcom', + owner: repo.gitHubRepository.owner, + login: repo.login || '', + } } return { kind: 'other' } } diff --git a/app/src/ui/repositories-list/repositories-list.tsx b/app/src/ui/repositories-list/repositories-list.tsx index 33e12744b19..2734b5242e4 100644 --- a/app/src/ui/repositories-list/repositories-list.tsx +++ b/app/src/ui/repositories-list/repositories-list.tsx @@ -250,7 +250,7 @@ export class RepositoriesList extends React.Component< } else if (kind === 'other') { return 'Other' } else if (kind === 'dotcom') { - return group.owner.login + return group.owner.login + (group.login ? ' (' + group.login + ')' : '') } else if (kind === 'recent') { return 'Recent' } else { diff --git a/app/src/ui/repository-settings/remote.tsx b/app/src/ui/repository-settings/remote.tsx index 7257923ba55..f04280ccc66 100644 --- a/app/src/ui/repository-settings/remote.tsx +++ b/app/src/ui/repository-settings/remote.tsx @@ -2,11 +2,21 @@ import * as React from 'react' import { IRemote } from '../../models/remote' import { TextBox } from '../lib/text-box' import { DialogContent } from '../dialog' +import { Account } from '../../models/account' +import { AccountPicker } from '../account-picker' +import { Repository } from '../../models/repository' +import { getDotComAPIEndpoint, getEndpointForRepository } from '../../lib/api' interface IRemoteProps { /** The remote being shown. */ readonly remote: IRemote + readonly repository: Repository + + readonly account: Account | null + + readonly accounts: ReadonlyArray + /** The default branch being shown. */ readonly defaultBranch: string | undefined @@ -15,14 +25,57 @@ interface IRemoteProps { /** The function to call when the default branch is changed by the user. */ readonly onDefaultBranchChanged: (branch: string) => void + + /** The function to call when the account is changed by the user. */ + readonly onSelectedAccountChanged: (account: Account) => void } /** The Remote component. */ export class Remote extends React.Component { public render() { const { remote, defaultBranch } = this.props + + const endpoint = this.props.repository.url + ? getEndpointForRepository(this.props.repository.url) + : getDotComAPIEndpoint() + const noaccount = new Account( + 'no-account', + endpoint, + '', + '', + 0, + [], + 'No Account', + -1, + '', + 'free' + ) + + const account = this.props.account || noaccount + + const accounts: ReadonlyArray = [ + noaccount, + ...this.props.accounts.filter(a => a.endpoint === endpoint), + ] + return ( +
+ +
+
+ +
{ onValueChanged={this.props.onDefaultBranchChanged} />
+
+ +
) } diff --git a/app/src/ui/repository-settings/repository-settings.tsx b/app/src/ui/repository-settings/repository-settings.tsx index e1021499185..a5a722c1868 100644 --- a/app/src/ui/repository-settings/repository-settings.tsx +++ b/app/src/ui/repository-settings/repository-settings.tsx @@ -41,6 +41,7 @@ interface IRepositorySettingsProps { readonly remote: IRemote | null readonly repository: Repository readonly repositoryAccount: Account | null + readonly accounts: ReadonlyArray readonly onDismissed: () => void } @@ -76,6 +77,7 @@ interface IRepositorySettingsState { readonly selectedExternalEditor: string | null readonly useCustomEditor: boolean readonly customEditor: ICustomIntegration + readonly repositoryAccount: Account | null } export class RepositorySettings extends React.Component< @@ -114,6 +116,7 @@ export class RepositorySettings extends React.Component< path: '', arguments: '', }, + repositoryAccount: props.repositoryAccount, } } @@ -253,9 +256,13 @@ export class RepositorySettings extends React.Component< return ( ) } else { @@ -393,6 +400,20 @@ export class RepositorySettings extends React.Component< } } + // only update this if it will be different from what we have stored + if ( + this.state.repositoryAccount?.login !== + this.props.repositoryAccount?.login + ) { + await this.props.dispatcher.updateRepositoryAccount( + this.props.repository, + this.state.repositoryAccount && + this.state.repositoryAccount.login !== 'no-account' + ? this.state.repositoryAccount + : null + ) + } + // only update this if it will be different from what we have stored if ( this.state.forkContributionTarget !== @@ -508,6 +529,10 @@ export class RepositorySettings extends React.Component< this.setState({ defaultBranch: branch }) } + private onSelectedAccountChanged = (account: Account) => { + this.setState({ repositoryAccount: account }) + } + private onGitConfigLocationChanged = (value: GitConfigLocation) => { this.setState({ gitConfigLocation: value }) } diff --git a/app/test/helpers/github-repo-builder.ts b/app/test/helpers/github-repo-builder.ts index ade6bfb6ed0..1f813d34cf4 100644 --- a/app/test/helpers/github-repo-builder.ts +++ b/app/test/helpers/github-repo-builder.ts @@ -24,6 +24,7 @@ interface IGitHubRepoFixtureOptions { * clone url and html url from this, even if its ''. */ endpoint?: string + login?: string } /** @@ -39,6 +40,7 @@ export function gitHubRepoFixture({ parent, endpoint, isPrivate, + login, }: IGitHubRepoFixtureOptions): GitHubRepository { const htmlUrl = `${ endpoint !== undefined ? endpoint : 'https://github.com' @@ -58,6 +60,7 @@ export function gitHubRepoFixture({ null, null, null, - parent + parent, + login ) } diff --git a/app/test/helpers/repository-builder-branch-pruner.ts b/app/test/helpers/repository-builder-branch-pruner.ts index dbbaa25d63a..a1867dfd17f 100644 --- a/app/test/helpers/repository-builder-branch-pruner.ts +++ b/app/test/helpers/repository-builder-branch-pruner.ts @@ -66,15 +66,17 @@ export async function setupRepository( repositoriesStateCache: RepositoryStateCache, includesGhRepo: boolean, defaultBranchName: string, - lastPruneDate?: Date + lastPruneDate?: Date, + login?: string ) { - let repository = await repositoriesStore.addRepository(path) + let repository = await repositoriesStore.addRepository(path, undefined) if (includesGhRepo) { const apiRepo: IAPIFullRepository = { clone_url: 'string', ssh_url: 'string', html_url: 'string', name: 'string', + login: 'string', owner: { id: 0, html_url: '', @@ -99,7 +101,7 @@ export async function setupRepository( const endpoint = getDotComAPIEndpoint() repository = await repositoriesStore.setGitHubRepository( repository, - await repositoriesStore.upsertGitHubRepository(endpoint, apiRepo) + await repositoriesStore.upsertGitHubRepository(endpoint, apiRepo, login) ) } await primeCaches(repository, repositoriesStateCache) diff --git a/app/test/unit/stores/updates/update-remote-url-test.ts b/app/test/unit/stores/updates/update-remote-url-test.ts index 54d581aaf3e..a88602737a3 100644 --- a/app/test/unit/stores/updates/update-remote-url-test.ts +++ b/app/test/unit/stores/updates/update-remote-url-test.ts @@ -40,7 +40,8 @@ describe('Update remote url', () => { const createRepository = async ( t: TestContext, apiRepo: IAPIFullRepository, - remoteUrl: string | null = null + remoteUrl: string | null = null, + login?: string ) => { const db = new TestRepositoriesDatabase() await db.reset() @@ -49,7 +50,7 @@ describe('Update remote url', () => { const repoPath = await setupFixtureRepository(t, 'test-repo') const repository = await repositoriesStore.setGitHubRepository( await repositoriesStore.addRepository(repoPath), - await repositoriesStore.upsertGitHubRepository(endpoint, apiRepo) + await repositoriesStore.upsertGitHubRepository(endpoint, apiRepo, login) ) await addRemote(repository, 'origin', remoteUrl || apiRepo.clone_url) gitStore = new GitStore(repository, shell, new TestStatsStore()) diff --git a/app/test/unit/text-token-parser-test.ts b/app/test/unit/text-token-parser-test.ts index 321899f953a..aa30694c5ca 100644 --- a/app/test/unit/text-token-parser-test.ts +++ b/app/test/unit/text-token-parser-test.ts @@ -53,6 +53,7 @@ describe('Tokenizer', () => { const gitHubRepository = gitHubRepoFixture({ name, + login, owner: login, isPrivate: false, })