From 2da2bef7779135e77bd6f6b78eb9350740d90421 Mon Sep 17 00:00:00 2001 From: bo0tzz Date: Thu, 23 Apr 2026 12:22:27 +0200 Subject: [PATCH] fix: review notes, new register endpoint --- e2e/src/specs/server/api/oauth.e2e-spec.ts | 70 ++- i18n/en.json | 1 + open-api/immich-openapi-specs.json | 34 ++ open-api/typescript-sdk/src/fetch-client.ts | 14 + .../src/controllers/auth.controller.spec.ts | 65 +++ server/src/controllers/auth.controller.ts | 56 +- server/src/dtos/server.dto.ts | 1 + ...1776362646907-CreateOAuthLinkTokenTable.ts | 1 + .../schema/tables/oauth-link-token.table.ts | 11 + server/src/services/auth.service.spec.ts | 547 ++++++++++++------ server/src/services/auth.service.ts | 186 +++--- server/src/services/server.service.spec.ts | 1 + server/src/services/server.service.ts | 1 + web/src/routes/auth/link/+page.svelte | 42 +- 14 files changed, 736 insertions(+), 294 deletions(-) diff --git a/e2e/src/specs/server/api/oauth.e2e-spec.ts b/e2e/src/specs/server/api/oauth.e2e-spec.ts index e323952ea5..b4264199b8 100644 --- a/e2e/src/specs/server/api/oauth.e2e-spec.ts +++ b/e2e/src/specs/server/api/oauth.e2e-spec.ts @@ -205,30 +205,39 @@ describe(`/oauth`, () => { expect(status).toBeGreaterThanOrEqual(400); }); - it('should auto register the user by default', async () => { + it('should return a link token for a new OAuth user', async () => { const callbackParams = await loginWithOAuth('oauth-auto-register'); - const { status, body } = await request(app).post('/oauth/callback').send(callbackParams); - expect(status).toBe(201); - expect(body).toMatchObject({ - accessToken: expect.any(String), - isAdmin: false, - name: 'OAuth User', - userEmail: 'oauth-auto-register@immich.app', - userId: expect.any(String), - }); + const response = await request(app).post('/oauth/callback').send(callbackParams); + expect(response.status).toBe(403); + expect(response.body.message).toBe('oauth_account_link_required'); + const setCookie = response.headers['set-cookie'] as unknown as string[]; + expect(setCookie.some((cookie) => cookie.startsWith('immich_oauth_link_token='))).toBe(true); }); it('should allow passing state and codeVerifier via cookies', async () => { const { url, state, codeVerifier } = await loginWithOAuth('oauth-auto-register'); - const { status, body } = await request(app) + const response = await request(app) .post('/oauth/callback') .set('Cookie', [`immich_oauth_state=${state}`, `immich_oauth_code_verifier=${codeVerifier}`]) .send({ url }); - expect(status).toBe(201); - expect(body).toMatchObject({ + expect(response.status).toBe(403); + expect(response.body.message).toBe('oauth_account_link_required'); + }); + + it('should register a new user via POST /auth/register using the link token cookie', async () => { + const callbackParams = await loginWithOAuth('oauth-register-flow'); + const callbackResponse = await request(app).post('/oauth/callback').send(callbackParams); + expect(callbackResponse.status).toBe(403); + const setCookie = callbackResponse.headers['set-cookie'] as unknown as string[]; + const linkCookie = setCookie.find((cookie) => cookie.startsWith('immich_oauth_link_token=')); + expect(linkCookie).toBeDefined(); + + const registerResponse = await request(app).post('/auth/register').set('Cookie', linkCookie!); + expect(registerResponse.status).toBe(201); + expect(registerResponse.body).toMatchObject({ accessToken: expect.any(String), + userEmail: 'oauth-register-flow@immich.app', userId: expect.any(String), - userEmail: 'oauth-auto-register@immich.app', }); }); @@ -349,11 +358,11 @@ describe(`/oauth`, () => { }); }); - it('should not auto register the user', async () => { + it('should still create a link token when auto register is disabled', async () => { const callbackParams = await loginWithOAuth('oauth-no-auto-register'); - const { status, body } = await request(app).post('/oauth/callback').send(callbackParams); - expect(status).toBe(400); - expect(body).toEqual(errorDto.badRequest('User does not exist and auto registering is disabled.')); + const response = await request(app).post('/oauth/callback').send(callbackParams); + expect(response.status).toBe(403); + expect(response.body.message).toBe('oauth_account_link_required'); }); it('should not auto-link to an existing user by email', async () => { @@ -445,24 +454,18 @@ describe(`/oauth`, () => { expect(params.get('state')).toBeDefined(); }); - it('should auto register the user by default', async () => { + it('should return a link token for a new OAuth user via mobile redirect', async () => { const callbackParams = await loginWithOAuth('oauth-mobile-override', 'app.immich:///oauth-callback'); expect(callbackParams.url).toEqual(expect.stringContaining(mobileOverrideRedirectUri)); // simulate redirecting back to mobile app const url = callbackParams.url.replace(mobileOverrideRedirectUri, 'app.immich:///oauth-callback'); - const { status, body } = await request(app) + const response = await request(app) .post('/oauth/callback') .send({ ...callbackParams, url }); - expect(status).toBe(201); - expect(body).toMatchObject({ - accessToken: expect.any(String), - isAdmin: false, - name: 'OAuth User', - userEmail: 'oauth-mobile-override@immich.app', - userId: expect.any(String), - }); + expect(response.status).toBe(403); + expect(response.body.message).toBe('oauth_account_link_required'); }); }); @@ -474,14 +477,9 @@ describe(`/oauth`, () => { clientSecret: OAuthClient.DEFAULT, }); const callbackParams = await loginWithOAuth(OAuthUser.ID_TOKEN_CLAIMS); - const { status, body } = await request(app).post('/oauth/callback').send(callbackParams); - expect(status).toBe(201); - expect(body).toMatchObject({ - accessToken: expect.any(String), - name: 'ID Token User', - userEmail: 'oauth-id-token-claims@immich.app', - userId: expect.any(String), - }); + const response = await request(app).post('/oauth/callback').send(callbackParams); + expect(response.status).toBe(403); + expect(response.body.message).toBe('oauth_account_link_required'); }); }); diff --git a/i18n/en.json b/i18n/en.json index 2ff1123d42..1fe2f2b746 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -853,6 +853,7 @@ "create_link_to_share": "Create link to share", "create_link_to_share_description": "Let anyone with the link see the selected photo(s)", "create_new": "CREATE NEW", + "create_new_account": "Create new account", "create_new_face": "Create new face", "create_new_person": "Create new person", "create_new_person_hint": "Assign selected assets to a new person", diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index 282ae03b8c..a5fd08fd8c 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -4660,6 +4660,35 @@ "x-immich-state": "Stable" } }, + "/auth/register": { + "post": { + "description": "Create a new user from a pending OAuth link token (requires OAuth auto-register to be enabled).", + "operationId": "register", + "parameters": [], + "responses": { + "201": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/LoginResponseDto" + } + } + }, + "description": "" + } + }, + "summary": "Register via OAuth", + "tags": [ + "Authentication" + ], + "x-immich-history": [ + { + "version": "v2", + "state": "Added" + } + ] + } + }, "/auth/session/lock": { "post": { "description": "Remove elevated access to locked assets from the current session.", @@ -21130,6 +21159,10 @@ "description": "Whether OAuth auto-launch is enabled", "type": "boolean" }, + "oauthAutoRegister": { + "description": "Whether OAuth auto-register is enabled", + "type": "boolean" + }, "ocr": { "description": "Whether OCR is enabled", "type": "boolean" @@ -21168,6 +21201,7 @@ "map", "oauth", "oauthAutoLaunch", + "oauthAutoRegister", "ocr", "passwordLogin", "reverseGeocoding", diff --git a/open-api/typescript-sdk/src/fetch-client.ts b/open-api/typescript-sdk/src/fetch-client.ts index 8b6ac68976..5828f0d4a6 100644 --- a/open-api/typescript-sdk/src/fetch-client.ts +++ b/open-api/typescript-sdk/src/fetch-client.ts @@ -2047,6 +2047,8 @@ export type ServerFeaturesDto = { oauth: boolean; /** Whether OAuth auto-launch is enabled */ oauthAutoLaunch: boolean; + /** Whether OAuth auto-register is enabled */ + oauthAutoRegister: boolean; /** Whether OCR is enabled */ ocr: boolean; /** Whether password login is enabled */ @@ -4304,6 +4306,18 @@ export function changePinCode({ pinCodeChangeDto }: { body: pinCodeChangeDto }))); } +/** + * Register via OAuth + */ +export function register(opts?: Oazapfts.RequestOpts) { + return oazapfts.ok(oazapfts.fetchJson<{ + status: 201; + data: LoginResponseDto; + }>("/auth/register", { + ...opts, + method: "POST" + })); +} /** * Lock auth session */ diff --git a/server/src/controllers/auth.controller.spec.ts b/server/src/controllers/auth.controller.spec.ts index 79f239983d..9bb5900ef8 100644 --- a/server/src/controllers/auth.controller.spec.ts +++ b/server/src/controllers/auth.controller.spec.ts @@ -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'); diff --git a/server/src/controllers/auth.controller.ts b/server/src/controllers/auth.controller.ts index 37de5b5e75..40c0203f0f 100644 --- a/server/src/controllers/auth.controller.ts +++ b/server/src/controllers/auth.controller.ts @@ -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 { - 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 { + 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') diff --git a/server/src/dtos/server.dto.ts b/server/src/dtos/server.dto.ts index 57a58e1dd7..bcbaba5e5f 100644 --- a/server/src/dtos/server.dto.ts +++ b/server/src/dtos/server.dto.ts @@ -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'), diff --git a/server/src/schema/migrations/1776362646907-CreateOAuthLinkTokenTable.ts b/server/src/schema/migrations/1776362646907-CreateOAuthLinkTokenTable.ts index 36b9ed1945..c7acf8616c 100644 --- a/server/src/schema/migrations/1776362646907-CreateOAuthLinkTokenTable.ts +++ b/server/src/schema/migrations/1776362646907-CreateOAuthLinkTokenTable.ts @@ -8,6 +8,7 @@ export async function up(db: Kysely): Promise { "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() ); diff --git a/server/src/schema/tables/oauth-link-token.table.ts b/server/src/schema/tables/oauth-link-token.table.ts index 2477c0391d..6fa41e034a 100644 --- a/server/src/schema/tables/oauth-link-token.table.ts +++ b/server/src/schema/tables/oauth-link-token.table.ts @@ -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; diff --git a/server/src/services/auth.service.spec.ts b/server/src/services/auth.service.spec.ts index 52dabb0009..83b7d7852c 100644 --- a/server/src/services/auth.service.spec.ts +++ b/server/src/services/auth.service.spec.ts @@ -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 })); }); }); diff --git a/server/src/services/auth.service.ts b/server/src/services/auth.service.ts index e6799ee460..b47f364a09 100644 --- a/server/src/services/auth.service.ts +++ b/server/src/services/auth.service.ts @@ -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 { 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) { diff --git a/server/src/services/server.service.spec.ts b/server/src/services/server.service.spec.ts index 6e1187a900..33d5cd2304 100644 --- a/server/src/services/server.service.spec.ts +++ b/server/src/services/server.service.spec.ts @@ -141,6 +141,7 @@ describe(ServerService.name, () => { reverseGeocoding: true, oauth: false, oauthAutoLaunch: false, + oauthAutoRegister: true, ocr: true, passwordLogin: true, search: true, diff --git a/server/src/services/server.service.ts b/server/src/services/server.service.ts index ff9e90f7e0..3f1bde713d 100644 --- a/server/src/services/server.service.ts +++ b/server/src/services/server.service.ts @@ -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, diff --git a/web/src/routes/auth/link/+page.svelte b/web/src/routes/auth/link/+page.svelte index e6d1bdc260..4ce5b6de12 100644 --- a/web/src/routes/auth/link/+page.svelte +++ b/web/src/routes/auth/link/+page.svelte @@ -5,8 +5,8 @@ import { eventManager } from '$lib/managers/event-manager.svelte'; import { featureFlagsManager } from '$lib/managers/feature-flags-manager.svelte'; import { Route } from '$lib/route'; - import { getServerErrorMessage } from '$lib/utils/handle-error'; - import { login } from '@immich/sdk'; + import { getServerErrorMessage, handleError } from '$lib/utils/handle-error'; + import { login, register } from '@immich/sdk'; import { Alert, Button, Field, Input, PasswordInput, Stack, toastManager } from '@immich/ui'; import { t } from 'svelte-i18n'; import type { PageData } from './$types'; @@ -21,6 +21,7 @@ let password = $state(''); let errorMessage = $state(''); let loading = $state(false); + let registering = $state(false); const handleSubmit = async (event: Event) => { event.preventDefault(); @@ -37,6 +38,19 @@ loading = false; } }; + + const handleRegister = async () => { + try { + registering = true; + const user = await register(); + eventManager.emit('AuthLogin', user); + await authManager.refresh(); + await goto(Route.photos(), { invalidateAll: true }); + } catch (error) { + handleError(error, $t('errors.unable_to_create_user')); + registering = false; + } + }; @@ -68,5 +82,29 @@ {$t('oauth_link_password_login_required')} {/if} + + {#if featureFlagsManager.value.oauthAutoRegister} + {#if featureFlagsManager.value.passwordLogin} +
+
+ + {$t('or')} + +
+ {/if} + + + {/if}