From 640ad1c43e694dd2fcbbb14d6200419eeee7f672 Mon Sep 17 00:00:00 2001 From: Thomas Cooper Date: Fri, 19 Jun 2026 18:05:47 -0400 Subject: [PATCH 1/2] feat(proxy): resolve push identity from token via SCM provider API (#1400) parsePush incorrectly uses the last commit's committer as the push user. This adds a new chain processor that extracts the token from HTTP Basic auth, calls the SCM provider's user API (GitHub GET /user for now), and maps the SCM login to a git-proxy user via the gitAccount field. - TokenIdentityProvider interface with hostname-based dispatch - GitHubTokenIdentityProvider calling api.github.com/user - resolveUserFromToken chain processor (non-blocking on failure) - findUserByGitAccount DB lookup (file + mongo) - GET/PUT /api/v1/user/:username/git-account endpoints --- src/db/file/index.ts | 1 + src/db/file/users.ts | 13 + src/db/index.ts | 2 + src/db/mongo/index.ts | 1 + src/db/mongo/users.ts | 6 + src/db/types.ts | 1 + src/proxy/chain.ts | 1 + src/proxy/processors/push-action/index.ts | 2 + .../push-action/resolveUserFromToken.ts | 125 ++++++ .../processors/push-action/tokenIdentity.ts | 114 +++++ src/service/routes/users.ts | 52 +++ test/chain.test.ts | 1 + test/db/file/users.test.ts | 49 +++ test/db/mongo/user.test.ts | 34 ++ test/processors/resolveUserFromToken.test.ts | 395 ++++++++++++++++++ test/services/routes/users.test.ts | 140 +++++++ 16 files changed, 937 insertions(+) create mode 100644 src/proxy/processors/push-action/resolveUserFromToken.ts create mode 100644 src/proxy/processors/push-action/tokenIdentity.ts create mode 100644 test/processors/resolveUserFromToken.test.ts diff --git a/src/db/file/index.ts b/src/db/file/index.ts index e6b422027..0693e860e 100644 --- a/src/db/file/index.ts +++ b/src/db/file/index.ts @@ -39,6 +39,7 @@ export const { export const { findUser, findUserByEmail, + findUserByGitAccount, findUserByOIDC, findUserBySSHKey, getUsers, diff --git a/src/db/file/users.ts b/src/db/file/users.ts index 9e30e3445..92909df83 100644 --- a/src/db/file/users.ts +++ b/src/db/file/users.ts @@ -92,6 +92,19 @@ export const findUserByEmail = (email: string): Promise => { }); }; +export const findUserByGitAccount = (gitAccount: string): Promise => { + return new Promise((resolve, reject) => { + db.findOne({ gitAccount: gitAccount.toLowerCase() }, (err: Error | null, doc: User) => { + /* istanbul ignore if */ + if (err) { + reject(err); + } else { + resolve(doc ?? null); + } + }); + }); +}; + export const findUserByOIDC = function (oidcId: string): Promise { return new Promise((resolve, reject) => { db.findOne({ oidcId: oidcId }, (err: Error | null, doc: User) => { diff --git a/src/db/index.ts b/src/db/index.ts index e29e6282a..7b3ff6896 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -204,6 +204,8 @@ export const deleteRepo = (_id: string): Promise => start().deleteRepo(_id export const findUser = (username: string): Promise => start().findUser(username); export const findUserByEmail = (email: string): Promise => start().findUserByEmail(email); +export const findUserByGitAccount = (gitAccount: string): Promise => + start().findUserByGitAccount(gitAccount); export const findUserByOIDC = (oidcId: string): Promise => start().findUserByOIDC(oidcId); export const findUserBySSHKey = (sshKey: string): Promise => diff --git a/src/db/mongo/index.ts b/src/db/mongo/index.ts index 1114b9a29..e5b0b9f25 100644 --- a/src/db/mongo/index.ts +++ b/src/db/mongo/index.ts @@ -39,6 +39,7 @@ export const { export const { findUser, findUserByEmail, + findUserByGitAccount, findUserByOIDC, findUserBySSHKey, getUsers, diff --git a/src/db/mongo/users.ts b/src/db/mongo/users.ts index 04d748c03..187e143e0 100644 --- a/src/db/mongo/users.ts +++ b/src/db/mongo/users.ts @@ -34,6 +34,12 @@ export const findUserByEmail = async function (email: string): Promise { + const collection = await connect(collectionName); + const doc = await collection.findOne({ gitAccount: { $eq: gitAccount.toLowerCase() } }); + return doc ? toClass(doc, User.prototype) : null; +}; + export const findUserByOIDC = async function (oidcId: string): Promise { const collection = await connect(collectionName); const doc = await collection.findOne({ oidcId: { $eq: oidcId } }); diff --git a/src/db/types.ts b/src/db/types.ts index a77838300..ac6ec3125 100644 --- a/src/db/types.ts +++ b/src/db/types.ts @@ -138,6 +138,7 @@ export interface Sink { deleteRepo: (_id: string) => Promise; findUser: (username: string) => Promise; findUserByEmail: (email: string) => Promise; + findUserByGitAccount: (gitAccount: string) => Promise; findUserByOIDC: (oidcId: string) => Promise; findUserBySSHKey: (sshKey: string) => Promise; getUsers: (query?: Partial) => Promise; diff --git a/src/proxy/chain.ts b/src/proxy/chain.ts index ab63f1f8d..cd24fe333 100644 --- a/src/proxy/chain.ts +++ b/src/proxy/chain.ts @@ -24,6 +24,7 @@ import { handleErrorAndLog } from '../utils/errors'; const pushActionChain: ((req: Request, action: Action) => Promise)[] = [ proc.push.parsePush, + proc.push.resolveUserFromToken, proc.push.checkEmptyBranch, proc.push.checkRepoInAuthorisedList, proc.push.checkCommitMessages, diff --git a/src/proxy/processors/push-action/index.ts b/src/proxy/processors/push-action/index.ts index 5bcb0d0f3..303fea941 100644 --- a/src/proxy/processors/push-action/index.ts +++ b/src/proxy/processors/push-action/index.ts @@ -29,6 +29,7 @@ import { exec as checkIfWaitingAuth } from './checkIfWaitingAuth'; import { exec as checkCommitMessages } from './checkCommitMessages'; import { exec as checkAuthorEmails } from './checkAuthorEmails'; import { exec as checkUserPushPermission } from './checkUserPushPermission'; +import { exec as resolveUserFromToken } from './resolveUserFromToken'; import { exec as checkEmptyBranch } from './checkEmptyBranch'; @@ -47,5 +48,6 @@ export { checkCommitMessages, checkAuthorEmails, checkUserPushPermission, + resolveUserFromToken, checkEmptyBranch, }; diff --git a/src/proxy/processors/push-action/resolveUserFromToken.ts b/src/proxy/processors/push-action/resolveUserFromToken.ts new file mode 100644 index 000000000..c72aab737 --- /dev/null +++ b/src/proxy/processors/push-action/resolveUserFromToken.ts @@ -0,0 +1,125 @@ +/** + * Copyright 2026 GitProxy Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Request } from 'express'; + +import { Action, Step } from '../../actions'; +import { getProviderForHost, scmTokenCache } from './tokenIdentity'; +import { findUserByGitAccount } from '../../../db'; +import { getErrorMessage } from '../../../utils/errors'; + +async function exec(req: Request, action: Action): Promise { + const step = new Step('resolveUserFromToken'); + + if (req.user) { + step.log(`User already resolved via session auth: ${action.user}`); + action.addStep(step); + return action; + } + + try { + const authHeader = req.headers?.authorization; + if (!authHeader) { + step.log('No Authorization header — cannot resolve push identity from token'); + action.addStep(step); + return action; + } + + const [scheme, encoded] = authHeader.split(' '); + if (!scheme || !encoded || scheme.toLowerCase() !== 'basic') { + step.log('Authorization header is not Basic — cannot resolve push identity'); + action.addStep(step); + return action; + } + + const credentials = Buffer.from(encoded, 'base64').toString(); + const separatorIndex = credentials.indexOf(':'); + if (separatorIndex === -1) { + step.log('Malformed Basic auth credentials'); + action.addStep(step); + return action; + } + + const token = credentials.slice(separatorIndex + 1); + + let hostname: string; + try { + hostname = new URL(action.url).hostname; + } catch { + step.log(`Cannot parse hostname from action URL: ${action.url}`); + action.addStep(step); + return action; + } + + const provider = getProviderForHost(hostname); + if (!provider) { + step.log(`No token identity provider for host '${hostname}' — identity resolution skipped`); + action.addStep(step); + return action; + } + + const cached = scmTokenCache.lookup(provider.name, token); + if (cached) { + step.log(`${provider.name}: resolved push identity from cache: ${cached}`); + action.user = cached; + action.addStep(step); + return action; + } + + const identity = await provider.fetchScmIdentity(token); + if (!identity) { + step.log( + `${provider.name}: failed to resolve identity from token (invalid token or missing scope?)`, + ); + action.addStep(step); + return action; + } + + step.log( + `${provider.name}: resolved SCM identity from token: ${identity.login} (${identity.email ?? 'no public email'})`, + ); + + const user = await findUserByGitAccount(identity.login); + if (user) { + step.log( + `Mapped SCM identity '${identity.login}' to git-proxy user '${user.username}' (${user.email})`, + ); + action.user = user.username; + action.userEmail = user.email; + scmTokenCache.store(provider.name, token, user.username); + } else { + step.log( + `No git-proxy user has gitAccount '${identity.login}' — ` + + `falling back to SCM identity. ` + + `Users can associate their account via PUT /api/v1/user/:username/git-account`, + ); + action.user = identity.login; + if (identity.email) { + action.userEmail = identity.email; + } + } + } catch (error: unknown) { + const msg = getErrorMessage(error); + step.log(`Failed to resolve push identity from token: ${msg}`); + } + + action.addStep(step); + return action; +} + +exec.displayName = 'resolveUserFromToken.exec'; + +export { exec }; diff --git a/src/proxy/processors/push-action/tokenIdentity.ts b/src/proxy/processors/push-action/tokenIdentity.ts new file mode 100644 index 000000000..edbd691d8 --- /dev/null +++ b/src/proxy/processors/push-action/tokenIdentity.ts @@ -0,0 +1,114 @@ +/** + * Copyright 2026 GitProxy Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import crypto from 'crypto'; + +export type ScmUserInfo = { + login: string; + email?: string; +}; + +type CacheEntry = { username: string; provider: string; cachedAt: number }; +const DEFAULT_TTL_MS = 5 * 60 * 1000; + +export class ScmTokenCache { + private readonly cache = new Map(); + private readonly ttlMs: number; + + constructor(ttlMs = DEFAULT_TTL_MS) { + this.ttlMs = ttlMs; + } + + private key(provider: string, token: string): string { + return crypto.createHash('sha512').update(`${provider}:${token}`).digest('hex'); + } + + lookup(provider: string, token: string): string | null { + const k = this.key(provider, token); + const entry = this.cache.get(k); + if (!entry) return null; + if (Date.now() - entry.cachedAt > this.ttlMs) { + this.cache.delete(k); + return null; + } + return entry.username; + } + + store(provider: string, token: string, username: string): void { + this.cache.set(this.key(provider, token), { username, provider, cachedAt: Date.now() }); + } + + evictByUsername(provider: string, username: string): void { + for (const [k, entry] of this.cache.entries()) { + if (entry.username === username && entry.provider === provider) this.cache.delete(k); + } + } +} + +export const scmTokenCache = new ScmTokenCache(); + +export interface TokenIdentityProvider { + readonly name: string; + matches(hostname: string): boolean; + fetchScmIdentity(token: string): Promise; +} + +type GitHubUserResponse = { + login: string; + id: number; + email: string | null; +}; + +export class GitHubTokenIdentityProvider implements TokenIdentityProvider { + readonly name = 'github'; + + matches(hostname: string): boolean { + return hostname === 'github.com'; + } + + async fetchScmIdentity(token: string): Promise { + try { + const response = await fetch('https://api.github.com/user', { + headers: { + Authorization: `token ${token}`, + Accept: 'application/vnd.github+json', + }, + }); + + if (!response.ok) { + console.warn( + `GitHub /user API returned ${response.status} — token may be invalid or lack read:user scope`, + ); + return null; + } + + const user: GitHubUserResponse = await response.json(); + return { + login: user.login, + email: user.email ?? undefined, + }; + } catch (e) { + console.warn(`Failed to fetch GitHub identity: ${e}`); + return null; + } + } +} + +const providers: TokenIdentityProvider[] = [new GitHubTokenIdentityProvider()]; + +export function getProviderForHost(hostname: string): TokenIdentityProvider | null { + return providers.find((p) => p.matches(hostname)) ?? null; +} diff --git a/src/service/routes/users.ts b/src/service/routes/users.ts index 385ad1da0..3db8e6688 100644 --- a/src/service/routes/users.ts +++ b/src/service/routes/users.ts @@ -19,6 +19,7 @@ import crypto from 'crypto'; import * as db from '../../db'; import { toPublicUser } from './utils'; +import { scmTokenCache } from '../../proxy/processors/push-action/tokenIdentity'; const router = express.Router(); @@ -62,6 +63,57 @@ router.get('/:id', async (req: Request<{ id: string }>, res: Response) => { res.send(toPublicUser(user)); }); +// Get git account (SCM identity) for a user +router.get('/:username/git-account', async (req: Request<{ username: string }>, res: Response) => { + const targetUsername = req.params.username.toLowerCase(); + const user = await db.findUser(targetUsername); + if (!user) { + res.status(404).json({ error: `User ${targetUsername} not found` }); + return; + } + res.json({ username: user.username, gitAccount: user.gitAccount }); +}); + +// Set git account (SCM identity) for a user +router.put('/:username/git-account', async (req: Request<{ username: string }>, res: Response) => { + if (!req.user) { + res.status(401).json({ error: 'Authentication required' }); + return; + } + + const { username, admin } = req.user as { username: string; admin: boolean }; + const targetUsername = req.params.username.toLowerCase(); + + if (username !== targetUsername && !admin) { + res.status(403).json({ error: 'Not authorized to update git account for this user' }); + return; + } + + const { gitAccount } = req.body; + if (!gitAccount || typeof gitAccount !== 'string' || !gitAccount.trim()) { + res.status(400).json({ error: 'gitAccount is required' }); + return; + } + + const existing = await db.findUser(targetUsername); + if (!existing) { + res.status(404).json({ error: `User ${targetUsername} not found` }); + return; + } + + const conflict = await db.findUserByGitAccount(gitAccount.trim()); + if (conflict && conflict.username !== targetUsername) { + res + .status(409) + .json({ error: `Git account '${gitAccount}' is already associated with another user` }); + return; + } + + await db.updateUser({ username: targetUsername, gitAccount: gitAccount.trim() }); + scmTokenCache.evictByUsername('github', targetUsername); + res.json({ username: targetUsername, gitAccount: gitAccount.trim() }); +}); + // Get SSH key fingerprints for a user router.get( '/:username/ssh-key-fingerprints', diff --git a/test/chain.test.ts b/test/chain.test.ts index 44eb6750f..c9b83b488 100644 --- a/test/chain.test.ts +++ b/test/chain.test.ts @@ -35,6 +35,7 @@ const initMockPushProcessors = () => { checkCommitMessages: vi.fn(), checkAuthorEmails: vi.fn(), checkUserPushPermission: vi.fn(), + resolveUserFromToken: vi.fn(), checkIfWaitingAuth: vi.fn(), checkHiddenCommits: vi.fn(), pullRemote: vi.fn(), diff --git a/test/db/file/users.test.ts b/test/db/file/users.test.ts index b099cbc5a..8989cdddf 100644 --- a/test/db/file/users.test.ts +++ b/test/db/file/users.test.ts @@ -18,6 +18,55 @@ import { describe, it, expect, beforeEach } from 'vitest'; import * as dbUsers from '../../../src/db/file/users'; import { User, PublicKeyRecord } from '../../../src/db/types'; +describe('db/file/users findUserByGitAccount', () => { + beforeEach(async () => { + const allUsers = await dbUsers.getUsers(); + for (const user of allUsers) { + await dbUsers.deleteUser(user.username); + } + }); + + it('should find user by gitAccount', async () => { + const testUser: User = { + username: 'testuser', + password: 'password', + email: 'test@example.com', + publicKeys: [], + gitAccount: 'octocat', + admin: false, + }; + + await dbUsers.createUser(testUser); + + const found = await dbUsers.findUserByGitAccount('octocat'); + expect(found).toBeDefined(); + expect(found?.username).toBe('testuser'); + expect(found?.gitAccount).toBe('octocat'); + }); + + it('should be case-insensitive', async () => { + const testUser: User = { + username: 'testuser', + password: 'password', + email: 'test@example.com', + publicKeys: [], + gitAccount: 'octocat', + admin: false, + }; + + await dbUsers.createUser(testUser); + + const found = await dbUsers.findUserByGitAccount('Octocat'); + expect(found).toBeDefined(); + expect(found?.username).toBe('testuser'); + }); + + it('should return null when no user has the gitAccount', async () => { + const found = await dbUsers.findUserByGitAccount('nonexistent'); + expect(found).toBeNull(); + }); +}); + describe('db/file/users SSH Key Functions', () => { beforeEach(async () => { // Clear the database before each test diff --git a/test/db/mongo/user.test.ts b/test/db/mongo/user.test.ts index b6a35c7b8..2e2c9be15 100644 --- a/test/db/mongo/user.test.ts +++ b/test/db/mongo/user.test.ts @@ -48,6 +48,7 @@ describe('MongoDB User', async () => { const { findUser, findUserByEmail, + findUserByGitAccount, findUserByOIDC, getUsers, deleteUser, @@ -142,6 +143,39 @@ describe('MongoDB User', async () => { }); }); + describe('findUserByGitAccount', () => { + it('should find user by git account', async () => { + const userData = { ...TEST_USER, gitAccount: 'octocat' }; + mockFindOne.mockResolvedValue(userData); + mockToClass.mockReturnValue(userData); + + const result = await findUserByGitAccount('Octocat'); + + expect(mockConnect).toHaveBeenCalledWith('users'); + expect(mockFindOne).toHaveBeenCalledWith({ gitAccount: { $eq: 'octocat' } }); + expect(mockToClass).toHaveBeenCalledWith(userData, User.prototype); + expect(result).toEqual(userData); + }); + + it('should convert git account to lowercase', async () => { + mockFindOne.mockResolvedValue(TEST_USER); + mockToClass.mockReturnValue(TEST_USER); + + await findUserByGitAccount('UPPERCASE'); + + expect(mockFindOne).toHaveBeenCalledWith({ gitAccount: { $eq: 'uppercase' } }); + }); + + it('should return null when user not found', async () => { + mockFindOne.mockResolvedValue(null); + + const result = await findUserByGitAccount('nonexistent'); + + expect(result).toBeNull(); + expect(mockToClass).not.toHaveBeenCalled(); + }); + }); + describe('findUserByOIDC', () => { it('should find user by OIDC ID', async () => { const userData = { ...TEST_USER }; diff --git a/test/processors/resolveUserFromToken.test.ts b/test/processors/resolveUserFromToken.test.ts new file mode 100644 index 000000000..09a9d8129 --- /dev/null +++ b/test/processors/resolveUserFromToken.test.ts @@ -0,0 +1,395 @@ +/** + * Copyright 2026 GitProxy Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { Action } from '../../src/proxy/actions'; +import { + GitHubTokenIdentityProvider, + getProviderForHost, + ScmTokenCache, +} from '../../src/proxy/processors/push-action/tokenIdentity'; +import { Request } from 'express'; + +function makeAction(url: string): Action { + return new Action('test-id', 'push', 'POST', Date.now(), url); +} + +function makeRequest(overrides: Partial = {}): Request { + const token = 'ghp_testtoken123'; + const encoded = Buffer.from(`x-access-token:${token}`).toString('base64'); + return { + headers: { + authorization: `Basic ${encoded}`, + }, + ...overrides, + } as unknown as Request; +} + +describe('GitHubTokenIdentityProvider', () => { + const provider = new GitHubTokenIdentityProvider(); + let fetchSpy: ReturnType; + + beforeEach(() => { + fetchSpy = vi.spyOn(globalThis, 'fetch') as ReturnType; + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + describe('matches', () => { + it('should match github.com', () => { + expect(provider.matches('github.com')).toBe(true); + }); + + it('should not match gitlab.com', () => { + expect(provider.matches('gitlab.com')).toBe(false); + }); + + it('should not match bitbucket.org', () => { + expect(provider.matches('bitbucket.org')).toBe(false); + }); + + it('should not match self-hosted github enterprise', () => { + expect(provider.matches('github.mycompany.com')).toBe(false); + }); + }); + + describe('fetchScmIdentity', () => { + it('should return login and email on success', async () => { + fetchSpy.mockResolvedValueOnce({ + ok: true, + json: async () => ({ login: 'octocat', id: 1, email: 'octocat@github.com' }), + } as Response); + + const result = await provider.fetchScmIdentity('ghp_token123'); + + expect(result).toEqual({ login: 'octocat', email: 'octocat@github.com' }); + expect(fetchSpy).toHaveBeenCalledWith('https://api.github.com/user', { + headers: { + Authorization: 'token ghp_token123', + Accept: 'application/vnd.github+json', + }, + }); + }); + + it('should return login with undefined email when GitHub returns null email', async () => { + fetchSpy.mockResolvedValueOnce({ + ok: true, + json: async () => ({ login: 'private-user', id: 2, email: null }), + } as Response); + + const result = await provider.fetchScmIdentity('ghp_token456'); + + expect(result).toEqual({ login: 'private-user', email: undefined }); + }); + + it('should return null on non-OK response', async () => { + fetchSpy.mockResolvedValueOnce({ + ok: false, + status: 401, + } as Response); + + const result = await provider.fetchScmIdentity('ghp_bad_token'); + + expect(result).toBeNull(); + }); + + it('should return null on network error', async () => { + fetchSpy.mockRejectedValueOnce(new Error('ECONNREFUSED')); + + const result = await provider.fetchScmIdentity('ghp_token789'); + + expect(result).toBeNull(); + }); + }); +}); + +describe('getProviderForHost', () => { + it('should return GitHubTokenIdentityProvider for github.com', () => { + const provider = getProviderForHost('github.com'); + expect(provider).not.toBeNull(); + expect(provider!.name).toBe('github'); + }); + + it('should return null for unsupported hosts', () => { + expect(getProviderForHost('gitlab.com')).toBeNull(); + expect(getProviderForHost('bitbucket.org')).toBeNull(); + expect(getProviderForHost('my-git.internal.com')).toBeNull(); + }); +}); + +describe('ScmTokenCache', () => { + it('should return null on cache miss', () => { + const cache = new ScmTokenCache(); + expect(cache.lookup('github', 'sometoken')).toBeNull(); + }); + + it('should return username on cache hit', () => { + const cache = new ScmTokenCache(); + cache.store('github', 'sometoken', 'octocat'); + expect(cache.lookup('github', 'sometoken')).toBe('octocat'); + }); + + it('should return null after TTL expires', () => { + const cache = new ScmTokenCache(100); // 100ms TTL + cache.store('github', 'sometoken', 'octocat'); + // manually backdate the cache entry + const key = (cache as any).key('github', 'sometoken'); + (cache as any).cache.set(key, { username: 'octocat', cachedAt: Date.now() - 200 }); + expect(cache.lookup('github', 'sometoken')).toBeNull(); + }); + + it('should not share entries across providers', () => { + const cache = new ScmTokenCache(); + cache.store('github', 'sometoken', 'octocat'); + expect(cache.lookup('gitlab', 'sometoken')).toBeNull(); + }); + + it('should evict entries by username', () => { + const cache = new ScmTokenCache(); + cache.store('github', 'token1', 'alice'); + cache.store('github', 'token2', 'alice'); + cache.store('github', 'token3', 'bob'); + cache.evictByUsername('github', 'alice'); + expect(cache.lookup('github', 'token1')).toBeNull(); + expect(cache.lookup('github', 'token2')).toBeNull(); + expect(cache.lookup('github', 'token3')).toBe('bob'); + }); + + it('should not evict across providers', () => { + const cache = new ScmTokenCache(); + cache.store('github', 'sometoken', 'alice'); + cache.evictByUsername('gitlab', 'alice'); + expect(cache.lookup('github', 'sometoken')).toBe('alice'); + }); +}); + +describe('resolveUserFromToken', () => { + let fetchSpy: ReturnType; + let exec: typeof import('../../src/proxy/processors/push-action/resolveUserFromToken').exec; + + beforeEach(async () => { + vi.resetModules(); + + vi.doMock('../../src/db', () => ({ + findUserByGitAccount: vi.fn().mockResolvedValue(null), + })); + + fetchSpy = vi.spyOn(globalThis, 'fetch') as ReturnType; + + const mod = await import('../../src/proxy/processors/push-action/resolveUserFromToken'); + exec = mod.exec; + }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.resetModules(); + }); + + it('should skip when req.user is set (session auth)', async () => { + const req = makeRequest({ user: { username: 'session-user', email: 'a@b.com' } } as any); + const action = makeAction('https://github.com/finos/git-proxy.git'); + action.user = 'session-user'; + + const result = await exec(req, action); + + expect(result.user).toBe('session-user'); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it('should resolve GitHub identity from token and fall back to SCM identity when no DB user', async () => { + fetchSpy.mockResolvedValueOnce({ + ok: true, + json: async () => ({ login: 'octocat', id: 1, email: 'octocat@github.com' }), + } as Response); + + const req = makeRequest(); + const action = makeAction('https://github.com/finos/git-proxy.git'); + + const result = await exec(req, action); + + expect(result.user).toBe('octocat'); + expect(result.userEmail).toBe('octocat@github.com'); + expect(fetchSpy).toHaveBeenCalledWith('https://api.github.com/user', { + headers: { + Authorization: 'token ghp_testtoken123', + Accept: 'application/vnd.github+json', + }, + }); + }); + + it('should map SCM identity to git-proxy user when gitAccount matches', async () => { + vi.resetModules(); + + vi.doMock('../../src/db', () => ({ + findUserByGitAccount: vi.fn().mockResolvedValue({ + username: 'tcooper', + email: 'thomas.cooper@example.com', + gitAccount: 'octocat', + }), + })); + + const mod = await import('../../src/proxy/processors/push-action/resolveUserFromToken'); + + fetchSpy.mockResolvedValueOnce({ + ok: true, + json: async () => ({ login: 'octocat', id: 1, email: 'octocat@github.com' }), + } as Response); + + const req = makeRequest(); + const action = makeAction('https://github.com/finos/git-proxy.git'); + + const result = await mod.exec(req, action); + + expect(result.user).toBe('tcooper'); + expect(result.userEmail).toBe('thomas.cooper@example.com'); + }); + + it('should handle GitHub identity with null email gracefully', async () => { + fetchSpy.mockResolvedValueOnce({ + ok: true, + json: async () => ({ login: 'octocat', id: 1, email: null }), + } as Response); + + const req = makeRequest(); + const action = makeAction('https://github.com/finos/git-proxy.git'); + action.userEmail = 'committer@example.com'; + + const result = await exec(req, action); + + expect(result.user).toBe('octocat'); + expect(result.userEmail).toBe('committer@example.com'); + }); + + it('should not call fetch for non-GitHub hosts', async () => { + const req = makeRequest(); + const action = makeAction('https://gitlab.com/finos/git-proxy.git'); + + const result = await exec(req, action); + + expect(fetchSpy).not.toHaveBeenCalled(); + expect(result.user).toBeUndefined(); + }); + + it('should handle GitHub API errors gracefully without blocking', async () => { + fetchSpy.mockResolvedValueOnce({ + ok: false, + status: 401, + } as Response); + + const req = makeRequest(); + const action = makeAction('https://github.com/finos/git-proxy.git'); + action.user = 'committer-fallback'; + + const result = await exec(req, action); + + expect(result.user).toBe('committer-fallback'); + expect(result.error).toBe(false); + }); + + it('should handle network errors gracefully without blocking', async () => { + fetchSpy.mockRejectedValueOnce(new Error('ECONNREFUSED')); + + const req = makeRequest(); + const action = makeAction('https://github.com/finos/git-proxy.git'); + + const result = await exec(req, action); + + expect(result.error).toBe(false); + }); + + it('should skip when no Authorization header is present', async () => { + const req = { headers: {} } as unknown as Request; + const action = makeAction('https://github.com/finos/git-proxy.git'); + + const result = await exec(req, action); + + expect(fetchSpy).not.toHaveBeenCalled(); + expect(result.user).toBeUndefined(); + }); + + it('should skip when Authorization header is not Basic', async () => { + const req = { + headers: { authorization: 'Bearer some-jwt' }, + } as unknown as Request; + const action = makeAction('https://github.com/finos/git-proxy.git'); + + await exec(req, action); + + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it('should skip when Basic auth credentials have no colon separator', async () => { + const encoded = Buffer.from('no-colon-here').toString('base64'); + const req = { + headers: { authorization: `Basic ${encoded}` }, + } as unknown as Request; + const action = makeAction('https://github.com/finos/git-proxy.git'); + + const result = await exec(req, action); + + expect(fetchSpy).not.toHaveBeenCalled(); + expect(result.error).toBe(false); + }); + + it('should skip when action URL is unparseable', async () => { + const req = makeRequest(); + const action = makeAction('https://github.com/finos/git-proxy.git'); + action.url = 'not-a-valid-url'; + + const result = await exec(req, action); + + expect(fetchSpy).not.toHaveBeenCalled(); + expect(result.error).toBe(false); + }); +}); + +describe('resolveUserFromToken cache integration', () => { + let fetchSpy: ReturnType; + + beforeEach(() => { + vi.resetModules(); + fetchSpy = vi.spyOn(globalThis, 'fetch') as ReturnType; + }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.resetModules(); + }); + + it('should return cached identity without calling the API', async () => { + vi.doMock('../../src/db', () => ({ + findUserByGitAccount: vi.fn(), + })); + vi.doMock('../../src/proxy/processors/push-action/tokenIdentity', async () => { + const real = await vi.importActual< + typeof import('../../src/proxy/processors/push-action/tokenIdentity') + >('../../src/proxy/processors/push-action/tokenIdentity'); + const cache = new real.ScmTokenCache(); + cache.store('github', 'ghp_testtoken123', 'cached-user'); + return { ...real, scmTokenCache: cache }; + }); + const mod = await import('../../src/proxy/processors/push-action/resolveUserFromToken'); + const req = makeRequest(); + const action = makeAction('https://github.com/finos/git-proxy.git'); + + const result = await mod.exec(req, action); + + expect(result.user).toBe('cached-user'); + expect(fetchSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/test/services/routes/users.test.ts b/test/services/routes/users.test.ts index 6df9e3a57..41d2054b7 100644 --- a/test/services/routes/users.test.ts +++ b/test/services/routes/users.test.ts @@ -86,6 +86,146 @@ describe('Users API', () => { }); }); + describe('Git Account Management', () => { + beforeEach(() => { + vi.spyOn(db, 'findUserByGitAccount').mockResolvedValue(null); + vi.spyOn(db, 'updateUser').mockResolvedValue(undefined); + }); + + describe('GET /users/:username/git-account', () => { + it('should return git account for existing user', async () => { + const res = await request(app).get('/users/bob/git-account'); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ username: 'bob', gitAccount: '' }); + }); + + it('should return 404 for non-existent user', async () => { + vi.spyOn(db, 'findUser').mockResolvedValueOnce(null); + + const res = await request(app).get('/users/nonexistent/git-account'); + + expect(res.status).toBe(404); + }); + }); + + describe('PUT /users/:username/git-account', () => { + it('should return 401 when not authenticated', async () => { + const res = await request(app) + .put('/users/bob/git-account') + .send({ gitAccount: 'octocat' }); + + expect(res.status).toBe(401); + }); + + it('should return 403 when non-admin tries to update other user', async () => { + const testApp = express(); + testApp.use(express.json()); + testApp.use((req, _res, next) => { + req.user = { username: 'alice', admin: false }; + next(); + }); + testApp.use('/users', usersRouter); + + const res = await request(testApp) + .put('/users/bob/git-account') + .send({ gitAccount: 'octocat' }); + + expect(res.status).toBe(403); + }); + + it('should allow user to set their own git account', async () => { + const testApp = express(); + testApp.use(express.json()); + testApp.use((req, _res, next) => { + req.user = { username: 'bob', admin: false }; + next(); + }); + testApp.use('/users', usersRouter); + + const res = await request(testApp) + .put('/users/bob/git-account') + .send({ gitAccount: 'octocat' }); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ username: 'bob', gitAccount: 'octocat' }); + expect(db.updateUser).toHaveBeenCalledWith({ username: 'bob', gitAccount: 'octocat' }); + }); + + it('should allow admin to set any user git account', async () => { + const testApp = express(); + testApp.use(express.json()); + testApp.use((req, _res, next) => { + req.user = { username: 'admin', admin: true }; + next(); + }); + testApp.use('/users', usersRouter); + + const res = await request(testApp) + .put('/users/bob/git-account') + .send({ gitAccount: 'octocat' }); + + expect(res.status).toBe(200); + }); + + it('should return 400 when gitAccount is missing', async () => { + const testApp = express(); + testApp.use(express.json()); + testApp.use((req, _res, next) => { + req.user = { username: 'bob', admin: false }; + next(); + }); + testApp.use('/users', usersRouter); + + const res = await request(testApp).put('/users/bob/git-account').send({}); + + expect(res.status).toBe(400); + }); + + it('should return 409 when gitAccount is taken by another user', async () => { + vi.spyOn(db, 'findUserByGitAccount').mockResolvedValueOnce({ + username: 'alice', + password: null, + email: 'alice@example.com', + gitAccount: 'octocat', + admin: false, + }); + + const testApp = express(); + testApp.use(express.json()); + testApp.use((req, _res, next) => { + req.user = { username: 'bob', admin: false }; + next(); + }); + testApp.use('/users', usersRouter); + + const res = await request(testApp) + .put('/users/bob/git-account') + .send({ gitAccount: 'octocat' }); + + expect(res.status).toBe(409); + }); + + it('should return 404 when target user does not exist', async () => { + vi.spyOn(db, 'findUser').mockResolvedValueOnce(null); + + const testApp = express(); + testApp.use(express.json()); + testApp.use((req, _res, next) => { + req.user = { username: 'nonexistent', admin: false }; + next(); + }); + testApp.use('/users', usersRouter); + + const res = await request(testApp) + .put('/users/nonexistent/git-account') + .send({ gitAccount: 'octocat' }); + + expect(res.status).toBe(404); + }); + }); + }); + describe('SSH Key Management', () => { beforeEach(() => { // Mock SSH key operations From 2a00c7c462deb66e3fbc30e15da5fc39bdb166c3 Mon Sep 17 00:00:00 2001 From: Thomas Cooper Date: Wed, 24 Jun 2026 17:48:04 -0400 Subject: [PATCH 2/2] fix(proxy): remove dead email fallback and add fetch timeout in token identity resolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitHub's GET /user only returns email if the user has explicitly made it public — effectively never. Remove the if (identity.email) branch and the email field from ScmUserInfo to avoid the misleading implication that an email fallback exists. Add AbortSignal.timeout(5000) to the GitHub API fetch to prevent the push chain from hanging if the API is slow or unreachable. --- .../push-action/resolveUserFromToken.ts | 7 +---- .../processors/push-action/tokenIdentity.ts | 5 +--- test/processors/resolveUserFromToken.test.ts | 29 +++++++------------ 3 files changed, 12 insertions(+), 29 deletions(-) diff --git a/src/proxy/processors/push-action/resolveUserFromToken.ts b/src/proxy/processors/push-action/resolveUserFromToken.ts index c72aab737..5dbb0cf78 100644 --- a/src/proxy/processors/push-action/resolveUserFromToken.ts +++ b/src/proxy/processors/push-action/resolveUserFromToken.ts @@ -88,9 +88,7 @@ async function exec(req: Request, action: Action): Promise { return action; } - step.log( - `${provider.name}: resolved SCM identity from token: ${identity.login} (${identity.email ?? 'no public email'})`, - ); + step.log(`${provider.name}: resolved SCM identity from token: ${identity.login}`); const user = await findUserByGitAccount(identity.login); if (user) { @@ -107,9 +105,6 @@ async function exec(req: Request, action: Action): Promise { `Users can associate their account via PUT /api/v1/user/:username/git-account`, ); action.user = identity.login; - if (identity.email) { - action.userEmail = identity.email; - } } } catch (error: unknown) { const msg = getErrorMessage(error); diff --git a/src/proxy/processors/push-action/tokenIdentity.ts b/src/proxy/processors/push-action/tokenIdentity.ts index edbd691d8..5370287a5 100644 --- a/src/proxy/processors/push-action/tokenIdentity.ts +++ b/src/proxy/processors/push-action/tokenIdentity.ts @@ -18,7 +18,6 @@ import crypto from 'crypto'; export type ScmUserInfo = { login: string; - email?: string; }; type CacheEntry = { username: string; provider: string; cachedAt: number }; @@ -68,8 +67,6 @@ export interface TokenIdentityProvider { type GitHubUserResponse = { login: string; - id: number; - email: string | null; }; export class GitHubTokenIdentityProvider implements TokenIdentityProvider { @@ -86,6 +83,7 @@ export class GitHubTokenIdentityProvider implements TokenIdentityProvider { Authorization: `token ${token}`, Accept: 'application/vnd.github+json', }, + signal: AbortSignal.timeout(5000), }); if (!response.ok) { @@ -98,7 +96,6 @@ export class GitHubTokenIdentityProvider implements TokenIdentityProvider { const user: GitHubUserResponse = await response.json(); return { login: user.login, - email: user.email ?? undefined, }; } catch (e) { console.warn(`Failed to fetch GitHub identity: ${e}`); diff --git a/test/processors/resolveUserFromToken.test.ts b/test/processors/resolveUserFromToken.test.ts index 09a9d8129..3e8120a69 100644 --- a/test/processors/resolveUserFromToken.test.ts +++ b/test/processors/resolveUserFromToken.test.ts @@ -69,34 +69,24 @@ describe('GitHubTokenIdentityProvider', () => { }); describe('fetchScmIdentity', () => { - it('should return login and email on success', async () => { + it('should return login on success', async () => { fetchSpy.mockResolvedValueOnce({ ok: true, - json: async () => ({ login: 'octocat', id: 1, email: 'octocat@github.com' }), + json: async () => ({ login: 'octocat' }), } as Response); const result = await provider.fetchScmIdentity('ghp_token123'); - expect(result).toEqual({ login: 'octocat', email: 'octocat@github.com' }); + expect(result).toEqual({ login: 'octocat' }); expect(fetchSpy).toHaveBeenCalledWith('https://api.github.com/user', { headers: { Authorization: 'token ghp_token123', Accept: 'application/vnd.github+json', }, + signal: expect.any(AbortSignal), }); }); - it('should return login with undefined email when GitHub returns null email', async () => { - fetchSpy.mockResolvedValueOnce({ - ok: true, - json: async () => ({ login: 'private-user', id: 2, email: null }), - } as Response); - - const result = await provider.fetchScmIdentity('ghp_token456'); - - expect(result).toEqual({ login: 'private-user', email: undefined }); - }); - it('should return null on non-OK response', async () => { fetchSpy.mockResolvedValueOnce({ ok: false, @@ -214,7 +204,7 @@ describe('resolveUserFromToken', () => { it('should resolve GitHub identity from token and fall back to SCM identity when no DB user', async () => { fetchSpy.mockResolvedValueOnce({ ok: true, - json: async () => ({ login: 'octocat', id: 1, email: 'octocat@github.com' }), + json: async () => ({ login: 'octocat' }), } as Response); const req = makeRequest(); @@ -223,12 +213,13 @@ describe('resolveUserFromToken', () => { const result = await exec(req, action); expect(result.user).toBe('octocat'); - expect(result.userEmail).toBe('octocat@github.com'); + expect(result.userEmail).toBeUndefined(); expect(fetchSpy).toHaveBeenCalledWith('https://api.github.com/user', { headers: { Authorization: 'token ghp_testtoken123', Accept: 'application/vnd.github+json', }, + signal: expect.any(AbortSignal), }); }); @@ -247,7 +238,7 @@ describe('resolveUserFromToken', () => { fetchSpy.mockResolvedValueOnce({ ok: true, - json: async () => ({ login: 'octocat', id: 1, email: 'octocat@github.com' }), + json: async () => ({ login: 'octocat' }), } as Response); const req = makeRequest(); @@ -259,10 +250,10 @@ describe('resolveUserFromToken', () => { expect(result.userEmail).toBe('thomas.cooper@example.com'); }); - it('should handle GitHub identity with null email gracefully', async () => { + it('should leave userEmail from parsePush untouched when no gitAccount match', async () => { fetchSpy.mockResolvedValueOnce({ ok: true, - json: async () => ({ login: 'octocat', id: 1, email: null }), + json: async () => ({ login: 'octocat' }), } as Response); const req = makeRequest();