From 7dc12dea1e3c7bec5429ab0824efd3accafec8cd Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Mon, 26 Dec 2022 10:35:52 -0500 Subject: [PATCH] feat(web,server): link/unlink oauth account (#1154) * feat(web,server): link/unlink oauth account * chore: linting * fix: broken oauth callback * fix: user core bugs * fix: tests * fix: use user response * chore: update docs * feat: prevent the same oauth account from being linked twice * chore: mock logger --- docs/docs/features/oauth.md | 33 +++- mobile/openapi/README.md | Bin 12067 -> 12219 bytes mobile/openapi/doc/OAuthApi.md | Bin 2311 -> 4092 bytes mobile/openapi/doc/UserResponseDto.md | Bin 706 -> 737 bytes mobile/openapi/lib/api/o_auth_api.dart | Bin 3715 -> 6681 bytes .../openapi/lib/model/user_response_dto.dart | Bin 5910 -> 6142 bytes mobile/openapi/test/o_auth_api_test.dart | Bin 748 -> 983 bytes .../openapi/test/user_response_dto_test.dart | Bin 1401 -> 1500 bytes .../immich/src/api-v1/auth/auth.service.ts | 4 +- .../src/api-v1/oauth/oauth.controller.ts | 20 +- .../src/api-v1/oauth/oauth.service.spec.ts | 88 +++++++-- .../immich/src/api-v1/oauth/oauth.service.ts | 33 +++- .../user/response-dto/user-response.dto.ts | 2 + .../apps/immich/src/api-v1/user/user.core.ts | 21 +-- .../src/api-v1/user/user.service.spec.ts | 19 ++ .../immich/src/api-v1/user/user.service.ts | 6 +- server/apps/immich/test/user.e2e-spec.ts | 2 + server/immich-openapi-specs.json | 58 +++++- .../libs/database/src/entities/user.entity.ts | 2 +- web/src/api/open-api/api.ts | 127 +++++++++++++ web/src/api/utils.ts | 27 +++ .../lib/components/forms/login-form.svelte | 11 +- .../change-password-settings.svelte | 78 ++++++++ .../user-settings-page/oauth-settings.svelte | 86 +++++++++ .../user-profile-settings.svelte | 81 +++++++++ .../user-settings-list.svelte | 171 +++--------------- web/src/lib/utils/handle-error.ts | 13 ++ 27 files changed, 680 insertions(+), 202 deletions(-) create mode 100644 web/src/lib/components/user-settings-page/change-password-settings.svelte create mode 100644 web/src/lib/components/user-settings-page/oauth-settings.svelte create mode 100644 web/src/lib/components/user-settings-page/user-profile-settings.svelte create mode 100644 web/src/lib/utils/handle-error.ts diff --git a/docs/docs/features/oauth.md b/docs/docs/features/oauth.md index c84cea471..e4c9ac927 100644 --- a/docs/docs/features/oauth.md +++ b/docs/docs/features/oauth.md @@ -26,14 +26,33 @@ Before enabling OAuth in Immich, a new client application needs to be configured The **Sign-in redirect URIs** should include: -- All URLs that will be used to access the login page of the Immich web client (eg. `http://localhost:2283/auth/login`, `http://192.168.0.200:2283/auth/login`, `https://immich.example.com/auth/login`) -- Mobile app redirect URL `app.immich:/` +- `app.immich:/` - for logging in with OAuth from the [Mobile App](/docs/features/mobile-app.mdx) +- `http://DOMAIN:PORT/auth/login` - for logging in with OAuth from the Web Client +- `http://DOMAIN:PORT/user-settings` - for manually linking OAuth in the Web Client -:::caution -You **MUST** include `app.immich:/` as the redirect URI for iOS and Android mobile app to work properly. +:::info Redirect URIs + +Redirect URIs should contain all the domains you will be using to access Immich. Some examples include: + +Mobile + +- `app.immich:/` (You **MUST** include this for iOS and Android mobile apps to work properly) + +Localhost + +- `http://localhost:2283/auth/login` +- `http://localhost:2283/user-settings` + +Local IP + +- `http://192.168.0.200:2283/auth/login` +- `http://192.168.0.200:2283/user-settings` + +Hostname + +- `https://immich.example.com/auth/login`) +- `https://immich.example.com/user-settings`) -**Authentik example** - ::: ## Enable OAuth @@ -42,7 +61,7 @@ Once you have a new OAuth client application configured, Immich can be configure | Setting | Type | Default | Description | | ------------- | ------- | -------------------- | ------------------------------------------------------------------------- | -| Enabled | boolean | false | Enable/disable OAuth | +| Enabled | boolean | false | Enable/disable OAuth | | Issuer URL | URL | (required) | Required. Self-discovery URL for client (from previous step) | | Client ID | string | (required) | Required. Client ID (from previous step) | | Client secret | string | (required) | Required. Client Secret (previous step) | diff --git a/mobile/openapi/README.md b/mobile/openapi/README.md index 49b02442005a40d6f287a70baf34b2ef66e8aeae..0a6b958f8fe20550b1f46526ac9462bc2411523d 100644 GIT binary patch delta 130 zcmZ1+w>y5rJ8>a@$I_Av$AU~Pg&KuuEv=l)yvYILVx~YkTN5axr4`^G9HOPApr4-z jRHzRU2MTj(q3J8lgX&ZOv#{!g$V@&cA-kDPLRK6A`@1Q@ delta 12 TcmdlTzc_BgJMqnJ5~kt+DSZWQ diff --git a/mobile/openapi/doc/OAuthApi.md b/mobile/openapi/doc/OAuthApi.md index 255f71db4fd94f7f6d55bc7cab3847930920e1d3..1ed0598830f3c1863d9ceba4a53d4c3c75e94116 100644 GIT binary patch delta 399 zcmZn{`Xj#~nXx`vODiWcFI!71R>R-1v?Rl^AX6_lMHwWfsZgV!r4`^G9HOPApr4-z zl+_1`1BJPO+Dh}VX@rPk(+ZIRY3AZm1{$24n3I#VIgfEBy8?&{F^tPjA+$KPC@8hK zAV04-)ukj~0VpxKkMTA$h%R7C0@F8{9*U!>1Ni_Xw0R?Y7^513D!q!=EOky)&enwL_LpP5&pkdmKTtdN&qqL7!Gnxc@HSE*2t zSd^HXT9R6%2lSD$vO-X5Noi4@LP=#oDitM)o{zKcGP=scD&csVS2KS#AIT D#6*U9 delta 12 Tcmew(-!8NvnQ`-V#wF|kBY_0} diff --git a/mobile/openapi/doc/UserResponseDto.md b/mobile/openapi/doc/UserResponseDto.md index 4d6fb1c669c4e0b6d393b4455a9327b6313d830d..f484c18ab0ec1a2deb724ff96f8c5ff2f3bd9dd7 100644 GIT binary patch delta 41 tcmX@a`jB;lER(F3R(@h>Nrq>NmX<<|f|gcrNl|8AI+&wSGg+ExIRFmc44MD{ delta 11 ScmaFJdWdy{EYoBgrlkNH69e-A diff --git a/mobile/openapi/lib/api/o_auth_api.dart b/mobile/openapi/lib/api/o_auth_api.dart index 42823338517da089f0d0ea9305d2ed15781f5d15..e61b047cc4f3257ac7ef03eb7c5d6010c3c4dc0f 100644 GIT binary patch delta 288 zcmZpcooTY6k#90LhbnVUX5Qqpj6GmFfvJ`q$j?@{=9>JULwxgpCP5~#(Bjmhpw!}m z{Ji2+my&#YI|Y!E$+=91lVe!LLHagVv3f8`p{Y~X(Nst*Rse}kzQ-QKTABy3Nn6FO zw4}5s)dps!odQ@YJ3O-_!=t36z%wr`Ut{tFCSOj7IMgwd|1&90p31pwvM<*;1*j2d dRsx*{(V{Wgk5yF|i?N$$vT71=sV-A37XYUlX0QMN delta 10 RcmbPf(k#27k&mgC3jh?N12zBv diff --git a/mobile/openapi/lib/model/user_response_dto.dart b/mobile/openapi/lib/model/user_response_dto.dart index 432ec9b1595efb653436930ef6462a22be271687..f00efadf27e3ac838880f73e4fddf3551958ec1d 100644 GIT binary patch delta 216 zcmbQH_fLOAJ=5f=i~{WWiKQhOo++EFnKYRAf=h}r^U@VyV)IybF)Avksc|WQKz>O^ zYLOmPPQlg|t}KErl969Q8>~tLrgu3zFQbqSOpUDyOneK68k3xYLVQ+neqMBuI#gIa wRsn9F_2#QwGg&!-7DAjhIfq}73#Lv}adVo$DmGCC1-MonpvAS;T(w+W0DbsFrT_o{ delta 49 zcmV-10M7sZFP1K_e*&|b0we;n(FC;tv!@1G0kath0|B!x2_^!wcMFsSvqur41_pgQ H3VjL+wcHO> diff --git a/mobile/openapi/test/o_auth_api_test.dart b/mobile/openapi/test/o_auth_api_test.dart index 01103d63ce584ee6584589f388515a874e7a54ac..5940bb6f18e9a10786016edd4369d12fe96d1d78 100644 GIT binary patch delta 107 zcmaFEdYyg4Gp5PjOxz-&#i>O>sl^5PdBv$NCHZy=IhlErA2C|70O{<>`3@NE&wcR1mFMw diff --git a/mobile/openapi/test/user_response_dto_test.dart b/mobile/openapi/test/user_response_dto_test.dart index 9b9ef3253bea6003958b9726e11d2d342ed49aa8..f1dda093cfc0e96cd8d19efb2d1a3ae09979149a 100644 GIT binary patch delta 39 ncmey#b%%RHKI>#ZWmknH1sd2P|rn^I5q7B@Ye` delta 18 Zcmcb^{gZ1$J}VcOf { diff --git a/server/apps/immich/src/api-v1/oauth/oauth.controller.ts b/server/apps/immich/src/api-v1/oauth/oauth.controller.ts index 85cfa2ecd..bd631e195 100644 --- a/server/apps/immich/src/api-v1/oauth/oauth.controller.ts +++ b/server/apps/immich/src/api-v1/oauth/oauth.controller.ts @@ -3,8 +3,10 @@ import { ApiTags } from '@nestjs/swagger'; import { Response } from 'express'; import { AuthType } from '../../constants/jwt.constant'; import { AuthUserDto, GetAuthUser } from '../../decorators/auth-user.decorator'; +import { Authenticated } from '../../decorators/authenticated.decorator'; import { ImmichJwtService } from '../../modules/immich-jwt/immich-jwt.service'; import { LoginResponseDto } from '../auth/response-dto/login-response.dto'; +import { UserResponseDto } from '../user/response-dto/user-response.dto'; import { OAuthCallbackDto } from './dto/oauth-auth-code.dto'; import { OAuthConfigDto } from './dto/oauth-config.dto'; import { OAuthService } from './oauth.service'; @@ -22,12 +24,26 @@ export class OAuthController { @Post('/callback') public async callback( - @GetAuthUser() authUser: AuthUserDto, @Res({ passthrough: true }) response: Response, @Body(ValidationPipe) dto: OAuthCallbackDto, ): Promise { - const loginResponse = await this.oauthService.callback(authUser, dto); + const loginResponse = await this.oauthService.login(dto); response.setHeader('Set-Cookie', this.immichJwtService.getCookies(loginResponse, AuthType.OAUTH)); return loginResponse; } + + @Authenticated() + @Post('link') + public async link( + @GetAuthUser() authUser: AuthUserDto, + @Body(ValidationPipe) dto: OAuthCallbackDto, + ): Promise { + return this.oauthService.link(authUser, dto); + } + + @Authenticated() + @Post('unlink') + public async unlink(@GetAuthUser() authUser: AuthUserDto): Promise { + return this.oauthService.unlink(authUser); + } } diff --git a/server/apps/immich/src/api-v1/oauth/oauth.service.spec.ts b/server/apps/immich/src/api-v1/oauth/oauth.service.spec.ts index 6018ea98e..29af0fdd9 100644 --- a/server/apps/immich/src/api-v1/oauth/oauth.service.spec.ts +++ b/server/apps/immich/src/api-v1/oauth/oauth.service.spec.ts @@ -1,7 +1,7 @@ import { SystemConfig } from '@app/database/entities/system-config.entity'; import { UserEntity } from '@app/database/entities/user.entity'; import { ImmichConfigService } from '@app/immich-config'; -import { BadRequestException } from '@nestjs/common'; +import { BadRequestException, Logger } from '@nestjs/common'; import { generators, Issuer } from 'openid-client'; import { AuthUserDto } from '../../decorators/auth-user.decorator'; import { ImmichJwtService } from '../../modules/immich-jwt/immich-jwt.service'; @@ -32,6 +32,21 @@ const loginResponse = { userEmail: 'user@immich.com,', } as LoginResponseDto; +jest.mock('@nestjs/common', () => { + return { + ...jest.requireActual('@nestjs/common'), + Logger: function MockLogger() { + Object.assign(this as Logger, { + verbose: jest.fn(), + debug: jest.fn(), + log: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }); + }, + }; +}); + describe('OAuthService', () => { let sut: OAuthService; let userRepositoryMock: jest.Mocked; @@ -109,9 +124,9 @@ describe('OAuthService', () => { }); }); - describe('callback', () => { + describe('login', () => { it('should throw an error if OAuth is not enabled', async () => { - await expect(sut.callback(authUser, { url: '' })).rejects.toBeInstanceOf(BadRequestException); + await expect(sut.login({ url: '' })).rejects.toBeInstanceOf(BadRequestException); }); it('should not allow auto registering', async () => { @@ -122,10 +137,8 @@ describe('OAuthService', () => { }, } as SystemConfig); sut = new OAuthService(immichJwtServiceMock, immichConfigServiceMock, userRepositoryMock); - jest.spyOn(sut['logger'], 'debug').mockImplementation(() => null); - jest.spyOn(sut['logger'], 'warn').mockImplementation(() => null); userRepositoryMock.getByEmail.mockResolvedValue(null); - await expect(sut.callback(authUser, { url: 'http://immich/auth/login?code=abc123' })).rejects.toBeInstanceOf( + await expect(sut.login({ url: 'http://immich/auth/login?code=abc123' })).rejects.toBeInstanceOf( BadRequestException, ); expect(userRepositoryMock.getByEmail).toHaveBeenCalledTimes(1); @@ -139,15 +152,11 @@ describe('OAuthService', () => { }, } as SystemConfig); sut = new OAuthService(immichJwtServiceMock, immichConfigServiceMock, userRepositoryMock); - jest.spyOn(sut['logger'], 'debug').mockImplementation(() => null); - jest.spyOn(sut['logger'], 'warn').mockImplementation(() => null); userRepositoryMock.getByEmail.mockResolvedValue(user); userRepositoryMock.update.mockResolvedValue(user); immichJwtServiceMock.createLoginResponse.mockResolvedValue(loginResponse); - await expect(sut.callback(authUser, { url: 'http://immich/auth/login?code=abc123' })).resolves.toEqual( - loginResponse, - ); + await expect(sut.login({ url: 'http://immich/auth/login?code=abc123' })).resolves.toEqual(loginResponse); expect(userRepositoryMock.getByEmail).toHaveBeenCalledTimes(1); expect(userRepositoryMock.update).toHaveBeenCalledWith(user.id, { oauthId: sub }); @@ -161,16 +170,12 @@ describe('OAuthService', () => { }, } as SystemConfig); sut = new OAuthService(immichJwtServiceMock, immichConfigServiceMock, userRepositoryMock); - jest.spyOn(sut['logger'], 'debug').mockImplementation(() => null); - jest.spyOn(sut['logger'], 'log').mockImplementation(() => null); userRepositoryMock.getByEmail.mockResolvedValue(null); userRepositoryMock.getAdmin.mockResolvedValue(user); userRepositoryMock.create.mockResolvedValue(user); immichJwtServiceMock.createLoginResponse.mockResolvedValue(loginResponse); - await expect(sut.callback(authUser, { url: 'http://immich/auth/login?code=abc123' })).resolves.toEqual( - loginResponse, - ); + await expect(sut.login({ url: 'http://immich/auth/login?code=abc123' })).resolves.toEqual(loginResponse); expect(userRepositoryMock.getByEmail).toHaveBeenCalledTimes(2); // second call is for domain check before create expect(userRepositoryMock.create).toHaveBeenCalledTimes(1); @@ -178,6 +183,57 @@ describe('OAuthService', () => { }); }); + describe('link', () => { + it('should link an account', async () => { + immichConfigServiceMock.getConfig.mockResolvedValue({ + oauth: { + enabled: true, + autoRegister: true, + }, + } as SystemConfig); + + userRepositoryMock.update.mockResolvedValue(user); + + await sut.link(authUser, { url: 'http://immich/user-settings?code=abc123' }); + + expect(userRepositoryMock.update).toHaveBeenCalledWith(authUser.id, { oauthId: sub }); + }); + + it('should not link an already linked oauth.sub', async () => { + immichConfigServiceMock.getConfig.mockResolvedValue({ + oauth: { + enabled: true, + autoRegister: true, + }, + } as SystemConfig); + + userRepositoryMock.getByOAuthId.mockResolvedValue({ id: 'other-user' } as UserEntity); + + await expect(sut.link(authUser, { url: 'http://immich/user-settings?code=abc123' })).rejects.toBeInstanceOf( + BadRequestException, + ); + + expect(userRepositoryMock.update).not.toHaveBeenCalled(); + }); + }); + + describe('unlink', () => { + it('should unlink an account', async () => { + immichConfigServiceMock.getConfig.mockResolvedValue({ + oauth: { + enabled: true, + autoRegister: true, + }, + } as SystemConfig); + + userRepositoryMock.update.mockResolvedValue(user); + + await sut.unlink(authUser); + + expect(userRepositoryMock.update).toHaveBeenCalledWith(authUser.id, { oauthId: '' }); + }); + }); + describe('getLogoutEndpoint', () => { it('should return null if OAuth is not configured', async () => { await expect(sut.getLogoutEndpoint()).resolves.toBeNull(); diff --git a/server/apps/immich/src/api-v1/oauth/oauth.service.ts b/server/apps/immich/src/api-v1/oauth/oauth.service.ts index ce7b10927..4b3663a54 100644 --- a/server/apps/immich/src/api-v1/oauth/oauth.service.ts +++ b/server/apps/immich/src/api-v1/oauth/oauth.service.ts @@ -4,6 +4,7 @@ import { ClientMetadata, custom, generators, Issuer, UserinfoResponse } from 'op import { AuthUserDto } from '../../decorators/auth-user.decorator'; import { ImmichJwtService } from '../../modules/immich-jwt/immich-jwt.service'; import { LoginResponseDto } from '../auth/response-dto/login-response.dto'; +import { UserResponseDto } from '../user/response-dto/user-response.dto'; import { IUserRepository, USER_REPOSITORY } from '../user/user-repository'; import { UserCore } from '../user/user.core'; import { OAuthCallbackDto } from './dto/oauth-auth-code.dto'; @@ -47,12 +48,8 @@ export class OAuthService { return { enabled: true, buttonText, url }; } - public async callback(authUser: AuthUserDto, dto: OAuthCallbackDto): Promise { - const redirectUri = dto.url.split('?')[0]; - const client = await this.getClient(); - const params = client.callbackParams(dto.url); - const tokens = await client.callback(redirectUri, params, { state: params.state }); - const profile = await client.userinfo(tokens.access_token || ''); + public async login(dto: OAuthCallbackDto): Promise { + const profile = await this.callback(dto.url); this.logger.debug(`Logging in with OAuth: ${JSON.stringify(profile)}`); let user = await this.userCore.getByOAuthId(profile.sub); @@ -61,7 +58,7 @@ export class OAuthService { if (!user) { const emailUser = await this.userCore.getByEmail(profile.email); if (emailUser) { - user = await this.userCore.updateUser(authUser, emailUser, { oauthId: profile.sub }); + user = await this.userCore.updateUser(emailUser, emailUser.id, { oauthId: profile.sub }); } } @@ -88,6 +85,20 @@ export class OAuthService { return this.immichJwtService.createLoginResponse(user); } + public async link(user: AuthUserDto, dto: OAuthCallbackDto): Promise { + const { sub: oauthId } = await this.callback(dto.url); + const duplicate = await this.userCore.getByOAuthId(oauthId); + if (duplicate && duplicate.id !== user.id) { + this.logger.warn(`OAuth link account failed: sub is already linked to another user (${duplicate.email}).`); + throw new BadRequestException('This OAuth account has already been linked to another user.'); + } + return this.userCore.updateUser(user, user.id, { oauthId }); + } + + public async unlink(user: AuthUserDto): Promise { + return this.userCore.updateUser(user, user.id, { oauthId: '' }); + } + public async getLogoutEndpoint(): Promise { const config = await this.immichConfigService.getConfig(); const { enabled } = config.oauth; @@ -98,6 +109,14 @@ export class OAuthService { return (await this.getClient()).issuer.metadata.end_session_endpoint || null; } + private async callback(url: string): Promise { + const redirectUri = url.split('?')[0]; + const client = await this.getClient(); + const params = client.callbackParams(url); + const tokens = await client.callback(redirectUri, params, { state: params.state }); + return await client.userinfo(tokens.access_token || ''); + } + private async getClient() { const config = await this.immichConfigService.getConfig(); const { enabled, clientId, clientSecret, issuerUrl } = config.oauth; diff --git a/server/apps/immich/src/api-v1/user/response-dto/user-response.dto.ts b/server/apps/immich/src/api-v1/user/response-dto/user-response.dto.ts index 145b06841..502387b6a 100644 --- a/server/apps/immich/src/api-v1/user/response-dto/user-response.dto.ts +++ b/server/apps/immich/src/api-v1/user/response-dto/user-response.dto.ts @@ -10,6 +10,7 @@ export class UserResponseDto { shouldChangePassword!: boolean; isAdmin!: boolean; deletedAt?: Date; + oauthId!: string; } export function mapUser(entity: UserEntity): UserResponseDto { @@ -23,5 +24,6 @@ export function mapUser(entity: UserEntity): UserResponseDto { shouldChangePassword: entity.shouldChangePassword, isAdmin: entity.isAdmin, deletedAt: entity.deletedAt, + oauthId: entity.oauthId, }; } diff --git a/server/apps/immich/src/api-v1/user/user.core.ts b/server/apps/immich/src/api-v1/user/user.core.ts index 464c75315..27aa3339e 100644 --- a/server/apps/immich/src/api-v1/user/user.core.ts +++ b/server/apps/immich/src/api-v1/user/user.core.ts @@ -25,27 +25,22 @@ export class UserCore { return hash(password, salt); } - async updateUser(authUser: AuthUserDto, userToUpdate: UserEntity, data: Partial): Promise { - if (!authUser.isAdmin && (authUser.id !== userToUpdate.id || userToUpdate.id != data.id)) { + async updateUser(authUser: AuthUserDto, id: string, dto: Partial): Promise { + if (!(authUser.isAdmin || authUser.id === id)) { throw new ForbiddenException('You are not allowed to update this user'); } - // TODO: can this happen? If so we should implement a test case, otherwise remove it (also from DTO) - if (userToUpdate.isAdmin) { - const adminUser = await this.userRepository.getAdmin(); - if (adminUser && adminUser.id !== userToUpdate.id) { - throw new BadRequestException('Admin user exists'); - } + if (dto.isAdmin && authUser.isAdmin && authUser.id !== id) { + throw new BadRequestException('Admin user exists'); } try { - const payload: Partial = { ...data }; - if (payload.password) { + if (dto.password) { const salt = await this.generateSalt(); - payload.salt = salt; - payload.password = await this.hashPassword(payload.password, salt); + dto.salt = salt; + dto.password = await this.hashPassword(dto.password, salt); } - return this.userRepository.update(userToUpdate.id, payload); + return this.userRepository.update(id, dto); } catch (e) { Logger.error(e, 'Failed to update user info'); throw new InternalServerErrorException('Failed to update user info'); diff --git a/server/apps/immich/src/api-v1/user/user.service.spec.ts b/server/apps/immich/src/api-v1/user/user.service.spec.ts index 7306acace..f50febdf9 100644 --- a/server/apps/immich/src/api-v1/user/user.service.spec.ts +++ b/server/apps/immich/src/api-v1/user/user.service.spec.ts @@ -141,6 +141,25 @@ describe('UserService', () => { }); describe('Create user', () => { + it('should let the admin update himself', async () => { + const dto = { id: adminUser.id, shouldChangePassword: true, isAdmin: true }; + + when(userRepositoryMock.get).calledWith(adminUser.id).mockResolvedValueOnce(null); + when(userRepositoryMock.update).calledWith(adminUser.id, dto).mockResolvedValueOnce(adminUser); + + await sut.updateUser(adminUser, dto); + + expect(userRepositoryMock.update).toHaveBeenCalledWith(adminUser.id, dto); + }); + + it('should not let the another user become an admin', async () => { + const dto = { id: immichUser.id, shouldChangePassword: true, isAdmin: true }; + + when(userRepositoryMock.get).calledWith(immichUser.id).mockResolvedValueOnce(immichUser); + + await expect(sut.updateUser(adminUser, dto)).rejects.toBeInstanceOf(BadRequestException); + }); + it('should not create a user if there is no local admin account', async () => { when(userRepositoryMock.getAdmin).calledWith().mockResolvedValueOnce(null); diff --git a/server/apps/immich/src/api-v1/user/user.service.ts b/server/apps/immich/src/api-v1/user/user.service.ts index f3f3c9e75..559c1e42c 100644 --- a/server/apps/immich/src/api-v1/user/user.service.ts +++ b/server/apps/immich/src/api-v1/user/user.service.ts @@ -65,12 +65,12 @@ export class UserService { return mapUser(createdUser); } - async updateUser(authUser: AuthUserDto, updateUserDto: UpdateUserDto): Promise { - const user = await this.userCore.get(updateUserDto.id); + async updateUser(authUser: AuthUserDto, dto: UpdateUserDto): Promise { + const user = await this.userCore.get(dto.id); if (!user) { throw new NotFoundException('User not found'); } - const updatedUser = await this.userCore.updateUser(authUser, user, updateUserDto); + const updatedUser = await this.userCore.updateUser(authUser, dto.id, dto); return mapUser(updatedUser); } diff --git a/server/apps/immich/test/user.e2e-spec.ts b/server/apps/immich/test/user.e2e-spec.ts index 00c1e5b5f..5342897e6 100644 --- a/server/apps/immich/test/user.e2e-spec.ts +++ b/server/apps/immich/test/user.e2e-spec.ts @@ -107,6 +107,7 @@ describe('User', () => { shouldChangePassword: true, profileImagePath: '', deletedAt: null, + oauthId: '', }, { email: userTwoEmail, @@ -118,6 +119,7 @@ describe('User', () => { shouldChangePassword: true, profileImagePath: '', deletedAt: null, + oauthId: '', }, ]), ); diff --git a/server/immich-openapi-specs.json b/server/immich-openapi-specs.json index d01a7d1a7..bb13cf0c0 100644 --- a/server/immich-openapi-specs.json +++ b/server/immich-openapi-specs.json @@ -1826,6 +1826,58 @@ ] } }, + "/oauth/link": { + "post": { + "operationId": "link", + "parameters": [], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/OAuthCallbackDto" + } + } + } + }, + "responses": { + "201": { + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UserResponseDto" + } + } + } + } + }, + "tags": [ + "OAuth" + ] + } + }, + "/oauth/unlink": { + "post": { + "operationId": "unlink", + "parameters": [], + "responses": { + "201": { + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UserResponseDto" + } + } + } + } + }, + "tags": [ + "OAuth" + ] + } + }, "/device-info": { "post": { "operationId": "createDeviceInfo", @@ -2287,6 +2339,9 @@ "deletedAt": { "format": "date-time", "type": "string" + }, + "oauthId": { + "type": "string" } }, "required": [ @@ -2297,7 +2352,8 @@ "createdAt", "profileImagePath", "shouldChangePassword", - "isAdmin" + "isAdmin", + "oauthId" ] }, "CreateUserDto": { diff --git a/server/libs/database/src/entities/user.entity.ts b/server/libs/database/src/entities/user.entity.ts index 1d0a659bd..024d6ccda 100644 --- a/server/libs/database/src/entities/user.entity.ts +++ b/server/libs/database/src/entities/user.entity.ts @@ -24,7 +24,7 @@ export class UserEntity { @Column({ default: '', select: false }) salt?: string; - @Column({ default: '', select: false }) + @Column({ default: '' }) oauthId!: string; @Column({ default: '' }) diff --git a/web/src/api/open-api/api.ts b/web/src/api/open-api/api.ts index 38b5be8cd..4fed33138 100644 --- a/web/src/api/open-api/api.ts +++ b/web/src/api/open-api/api.ts @@ -1951,6 +1951,12 @@ export interface UserResponseDto { * @memberof UserResponseDto */ 'deletedAt'?: string; + /** + * + * @type {string} + * @memberof UserResponseDto + */ + 'oauthId': string; } /** * @@ -5059,6 +5065,70 @@ export const OAuthApiAxiosParamCreator = function (configuration?: Configuration localVarRequestOptions.headers = {...localVarHeaderParameter, ...headersFromBaseOptions, ...options.headers}; localVarRequestOptions.data = serializeDataIfNeeded(oAuthConfigDto, localVarRequestOptions, configuration) + return { + url: toPathString(localVarUrlObj), + options: localVarRequestOptions, + }; + }, + /** + * + * @param {OAuthCallbackDto} oAuthCallbackDto + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + link: async (oAuthCallbackDto: OAuthCallbackDto, options: AxiosRequestConfig = {}): Promise => { + // verify required parameter 'oAuthCallbackDto' is not null or undefined + assertParamExists('link', 'oAuthCallbackDto', oAuthCallbackDto) + const localVarPath = `/oauth/link`; + // use dummy base URL string because the URL constructor only accepts absolute URLs. + const localVarUrlObj = new URL(localVarPath, DUMMY_BASE_URL); + let baseOptions; + if (configuration) { + baseOptions = configuration.baseOptions; + } + + const localVarRequestOptions = { method: 'POST', ...baseOptions, ...options}; + const localVarHeaderParameter = {} as any; + const localVarQueryParameter = {} as any; + + + + localVarHeaderParameter['Content-Type'] = 'application/json'; + + setSearchParams(localVarUrlObj, localVarQueryParameter); + let headersFromBaseOptions = baseOptions && baseOptions.headers ? baseOptions.headers : {}; + localVarRequestOptions.headers = {...localVarHeaderParameter, ...headersFromBaseOptions, ...options.headers}; + localVarRequestOptions.data = serializeDataIfNeeded(oAuthCallbackDto, localVarRequestOptions, configuration) + + return { + url: toPathString(localVarUrlObj), + options: localVarRequestOptions, + }; + }, + /** + * + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + unlink: async (options: AxiosRequestConfig = {}): Promise => { + const localVarPath = `/oauth/unlink`; + // use dummy base URL string because the URL constructor only accepts absolute URLs. + const localVarUrlObj = new URL(localVarPath, DUMMY_BASE_URL); + let baseOptions; + if (configuration) { + baseOptions = configuration.baseOptions; + } + + const localVarRequestOptions = { method: 'POST', ...baseOptions, ...options}; + const localVarHeaderParameter = {} as any; + const localVarQueryParameter = {} as any; + + + + setSearchParams(localVarUrlObj, localVarQueryParameter); + let headersFromBaseOptions = baseOptions && baseOptions.headers ? baseOptions.headers : {}; + localVarRequestOptions.headers = {...localVarHeaderParameter, ...headersFromBaseOptions, ...options.headers}; + return { url: toPathString(localVarUrlObj), options: localVarRequestOptions, @@ -5094,6 +5164,25 @@ export const OAuthApiFp = function(configuration?: Configuration) { const localVarAxiosArgs = await localVarAxiosParamCreator.generateConfig(oAuthConfigDto, options); return createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration); }, + /** + * + * @param {OAuthCallbackDto} oAuthCallbackDto + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + async link(oAuthCallbackDto: OAuthCallbackDto, options?: AxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise> { + const localVarAxiosArgs = await localVarAxiosParamCreator.link(oAuthCallbackDto, options); + return createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration); + }, + /** + * + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + async unlink(options?: AxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise> { + const localVarAxiosArgs = await localVarAxiosParamCreator.unlink(options); + return createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration); + }, } }; @@ -5122,6 +5211,23 @@ export const OAuthApiFactory = function (configuration?: Configuration, basePath generateConfig(oAuthConfigDto: OAuthConfigDto, options?: any): AxiosPromise { return localVarFp.generateConfig(oAuthConfigDto, options).then((request) => request(axios, basePath)); }, + /** + * + * @param {OAuthCallbackDto} oAuthCallbackDto + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + link(oAuthCallbackDto: OAuthCallbackDto, options?: any): AxiosPromise { + return localVarFp.link(oAuthCallbackDto, options).then((request) => request(axios, basePath)); + }, + /** + * + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + unlink(options?: any): AxiosPromise { + return localVarFp.unlink(options).then((request) => request(axios, basePath)); + }, }; }; @@ -5153,6 +5259,27 @@ export class OAuthApi extends BaseAPI { public generateConfig(oAuthConfigDto: OAuthConfigDto, options?: AxiosRequestConfig) { return OAuthApiFp(this.configuration).generateConfig(oAuthConfigDto, options).then((request) => request(this.axios, this.basePath)); } + + /** + * + * @param {OAuthCallbackDto} oAuthCallbackDto + * @param {*} [options] Override http request option. + * @throws {RequiredError} + * @memberof OAuthApi + */ + public link(oAuthCallbackDto: OAuthCallbackDto, options?: AxiosRequestConfig) { + return OAuthApiFp(this.configuration).link(oAuthCallbackDto, options).then((request) => request(this.axios, this.basePath)); + } + + /** + * + * @param {*} [options] Override http request option. + * @throws {RequiredError} + * @memberof OAuthApi + */ + public unlink(options?: AxiosRequestConfig) { + return OAuthApiFp(this.configuration).unlink(options).then((request) => request(this.axios, this.basePath)); + } } diff --git a/web/src/api/utils.ts b/web/src/api/utils.ts index ae08ad1ef..d2381c9e9 100644 --- a/web/src/api/utils.ts +++ b/web/src/api/utils.ts @@ -1,3 +1,7 @@ +import { AxiosError, AxiosPromise } from 'axios'; +import { api } from './api'; +import { UserResponseDto } from './open-api'; + const _basePath = '/api'; export function getFileUrl(assetId: string, isThumb?: boolean, isWeb?: boolean) { @@ -9,3 +13,26 @@ export function getFileUrl(assetId: string, isThumb?: boolean, isWeb?: boolean) return urlObj.href; } + +export type ApiError = AxiosError<{ message: string }>; + +export const oauth = { + isCallback: (location: Location) => { + const search = location.search; + return search.includes('code=') || search.includes('error='); + }, + getConfig: (location: Location) => { + const redirectUri = location.href.split('?')[0]; + console.log(`OAuth Redirect URI: ${redirectUri}`); + return api.oauthApi.generateConfig({ redirectUri }); + }, + login: (location: Location) => { + return api.oauthApi.callback({ url: location.href }); + }, + link: (location: Location): AxiosPromise => { + return api.oauthApi.link({ url: location.href }); + }, + unlink: () => { + return api.oauthApi.unlink(); + } +}; diff --git a/web/src/lib/components/forms/login-form.svelte b/web/src/lib/components/forms/login-form.svelte index 7ba30cb2f..e1ada7de9 100644 --- a/web/src/lib/components/forms/login-form.svelte +++ b/web/src/lib/components/forms/login-form.svelte @@ -1,7 +1,7 @@ + +
+
+
+
+ + + + + + +
+ +
+
+
+
+
diff --git a/web/src/lib/components/user-settings-page/oauth-settings.svelte b/web/src/lib/components/user-settings-page/oauth-settings.svelte new file mode 100644 index 000000000..e779a5153 --- /dev/null +++ b/web/src/lib/components/user-settings-page/oauth-settings.svelte @@ -0,0 +1,86 @@ + + +
+
+
+ {#if loading} +
+ +
+ {:else if config.enabled} + {#if user.oauthId} + + {:else} + + + + {/if} + {/if} +
+
+
diff --git a/web/src/lib/components/user-settings-page/user-profile-settings.svelte b/web/src/lib/components/user-settings-page/user-profile-settings.svelte new file mode 100644 index 000000000..0f75a687e --- /dev/null +++ b/web/src/lib/components/user-settings-page/user-profile-settings.svelte @@ -0,0 +1,81 @@ + + +
+
+
+
+ + + + + + + + +
+ +
+
+
+
+
diff --git a/web/src/lib/components/user-settings-page/user-settings-list.svelte b/web/src/lib/components/user-settings-page/user-settings-list.svelte index 83aab28d7..d89eb3843 100644 --- a/web/src/lib/components/user-settings-page/user-settings-list.svelte +++ b/web/src/lib/components/user-settings-page/user-settings-list.svelte @@ -1,156 +1,43 @@ -
-
-
-
- - - - - - - - -
- -
-
-
-
-
+ -
-
-
-
- - - - - - -
- -
-
-
-
-
+ + +{#if oauthEnabled} + + + +{/if} diff --git a/web/src/lib/utils/handle-error.ts b/web/src/lib/utils/handle-error.ts new file mode 100644 index 000000000..786719759 --- /dev/null +++ b/web/src/lib/utils/handle-error.ts @@ -0,0 +1,13 @@ +import { ApiError } from '../../api'; +import { + notificationController, + NotificationType +} from '../components/shared-components/notification/notification'; + +export function handleError(error: unknown, message: string) { + console.error(`[handleError]: ${message}`, error); + notificationController.show({ + message: (error as ApiError)?.response?.data?.message || message, + type: NotificationType.Error + }); +}