From 7303fab9d948305a36ab83e7265ca2d388c94c2f Mon Sep 17 00:00:00 2001 From: Sam Holton Date: Fri, 1 Mar 2024 19:46:07 -0500 Subject: [PATCH] feat(server/web): add oauth defaultStorageQuota and storageQuotaClaim (#7548) * feat(server/web): add oauth defaultStorageQuota and storageQuotaClaim * feat(server/web): fix format and use domain.util constants * address some pr feedback * simplify oauth storage quota logic * adding tests and pr feedback * chore: cleanup --------- Co-authored-by: Jason Rasmussen --- docs/docs/administration/oauth.md | 4 +- docs/docs/install/config-file.md | 9 +- mobile/openapi/doc/SystemConfigOAuthDto.md | Bin 810 -> 891 bytes .../lib/model/system_config_o_auth_dto.dart | Bin 6162 -> 6884 bytes .../test/system_config_o_auth_dto_test.dart | Bin 1783 -> 2022 bytes open-api/immich-openapi-specs.json | 10 ++- open-api/typescript-sdk/src/fetch-client.ts | 2 + server/src/domain/auth/auth.service.spec.ts | 78 ++++++++++++++++-- server/src/domain/auth/auth.service.ts | 34 ++++++-- .../dto/system-config-oauth.dto.ts | 9 +- .../system-config/system-config.core.ts | 2 + .../system-config.service.spec.ts | 2 + .../infra/entities/system-config.entity.ts | 4 + server/test/fixtures/system-config.stub.ts | 5 ++ server/test/fixtures/user.stub.ts | 14 ++++ .../jobs/storage-migration-description.svelte | 2 +- .../settings/oauth/oauth-settings.svelte | 20 +++++ 17 files changed, 177 insertions(+), 18 deletions(-) diff --git a/docs/docs/administration/oauth.md b/docs/docs/administration/oauth.md index c1ac3c577..66a752f0b 100644 --- a/docs/docs/administration/oauth.md +++ b/docs/docs/administration/oauth.md @@ -67,9 +67,11 @@ Once you have a new OAuth client application configured, Immich can be configure | Client Secret | string | (required) | Required. Client Secret (previous step) | | Scope | string | openid email profile | Full list of scopes to send with the request (space delimited) | | Signing Algorithm | string | RS256 | The algorithm used to sign the id token (examples: RS256, HS256) | +| Storage Label Claim | string | preferred_username | Claim mapping for the user's storage label | +| Storage Quota Claim | string | immich_quota | Claim mapping for the user's storage | +| Default Storage Quota (GiB) | number | 0 | Default quota for user without storage quota claim (Enter 0 for unlimited quota) | | Button Text | string | Login with OAuth | Text for the OAuth button on the web | | Auto Register | boolean | true | When true, will automatically register a user the first time they sign in | -| Storage Claim | string | preferred_username | Claim mapping for the user's storage label | | [Auto Launch](#auto-launch) | boolean | false | When true, will skip the login page and automatically start the OAuth login process | | [Mobile Redirect URI Override](#mobile-redirect-uri) | URL | (empty) | Http(s) alternative mobile redirect URI | diff --git a/docs/docs/install/config-file.md b/docs/docs/install/config-file.md index 755722d9e..8a7776a42 100644 --- a/docs/docs/install/config-file.md +++ b/docs/docs/install/config-file.md @@ -95,13 +95,16 @@ The default configuration looks like this: "issuerUrl": "", "clientId": "", "clientSecret": "", - "mobileOverrideEnabled": false, - "mobileRedirectUri": "", "scope": "openid email profile", + "signingAlgorithm": "RS256", "storageLabelClaim": "preferred_username", + "storageQuotaClaim": "immich_quota", + "defaultStorageQuota": 0, "buttonText": "Login with OAuth", "autoRegister": true, - "autoLaunch": false + "autoLaunch": false, + "mobileOverrideEnabled": false, + "mobileRedirectUri": "" }, "passwordLogin": { "enabled": true diff --git a/mobile/openapi/doc/SystemConfigOAuthDto.md b/mobile/openapi/doc/SystemConfigOAuthDto.md index c02dae9b732ff4a88b398ec40b4782c9960340fc..43694f6fc265e436b710371bfb4a4258675b4979 100644 GIT binary patch delta 65 zcmZ3*_M2@(JELq$YFc7xPDyY{eop{EoLpcD S=bXgM+{uhgVv}2$mI43(YZe#) delta 15 Xcmey(wu)^-JLBZ_jQ=KIV_FITIJE~m diff --git a/mobile/openapi/lib/model/system_config_o_auth_dto.dart b/mobile/openapi/lib/model/system_config_o_auth_dto.dart index 603ff8a952cb2213a49e9bbee759ab9809a33483..1e7fe7f0001f6deaa6c27bad676359fe56c28a12 100644 GIT binary patch delta 593 zcmbPa@Wgb38Iy2IYFc7xPDyY{eo@Kjm2n{Iahx43ba>QbUl>#sx z^a>J-ic>YzRjRX!^YfyM)Uha3kF8bL+$_%jk5Nbg!#9&V1eB2@cr&YD85`DsEEb^6wAb%60yYt2>5#RUN3QQ8jx delta 85 zcmV-b0IL7wHIgu}HUg7f0@#yU19Ow<1FW;M1T_J(9|kA^laK@jlYj@>lYa=(lXMBV rlU54SvrY>?0kg~uUIDXZ4!;7kZxQqXv$GRs29q=zkp_J`3VjL+iN+sF diff --git a/mobile/openapi/test/system_config_o_auth_dto_test.dart b/mobile/openapi/test/system_config_o_auth_dto_test.dart index 2bcbb64efd7a343f8cee4d77f43db14f41782ad3..e855ff96087d88d777c18aeb24ae12326f37548a 100644 GIT binary patch delta 107 zcmey)`;33XTW0aR(p-g<)U?FXoRZ*@{G!D4)WFjGlElgH85M=Gi7T?`E`H0*JlTPX heR2acJ5MpPrU`7iTwv|aIf { let userTokenMock: jest.Mocked; let shareMock: jest.Mocked; let keyMock: jest.Mocked; - let callbackMock: jest.Mock; - afterEach(() => { - jest.resetModules(); - }); + let callbackMock: jest.Mock; + let userinfoMock: jest.Mock; beforeEach(async () => { callbackMock = jest.fn().mockReturnValue({ access_token: 'access-token' }); + userinfoMock = jest.fn().mockResolvedValue({ sub, email }); jest.spyOn(generators, 'state').mockReturnValue('state'); jest.spyOn(Issuer, 'discover').mockResolvedValue({ @@ -83,7 +83,7 @@ describe('AuthService', () => { authorizationUrl: jest.fn().mockReturnValue('http://authorization-url'), callbackParams: jest.fn().mockReturnValue({ state: 'state' }), callback: callbackMock, - userinfo: jest.fn().mockResolvedValue({ sub, email }), + userinfo: userinfoMock, }), } as any); @@ -491,6 +491,74 @@ describe('AuthService', () => { expect(callbackMock).toHaveBeenCalledWith('http://mobile-redirect', { state: 'state' }, { state: 'state' }); }); + + it('should use the default quota', async () => { + configMock.load.mockResolvedValue(systemConfigStub.withDefaultStorageQuota); + userMock.getByEmail.mockResolvedValue(null); + userMock.getAdmin.mockResolvedValue(userStub.user1); + userMock.create.mockResolvedValue(userStub.user1); + + await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' }, loginDetails)).resolves.toEqual( + loginResponseStub.user1oauth, + ); + + expect(userMock.create).toHaveBeenCalledWith(userDto.userWithDefaultStorageQuota); + }); + + it('should ignore an invalid storage quota', async () => { + configMock.load.mockResolvedValue(systemConfigStub.withDefaultStorageQuota); + userMock.getByEmail.mockResolvedValue(null); + userMock.getAdmin.mockResolvedValue(userStub.user1); + userMock.create.mockResolvedValue(userStub.user1); + userinfoMock.mockResolvedValue({ sub, email, immich_quota: 'abc' }); + + await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' }, loginDetails)).resolves.toEqual( + loginResponseStub.user1oauth, + ); + + expect(userMock.create).toHaveBeenCalledWith(userDto.userWithDefaultStorageQuota); + }); + it('should ignore a negative quota', async () => { + configMock.load.mockResolvedValue(systemConfigStub.withDefaultStorageQuota); + userMock.getByEmail.mockResolvedValue(null); + userMock.getAdmin.mockResolvedValue(userStub.user1); + userMock.create.mockResolvedValue(userStub.user1); + userinfoMock.mockResolvedValue({ sub, email, immich_quota: -5 }); + + await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' }, loginDetails)).resolves.toEqual( + loginResponseStub.user1oauth, + ); + + expect(userMock.create).toHaveBeenCalledWith(userDto.userWithDefaultStorageQuota); + }); + + it('should ignore a 0 quota', async () => { + configMock.load.mockResolvedValue(systemConfigStub.withDefaultStorageQuota); + userMock.getByEmail.mockResolvedValue(null); + userMock.getAdmin.mockResolvedValue(userStub.user1); + userMock.create.mockResolvedValue(userStub.user1); + userinfoMock.mockResolvedValue({ sub, email, immich_quota: 0 }); + + await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' }, loginDetails)).resolves.toEqual( + loginResponseStub.user1oauth, + ); + + expect(userMock.create).toHaveBeenCalledWith(userDto.userWithDefaultStorageQuota); + }); + + it('should use a valid storage quota', async () => { + configMock.load.mockResolvedValue(systemConfigStub.withDefaultStorageQuota); + userMock.getByEmail.mockResolvedValue(null); + userMock.getAdmin.mockResolvedValue(userStub.user1); + userMock.create.mockResolvedValue(userStub.user1); + userinfoMock.mockResolvedValue({ sub, email, immich_quota: 5 }); + + await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' }, loginDetails)).resolves.toEqual( + loginResponseStub.user1oauth, + ); + + expect(userMock.create).toHaveBeenCalledWith(userDto.userWithStorageQuotaClaim); + }); }); describe('link', () => { diff --git a/server/src/domain/auth/auth.service.ts b/server/src/domain/auth/auth.service.ts index a2c0d2df9..f0fc4585c 100644 --- a/server/src/domain/auth/auth.service.ts +++ b/server/src/domain/auth/auth.service.ts @@ -7,11 +7,13 @@ import { InternalServerErrorException, UnauthorizedException, } from '@nestjs/common'; +import { isNumber, isString } from 'class-validator'; import cookieParser from 'cookie'; import { DateTime } from 'luxon'; import { IncomingHttpHeaders } from 'node:http'; import { ClientMetadata, Issuer, UserinfoResponse, custom, generators } from 'openid-client'; import { AccessCore, Permission } from '../access'; +import { HumanReadableSize } from '../domain.util'; import { IAccessRepository, ICryptoRepository, @@ -64,6 +66,12 @@ interface OAuthProfile extends UserinfoResponse { email: string; } +interface ClaimOptions { + key: string; + default: T; + isValid: (value: unknown) => boolean; +} + @Injectable() export class AuthService { private access: AccessCore; @@ -234,9 +242,11 @@ export class AuthService { } } + const { autoRegister, defaultStorageQuota, storageLabelClaim, storageQuotaClaim } = config.oauth; + // register new user if (!user) { - if (!config.oauth.autoRegister) { + if (!autoRegister) { this.logger.warn( `Unable to register ${profile.email}. To enable set OAuth Auto Register to true in admin settings.`, ); @@ -246,17 +256,24 @@ export class AuthService { this.logger.log(`Registering new user: ${profile.email}/${profile.sub}`); this.logger.verbose(`OAuth Profile: ${JSON.stringify(profile)}`); - let storageLabel: string | null = profile[config.oauth.storageLabelClaim as keyof OAuthProfile] as string; - if (typeof storageLabel !== 'string') { - storageLabel = null; - } + const storageLabel = this.getClaim(profile, { + key: storageLabelClaim, + default: '', + isValid: isString, + }); + const storageQuota = this.getClaim(profile, { + key: storageQuotaClaim, + default: defaultStorageQuota, + isValid: (value: unknown) => isNumber(value) && value > 0, + }); const userName = profile.name ?? `${profile.given_name || ''} ${profile.family_name || ''}`; user = await this.userCore.createUser({ name: userName, email: profile.email, oauthId: profile.sub, - storageLabel, + quotaSizeInBytes: storageQuota * HumanReadableSize.GiB || null, + storageLabel: storageLabel || null, }); } @@ -443,4 +460,9 @@ export class AuthService { } return [accessTokenCookie, authTypeCookie, isAuthenticatedCookie]; } + + private getClaim(profile: OAuthProfile, options: ClaimOptions): T { + const value = profile[options.key as keyof OAuthProfile]; + return options.isValid(value) ? (value as T) : options.default; + } } diff --git a/server/src/domain/system-config/dto/system-config-oauth.dto.ts b/server/src/domain/system-config/dto/system-config-oauth.dto.ts index 52aa47a7e..04159b8d3 100644 --- a/server/src/domain/system-config/dto/system-config-oauth.dto.ts +++ b/server/src/domain/system-config/dto/system-config-oauth.dto.ts @@ -1,4 +1,4 @@ -import { IsBoolean, IsNotEmpty, IsString, IsUrl, ValidateIf } from 'class-validator'; +import { IsBoolean, IsNotEmpty, IsNumber, IsString, IsUrl, Min, ValidateIf } from 'class-validator'; const isEnabled = (config: SystemConfigOAuthDto) => config.enabled; const isOverrideEnabled = (config: SystemConfigOAuthDto) => config.mobileOverrideEnabled; @@ -23,6 +23,10 @@ export class SystemConfigOAuthDto { @IsString() clientSecret!: string; + @IsNumber() + @Min(0) + defaultStorageQuota!: number; + @IsBoolean() enabled!: boolean; @@ -47,4 +51,7 @@ export class SystemConfigOAuthDto { @IsString() storageLabelClaim!: string; + + @IsString() + storageQuotaClaim!: string; } diff --git a/server/src/domain/system-config/system-config.core.ts b/server/src/domain/system-config/system-config.core.ts index d32309955..a9d41d76d 100644 --- a/server/src/domain/system-config/system-config.core.ts +++ b/server/src/domain/system-config/system-config.core.ts @@ -93,6 +93,7 @@ export const defaults = Object.freeze({ buttonText: 'Login with OAuth', clientId: '', clientSecret: '', + defaultStorageQuota: 0, enabled: false, issuerUrl: '', mobileOverrideEnabled: false, @@ -100,6 +101,7 @@ export const defaults = Object.freeze({ scope: 'openid email profile', signingAlgorithm: 'RS256', storageLabelClaim: 'preferred_username', + storageQuotaClaim: 'immich_quota', }, passwordLogin: { enabled: true, diff --git a/server/src/domain/system-config/system-config.service.spec.ts b/server/src/domain/system-config/system-config.service.spec.ts index 67a6418f4..a3d29b1ee 100644 --- a/server/src/domain/system-config/system-config.service.spec.ts +++ b/server/src/domain/system-config/system-config.service.spec.ts @@ -93,6 +93,7 @@ const updatedConfig = Object.freeze({ buttonText: 'Login with OAuth', clientId: '', clientSecret: '', + defaultStorageQuota: 0, enabled: false, issuerUrl: '', mobileOverrideEnabled: false, @@ -100,6 +101,7 @@ const updatedConfig = Object.freeze({ scope: 'openid email profile', signingAlgorithm: 'RS256', storageLabelClaim: 'preferred_username', + storageQuotaClaim: 'immich_quota', }, passwordLogin: { enabled: true, diff --git a/server/src/infra/entities/system-config.entity.ts b/server/src/infra/entities/system-config.entity.ts index edf473435..e2d0c71f6 100644 --- a/server/src/infra/entities/system-config.entity.ts +++ b/server/src/infra/entities/system-config.entity.ts @@ -80,6 +80,7 @@ export enum SystemConfigKey { OAUTH_BUTTON_TEXT = 'oauth.buttonText', OAUTH_CLIENT_ID = 'oauth.clientId', OAUTH_CLIENT_SECRET = 'oauth.clientSecret', + OAUTH_DEFAULT_STORAGE_QUOTA = 'oauth.defaultStorageQuota', OAUTH_ENABLED = 'oauth.enabled', OAUTH_ISSUER_URL = 'oauth.issuerUrl', OAUTH_MOBILE_OVERRIDE_ENABLED = 'oauth.mobileOverrideEnabled', @@ -87,6 +88,7 @@ export enum SystemConfigKey { OAUTH_SCOPE = 'oauth.scope', OAUTH_SIGNING_ALGORITHM = 'oauth.signingAlgorithm', OAUTH_STORAGE_LABEL_CLAIM = 'oauth.storageLabelClaim', + OAUTH_STORAGE_QUOTA_CLAIM = 'oauth.storageQuotaClaim', PASSWORD_LOGIN_ENABLED = 'passwordLogin.enabled', @@ -227,6 +229,7 @@ export interface SystemConfig { buttonText: string; clientId: string; clientSecret: string; + defaultStorageQuota: number; enabled: boolean; issuerUrl: string; mobileOverrideEnabled: boolean; @@ -234,6 +237,7 @@ export interface SystemConfig { scope: string; signingAlgorithm: string; storageLabelClaim: string; + storageQuotaClaim: string; }; passwordLogin: { enabled: boolean; diff --git a/server/test/fixtures/system-config.stub.ts b/server/test/fixtures/system-config.stub.ts index 721bb181c..0e99fb07a 100644 --- a/server/test/fixtures/system-config.stub.ts +++ b/server/test/fixtures/system-config.stub.ts @@ -22,6 +22,11 @@ export const systemConfigStub: Record = { { key: SystemConfigKey.OAUTH_MOBILE_REDIRECT_URI, value: 'http://mobile-redirect' }, { key: SystemConfigKey.OAUTH_BUTTON_TEXT, value: 'OAuth' }, ], + withDefaultStorageQuota: [ + { key: SystemConfigKey.OAUTH_ENABLED, value: true }, + { key: SystemConfigKey.OAUTH_AUTO_REGISTER, value: true }, + { key: SystemConfigKey.OAUTH_DEFAULT_STORAGE_QUOTA, value: 1 }, + ], libraryWatchEnabled: [{ key: SystemConfigKey.LIBRARY_WATCH_ENABLED, value: true }], libraryWatchDisabled: [{ key: SystemConfigKey.LIBRARY_WATCH_ENABLED, value: false }], }; diff --git a/server/test/fixtures/user.stub.ts b/server/test/fixtures/user.stub.ts index e0d9113c6..e89e97bfd 100644 --- a/server/test/fixtures/user.stub.ts +++ b/server/test/fixtures/user.stub.ts @@ -23,6 +23,20 @@ export const userDto = { name: 'User with quota', quotaSizeInBytes: 42, }, + userWithDefaultStorageQuota: { + email: 'test@immich.com', + name: ' ', + oauthId: 'my-auth-user-sub', + quotaSizeInBytes: 1_073_741_824, + storageLabel: null, + }, + userWithStorageQuotaClaim: { + email: 'test@immich.com', + name: ' ', + oauthId: 'my-auth-user-sub', + quotaSizeInBytes: 5_368_709_120, + storageLabel: null, + }, }; export const userStub = { diff --git a/web/src/lib/components/admin-page/jobs/storage-migration-description.svelte b/web/src/lib/components/admin-page/jobs/storage-migration-description.svelte index 550a907c4..cc8ebddec 100644 --- a/web/src/lib/components/admin-page/jobs/storage-migration-description.svelte +++ b/web/src/lib/components/admin-page/jobs/storage-migration-description.svelte @@ -3,7 +3,7 @@ Apply the current -Storage template to previously uploaded assets diff --git a/web/src/lib/components/admin-page/settings/oauth/oauth-settings.svelte b/web/src/lib/components/admin-page/settings/oauth/oauth-settings.svelte index 10716728c..217017447 100644 --- a/web/src/lib/components/admin-page/settings/oauth/oauth-settings.svelte +++ b/web/src/lib/components/admin-page/settings/oauth/oauth-settings.svelte @@ -130,6 +130,26 @@ isEdited={!(config.oauth.storageLabelClaim == savedConfig.oauth.storageLabelClaim)} /> + + + +