From f2bd7e54d515ecf242e08a1a33300503809b19fa Mon Sep 17 00:00:00 2001 From: Thomas Cooper Date: Fri, 19 Jun 2026 18:05:47 -0400 Subject: [PATCH 1/5] 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 | 2 + 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, 938 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 a9dbc3fce..8c6a19c72 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -206,6 +206,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 5c9b59866..495dd98b2 100644 --- a/src/db/types.ts +++ b/src/db/types.ts @@ -165,6 +165,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 5a7e241e8..f566ad261 100644 --- a/src/proxy/chain.ts +++ b/src/proxy/chain.ts @@ -25,6 +25,8 @@ import { handleErrorAndLog } from '../utils/errors'; import { createProgressWriter } from './sideband'; const branchPushChain: Processor['exec'][] = [ + proc.push.parsePush, + proc.push.resolveUserFromToken, proc.push.checkEmptyBranch, proc.push.checkRepoInAuthorisedList, proc.push.checkMessages, diff --git a/src/proxy/processors/push-action/index.ts b/src/proxy/processors/push-action/index.ts index 721b29c60..04396097d 100644 --- a/src/proxy/processors/push-action/index.ts +++ b/src/proxy/processors/push-action/index.ts @@ -28,6 +28,7 @@ import { exec as checkIfWaitingAuth } from './checkIfWaitingAuth'; import { exec as checkMessages } from './checkMessages'; import { exec as checkAuthorEmails } from './checkAuthorEmails'; import { exec as checkUserPushPermission } from './checkUserPushPermission'; +import { exec as resolveUserFromToken } from './resolveUserFromToken'; import { exec as checkEmptyBranch } from './checkEmptyBranch'; @@ -45,5 +46,6 @@ export { checkMessages, 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 215da7632..7852103cc 100644 --- a/test/chain.test.ts +++ b/test/chain.test.ts @@ -34,6 +34,7 @@ const initMockPushProcessors = () => { checkMessages: 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 45efa1fec8928690fc858fb085b452855e18cb51 Mon Sep 17 00:00:00 2001 From: Thomas Cooper Date: Wed, 24 Jun 2026 17:48:04 -0400 Subject: [PATCH 2/5] 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(); From 10ed5bc3e08b6121305ddc7a07d9164ad94e51a7 Mon Sep 17 00:00:00 2001 From: Thomas Cooper Date: Wed, 24 Jun 2026 18:25:20 -0400 Subject: [PATCH 3/5] fix(proxy): extend token cache TTL to 7 days with sliding expiry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PATs are rarely rotated more frequently than 7 days in practice. The cache is a rate-limit optimization only — keys are one-way SHA-512 hashes, not recoverable tokens. Sliding TTL: a cache hit resets the expiry window, so a developer pushing regularly never sees an unnecessary API round-trip. --- .../processors/push-action/tokenIdentity.ts | 5 ++++- test/processors/resolveUserFromToken.test.ts | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/proxy/processors/push-action/tokenIdentity.ts b/src/proxy/processors/push-action/tokenIdentity.ts index 5370287a5..993ee6287 100644 --- a/src/proxy/processors/push-action/tokenIdentity.ts +++ b/src/proxy/processors/push-action/tokenIdentity.ts @@ -21,7 +21,9 @@ export type ScmUserInfo = { }; type CacheEntry = { username: string; provider: string; cachedAt: number }; -const DEFAULT_TTL_MS = 5 * 60 * 1000; +// 7 days — PATs are rarely rotated more frequently than this in practice; the cache is a +// rate-limit optimization only (keys are one-way SHA-512 hashes, not recoverable tokens). +const DEFAULT_TTL_MS = 7 * 24 * 60 * 60 * 1000; export class ScmTokenCache { private readonly cache = new Map(); @@ -43,6 +45,7 @@ export class ScmTokenCache { this.cache.delete(k); return null; } + entry.cachedAt = Date.now(); return entry.username; } diff --git a/test/processors/resolveUserFromToken.test.ts b/test/processors/resolveUserFromToken.test.ts index 3e8120a69..14eec0c72 100644 --- a/test/processors/resolveUserFromToken.test.ts +++ b/test/processors/resolveUserFromToken.test.ts @@ -143,6 +143,22 @@ describe('ScmTokenCache', () => { expect(cache.lookup('github', 'sometoken')).toBeNull(); }); + it('should reset TTL on hit (sliding expiry)', () => { + const cache = new ScmTokenCache(100); // 100ms TTL + cache.store('github', 'sometoken', 'octocat'); + const key = (cache as any).key('github', 'sometoken'); + // backdate to 90ms ago — still valid, but would expire in 10ms without a hit + (cache as any).cache.set(key, { + username: 'octocat', + provider: 'github', + cachedAt: Date.now() - 90, + }); + expect(cache.lookup('github', 'sometoken')).toBe('octocat'); // hit resets cachedAt + // backdate again to 90ms — if TTL had not been reset, this would be 180ms total (expired) + (cache as any).cache.get(key).cachedAt = Date.now() - 90; + expect(cache.lookup('github', 'sometoken')).toBe('octocat'); // still valid because TTL was reset + }); + it('should not share entries across providers', () => { const cache = new ScmTokenCache(); cache.store('github', 'sometoken', 'octocat'); From c33a413242d915e6514e89289fd081e3c10e6d00 Mon Sep 17 00:00:00 2001 From: Thomas Cooper Date: Thu, 16 Jul 2026 16:46:44 -0400 Subject: [PATCH 4/5] fix: remove parsePush from core logic chain, it is explicitly called as a pre-step --- src/proxy/chain.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/proxy/chain.ts b/src/proxy/chain.ts index f566ad261..2760645b9 100644 --- a/src/proxy/chain.ts +++ b/src/proxy/chain.ts @@ -25,7 +25,6 @@ import { handleErrorAndLog } from '../utils/errors'; import { createProgressWriter } from './sideband'; const branchPushChain: Processor['exec'][] = [ - proc.push.parsePush, proc.push.resolveUserFromToken, proc.push.checkEmptyBranch, proc.push.checkRepoInAuthorisedList, From 33712faf4e92f80b32e8d8fd78490596b6ed592f Mon Sep 17 00:00:00 2001 From: Thomas Cooper Date: Thu, 16 Jul 2026 17:30:07 -0400 Subject: [PATCH 5/5] refactor(api): remove redundant git-account endpoints, evict token cache in auth route Signed-off-by: Thomas Cooper --- .../push-action/resolveUserFromToken.ts | 2 +- src/service/routes/auth.ts | 2 + src/service/routes/users.ts | 52 ------- test/services/routes/users.test.ts | 140 ------------------ 4 files changed, 3 insertions(+), 193 deletions(-) diff --git a/src/proxy/processors/push-action/resolveUserFromToken.ts b/src/proxy/processors/push-action/resolveUserFromToken.ts index 5dbb0cf78..54c8e2099 100644 --- a/src/proxy/processors/push-action/resolveUserFromToken.ts +++ b/src/proxy/processors/push-action/resolveUserFromToken.ts @@ -102,7 +102,7 @@ async function exec(req: Request, action: Action): Promise { 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`, + `Users can associate their account via POST /api/auth/gitAccount`, ); action.user = identity.login; } diff --git a/src/service/routes/auth.ts b/src/service/routes/auth.ts index a9a83beb8..4bdc66f0d 100644 --- a/src/service/routes/auth.ts +++ b/src/service/routes/auth.ts @@ -27,6 +27,7 @@ import { User } from '../../db/types'; import { AuthenticationElement } from '../../config/generated/config'; import { isAdminUser, mustChangePassword, toPublicUser } from './utils'; import { handleErrorAndLog } from '../../utils/errors'; +import { scmTokenCache } from '../../proxy/processors/push-action/tokenIdentity'; const router = express.Router(); const passport = getPassport(); @@ -325,6 +326,7 @@ router.post('/gitAccount', async (req: Request, res: Response) => { user.gitAccount = req.body.gitAccount; await db.updateUser(user); + scmTokenCache.evictByUsername('github', user.username); return res.status(200).send({ message: 'Git account updated successfully' }).end(); } catch (error: unknown) { const msg = handleErrorAndLog(error, 'Failed to update git account'); diff --git a/src/service/routes/users.ts b/src/service/routes/users.ts index 3db8e6688..385ad1da0 100644 --- a/src/service/routes/users.ts +++ b/src/service/routes/users.ts @@ -19,7 +19,6 @@ 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(); @@ -63,57 +62,6 @@ 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/services/routes/users.test.ts b/test/services/routes/users.test.ts index 41d2054b7..6df9e3a57 100644 --- a/test/services/routes/users.test.ts +++ b/test/services/routes/users.test.ts @@ -86,146 +86,6 @@ 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