mirror of
https://github.com/immich-app/immich.git
synced 2026-02-25 12:40:12 -05:00
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>
This commit is contained in:
parent
4b8f90aa55
commit
f07e2b58f0
@ -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) => [
|
||||
|
||||
@ -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) {
|
||||
|
||||
@ -48,7 +48,7 @@ export class SessionRepository {
|
||||
}
|
||||
|
||||
@GenerateSql({ params: [DummyValue.STRING] })
|
||||
getByToken(token: string) {
|
||||
getByToken(token: Buffer) {
|
||||
return this.db
|
||||
.selectFrom('session')
|
||||
.select((eb) => [
|
||||
|
||||
@ -0,0 +1,15 @@
|
||||
import { Kysely, sql } from 'kysely';
|
||||
|
||||
export async function up(db: Kysely<any>): Promise<void> {
|
||||
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<any>): Promise<void> {
|
||||
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);
|
||||
}
|
||||
@ -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;
|
||||
|
||||
@ -17,9 +17,8 @@ export class SessionTable {
|
||||
@PrimaryGeneratedColumn()
|
||||
id!: Generated<string>;
|
||||
|
||||
// TODO convert to byte[]
|
||||
@Column()
|
||||
token!: string;
|
||||
@Column({ type: 'bytea', index: true })
|
||||
token!: Buffer;
|
||||
|
||||
@CreateDateColumn()
|
||||
createdAt!: Generated<Timestamp>;
|
||||
|
||||
@ -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,
|
||||
|
||||
@ -10,14 +10,14 @@ import { isGranted } from 'src/utils/access';
|
||||
export class ApiKeyService extends BaseService {
|
||||
async create(auth: AuthDto, dto: APIKeyCreateDto): Promise<APIKeyCreateResponseDto> {
|
||||
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,
|
||||
|
||||
@ -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)'));
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@ -456,8 +456,8 @@ export class AuthService extends BaseService {
|
||||
}
|
||||
|
||||
private async validateApiKey(key: string): Promise<AuthDto> {
|
||||
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<AuthDto> {
|
||||
const hashedToken = this.cryptoRepository.hashSha256(tokenValue);
|
||||
const session = await this.sessionRepository.getByToken(hashedToken);
|
||||
private async validateSession(token: string, headers: IncomingHttpHeaders): Promise<AuthDto> {
|
||||
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,
|
||||
|
||||
@ -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 };
|
||||
|
||||
@ -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,
|
||||
|
||||
@ -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');
|
||||
}
|
||||
}
|
||||
|
||||
@ -634,7 +634,7 @@ const personInsert = (person: Partial<Insertable<PersonTable>> & { 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(),
|
||||
|
||||
@ -8,7 +8,7 @@ export const newCryptoRepositoryMock = (): Mocked<RepositoryInterface<CryptoRepo
|
||||
randomBytes: vitest.fn().mockReturnValue(Buffer.from('random-bytes', 'utf8')),
|
||||
compareBcrypt: vitest.fn().mockReturnValue(true),
|
||||
hashBcrypt: vitest.fn().mockImplementation((input) => 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)`),
|
||||
|
||||
@ -148,7 +148,7 @@ const sessionFactory = (session: Partial<Session> = {}) => ({
|
||||
updateId: newUuidV7(),
|
||||
deviceOS: 'android',
|
||||
deviceType: 'mobile',
|
||||
token: 'abc123',
|
||||
token: Buffer.from('abc123'),
|
||||
parentId: null,
|
||||
expiresAt: null,
|
||||
userId: newUuid(),
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user