fix: review notes, new register endpoint

This commit is contained in:
bo0tzz
2026-04-23 12:22:27 +02:00
parent fd52481582
commit 2da2bef777
14 changed files with 736 additions and 294 deletions
@@ -137,6 +137,44 @@ describe(AuthController.name, () => {
);
});
it('should clear the link token cookie on successful login when it was present', async () => {
const loginResponse = mediumFactory.loginResponse();
service.login.mockResolvedValue(loginResponse);
const { status, headers } = await request(ctx.getHttpServer())
.post('/auth/login')
.set('Cookie', 'immich_oauth_link_token=plain')
.send({ name: 'admin', email: 'admin@local', password: 'password' });
expect(status).toEqual(201);
const cookies = (headers['set-cookie'] as unknown as string[]).join('\n');
expect(cookies).toMatch(/immich_oauth_link_token=;/);
});
it('should clear the link token cookie when login fails', async () => {
service.login.mockRejectedValue(new Error('Incorrect email or password'));
const { headers } = await request(ctx.getHttpServer())
.post('/auth/login')
.set('Cookie', 'immich_oauth_link_token=plain')
.send({ name: 'admin', email: 'admin@local', password: 'wrong' });
const cookies = (headers['set-cookie'] as unknown as string[] | undefined)?.join('\n') ?? '';
expect(cookies).toMatch(/immich_oauth_link_token=;/);
});
it('should not set a link token cookie header when no link token was present', async () => {
const loginResponse = mediumFactory.loginResponse();
service.login.mockResolvedValue(loginResponse);
const { headers } = await request(ctx.getHttpServer())
.post('/auth/login')
.send({ name: 'admin', email: 'admin@local', password: 'password' });
const cookies = (headers['set-cookie'] as unknown as string[]).join('\n');
expect(cookies).not.toMatch(/immich_oauth_link_token=/);
});
it('should auth cookies on a secure connection', async () => {
const loginResponse = mediumFactory.loginResponse();
service.login.mockResolvedValue(loginResponse);
@@ -175,6 +213,33 @@ describe(AuthController.name, () => {
});
});
describe('POST /auth/register', () => {
it('should clear the link token cookie on successful register', async () => {
const loginResponse = mediumFactory.loginResponse();
service.register.mockResolvedValue(loginResponse);
const { headers } = await request(ctx.getHttpServer())
.post('/auth/register')
.set('Cookie', 'immich_oauth_link_token=plain')
.send({});
const cookies = (headers['set-cookie'] as unknown as string[]).join('\n');
expect(cookies).toMatch(/immich_oauth_link_token=;/);
});
it('should clear the link token cookie when register fails', async () => {
service.register.mockRejectedValue(new Error('Missing OAuth link token'));
const { headers } = await request(ctx.getHttpServer())
.post('/auth/register')
.set('Cookie', 'immich_oauth_link_token=plain')
.send({});
const cookies = (headers['set-cookie'] as unknown as string[] | undefined)?.join('\n') ?? '';
expect(cookies).toMatch(/immich_oauth_link_token=;/);
});
});
describe('POST /auth/logout', () => {
it('should be an authenticated route', async () => {
await request(ctx.getHttpServer()).post('/auth/logout');
+45 -11
View File
@@ -1,5 +1,6 @@
import { Body, Controller, Delete, Get, HttpCode, HttpStatus, Post, Put, Req, Res } from '@nestjs/common';
import { ApiTags } from '@nestjs/swagger';
import { parse as parseCookie } from 'cookie';
import { Request, Response } from 'express';
import { Endpoint, HistoryBuilder } from 'src/decorators';
import {
@@ -39,18 +40,51 @@ export class AuthController {
@Body() loginCredential: LoginCredentialDto,
@GetLoginDetails() loginDetails: LoginDetails,
): Promise<LoginResponseDto> {
const body = await this.service.login(loginCredential, loginDetails, request.headers);
if (request.cookies?.[ImmichCookie.OAuthLinkToken]) {
res.clearCookie(ImmichCookie.OAuthLinkToken);
const hadLinkCookie = !!parseCookie(request.headers.cookie || '')[ImmichCookie.OAuthLinkToken];
try {
const body = await this.service.login(loginCredential, loginDetails, request.headers);
return respondWithCookie(res, body, {
isSecure: loginDetails.isSecure,
values: [
{ key: ImmichCookie.AccessToken, value: body.accessToken },
{ key: ImmichCookie.AuthType, value: AuthType.Password },
{ key: ImmichCookie.IsAuthenticated, value: 'true' },
],
});
} finally {
if (hadLinkCookie) {
res.clearCookie(ImmichCookie.OAuthLinkToken);
}
}
}
@Post('register')
@Endpoint({
summary: 'Register via OAuth',
description: 'Create a new user from a pending OAuth link token (requires OAuth auto-register to be enabled).',
history: new HistoryBuilder().added('v2'),
})
async register(
@Req() request: Request,
@Res({ passthrough: true }) res: Response,
@GetLoginDetails() loginDetails: LoginDetails,
): Promise<LoginResponseDto> {
const hadLinkCookie = !!parseCookie(request.headers.cookie || '')[ImmichCookie.OAuthLinkToken];
try {
const body = await this.service.register(loginDetails, request.headers);
return respondWithCookie(res, body, {
isSecure: loginDetails.isSecure,
values: [
{ key: ImmichCookie.AccessToken, value: body.accessToken },
{ key: ImmichCookie.AuthType, value: AuthType.OAuth },
{ key: ImmichCookie.IsAuthenticated, value: 'true' },
],
});
} finally {
if (hadLinkCookie) {
res.clearCookie(ImmichCookie.OAuthLinkToken);
}
}
return respondWithCookie(res, body, {
isSecure: loginDetails.isSecure,
values: [
{ key: ImmichCookie.AccessToken, value: body.accessToken },
{ key: ImmichCookie.AuthType, value: AuthType.Password },
{ key: ImmichCookie.IsAuthenticated, value: 'true' },
],
});
}
@Post('admin-sign-up')
+1
View File
@@ -132,6 +132,7 @@ const ServerFeaturesSchema = z
importFaces: z.boolean().describe('Whether face import is enabled'),
oauth: z.boolean().describe('Whether OAuth is enabled'),
oauthAutoLaunch: z.boolean().describe('Whether OAuth auto-launch is enabled'),
oauthAutoRegister: z.boolean().describe('Whether OAuth auto-register is enabled'),
passwordLogin: z.boolean().describe('Whether password login is enabled'),
sidecar: z.boolean().describe('Whether sidecar files are supported'),
search: z.boolean().describe('Whether search is enabled'),
@@ -8,6 +8,7 @@ export async function up(db: Kysely<any>): Promise<void> {
"oauthSub" varchar NOT NULL,
"oauthSid" varchar,
"email" varchar NOT NULL,
"profile" jsonb NOT NULL,
"expiresAt" timestamp with time zone NOT NULL,
"createdAt" timestamp with time zone NOT NULL DEFAULT now()
);
@@ -1,5 +1,13 @@
import { Column, CreateDateColumn, Generated, PrimaryGeneratedColumn, Table, Timestamp } from '@immich/sql-tools';
export type OAuthLinkTokenProfile = {
name: string;
storageLabel: string | null;
storageQuotaInGiB: number | null;
isAdmin: boolean;
picture: string | null;
};
@Table({ name: 'oauth_link_token' })
export class OAuthLinkTokenTable {
@PrimaryGeneratedColumn()
@@ -17,6 +25,9 @@ export class OAuthLinkTokenTable {
@Column()
email!: string;
@Column({ type: 'jsonb' })
profile!: OAuthLinkTokenProfile;
@Column({ type: 'timestamp with time zone' })
expiresAt!: Timestamp;
+374 -173
View File
@@ -99,6 +99,13 @@ describe(AuthService.name, () => {
oauthSub: 'oauth-sub-123',
oauthSid: null,
email: user.email,
profile: {
name: 'OAuth User',
storageLabel: null,
storageQuotaInGiB: null,
isAdmin: false,
picture: null,
},
token: Buffer.from('hashed'),
expiresAt: new Date(Date.now() + 600_000),
createdAt: new Date(),
@@ -108,7 +115,7 @@ describe(AuthService.name, () => {
await sut.login(dto, loginDetails, { cookie: 'immich_oauth_link_token=plain-token' });
expect(mocks.oauthLinkToken.consumeToken).toHaveBeenCalledTimes(1);
expect(mocks.user.update).toHaveBeenCalledWith(user.id, { oauthId: 'oauth-sub-123' });
expect(mocks.user.update).toHaveBeenCalledWith(user.id, expect.objectContaining({ oauthId: 'oauth-sub-123' }));
});
it('should propagate oauthSid from link token to the session', async () => {
@@ -121,6 +128,13 @@ describe(AuthService.name, () => {
oauthSub: 'oauth-sub-123',
oauthSid: 'idp-sid-456',
email: user.email,
profile: {
name: 'OAuth User',
storageLabel: null,
storageQuotaInGiB: null,
isAdmin: false,
picture: null,
},
token: Buffer.from('hashed'),
expiresAt: new Date(Date.now() + 600_000),
createdAt: new Date(),
@@ -132,14 +146,253 @@ describe(AuthService.name, () => {
expect(mocks.session.create).toHaveBeenCalledWith(expect.objectContaining({ oauthSid: 'idp-sid-456' }));
});
it('should reject login with invalid link token cookie', async () => {
it('should silently fall back to normal login when the link token is invalid or expired', async () => {
const user = UserFactory.create({ password: 'immich_password' });
const session = SessionFactory.create();
mocks.user.getByEmail.mockResolvedValue(user);
mocks.session.create.mockResolvedValue(session);
mocks.oauthLinkToken.consumeToken.mockResolvedValue(null as any);
await expect(sut.login(dto, loginDetails, { cookie: 'immich_oauth_link_token=bad-token' })).rejects.toThrow(
'Invalid or expired link token',
await expect(
sut.login(dto, loginDetails, { cookie: 'immich_oauth_link_token=bad-token' }),
).resolves.toMatchObject({ userId: user.id });
expect(mocks.oauthLinkToken.consumeToken).toHaveBeenCalledTimes(1);
expect(mocks.user.update).not.toHaveBeenCalled();
expect(mocks.session.create).toHaveBeenCalledWith(expect.objectContaining({ oauthSid: null }));
});
it('should reject when the link token points to a sub already linked to another user', async () => {
const user = UserFactory.create({ password: 'immich_password' });
const otherUser = UserFactory.create({ oauthId: 'oauth-sub-123' });
mocks.user.getByEmail.mockResolvedValue(user);
mocks.oauthLinkToken.consumeToken.mockResolvedValue({
id: 'token-id',
oauthSub: 'oauth-sub-123',
oauthSid: null,
email: user.email,
profile: {
name: 'OAuth User',
storageLabel: null,
storageQuotaInGiB: null,
isAdmin: false,
picture: null,
},
token: Buffer.from('hashed'),
expiresAt: new Date(Date.now() + 600_000),
createdAt: new Date(),
});
mocks.user.getByOAuthId.mockResolvedValue(otherUser);
await expect(sut.login(dto, loginDetails, { cookie: 'immich_oauth_link_token=plain-token' })).rejects.toThrow(
'This OAuth account has already been linked to another user.',
);
expect(mocks.user.update).not.toHaveBeenCalled();
});
it('should sanitize the storage label when linking from an OAuth profile', async () => {
const user = UserFactory.create({ password: 'immich_password' });
const session = SessionFactory.create();
mocks.user.getByEmail.mockResolvedValue(user);
mocks.session.create.mockResolvedValue(session);
mocks.oauthLinkToken.consumeToken.mockResolvedValue({
id: 'token-id',
oauthSub: 'oauth-sub-123',
oauthSid: null,
email: user.email,
profile: {
name: 'OAuth User',
storageLabel: '../evil/path',
storageQuotaInGiB: null,
isAdmin: false,
picture: null,
},
token: Buffer.from('hashed'),
expiresAt: new Date(Date.now() + 600_000),
createdAt: new Date(),
});
mocks.user.update.mockResolvedValue(user);
await sut.login(dto, loginDetails, { cookie: 'immich_oauth_link_token=plain-token' });
const updateCall = mocks.user.update.mock.calls[0][1];
expect(updateCall.storageLabel).not.toContain('/');
expect(updateCall.storageLabel).not.toContain('.');
});
});
describe('register', () => {
it('should throw if auto-register is disabled', async () => {
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthEnabled);
await expect(sut.register(loginDetails, { cookie: 'immich_oauth_link_token=plain' })).rejects.toThrow(
'OAuth auto-register is disabled',
);
});
it('should throw if link token cookie is missing', async () => {
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithAutoRegister);
await expect(sut.register(loginDetails, {})).rejects.toThrow('Missing OAuth link token');
});
it('should throw if the sub is already linked', async () => {
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithAutoRegister);
mocks.oauthLinkToken.consumeToken.mockResolvedValue({
id: 'token-id',
oauthSub: 'oauth-sub-123',
oauthSid: null,
email: 'new@immich.cloud',
profile: {
name: 'New User',
storageLabel: null,
storageQuotaInGiB: null,
isAdmin: false,
picture: null,
},
token: Buffer.from('hashed'),
expiresAt: new Date(Date.now() + 600_000),
createdAt: new Date(),
});
mocks.user.getByOAuthId.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-sub-123' }));
await expect(sut.register(loginDetails, { cookie: 'immich_oauth_link_token=plain' })).rejects.toThrow(
'This OAuth account has already been linked to another user',
);
});
it('should create a user from the link token and apply the profile', async () => {
const newUser = UserFactory.create();
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithAutoRegister);
mocks.oauthLinkToken.consumeToken.mockResolvedValue({
id: 'token-id',
oauthSub: 'oauth-sub-123',
oauthSid: 'idp-sid',
email: 'new@immich.cloud',
profile: {
name: 'New User',
storageLabel: 'shiny',
storageQuotaInGiB: 5,
isAdmin: true,
picture: null,
},
token: Buffer.from('hashed'),
expiresAt: new Date(Date.now() + 600_000),
createdAt: new Date(),
});
mocks.user.getByOAuthId.mockResolvedValue(void 0);
mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true }));
mocks.user.create.mockResolvedValue(newUser);
mocks.user.update.mockResolvedValue(newUser);
mocks.session.create.mockResolvedValue(SessionFactory.create());
await sut.register(loginDetails, { cookie: 'immich_oauth_link_token=plain' });
expect(mocks.user.create).toHaveBeenCalledWith(
expect.objectContaining({ email: 'new@immich.cloud', name: 'New User', isAdmin: true }),
);
expect(mocks.user.update).toHaveBeenCalledWith(
newUser.id,
expect.objectContaining({
oauthId: 'oauth-sub-123',
storageLabel: 'shiny',
quotaSizeInBytes: 5 * 1024 * 1024 * 1024,
isAdmin: true,
}),
);
expect(mocks.session.create).toHaveBeenCalledWith(expect.objectContaining({ oauthSid: 'idp-sid' }));
});
it('should allow the first OAuth admin to bootstrap the instance', async () => {
const newUser = UserFactory.create({ isAdmin: true });
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithAutoRegister);
mocks.oauthLinkToken.consumeToken.mockResolvedValue({
id: 'token-id',
oauthSub: 'oauth-sub-123',
oauthSid: null,
email: 'first@immich.cloud',
profile: {
name: 'First Admin',
storageLabel: null,
storageQuotaInGiB: null,
isAdmin: true,
picture: null,
},
token: Buffer.from('hashed'),
expiresAt: new Date(Date.now() + 600_000),
createdAt: new Date(),
});
mocks.user.getByOAuthId.mockResolvedValue(void 0);
mocks.user.getAdmin.mockResolvedValue(void 0);
mocks.user.create.mockResolvedValue(newUser);
mocks.user.update.mockResolvedValue(newUser);
mocks.session.create.mockResolvedValue(SessionFactory.create());
await sut.register(loginDetails, { cookie: 'immich_oauth_link_token=plain' });
expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ isAdmin: true }));
expect(mocks.user.getAdmin).not.toHaveBeenCalled();
});
it('should reject a non-admin OAuth register when no admin exists yet', async () => {
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithAutoRegister);
mocks.oauthLinkToken.consumeToken.mockResolvedValue({
id: 'token-id',
oauthSub: 'oauth-sub-123',
oauthSid: null,
email: 'first@immich.cloud',
profile: {
name: 'Regular User',
storageLabel: null,
storageQuotaInGiB: null,
isAdmin: false,
picture: null,
},
token: Buffer.from('hashed'),
expiresAt: new Date(Date.now() + 600_000),
createdAt: new Date(),
});
mocks.user.getByOAuthId.mockResolvedValue(void 0);
mocks.user.getAdmin.mockResolvedValue(void 0);
await expect(sut.register(loginDetails, { cookie: 'immich_oauth_link_token=plain' })).rejects.toThrow(
'The first registered account must the administrator.',
);
expect(mocks.user.create).not.toHaveBeenCalled();
});
it('should sanitize the storage label on register', async () => {
const newUser = UserFactory.create();
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithAutoRegister);
mocks.oauthLinkToken.consumeToken.mockResolvedValue({
id: 'token-id',
oauthSub: 'oauth-sub-123',
oauthSid: null,
email: 'new@immich.cloud',
profile: {
name: 'New User',
storageLabel: '../sneaky',
storageQuotaInGiB: null,
isAdmin: false,
picture: null,
},
token: Buffer.from('hashed'),
expiresAt: new Date(Date.now() + 600_000),
createdAt: new Date(),
});
mocks.user.getByOAuthId.mockResolvedValue(void 0);
mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true }));
mocks.user.create.mockResolvedValue(newUser);
mocks.user.update.mockResolvedValue(newUser);
mocks.session.create.mockResolvedValue(SessionFactory.create());
await sut.register(loginDetails, { cookie: 'immich_oauth_link_token=plain' });
const updateCall = mocks.user.update.mock.calls[0][1];
expect(updateCall.storageLabel).not.toContain('/');
expect(updateCall.storageLabel).not.toContain('.');
});
});
@@ -739,29 +992,11 @@ describe(AuthService.name, () => {
).rejects.toBeInstanceOf(BadRequestException);
});
it('should not allow auto registering', async () => {
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthEnabled);
mocks.user.getByEmail.mockResolvedValue(void 0);
mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ profile: OAuthProfileFactory.create() });
await expect(
sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
),
).rejects.toBeInstanceOf(BadRequestException);
expect(mocks.user.getByEmail).toHaveBeenCalledTimes(1);
});
it('should reject when existing user found by email and create a link token', async () => {
const user = UserFactory.create();
it('should create a link token when the oauth sub is not yet linked', async () => {
const profile = OAuthProfileFactory.create();
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthEnabled);
mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ profile, sid: 'idp-sid-789' });
mocks.user.getByEmail.mockResolvedValue(user);
mocks.oauthLinkToken.create.mockResolvedValue({} as any);
await expect(
@@ -772,20 +1007,18 @@ describe(AuthService.name, () => {
),
).rejects.toThrow(OAuthLinkRequiredException);
expect(mocks.user.getByEmail).toHaveBeenCalledTimes(1);
expect(mocks.user.update).not.toHaveBeenCalled();
expect(mocks.user.create).not.toHaveBeenCalled();
expect(mocks.oauthLinkToken.create).toHaveBeenCalledWith(
expect.objectContaining({ oauthSub: profile.sub, oauthSid: 'idp-sid-789' }),
expect.objectContaining({ oauthSub: profile.sub, oauthSid: 'idp-sid-789', email: profile.email }),
);
});
it('should normalize the email from the OAuth profile before looking up user', async () => {
const user = UserFactory.create();
it('should normalize the email from the OAuth profile before storing in the link token', async () => {
const profile = OAuthProfileFactory.create({ email: ' TEST@IMMICH.CLOUD ' });
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthEnabled);
mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ profile });
mocks.user.getByEmail.mockResolvedValue(user);
mocks.oauthLinkToken.create.mockResolvedValue({} as any);
await expect(
@@ -794,52 +1027,12 @@ describe(AuthService.name, () => {
{},
loginDetails,
),
).rejects.toThrow(ForbiddenException);
).rejects.toThrow(OAuthLinkRequiredException);
expect(mocks.user.getByEmail).toHaveBeenCalledWith('test@immich.cloud');
expect(mocks.user.update).not.toHaveBeenCalled();
expect(mocks.oauthLinkToken.create).toHaveBeenCalledWith(expect.objectContaining({ email: 'test@immich.cloud' }));
});
it('should not link to a user with a different oauth sub', async () => {
const user = UserFactory.create({ oauthId: 'existing-sub' });
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithAutoRegister);
mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ profile: OAuthProfileFactory.create() });
mocks.user.getByEmail.mockResolvedValueOnce(user);
mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true }));
mocks.oauthLinkToken.create.mockResolvedValue({} as any);
await expect(
sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foobar' },
{},
loginDetails,
),
).rejects.toThrow(ForbiddenException);
expect(mocks.user.update).not.toHaveBeenCalled();
expect(mocks.user.create).not.toHaveBeenCalled();
});
it('should allow auto registering by default', async () => {
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.enabled);
mocks.user.getByEmail.mockResolvedValue(void 0);
mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true }));
mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' }));
mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ profile: OAuthProfileFactory.create() });
mocks.session.create.mockResolvedValue(SessionFactory.create());
await sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foobar' },
{},
loginDetails,
);
expect(mocks.user.getByEmail).toHaveBeenCalledTimes(2); // second call is for domain check before create
expect(mocks.user.create).toHaveBeenCalledTimes(1);
});
it('should throw an error if user should be auto registered but the email claim does not exist', async () => {
it('should throw an error if the OAuth profile does not have an email claim', async () => {
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.enabled);
mocks.user.getByEmail.mockResolvedValue(void 0);
mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true }));
@@ -883,19 +1076,20 @@ describe(AuthService.name, () => {
it('should use the default quota', async () => {
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota);
mocks.user.getByEmail.mockResolvedValue(void 0);
mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true }));
mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ profile: OAuthProfileFactory.create() });
mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' }));
mocks.session.create.mockResolvedValue(SessionFactory.create());
mocks.oauthLinkToken.create.mockResolvedValue({} as any);
await sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
await expect(
sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
),
).rejects.toThrow(OAuthLinkRequiredException);
expect(mocks.oauthLinkToken.create).toHaveBeenCalledWith(
expect.objectContaining({ profile: expect.objectContaining({ storageQuotaInGiB: 1 }) }),
);
expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ quotaSizeInBytes: 1_073_741_824 }));
});
it('should infer name from given and family names', async () => {
@@ -903,18 +1097,19 @@ describe(AuthService.name, () => {
mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({
profile: OAuthProfileFactory.create({ name: undefined, given_name: 'Given', family_name: 'Family' }),
});
mocks.user.getByEmail.mockResolvedValue(void 0);
mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true }));
mocks.user.create.mockResolvedValue(UserFactory.create());
mocks.session.create.mockResolvedValue(SessionFactory.create());
mocks.oauthLinkToken.create.mockResolvedValue({} as any);
await sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
await expect(
sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
),
).rejects.toThrow(OAuthLinkRequiredException);
expect(mocks.oauthLinkToken.create).toHaveBeenCalledWith(
expect.objectContaining({ profile: expect.objectContaining({ name: 'Given Family' }) }),
);
expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ name: 'Given Family' }));
});
it('should fallback to email when no username is provided', async () => {
@@ -922,18 +1117,19 @@ describe(AuthService.name, () => {
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.enabled);
mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ profile });
mocks.user.getByEmail.mockResolvedValue(void 0);
mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true }));
mocks.user.create.mockResolvedValue(UserFactory.create());
mocks.session.create.mockResolvedValue(SessionFactory.create());
mocks.oauthLinkToken.create.mockResolvedValue({} as any);
await sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
await expect(
sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
),
).rejects.toThrow(OAuthLinkRequiredException);
expect(mocks.oauthLinkToken.create).toHaveBeenCalledWith(
expect.objectContaining({ profile: expect.objectContaining({ name: profile.email }) }),
);
expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ name: profile.email }));
});
it('should ignore an invalid storage quota', async () => {
@@ -941,18 +1137,19 @@ describe(AuthService.name, () => {
mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({
profile: OAuthProfileFactory.create({ immich_quota: 'abc' }),
});
mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true }));
mocks.user.getByEmail.mockResolvedValue(void 0);
mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' }));
mocks.session.create.mockResolvedValue(SessionFactory.create());
mocks.oauthLinkToken.create.mockResolvedValue({} as any);
await sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
await expect(
sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
),
).rejects.toThrow(OAuthLinkRequiredException);
expect(mocks.oauthLinkToken.create).toHaveBeenCalledWith(
expect.objectContaining({ profile: expect.objectContaining({ storageQuotaInGiB: 1 }) }),
);
expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ quotaSizeInBytes: 1_073_741_824 }));
});
it('should ignore a negative quota', async () => {
@@ -960,53 +1157,55 @@ describe(AuthService.name, () => {
mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({
profile: OAuthProfileFactory.create({ immich_quota: -5 }),
});
mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true }));
mocks.user.getByEmail.mockResolvedValue(void 0);
mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' }));
mocks.session.create.mockResolvedValue(SessionFactory.create());
mocks.oauthLinkToken.create.mockResolvedValue({} as any);
await sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
await expect(
sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
),
).rejects.toThrow(OAuthLinkRequiredException);
expect(mocks.oauthLinkToken.create).toHaveBeenCalledWith(
expect.objectContaining({ profile: expect.objectContaining({ storageQuotaInGiB: 1 }) }),
);
expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ quotaSizeInBytes: 1_073_741_824 }));
});
it('should set quota for 0 quota', async () => {
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota);
mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ profile: OAuthProfileFactory.create({ immich_quota: 0 }) });
mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true }));
mocks.user.getByEmail.mockResolvedValue(void 0);
mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' }));
mocks.session.create.mockResolvedValue(SessionFactory.create());
mocks.oauthLinkToken.create.mockResolvedValue({} as any);
await sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
await expect(
sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
),
).rejects.toThrow(OAuthLinkRequiredException);
expect(mocks.oauthLinkToken.create).toHaveBeenCalledWith(
expect.objectContaining({ profile: expect.objectContaining({ storageQuotaInGiB: 0 }) }),
);
expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ quotaSizeInBytes: 0 }));
});
it('should use a valid storage quota', async () => {
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota);
mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ profile: OAuthProfileFactory.create({ immich_quota: 5 }) });
mocks.user.getByEmail.mockResolvedValue(void 0);
mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true }));
mocks.user.getByOAuthId.mockResolvedValue(void 0);
mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' }));
mocks.session.create.mockResolvedValue(SessionFactory.create());
mocks.oauthLinkToken.create.mockResolvedValue({} as any);
await sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
await expect(
sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
),
).rejects.toThrow(OAuthLinkRequiredException);
expect(mocks.oauthLinkToken.create).toHaveBeenCalledWith(
expect.objectContaining({ profile: expect.objectContaining({ storageQuotaInGiB: 5 }) }),
);
expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ quotaSizeInBytes: 5_368_709_120 }));
});
it('should sync the profile picture', async () => {
@@ -1100,19 +1299,19 @@ describe(AuthService.name, () => {
mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({
profile: OAuthProfileFactory.create({ immich_role: 'foo' }),
});
mocks.user.getByEmail.mockResolvedValue(void 0);
mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true }));
mocks.user.getByOAuthId.mockResolvedValue(void 0);
mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' }));
mocks.session.create.mockResolvedValue(SessionFactory.create());
mocks.oauthLinkToken.create.mockResolvedValue({} as any);
await sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
await expect(
sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
),
).rejects.toThrow(OAuthLinkRequiredException);
expect(mocks.oauthLinkToken.create).toHaveBeenCalledWith(
expect.objectContaining({ profile: expect.objectContaining({ isAdmin: false }) }),
);
expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ isAdmin: false }));
});
it('should create an admin user if the role claim is set to admin', async () => {
@@ -1120,18 +1319,19 @@ describe(AuthService.name, () => {
mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({
profile: OAuthProfileFactory.create({ immich_role: 'admin' }),
});
mocks.user.getByEmail.mockResolvedValue(void 0);
mocks.user.getByOAuthId.mockResolvedValue(void 0);
mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' }));
mocks.session.create.mockResolvedValue(SessionFactory.create());
mocks.oauthLinkToken.create.mockResolvedValue({} as any);
await sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
await expect(
sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
),
).rejects.toThrow(OAuthLinkRequiredException);
expect(mocks.oauthLinkToken.create).toHaveBeenCalledWith(
expect.objectContaining({ profile: expect.objectContaining({ isAdmin: true }) }),
);
expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ isAdmin: true }));
});
it('should accept a custom role claim', async () => {
@@ -1141,18 +1341,19 @@ describe(AuthService.name, () => {
mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({
profile: OAuthProfileFactory.create({ my_role: 'admin' }),
});
mocks.user.getByEmail.mockResolvedValue(void 0);
mocks.user.getByOAuthId.mockResolvedValue(void 0);
mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' }));
mocks.session.create.mockResolvedValue(SessionFactory.create());
mocks.oauthLinkToken.create.mockResolvedValue({} as any);
await sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
await expect(
sut.callback(
{ url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' },
{},
loginDetails,
),
).rejects.toThrow(OAuthLinkRequiredException);
expect(mocks.oauthLinkToken.create).toHaveBeenCalledWith(
expect.objectContaining({ profile: expect.objectContaining({ isAdmin: true }) }),
);
expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ isAdmin: true }));
});
});
+114 -72
View File
@@ -2,6 +2,8 @@ import { BadRequestException, ForbiddenException, Injectable, UnauthorizedExcept
import { parse } from 'cookie';
import { DateTime } from 'luxon';
import { IncomingHttpHeaders } from 'node:http';
import sanitize from 'sanitize-filename';
import { SystemConfig } from 'src/config';
import { LOGIN_URL, MOBILE_REDIRECT, SALT_ROUNDS } from 'src/constants';
import { AuthSharedLink, AuthUser, UserAdmin } from 'src/database';
import {
@@ -23,6 +25,7 @@ import {
import { UserAdminResponseDto, mapUserAdmin } from 'src/dtos/user.dto';
import { AuthType, ImmichCookie, ImmichHeader, ImmichQuery, JobName, Permission } from 'src/enum';
import { OAuthProfile } from 'src/repositories/oauth.repository';
import { OAuthLinkTokenProfile } from 'src/schema/tables/oauth-link-token.table';
import { BaseService } from 'src/services/base.service';
import { isGranted } from 'src/utils/access';
import { HumanReadableSize } from 'src/utils/bytes';
@@ -89,22 +92,46 @@ export class AuthService extends BaseService {
if (linkTokenCookie) {
const hashedToken = this.cryptoRepository.hashSha256(linkTokenCookie);
const record = await this.oauthLinkTokenRepository.consumeToken(hashedToken);
if (!record) {
throw new BadRequestException('Invalid or expired link token');
if (record) {
const duplicate = await this.userRepository.getByOAuthId(record.oauthSub);
if (duplicate && duplicate.id !== user.id) {
throw new BadRequestException('This OAuth account has already been linked to another user.');
}
user = await this.applyOAuthProfileToUser(user, record);
linkedOAuthSid = record.oauthSid ?? undefined;
}
const duplicate = await this.userRepository.getByOAuthId(record.oauthSub);
if (duplicate && duplicate.id !== user.id) {
throw new BadRequestException('This OAuth account has already been linked to another user.');
}
await this.userRepository.update(user.id, { oauthId: record.oauthSub });
linkedOAuthSid = record.oauthSid ?? undefined;
}
return this.createLoginResponse(user, details, linkedOAuthSid);
}
async register(details: LoginDetails, headers: IncomingHttpHeaders) {
const { oauth } = await this.getConfig({ withCache: false });
if (!oauth.enabled || !oauth.autoRegister) {
throw new BadRequestException('OAuth auto-register is disabled');
}
const linkTokenCookie = this.getCookieOAuthLinkToken(headers);
if (!linkTokenCookie) {
throw new BadRequestException('Missing OAuth link token');
}
const record = await this.consumeOAuthLinkToken(linkTokenCookie);
const existing = await this.userRepository.getByOAuthId(record.oauthSub);
if (existing) {
throw new BadRequestException('This OAuth account has already been linked to another user.');
}
this.logger.log(`Registering new user from OAuth: ${record.oauthSub}/${record.email}`);
const newUser = await this.createUser({
email: record.email,
name: record.profile.name,
isAdmin: record.profile.isAdmin,
});
const user = await this.applyOAuthProfileToUser(newUser, record);
return this.createLoginResponse(user, details, record.oauthSid ?? undefined);
}
async logout(auth: AuthDto, authType: AuthType): Promise<LogoutResponseDto> {
if (auth.session) {
await this.sessionRepository.delete(auth.session.id);
@@ -343,76 +370,91 @@ export class AuthService extends BaseService {
codeVerifier,
);
const normalizedEmail = profile.email ? profile.email.trim().toLowerCase() : undefined;
const { autoRegister, defaultStorageQuota, storageLabelClaim, storageQuotaClaim, roleClaim } = oauth;
this.logger.debug(`Logging in with OAuth: ${JSON.stringify(profile)}`);
let user: UserAdmin | undefined = await this.userRepository.getByOAuthId(profile.sub);
const user = await this.userRepository.getByOAuthId(profile.sub);
if (!user && normalizedEmail) {
const emailUser = await this.userRepository.getByEmail(normalizedEmail);
if (emailUser) {
const plainToken = this.cryptoRepository.randomBytesAsText(32);
const hashedToken = this.cryptoRepository.hashSha256(plainToken);
await this.oauthLinkTokenRepository.create({
token: hashedToken,
oauthSub: profile.sub,
oauthSid: oauthSid ?? null,
email: emailUser.email,
expiresAt: DateTime.now().plus({ minutes: 10 }).toJSDate(),
});
throw new OAuthLinkRequiredException(emailUser.email, plainToken);
if (user) {
if (!user.profileImagePath && profile.picture) {
await this.syncProfilePicture(user, profile.picture);
}
return this.createLoginResponse(user, loginDetails, oauthSid);
}
// register new user
if (!user) {
if (!autoRegister) {
this.logger.warn(
`Unable to register ${profile.sub}/${normalizedEmail || '(no email)'}. To enable set OAuth Auto Register to true in admin settings.`,
);
throw new BadRequestException(`User does not exist and auto registering is disabled.`);
}
if (!normalizedEmail) {
throw new BadRequestException('OAuth profile does not have an email address');
}
this.logger.log(`Registering new user: ${profile.sub}/${normalizedEmail}`);
const storageLabel = this.getClaim(profile, {
key: storageLabelClaim,
default: '',
isValid: (value: unknown): value is string => typeof value === 'string',
});
const storageQuota = this.getClaim(profile, {
key: storageQuotaClaim,
default: defaultStorageQuota,
isValid: (value: unknown) => Number(value) >= 0,
});
const role = this.getClaim<'admin' | 'user'>(profile, {
key: roleClaim,
default: 'user',
isValid: (value: unknown) => typeof value === 'string' && ['admin', 'user'].includes(value),
});
user = await this.createUser({
name:
profile.name ||
`${profile.given_name || ''} ${profile.family_name || ''}`.trim() ||
profile.preferred_username ||
normalizedEmail,
email: normalizedEmail,
oauthId: profile.sub,
quotaSizeInBytes: storageQuota === null ? null : storageQuota * HumanReadableSize.GiB,
storageLabel: storageLabel || null,
isAdmin: role === 'admin',
});
if (!normalizedEmail) {
throw new BadRequestException('OAuth profile does not have an email address');
}
if (!user.profileImagePath && profile.picture) {
await this.syncProfilePicture(user, profile.picture);
}
const resolvedProfile = this.resolveOAuthProfile(profile, normalizedEmail, oauth);
const plainToken = this.cryptoRepository.randomBytesAsText(32);
const hashedToken = this.cryptoRepository.hashSha256(plainToken);
await this.oauthLinkTokenRepository.create({
token: hashedToken,
oauthSub: profile.sub,
oauthSid: oauthSid ?? null,
email: normalizedEmail,
profile: resolvedProfile,
expiresAt: DateTime.now().plus({ minutes: 10 }).toJSDate(),
});
throw new OAuthLinkRequiredException(normalizedEmail, plainToken);
}
return this.createLoginResponse(user, loginDetails, oauthSid);
private resolveOAuthProfile(
profile: OAuthProfile,
normalizedEmail: string,
oauth: SystemConfig['oauth'],
): OAuthLinkTokenProfile {
const { defaultStorageQuota, storageLabelClaim, storageQuotaClaim, roleClaim } = oauth;
const storageLabel = this.getClaim(profile, {
key: storageLabelClaim,
default: '',
isValid: (value: unknown): value is string => typeof value === 'string',
});
const storageQuota = this.getClaim(profile, {
key: storageQuotaClaim,
default: defaultStorageQuota,
isValid: (value: unknown) => Number(value) >= 0,
});
const role = this.getClaim<'admin' | 'user'>(profile, {
key: roleClaim,
default: 'user',
isValid: (value: unknown) => typeof value === 'string' && ['admin', 'user'].includes(value),
});
return {
name:
profile.name ||
`${profile.given_name || ''} ${profile.family_name || ''}`.trim() ||
profile.preferred_username ||
normalizedEmail,
storageLabel: storageLabel || null,
storageQuotaInGiB: storageQuota,
isAdmin: role === 'admin',
picture: profile.picture ?? null,
};
}
private async consumeOAuthLinkToken(plainToken: string) {
const hashedToken = this.cryptoRepository.hashSha256(plainToken);
const record = await this.oauthLinkTokenRepository.consumeToken(hashedToken);
if (!record) {
throw new BadRequestException('Invalid or expired link token');
}
return record;
}
private async applyOAuthProfileToUser(user: UserAdmin, record: { oauthSub: string; profile: OAuthLinkTokenProfile }) {
const { profile } = record;
const storageLabel = profile.storageLabel ? sanitize(profile.storageLabel.replaceAll('.', '')) : null;
const updated = await this.userRepository.update(user.id, {
oauthId: record.oauthSub,
storageLabel,
quotaSizeInBytes: profile.storageQuotaInGiB === null ? null : profile.storageQuotaInGiB * HumanReadableSize.GiB,
isAdmin: profile.isAdmin,
});
if (!updated.profileImagePath && profile.picture) {
await this.syncProfilePicture(updated, profile.picture);
}
return updated;
}
private async syncProfilePicture(user: UserAdmin, url: string) {
@@ -141,6 +141,7 @@ describe(ServerService.name, () => {
reverseGeocoding: true,
oauth: false,
oauthAutoLaunch: false,
oauthAutoRegister: true,
ocr: true,
passwordLogin: true,
search: true,
+1
View File
@@ -102,6 +102,7 @@ export class ServerService extends BaseService {
trash: trash.enabled,
oauth: oauth.enabled,
oauthAutoLaunch: oauth.autoLaunch,
oauthAutoRegister: oauth.autoRegister,
ocr: isOcrEnabled(machineLearning),
passwordLogin: passwordLogin.enabled,
configFile: !!configFile,