diff --git a/mobile/openapi/doc/UpdateUserDto.md b/mobile/openapi/doc/UpdateUserDto.md index 043f2e6ab8..e40bf5eb77 100644 --- a/mobile/openapi/doc/UpdateUserDto.md +++ b/mobile/openapi/doc/UpdateUserDto.md @@ -8,14 +8,13 @@ import 'package:openapi/api.dart'; ## Properties Name | Type | Description | Notes ------------ | ------------- | ------------- | ------------- -**id** | **String** | | **email** | **String** | | [optional] **password** | **String** | | [optional] **firstName** | **String** | | [optional] **lastName** | **String** | | [optional] +**id** | **String** | | **isAdmin** | **bool** | | [optional] **shouldChangePassword** | **bool** | | [optional] -**profileImagePath** | **String** | | [optional] [[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/model/update_user_dto.dart b/mobile/openapi/lib/model/update_user_dto.dart index edc5298104..2d59db436e 100644 --- a/mobile/openapi/lib/model/update_user_dto.dart +++ b/mobile/openapi/lib/model/update_user_dto.dart @@ -13,18 +13,15 @@ part of openapi.api; class UpdateUserDto { /// Returns a new [UpdateUserDto] instance. UpdateUserDto({ - required this.id, this.email, this.password, this.firstName, this.lastName, + required this.id, this.isAdmin, this.shouldChangePassword, - this.profileImagePath, }); - String id; - /// /// Please note: This property should have been non-nullable! Since the specification file /// does not include a default value (using the "default:" property), however, the generated @@ -57,6 +54,8 @@ class UpdateUserDto { /// String? lastName; + String id; + /// /// Please note: This property should have been non-nullable! Since the specification file /// does not include a default value (using the "default:" property), however, the generated @@ -73,43 +72,32 @@ class UpdateUserDto { /// bool? shouldChangePassword; - /// - /// Please note: This property should have been non-nullable! Since the specification file - /// does not include a default value (using the "default:" property), however, the generated - /// source code must fall back to having a nullable type. - /// Consider adding a "default:" property in the specification file to hide this note. - /// - String? profileImagePath; - @override bool operator ==(Object other) => identical(this, other) || other is UpdateUserDto && - other.id == id && other.email == email && other.password == password && other.firstName == firstName && other.lastName == lastName && + other.id == id && other.isAdmin == isAdmin && - other.shouldChangePassword == shouldChangePassword && - other.profileImagePath == profileImagePath; + other.shouldChangePassword == shouldChangePassword; @override int get hashCode => // ignore: unnecessary_parenthesis - (id.hashCode) + (email == null ? 0 : email!.hashCode) + (password == null ? 0 : password!.hashCode) + (firstName == null ? 0 : firstName!.hashCode) + (lastName == null ? 0 : lastName!.hashCode) + + (id.hashCode) + (isAdmin == null ? 0 : isAdmin!.hashCode) + - (shouldChangePassword == null ? 0 : shouldChangePassword!.hashCode) + - (profileImagePath == null ? 0 : profileImagePath!.hashCode); + (shouldChangePassword == null ? 0 : shouldChangePassword!.hashCode); @override - String toString() => 'UpdateUserDto[id=$id, email=$email, password=$password, firstName=$firstName, lastName=$lastName, isAdmin=$isAdmin, shouldChangePassword=$shouldChangePassword, profileImagePath=$profileImagePath]'; + String toString() => 'UpdateUserDto[email=$email, password=$password, firstName=$firstName, lastName=$lastName, id=$id, isAdmin=$isAdmin, shouldChangePassword=$shouldChangePassword]'; Map toJson() { final json = {}; - json[r'id'] = this.id; if (this.email != null) { json[r'email'] = this.email; } else { @@ -130,6 +118,7 @@ class UpdateUserDto { } else { // json[r'lastName'] = null; } + json[r'id'] = this.id; if (this.isAdmin != null) { json[r'isAdmin'] = this.isAdmin; } else { @@ -140,11 +129,6 @@ class UpdateUserDto { } else { // json[r'shouldChangePassword'] = null; } - if (this.profileImagePath != null) { - json[r'profileImagePath'] = this.profileImagePath; - } else { - // json[r'profileImagePath'] = null; - } return json; } @@ -167,14 +151,13 @@ class UpdateUserDto { }()); return UpdateUserDto( - id: mapValueOfType(json, r'id')!, email: mapValueOfType(json, r'email'), password: mapValueOfType(json, r'password'), firstName: mapValueOfType(json, r'firstName'), lastName: mapValueOfType(json, r'lastName'), + id: mapValueOfType(json, r'id')!, isAdmin: mapValueOfType(json, r'isAdmin'), shouldChangePassword: mapValueOfType(json, r'shouldChangePassword'), - profileImagePath: mapValueOfType(json, r'profileImagePath'), ); } return null; diff --git a/mobile/openapi/test/update_user_dto_test.dart b/mobile/openapi/test/update_user_dto_test.dart index 5055f5fa9b..8cd9da9944 100644 --- a/mobile/openapi/test/update_user_dto_test.dart +++ b/mobile/openapi/test/update_user_dto_test.dart @@ -16,11 +16,6 @@ void main() { // final instance = UpdateUserDto(); group('test UpdateUserDto', () { - // String id - test('to test the property `id`', () async { - // TODO - }); - // String email test('to test the property `email`', () async { // TODO @@ -41,6 +36,11 @@ void main() { // TODO }); + // String id + test('to test the property `id`', () async { + // TODO + }); + // bool isAdmin test('to test the property `isAdmin`', () async { // TODO @@ -51,11 +51,6 @@ void main() { // TODO }); - // String profileImagePath - test('to test the property `profileImagePath`', () async { - // TODO - }); - }); diff --git a/server/apps/immich/src/controllers/user.controller.ts b/server/apps/immich/src/controllers/user.controller.ts index d426dff78a..9c180d2cb3 100644 --- a/server/apps/immich/src/controllers/user.controller.ts +++ b/server/apps/immich/src/controllers/user.controller.ts @@ -32,7 +32,7 @@ import { UserCountDto } from '@app/domain'; @ApiTags('User') @Controller('user') -@UsePipes(new ValidationPipe({ transform: true })) +@UsePipes(new ValidationPipe({ transform: true, whitelist: true })) export class UserController { constructor(private service: UserService) {} diff --git a/server/apps/immich/test/user.e2e-spec.ts b/server/apps/immich/test/user.e2e-spec.ts index bc1bc11a1d..e208c21b52 100644 --- a/server/apps/immich/test/user.e2e-spec.ts +++ b/server/apps/immich/test/user.e2e-spec.ts @@ -2,7 +2,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { INestApplication } from '@nestjs/common'; import request from 'supertest'; import { clearDb, authCustom } from './test-utils'; -import { CreateUserDto, UserService, AuthUserDto } from '@app/domain'; +import { CreateUserDto, UserService, AuthUserDto, UserResponseDto } from '@app/domain'; import { DataSource } from 'typeorm'; import { AuthService } from '@app/domain'; import { AppModule } from '../src/app.module'; @@ -39,10 +39,11 @@ describe('User', () => { }); }); - describe('with auth', () => { + describe('with admin auth', () => { let userService: UserService; let authService: AuthService; let authUser: AuthUserDto; + let userOne: UserResponseDto; beforeAll(async () => { const builder = Test.createTestingModule({ imports: [AppModule] }); @@ -69,7 +70,8 @@ describe('User', () => { password: '1234', }); authUser = { ...adminSignupResponseDto, isAdmin: true }; // TODO: find out why adminSignUp doesn't have isAdmin (maybe can just return UserResponseDto) - await Promise.allSettled([ + + [userOne] = await Promise.all([ _createUser(userService, { firstName: 'one', lastName: 'test', @@ -121,6 +123,67 @@ describe('User', () => { ); expect(body).toEqual(expect.not.arrayContaining([expect.objectContaining({ email: authUserEmail })])); }); + + it('disallows admin user from creating a second admin account', async () => { + const { status } = await request(app.getHttpServer()) + .put('/user') + .send({ + ...userOne, + isAdmin: true, + }); + expect(status).toEqual(400); + }); + + it('ignores updates to createdAt, updatedAt and deletedAt', async () => { + const { status, body } = await request(app.getHttpServer()) + .put('/user') + .send({ + ...userOne, + createdAt: '2023-01-01T00:00:00.000Z', + updatedAt: '2023-01-01T00:00:00.000Z', + deletedAt: '2023-01-01T00:00:00.000Z', + }); + expect(status).toEqual(200); + expect(body).toStrictEqual({ + ...userOne, + createdAt: new Date(userOne.createdAt).toISOString(), + updatedAt: expect.anything(), + }); + }); + + it('ignores updates to profileImagePath', async () => { + const { status, body } = await request(app.getHttpServer()) + .put('/user') + .send({ + ...userOne, + profileImagePath: 'invalid.jpg', + }); + expect(status).toEqual(200); + expect(body).toStrictEqual({ + ...userOne, + createdAt: new Date(userOne.createdAt).toISOString(), + updatedAt: expect.anything(), + }); + }); + + it('allows to update first and last name', async () => { + const { status, body } = await request(app.getHttpServer()) + .put('/user') + .send({ + ...userOne, + firstName: 'newFirstName', + lastName: 'newLastName', + }); + + expect(status).toEqual(200); + expect(body).toMatchObject({ + ...userOne, + createdAt: new Date(userOne.createdAt).toISOString(), + updatedAt: expect.anything(), + firstName: 'newFirstName', + lastName: 'newLastName', + }); + }); }); }); }); diff --git a/server/immich-openapi-specs.json b/server/immich-openapi-specs.json index 1a88ff1f8b..6920d0e9ef 100644 --- a/server/immich-openapi-specs.json +++ b/server/immich-openapi-specs.json @@ -4812,29 +4812,31 @@ "UpdateUserDto": { "type": "object", "properties": { - "id": { - "type": "string" - }, "email": { - "type": "string" + "type": "string", + "example": "testuser@email.com" }, "password": { - "type": "string" + "type": "string", + "example": "password" }, "firstName": { - "type": "string" + "type": "string", + "example": "John" }, "lastName": { - "type": "string" + "type": "string", + "example": "Doe" + }, + "id": { + "type": "string", + "format": "uuid" }, "isAdmin": { "type": "boolean" }, "shouldChangePassword": { "type": "boolean" - }, - "profileImagePath": { - "type": "string" } }, "required": [ diff --git a/server/libs/domain/src/user/dto/update-user.dto.ts b/server/libs/domain/src/user/dto/update-user.dto.ts index 73bcdf199b..f404e4d16f 100644 --- a/server/libs/domain/src/user/dto/update-user.dto.ts +++ b/server/libs/domain/src/user/dto/update-user.dto.ts @@ -1,28 +1,18 @@ -import { IsEmail, IsNotEmpty, IsOptional } from 'class-validator'; +import { IsBoolean, IsNotEmpty, IsOptional, IsUUID } from 'class-validator'; +import { CreateUserDto } from './create-user.dto'; +import { ApiProperty, PartialType } from '@nestjs/swagger'; -export class UpdateUserDto { +export class UpdateUserDto extends PartialType(CreateUserDto) { @IsNotEmpty() + @IsUUID('4') + @ApiProperty({ format: 'uuid' }) id!: string; - @IsEmail() - @IsOptional() - email?: string; - - @IsOptional() - password?: string; - - @IsOptional() - firstName?: string; - - @IsOptional() - lastName?: string; - @IsOptional() + @IsBoolean() isAdmin?: boolean; @IsOptional() + @IsBoolean() shouldChangePassword?: boolean; - - @IsOptional() - profileImagePath?: string; } diff --git a/server/libs/domain/src/user/user.core.ts b/server/libs/domain/src/user/user.core.ts index 0e7dd1dc59..2e75231568 100644 --- a/server/libs/domain/src/user/user.core.ts +++ b/server/libs/domain/src/user/user.core.ts @@ -21,12 +21,16 @@ export class UserCore { constructor(private userRepository: IUserRepository, private cryptoRepository: ICryptoRepository) {} async updateUser(authUser: AuthUserDto, id: string, dto: Partial): Promise { - if (!(authUser.isAdmin || authUser.id === id)) { + if (!authUser.isAdmin && authUser.id !== id) { throw new ForbiddenException('You are not allowed to update this user'); } - if (dto.isAdmin && authUser.isAdmin && authUser.id !== id) { - throw new BadRequestException('Admin user exists'); + if (!authUser.isAdmin) { + // Users can never update the isAdmin property. + delete dto.isAdmin; + } else if (dto.isAdmin && authUser.id !== id) { + // Admin cannot create another admin. + throw new BadRequestException('The server already has an admin'); } if (dto.email) { diff --git a/server/libs/domain/src/user/user.service.ts b/server/libs/domain/src/user/user.service.ts index 4bce62ed86..305d5e283a 100644 --- a/server/libs/domain/src/user/user.service.ts +++ b/server/libs/domain/src/user/user.service.ts @@ -90,6 +90,7 @@ export class UserService { if (!user) { throw new NotFoundException('User not found'); } + const updatedUser = await this.userCore.updateUser(authUser, dto.id, dto); return mapUser(updatedUser); } diff --git a/web/src/api/open-api/api.ts b/web/src/api/open-api/api.ts index afed1691ce..4bf1cd687f 100644 --- a/web/src/api/open-api/api.ts +++ b/web/src/api/open-api/api.ts @@ -2292,12 +2292,6 @@ export interface UpdateTagDto { * @interface UpdateUserDto */ export interface UpdateUserDto { - /** - * - * @type {string} - * @memberof UpdateUserDto - */ - 'id': string; /** * * @type {string} @@ -2322,6 +2316,12 @@ export interface UpdateUserDto { * @memberof UpdateUserDto */ 'lastName'?: string; + /** + * + * @type {string} + * @memberof UpdateUserDto + */ + 'id': string; /** * * @type {boolean} @@ -2334,12 +2334,6 @@ export interface UpdateUserDto { * @memberof UpdateUserDto */ 'shouldChangePassword'?: boolean; - /** - * - * @type {string} - * @memberof UpdateUserDto - */ - 'profileImagePath'?: string; } /** *