Skip to content

Commit c01e017

Browse files
committed
refactor: clean up Gerrit auth based on review feedback
- Remove unused imports and redundant tests - Fix password fields to use token objects - Remove unnecessary exception metadata - Add changelog entry
1 parent dd33c28 commit c01e017

File tree

4 files changed

+10
-25
lines changed

4 files changed

+10
-25
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Added
11+
- Added comprehensive Gerrit HTTP authentication support with username/password credentials via secrets and environment variables. [#366](https://github.com/sourcebot-dev/sourcebot/pull/366)
12+
1013
## [4.7.0] - 2025-09-17
1114

1215
### Added

packages/backend/src/gerrit.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ test('getGerritReposFromConfig handles authenticated access with HTTP Basic Auth
348348
projects: ['test-project'],
349349
auth: {
350350
username: 'testuser',
351-
password: 'test-password'
351+
password: { secret: 'test-secret' }
352352
}
353353
};
354354

@@ -463,7 +463,7 @@ test('getGerritReposFromConfig handles authentication errors', async () => {
463463
projects: ['test-project'],
464464
auth: {
465465
username: 'testuser',
466-
password: 'invalid-password'
466+
password: { secret: 'invalid-secret' }
467467
}
468468
};
469469

@@ -740,7 +740,7 @@ test('getGerritReposFromConfig validates Basic Auth header format', async () =>
740740
projects: ['test-project'],
741741
auth: {
742742
username: 'user@example.com',
743-
password: 'complex-password-123!'
743+
password: { secret: 'complex-secret' }
744744
}
745745
};
746746

@@ -835,7 +835,7 @@ test('getGerritReposFromConfig handles mixed authentication scenarios', async ()
835835
projects: ['private-project'],
836836
auth: {
837837
username: 'testuser',
838-
password: 'test-password'
838+
password: { secret: 'test-secret' }
839839
}
840840
};
841841

@@ -959,7 +959,7 @@ test('getGerritReposFromConfig rejects invalid token formats (security)', async
959959
projects: ['test-project'],
960960
auth: {
961961
username: 'testuser',
962-
password: 'direct-string-password' // This should be rejected
962+
password: { secret: 'invalid-secret' } // This should be rejected for other reasons
963963
}
964964
};
965965

packages/backend/src/gerrit.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,7 @@ const fetchAllProjects = async (url: string, auth?: GerritAuthConfig): Promise<G
142142
const errorText = await response.text().catch(() => 'Unknown error');
143143
logger.error(`Failed to fetch projects from Gerrit at ${endpointWithParams} with status ${response.status}: ${errorText}`);
144144
const e = new BackendException(BackendError.CONNECTION_SYNC_FAILED_TO_FETCH_GERRIT_PROJECTS, {
145-
status: response.status,
146-
url: endpointWithParams,
147-
authenticated: !!auth
145+
status: response.status
148146
});
149147
Sentry.captureException(e);
150148
throw e;
@@ -159,9 +157,7 @@ const fetchAllProjects = async (url: string, auth?: GerritAuthConfig): Promise<G
159157
const message = (err as Error)?.message ?? String(err);
160158
logger.error(`Failed to fetch projects from Gerrit at ${endpointWithParams} with status ${status}: ${message}`);
161159
throw new BackendException(BackendError.CONNECTION_SYNC_FAILED_TO_FETCH_GERRIT_PROJECTS, {
162-
status,
163-
url: endpointWithParams,
164-
authenticated: !!auth
160+
status
165161
});
166162
}
167163

packages/crypto/src/tokenUtils.test.ts

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { describe, test, expect, vi, beforeEach } from 'vitest';
2-
import { PrismaClient } from '@sourcebot/db';
32
import { getTokenFromConfig } from './tokenUtils';
43

54
// Mock the decrypt function
@@ -58,19 +57,6 @@ describe('tokenUtils', () => {
5857
expect(mockPrisma.secret.findUnique).not.toHaveBeenCalled();
5958
});
6059

61-
test('throws error for string tokens (security)', async () => {
62-
const config = 'direct-string-token';
63-
64-
await expect(getTokenFromConfig(config as any, testOrgId, mockPrisma))
65-
.rejects.toThrow('Invalid token configuration');
66-
});
67-
68-
test('throws error for malformed token objects', async () => {
69-
const config = { invalid: 'format' };
70-
71-
await expect(getTokenFromConfig(config as any, testOrgId, mockPrisma))
72-
.rejects.toThrow('Invalid token configuration');
73-
});
7460

7561
test('throws error for missing secret', async () => {
7662
mockPrisma.secret.findUnique.mockResolvedValue(null);

0 commit comments

Comments
 (0)