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 49b024420..0a6b958f8 100644 --- a/mobile/openapi/README.md +++ b/mobile/openapi/README.md @@ -108,6 +108,8 @@ Class | Method | HTTP request | Description *JobApi* | [**sendJobCommand**](doc//JobApi.md#sendjobcommand) | **PUT** /jobs/{jobId} | *OAuthApi* | [**callback**](doc//OAuthApi.md#callback) | **POST** /oauth/callback | *OAuthApi* | [**generateConfig**](doc//OAuthApi.md#generateconfig) | **POST** /oauth/config | +*OAuthApi* | [**link**](doc//OAuthApi.md#link) | **POST** /oauth/link | +*OAuthApi* | [**unlink**](doc//OAuthApi.md#unlink) | **POST** /oauth/unlink | *ServerInfoApi* | [**getServerInfo**](doc//ServerInfoApi.md#getserverinfo) | **GET** /server-info | *ServerInfoApi* | [**getServerVersion**](doc//ServerInfoApi.md#getserverversion) | **GET** /server-info/version | *ServerInfoApi* | [**getStats**](doc//ServerInfoApi.md#getstats) | **GET** /server-info/stats | diff --git a/mobile/openapi/doc/OAuthApi.md b/mobile/openapi/doc/OAuthApi.md index 255f71db4..1ed059883 100644 --- a/mobile/openapi/doc/OAuthApi.md +++ b/mobile/openapi/doc/OAuthApi.md @@ -11,6 +11,8 @@ Method | HTTP request | Description ------------- | ------------- | ------------- [**callback**](OAuthApi.md#callback) | **POST** /oauth/callback | [**generateConfig**](OAuthApi.md#generateconfig) | **POST** /oauth/config | +[**link**](OAuthApi.md#link) | **POST** /oauth/link | +[**unlink**](OAuthApi.md#unlink) | **POST** /oauth/unlink | # **callback** @@ -95,3 +97,81 @@ No authorization required [[Back to top]](#) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to Model list]](../README.md#documentation-for-models) [[Back to README]](../README.md) +# **link** +> UserResponseDto link(oAuthCallbackDto) + + + +### Example +```dart +import 'package:openapi/api.dart'; + +final api_instance = OAuthApi(); +final oAuthCallbackDto = OAuthCallbackDto(); // OAuthCallbackDto | + +try { + final result = api_instance.link(oAuthCallbackDto); + print(result); +} catch (e) { + print('Exception when calling OAuthApi->link: $e\n'); +} +``` + +### Parameters + +Name | Type | Description | Notes +------------- | ------------- | ------------- | ------------- + **oAuthCallbackDto** | [**OAuthCallbackDto**](OAuthCallbackDto.md)| | + +### Return type + +[**UserResponseDto**](UserResponseDto.md) + +### Authorization + +No authorization required + +### HTTP request headers + + - **Content-Type**: application/json + - **Accept**: application/json + +[[Back to top]](#) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to Model list]](../README.md#documentation-for-models) [[Back to README]](../README.md) + +# **unlink** +> UserResponseDto unlink() + + + +### Example +```dart +import 'package:openapi/api.dart'; + +final api_instance = OAuthApi(); + +try { + final result = api_instance.unlink(); + print(result); +} catch (e) { + print('Exception when calling OAuthApi->unlink: $e\n'); +} +``` + +### Parameters +This endpoint does not need any parameter. + +### Return type + +[**UserResponseDto**](UserResponseDto.md) + +### Authorization + +No authorization required + +### HTTP request headers + + - **Content-Type**: Not defined + - **Accept**: application/json + +[[Back to top]](#) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to Model list]](../README.md#documentation-for-models) [[Back to README]](../README.md) + diff --git a/mobile/openapi/doc/UserResponseDto.md b/mobile/openapi/doc/UserResponseDto.md index 4d6fb1c66..f484c18ab 100644 --- a/mobile/openapi/doc/UserResponseDto.md +++ b/mobile/openapi/doc/UserResponseDto.md @@ -17,6 +17,7 @@ Name | Type | Description | Notes **shouldChangePassword** | **bool** | | **isAdmin** | **bool** | | **deletedAt** | [**DateTime**](DateTime.md) | | [optional] +**oauthId** | **String** | | [[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md) diff --git a/mobile/openapi/lib/api/o_auth_api.dart b/mobile/openapi/lib/api/o_auth_api.dart index 428233385..e61b047cc 100644 --- a/mobile/openapi/lib/api/o_auth_api.dart +++ b/mobile/openapi/lib/api/o_auth_api.dart @@ -109,4 +109,92 @@ class OAuthApi { } return null; } + + /// Performs an HTTP 'POST /oauth/link' operation and returns the [Response]. + /// Parameters: + /// + /// * [OAuthCallbackDto] oAuthCallbackDto (required): + Future linkWithHttpInfo(OAuthCallbackDto oAuthCallbackDto,) async { + // ignore: prefer_const_declarations + final path = r'/oauth/link'; + + // ignore: prefer_final_locals + Object? postBody = oAuthCallbackDto; + + final queryParams = []; + final headerParams = {}; + final formParams = {}; + + const contentTypes = ['application/json']; + + + return apiClient.invokeAPI( + path, + 'POST', + queryParams, + postBody, + headerParams, + formParams, + contentTypes.isEmpty ? null : contentTypes.first, + ); + } + + /// Parameters: + /// + /// * [OAuthCallbackDto] oAuthCallbackDto (required): + Future link(OAuthCallbackDto oAuthCallbackDto,) async { + final response = await linkWithHttpInfo(oAuthCallbackDto,); + if (response.statusCode >= HttpStatus.badRequest) { + throw ApiException(response.statusCode, await _decodeBodyBytes(response)); + } + // When a remote server returns no body with a status of 204, we shall not decode it. + // At the time of writing this, `dart:convert` will throw an "Unexpected end of input" + // FormatException when trying to decode an empty string. + if (response.body.isNotEmpty && response.statusCode != HttpStatus.noContent) { + return await apiClient.deserializeAsync(await _decodeBodyBytes(response), 'UserResponseDto',) as UserResponseDto; + + } + return null; + } + + /// Performs an HTTP 'POST /oauth/unlink' operation and returns the [Response]. + Future unlinkWithHttpInfo() async { + // ignore: prefer_const_declarations + final path = r'/oauth/unlink'; + + // ignore: prefer_final_locals + Object? postBody; + + final queryParams = []; + final headerParams = {}; + final formParams = {}; + + const contentTypes = []; + + + return apiClient.invokeAPI( + path, + 'POST', + queryParams, + postBody, + headerParams, + formParams, + contentTypes.isEmpty ? null : contentTypes.first, + ); + } + + Future unlink() async { + final response = await unlinkWithHttpInfo(); + if (response.statusCode >= HttpStatus.badRequest) { + throw ApiException(response.statusCode, await _decodeBodyBytes(response)); + } + // When a remote server returns no body with a status of 204, we shall not decode it. + // At the time of writing this, `dart:convert` will throw an "Unexpected end of input" + // FormatException when trying to decode an empty string. + if (response.body.isNotEmpty && response.statusCode != HttpStatus.noContent) { + return await apiClient.deserializeAsync(await _decodeBodyBytes(response), 'UserResponseDto',) as UserResponseDto; + + } + return null; + } } diff --git a/mobile/openapi/lib/model/user_response_dto.dart b/mobile/openapi/lib/model/user_response_dto.dart index 432ec9b15..f00efadf2 100644 --- a/mobile/openapi/lib/model/user_response_dto.dart +++ b/mobile/openapi/lib/model/user_response_dto.dart @@ -22,6 +22,7 @@ class UserResponseDto { required this.shouldChangePassword, required this.isAdmin, this.deletedAt, + required this.oauthId, }); String id; @@ -48,6 +49,8 @@ class UserResponseDto { /// DateTime? deletedAt; + String oauthId; + @override bool operator ==(Object other) => identical(this, other) || other is UserResponseDto && other.id == id && @@ -58,7 +61,8 @@ class UserResponseDto { other.profileImagePath == profileImagePath && other.shouldChangePassword == shouldChangePassword && other.isAdmin == isAdmin && - other.deletedAt == deletedAt; + other.deletedAt == deletedAt && + other.oauthId == oauthId; @override int get hashCode => @@ -71,10 +75,11 @@ class UserResponseDto { (profileImagePath.hashCode) + (shouldChangePassword.hashCode) + (isAdmin.hashCode) + - (deletedAt == null ? 0 : deletedAt!.hashCode); + (deletedAt == null ? 0 : deletedAt!.hashCode) + + (oauthId.hashCode); @override - String toString() => 'UserResponseDto[id=$id, email=$email, firstName=$firstName, lastName=$lastName, createdAt=$createdAt, profileImagePath=$profileImagePath, shouldChangePassword=$shouldChangePassword, isAdmin=$isAdmin, deletedAt=$deletedAt]'; + String toString() => 'UserResponseDto[id=$id, email=$email, firstName=$firstName, lastName=$lastName, createdAt=$createdAt, profileImagePath=$profileImagePath, shouldChangePassword=$shouldChangePassword, isAdmin=$isAdmin, deletedAt=$deletedAt, oauthId=$oauthId]'; Map toJson() { final _json = {}; @@ -91,6 +96,7 @@ class UserResponseDto { } else { _json[r'deletedAt'] = null; } + _json[r'oauthId'] = oauthId; return _json; } @@ -122,6 +128,7 @@ class UserResponseDto { shouldChangePassword: mapValueOfType(json, r'shouldChangePassword')!, isAdmin: mapValueOfType(json, r'isAdmin')!, deletedAt: mapDateTime(json, r'deletedAt', ''), + oauthId: mapValueOfType(json, r'oauthId')!, ); } return null; @@ -179,6 +186,7 @@ class UserResponseDto { 'profileImagePath', 'shouldChangePassword', 'isAdmin', + 'oauthId', }; } diff --git a/mobile/openapi/test/o_auth_api_test.dart b/mobile/openapi/test/o_auth_api_test.dart index 01103d63c..5940bb6f1 100644 --- a/mobile/openapi/test/o_auth_api_test.dart +++ b/mobile/openapi/test/o_auth_api_test.dart @@ -27,5 +27,15 @@ void main() { // TODO }); + //Future link(OAuthCallbackDto oAuthCallbackDto) async + test('test link', () async { + // TODO + }); + + //Future unlink() async + test('test unlink', () async { + // TODO + }); + }); } diff --git a/mobile/openapi/test/user_response_dto_test.dart b/mobile/openapi/test/user_response_dto_test.dart index 9b9ef3253..f1dda093c 100644 --- a/mobile/openapi/test/user_response_dto_test.dart +++ b/mobile/openapi/test/user_response_dto_test.dart @@ -61,6 +61,11 @@ void main() { // TODO }); + // String oauthId + test('to test the property `oauthId`', () async { + // TODO + }); + }); diff --git a/server/apps/immich/src/api-v1/auth/auth.service.ts b/server/apps/immich/src/api-v1/auth/auth.service.ts index 84b538f52..539283a05 100644 --- a/server/apps/immich/src/api-v1/auth/auth.service.ts +++ b/server/apps/immich/src/api-v1/auth/auth.service.ts @@ -74,9 +74,7 @@ export class AuthService { throw new BadRequestException('Wrong password'); } - user.password = newPassword; - - return this.userCore.updateUser(authUser, user, dto); + return this.userCore.updateUser(authUser, authUser.id, { password: newPassword }); } public async adminSignUp(dto: SignUpDto): Promise { 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 + }); +}