From f07e2b58f0eb379cd4018fa7f3c1e5d26cda0fb3 Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Tue, 24 Feb 2026 08:26:36 -0500 Subject: [PATCH] refactor: prefer buffer (#26469) * refactor: prefer buffer * Update server/src/schema/tables/session.table.ts Co-authored-by: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com> --------- Co-authored-by: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com> --- server/src/repositories/api-key.repository.ts | 2 +- server/src/repositories/crypto.repository.ts | 2 +- server/src/repositories/session.repository.ts | 2 +- .../1771873044511-ChangesTokensToBuffers.ts | 15 +++++++++++++++ server/src/schema/tables/api-key.table.ts | 4 ++-- server/src/schema/tables/session.table.ts | 5 ++--- server/src/services/api-key.service.spec.ts | 4 ++-- server/src/services/api-key.service.ts | 4 ++-- server/src/services/auth.service.spec.ts | 4 ++-- server/src/services/auth.service.ts | 14 +++++++------- server/src/services/session.service.ts | 4 ++-- server/src/services/shared-link.service.spec.ts | 5 +++-- server/src/services/shared-link.service.ts | 2 +- server/test/medium.factory.ts | 2 +- .../test/repositories/crypto.repository.mock.ts | 2 +- server/test/small.factory.ts | 2 +- 16 files changed, 44 insertions(+), 29 deletions(-) create mode 100644 server/src/schema/migrations/1771873044511-ChangesTokensToBuffers.ts diff --git a/server/src/repositories/api-key.repository.ts b/server/src/repositories/api-key.repository.ts index 28307d7c83..1e0d13f16c 100644 --- a/server/src/repositories/api-key.repository.ts +++ b/server/src/repositories/api-key.repository.ts @@ -31,7 +31,7 @@ export class ApiKeyRepository { } @GenerateSql({ params: [DummyValue.STRING] }) - getKey(hashedToken: string) { + getKey(hashedToken: Buffer) { return this.db .selectFrom('api_key') .select((eb) => [ diff --git a/server/src/repositories/crypto.repository.ts b/server/src/repositories/crypto.repository.ts index bcd791ade2..9b093f6d79 100644 --- a/server/src/repositories/crypto.repository.ts +++ b/server/src/repositories/crypto.repository.ts @@ -23,7 +23,7 @@ export class CryptoRepository { } hashSha256(value: string) { - return createHash('sha256').update(value).digest('base64'); + return createHash('sha256').update(value).digest(); } verifySha256(value: string, encryptedValue: string, publicKey: string) { diff --git a/server/src/repositories/session.repository.ts b/server/src/repositories/session.repository.ts index 52292b8e4a..e008943f21 100644 --- a/server/src/repositories/session.repository.ts +++ b/server/src/repositories/session.repository.ts @@ -48,7 +48,7 @@ export class SessionRepository { } @GenerateSql({ params: [DummyValue.STRING] }) - getByToken(token: string) { + getByToken(token: Buffer) { return this.db .selectFrom('session') .select((eb) => [ diff --git a/server/src/schema/migrations/1771873044511-ChangesTokensToBuffers.ts b/server/src/schema/migrations/1771873044511-ChangesTokensToBuffers.ts new file mode 100644 index 0000000000..b0ed28a55c --- /dev/null +++ b/server/src/schema/migrations/1771873044511-ChangesTokensToBuffers.ts @@ -0,0 +1,15 @@ +import { Kysely, sql } from 'kysely'; + +export async function up(db: Kysely): Promise { + await sql`ALTER TABLE "api_key" ALTER COLUMN "key" TYPE bytea USING decode("key", 'base64');`.execute(db); + await sql`ALTER TABLE "session" ALTER COLUMN "token" TYPE bytea USING decode("token", 'base64');`.execute(db); + await sql`CREATE INDEX "api_key_key_idx" ON "api_key" ("key");`.execute(db); + await sql`CREATE INDEX "session_token_idx" ON "session" ("token");`.execute(db); +} + +export async function down(db: Kysely): Promise { + await sql`DROP INDEX "api_key_key_idx";`.execute(db); + await sql`DROP INDEX "session_token_idx";`.execute(db); + await sql`ALTER TABLE "api_key" ALTER COLUMN "key" TYPE character varying USING encode("key", 'base64');`.execute(db); + await sql`ALTER TABLE "session" ALTER COLUMN "token" TYPE character varying USING encode("token", 'base64');`.execute(db); +} diff --git a/server/src/schema/tables/api-key.table.ts b/server/src/schema/tables/api-key.table.ts index 6cb4d5026e..f0e33a9c71 100644 --- a/server/src/schema/tables/api-key.table.ts +++ b/server/src/schema/tables/api-key.table.ts @@ -21,8 +21,8 @@ export class ApiKeyTable { @Column() name!: string; - @Column() - key!: string; + @Column({ type: 'bytea', index: true }) + key!: Buffer; @ForeignKeyColumn(() => UserTable, { onUpdate: 'CASCADE', onDelete: 'CASCADE' }) userId!: string; diff --git a/server/src/schema/tables/session.table.ts b/server/src/schema/tables/session.table.ts index 396a847e7e..e57628d6da 100644 --- a/server/src/schema/tables/session.table.ts +++ b/server/src/schema/tables/session.table.ts @@ -17,9 +17,8 @@ export class SessionTable { @PrimaryGeneratedColumn() id!: Generated; - // TODO convert to byte[] - @Column() - token!: string; + @Column({ type: 'bytea', index: true }) + token!: Buffer; @CreateDateColumn() createdAt!: Generated; diff --git a/server/src/services/api-key.service.spec.ts b/server/src/services/api-key.service.spec.ts index 14544f454f..3a31dbbea1 100644 --- a/server/src/services/api-key.service.spec.ts +++ b/server/src/services/api-key.service.spec.ts @@ -24,7 +24,7 @@ describe(ApiKeyService.name, () => { await sut.create(auth, { name: apiKey.name, permissions: apiKey.permissions }); expect(mocks.apiKey.create).toHaveBeenCalledWith({ - key: 'super-secret (hashed)', + key: Buffer.from('super-secret (hashed)'), name: apiKey.name, permissions: apiKey.permissions, userId: apiKey.userId, @@ -44,7 +44,7 @@ describe(ApiKeyService.name, () => { await sut.create(auth, { permissions: [Permission.All] }); expect(mocks.apiKey.create).toHaveBeenCalledWith({ - key: 'super-secret (hashed)', + key: Buffer.from('super-secret (hashed)'), name: 'API Key', permissions: [Permission.All], userId: auth.user.id, diff --git a/server/src/services/api-key.service.ts b/server/src/services/api-key.service.ts index 492ee9c0fd..534de69107 100644 --- a/server/src/services/api-key.service.ts +++ b/server/src/services/api-key.service.ts @@ -10,14 +10,14 @@ import { isGranted } from 'src/utils/access'; export class ApiKeyService extends BaseService { async create(auth: AuthDto, dto: APIKeyCreateDto): Promise { const token = this.cryptoRepository.randomBytesAsText(32); - const tokenHashed = this.cryptoRepository.hashSha256(token); + const hashed = this.cryptoRepository.hashSha256(token); if (auth.apiKey && !isGranted({ requested: dto.permissions, current: auth.apiKey.permissions })) { throw new BadRequestException('Cannot grant permissions you do not have'); } const entity = await this.apiKeyRepository.create({ - key: tokenHashed, + key: hashed, name: dto.name || 'API Key', userId: auth.user.id, permissions: dto.permissions, diff --git a/server/src/services/auth.service.spec.ts b/server/src/services/auth.service.spec.ts index a34efedfb0..81f601da0a 100644 --- a/server/src/services/auth.service.spec.ts +++ b/server/src/services/auth.service.spec.ts @@ -513,7 +513,7 @@ describe(AuthService.name, () => { metadata: { adminRoute: false, sharedLinkRoute: false, uri: 'test' }, }), ).rejects.toBeInstanceOf(UnauthorizedException); - expect(mocks.apiKey.getKey).toHaveBeenCalledWith('auth_token (hashed)'); + expect(mocks.apiKey.getKey).toHaveBeenCalledWith(Buffer.from('auth_token (hashed)')); }); it('should throw an error if api key has insufficient permissions', async () => { @@ -574,7 +574,7 @@ describe(AuthService.name, () => { metadata: { adminRoute: false, sharedLinkRoute: false, uri: 'test' }, }), ).resolves.toEqual({ user: authUser, apiKey: expect.objectContaining(authApiKey) }); - expect(mocks.apiKey.getKey).toHaveBeenCalledWith('auth_token (hashed)'); + expect(mocks.apiKey.getKey).toHaveBeenCalledWith(Buffer.from('auth_token (hashed)')); }); }); diff --git a/server/src/services/auth.service.ts b/server/src/services/auth.service.ts index a6580f89dd..b8e1b78107 100644 --- a/server/src/services/auth.service.ts +++ b/server/src/services/auth.service.ts @@ -456,8 +456,8 @@ export class AuthService extends BaseService { } private async validateApiKey(key: string): Promise { - const hashedKey = this.cryptoRepository.hashSha256(key); - const apiKey = await this.apiKeyRepository.getKey(hashedKey); + const hashed = this.cryptoRepository.hashSha256(key); + const apiKey = await this.apiKeyRepository.getKey(hashed); if (apiKey?.user) { return { user: apiKey.user, @@ -476,9 +476,9 @@ export class AuthService extends BaseService { return this.cryptoRepository.compareBcrypt(inputSecret, existingHash); } - private async validateSession(tokenValue: string, headers: IncomingHttpHeaders): Promise { - const hashedToken = this.cryptoRepository.hashSha256(tokenValue); - const session = await this.sessionRepository.getByToken(hashedToken); + private async validateSession(token: string, headers: IncomingHttpHeaders): Promise { + const hashed = this.cryptoRepository.hashSha256(token); + const session = await this.sessionRepository.getByToken(hashed); if (session?.user) { const { appVersion, deviceOS, deviceType } = getUserAgentDetails(headers); const now = DateTime.now(); @@ -543,10 +543,10 @@ export class AuthService extends BaseService { private async createLoginResponse(user: UserAdmin, loginDetails: LoginDetails) { const token = this.cryptoRepository.randomBytesAsText(32); - const tokenHashed = this.cryptoRepository.hashSha256(token); + const hashed = this.cryptoRepository.hashSha256(token); await this.sessionRepository.create({ - token: tokenHashed, + token: hashed, deviceOS: loginDetails.deviceOS, deviceType: loginDetails.deviceType, appVersion: loginDetails.appVersion, diff --git a/server/src/services/session.service.ts b/server/src/services/session.service.ts index 2f477c0d6a..8b5bd13928 100644 --- a/server/src/services/session.service.ts +++ b/server/src/services/session.service.ts @@ -33,14 +33,14 @@ export class SessionService extends BaseService { } const token = this.cryptoRepository.randomBytesAsText(32); - const tokenHashed = this.cryptoRepository.hashSha256(token); + const hashed = this.cryptoRepository.hashSha256(token); const session = await this.sessionRepository.create({ parentId: auth.session.id, userId: auth.user.id, expiresAt: dto.duration ? DateTime.now().plus({ seconds: dto.duration }).toJSDate() : null, deviceType: dto.deviceType, deviceOS: dto.deviceOS, - token: tokenHashed, + token: hashed, }); return { ...mapSession(session), token }; diff --git a/server/src/services/shared-link.service.spec.ts b/server/src/services/shared-link.service.spec.ts index 5ad145af2b..07f31db4da 100644 --- a/server/src/services/shared-link.service.spec.ts +++ b/server/src/services/shared-link.service.spec.ts @@ -69,8 +69,9 @@ describe(SharedLinkService.name, () => { it('should accept a valid shared link auth token', async () => { mocks.sharedLink.get.mockResolvedValue({ ...sharedLinkStub.individual, password: '123' }); - mocks.crypto.hashSha256.mockReturnValue('hashed-auth-token'); - await expect(sut.getMine(authStub.adminSharedLink, ['hashed-auth-token'])).resolves.toBeDefined(); + const secret = Buffer.from('auth-token-123'); + mocks.crypto.hashSha256.mockReturnValue(secret); + await expect(sut.getMine(authStub.adminSharedLink, [secret.toString('base64')])).resolves.toBeDefined(); expect(mocks.sharedLink.get).toHaveBeenCalledWith( authStub.adminSharedLink.user.id, authStub.adminSharedLink.sharedLink?.id, diff --git a/server/src/services/shared-link.service.ts b/server/src/services/shared-link.service.ts index e321e4990d..b942c32326 100644 --- a/server/src/services/shared-link.service.ts +++ b/server/src/services/shared-link.service.ts @@ -236,6 +236,6 @@ export class SharedLinkService extends BaseService { } private asToken(sharedLink: { id: string; password: string }) { - return this.cryptoRepository.hashSha256(`${sharedLink.id}-${sharedLink.password}`); + return this.cryptoRepository.hashSha256(`${sharedLink.id}-${sharedLink.password}`).toString('base64'); } } diff --git a/server/test/medium.factory.ts b/server/test/medium.factory.ts index 55f219699e..51dde6b36b 100644 --- a/server/test/medium.factory.ts +++ b/server/test/medium.factory.ts @@ -634,7 +634,7 @@ const personInsert = (person: Partial> & { ownerId: stri }; }; -const sha256 = (value: string) => createHash('sha256').update(value).digest('base64'); +const sha256 = (value: string) => createHash('sha256').update(value).digest(); const sessionInsert = ({ id = newUuid(), diff --git a/server/test/repositories/crypto.repository.mock.ts b/server/test/repositories/crypto.repository.mock.ts index 773891206e..fb2b933d97 100644 --- a/server/test/repositories/crypto.repository.mock.ts +++ b/server/test/repositories/crypto.repository.mock.ts @@ -8,7 +8,7 @@ export const newCryptoRepositoryMock = (): Mocked Promise.resolve(`${input} (hashed)`)), - hashSha256: vitest.fn().mockImplementation((input) => `${input} (hashed)`), + hashSha256: vitest.fn().mockImplementation((input) => Buffer.from(`${input} (hashed)`)), verifySha256: vitest.fn().mockImplementation(() => true), hashSha1: vitest.fn().mockImplementation((input) => Buffer.from(`${input.toString()} (hashed)`)), hashFile: vitest.fn().mockImplementation((input) => `${input} (file-hashed)`), diff --git a/server/test/small.factory.ts b/server/test/small.factory.ts index 4169c6e9bd..06a5798405 100644 --- a/server/test/small.factory.ts +++ b/server/test/small.factory.ts @@ -148,7 +148,7 @@ const sessionFactory = (session: Partial = {}) => ({ updateId: newUuidV7(), deviceOS: 'android', deviceType: 'mobile', - token: 'abc123', + token: Buffer.from('abc123'), parentId: null, expiresAt: null, userId: newUuid(),