瀏覽代碼

fix(server): don't publicly reveal user count (#4409)

* fix: don't reveal user count publicly

* fix: mobile and user controller

* fix: update other frontend endpoints

* fix: revert openapi change

* chore: open api

* fix: initialize

* openapi

---------

Co-authored-by: Alex Tran <alex.tran1502@gmail.com>
Jonathan Jogenfors 1 年之前
父節點
當前提交
41befc0948

+ 15 - 0
cli/src/api/open-api/api.ts

@@ -2582,6 +2582,12 @@ export interface SearchResponseDto {
  * @interface ServerConfigDto
  * @interface ServerConfigDto
  */
  */
 export interface ServerConfigDto {
 export interface ServerConfigDto {
+    /**
+     * 
+     * @type {boolean}
+     * @memberof ServerConfigDto
+     */
+    'isInitialized': boolean;
     /**
     /**
      * 
      * 
      * @type {string}
      * @type {string}
@@ -15081,6 +15087,15 @@ export const UserApiAxiosParamCreator = function (configuration?: Configuration)
             const localVarHeaderParameter = {} as any;
             const localVarHeaderParameter = {} as any;
             const localVarQueryParameter = {} as any;
             const localVarQueryParameter = {} as any;
 
 
+            // authentication cookie required
+
+            // authentication api_key required
+            await setApiKeyToObject(localVarHeaderParameter, "x-api-key", configuration)
+
+            // authentication bearer required
+            // http bearer authentication required
+            await setBearerAuthToObject(localVarHeaderParameter, configuration)
+
             if (admin !== undefined) {
             if (admin !== undefined) {
                 localVarQueryParameter['admin'] = admin;
                 localVarQueryParameter['admin'] = admin;
             }
             }

+ 1 - 0
mobile/lib/shared/providers/server_info.provider.dart

@@ -34,6 +34,7 @@ class ServerInfoNotifier extends StateNotifier<ServerInfoState> {
               mapTileUrl: "https://tile.openstreetmap.org/{z}/{x}/{y}.png",
               mapTileUrl: "https://tile.openstreetmap.org/{z}/{x}/{y}.png",
               oauthButtonText: "",
               oauthButtonText: "",
               trashDays: 30,
               trashDays: 30,
+              isInitialized: false,
             ),
             ),
             isVersionMismatch: false,
             isVersionMismatch: false,
             versionMismatchErrorMessage: "",
             versionMismatchErrorMessage: "",

+ 1 - 0
mobile/openapi/doc/ServerConfigDto.md

@@ -8,6 +8,7 @@ import 'package:openapi/api.dart';
 ## Properties
 ## Properties
 Name | Type | Description | Notes
 Name | Type | Description | Notes
 ------------ | ------------- | ------------- | -------------
 ------------ | ------------- | ------------- | -------------
+**isInitialized** | **bool** |  | 
 **loginPageMessage** | **String** |  | 
 **loginPageMessage** | **String** |  | 
 **mapTileUrl** | **String** |  | 
 **mapTileUrl** | **String** |  | 
 **oauthButtonText** | **String** |  | 
 **oauthButtonText** | **String** |  | 

+ 15 - 1
mobile/openapi/doc/UserApi.md

@@ -410,6 +410,20 @@ Name | Type | Description  | Notes
 ### Example
 ### Example
 ```dart
 ```dart
 import 'package:openapi/api.dart';
 import 'package:openapi/api.dart';
+// TODO Configure API key authorization: cookie
+//defaultApiClient.getAuthentication<ApiKeyAuth>('cookie').apiKey = 'YOUR_API_KEY';
+// uncomment below to setup prefix (e.g. Bearer) for API key, if needed
+//defaultApiClient.getAuthentication<ApiKeyAuth>('cookie').apiKeyPrefix = 'Bearer';
+// TODO Configure API key authorization: api_key
+//defaultApiClient.getAuthentication<ApiKeyAuth>('api_key').apiKey = 'YOUR_API_KEY';
+// uncomment below to setup prefix (e.g. Bearer) for API key, if needed
+//defaultApiClient.getAuthentication<ApiKeyAuth>('api_key').apiKeyPrefix = 'Bearer';
+// TODO Configure HTTP Bearer authorization: bearer
+// Case 1. Use String Token
+//defaultApiClient.getAuthentication<HttpBearerAuth>('bearer').setAccessToken('YOUR_ACCESS_TOKEN');
+// Case 2. Use Function which generate token.
+// String yourTokenGeneratorFunction() { ... }
+//defaultApiClient.getAuthentication<HttpBearerAuth>('bearer').setAccessToken(yourTokenGeneratorFunction);
 
 
 final api_instance = UserApi();
 final api_instance = UserApi();
 final admin = true; // bool | 
 final admin = true; // bool | 
@@ -434,7 +448,7 @@ Name | Type | Description  | Notes
 
 
 ### Authorization
 ### Authorization
 
 
-No authorization required
+[cookie](../README.md#cookie), [api_key](../README.md#api_key), [bearer](../README.md#bearer)
 
 
 ### HTTP request headers
 ### HTTP request headers
 
 

+ 9 - 1
mobile/openapi/lib/model/server_config_dto.dart

@@ -13,12 +13,15 @@ part of openapi.api;
 class ServerConfigDto {
 class ServerConfigDto {
   /// Returns a new [ServerConfigDto] instance.
   /// Returns a new [ServerConfigDto] instance.
   ServerConfigDto({
   ServerConfigDto({
+    required this.isInitialized,
     required this.loginPageMessage,
     required this.loginPageMessage,
     required this.mapTileUrl,
     required this.mapTileUrl,
     required this.oauthButtonText,
     required this.oauthButtonText,
     required this.trashDays,
     required this.trashDays,
   });
   });
 
 
+  bool isInitialized;
+
   String loginPageMessage;
   String loginPageMessage;
 
 
   String mapTileUrl;
   String mapTileUrl;
@@ -29,6 +32,7 @@ class ServerConfigDto {
 
 
   @override
   @override
   bool operator ==(Object other) => identical(this, other) || other is ServerConfigDto &&
   bool operator ==(Object other) => identical(this, other) || other is ServerConfigDto &&
+     other.isInitialized == isInitialized &&
      other.loginPageMessage == loginPageMessage &&
      other.loginPageMessage == loginPageMessage &&
      other.mapTileUrl == mapTileUrl &&
      other.mapTileUrl == mapTileUrl &&
      other.oauthButtonText == oauthButtonText &&
      other.oauthButtonText == oauthButtonText &&
@@ -37,16 +41,18 @@ class ServerConfigDto {
   @override
   @override
   int get hashCode =>
   int get hashCode =>
     // ignore: unnecessary_parenthesis
     // ignore: unnecessary_parenthesis
+    (isInitialized.hashCode) +
     (loginPageMessage.hashCode) +
     (loginPageMessage.hashCode) +
     (mapTileUrl.hashCode) +
     (mapTileUrl.hashCode) +
     (oauthButtonText.hashCode) +
     (oauthButtonText.hashCode) +
     (trashDays.hashCode);
     (trashDays.hashCode);
 
 
   @override
   @override
-  String toString() => 'ServerConfigDto[loginPageMessage=$loginPageMessage, mapTileUrl=$mapTileUrl, oauthButtonText=$oauthButtonText, trashDays=$trashDays]';
+  String toString() => 'ServerConfigDto[isInitialized=$isInitialized, loginPageMessage=$loginPageMessage, mapTileUrl=$mapTileUrl, oauthButtonText=$oauthButtonText, trashDays=$trashDays]';
 
 
   Map<String, dynamic> toJson() {
   Map<String, dynamic> toJson() {
     final json = <String, dynamic>{};
     final json = <String, dynamic>{};
+      json[r'isInitialized'] = this.isInitialized;
       json[r'loginPageMessage'] = this.loginPageMessage;
       json[r'loginPageMessage'] = this.loginPageMessage;
       json[r'mapTileUrl'] = this.mapTileUrl;
       json[r'mapTileUrl'] = this.mapTileUrl;
       json[r'oauthButtonText'] = this.oauthButtonText;
       json[r'oauthButtonText'] = this.oauthButtonText;
@@ -62,6 +68,7 @@ class ServerConfigDto {
       final json = value.cast<String, dynamic>();
       final json = value.cast<String, dynamic>();
 
 
       return ServerConfigDto(
       return ServerConfigDto(
+        isInitialized: mapValueOfType<bool>(json, r'isInitialized')!,
         loginPageMessage: mapValueOfType<String>(json, r'loginPageMessage')!,
         loginPageMessage: mapValueOfType<String>(json, r'loginPageMessage')!,
         mapTileUrl: mapValueOfType<String>(json, r'mapTileUrl')!,
         mapTileUrl: mapValueOfType<String>(json, r'mapTileUrl')!,
         oauthButtonText: mapValueOfType<String>(json, r'oauthButtonText')!,
         oauthButtonText: mapValueOfType<String>(json, r'oauthButtonText')!,
@@ -113,6 +120,7 @@ class ServerConfigDto {
 
 
   /// The list of required keys that must be present in a JSON.
   /// The list of required keys that must be present in a JSON.
   static const requiredKeys = <String>{
   static const requiredKeys = <String>{
+    'isInitialized',
     'loginPageMessage',
     'loginPageMessage',
     'mapTileUrl',
     'mapTileUrl',
     'oauthButtonText',
     'oauthButtonText',

+ 5 - 0
mobile/openapi/test/server_config_dto_test.dart

@@ -16,6 +16,11 @@ void main() {
   // final instance = ServerConfigDto();
   // final instance = ServerConfigDto();
 
 
   group('test ServerConfigDto', () {
   group('test ServerConfigDto', () {
+    // bool isInitialized
+    test('to test the property `isInitialized`', () async {
+      // TODO
+    });
+
     // String loginPageMessage
     // String loginPageMessage
     test('to test the property `loginPageMessage`', () async {
     test('to test the property `loginPageMessage`', () async {
       // TODO
       // TODO

+ 16 - 1
server/immich-openapi-specs.json

@@ -5004,6 +5004,17 @@
             "description": ""
             "description": ""
           }
           }
         },
         },
+        "security": [
+          {
+            "bearer": []
+          },
+          {
+            "cookie": []
+          },
+          {
+            "api_key": []
+          }
+        ],
         "tags": [
         "tags": [
           "User"
           "User"
         ]
         ]
@@ -7340,6 +7351,9 @@
       },
       },
       "ServerConfigDto": {
       "ServerConfigDto": {
         "properties": {
         "properties": {
+          "isInitialized": {
+            "type": "boolean"
+          },
           "loginPageMessage": {
           "loginPageMessage": {
             "type": "string"
             "type": "string"
           },
           },
@@ -7357,7 +7371,8 @@
           "trashDays",
           "trashDays",
           "oauthButtonText",
           "oauthButtonText",
           "loginPageMessage",
           "loginPageMessage",
-          "mapTileUrl"
+          "mapTileUrl",
+          "isInitialized"
         ],
         ],
         "type": "object"
         "type": "object"
       },
       },

+ 1 - 0
server/src/domain/repositories/user.repository.ts

@@ -18,6 +18,7 @@ export const IUserRepository = 'IUserRepository';
 export interface IUserRepository {
 export interface IUserRepository {
   get(id: string, withDeleted?: boolean): Promise<UserEntity | null>;
   get(id: string, withDeleted?: boolean): Promise<UserEntity | null>;
   getAdmin(): Promise<UserEntity | null>;
   getAdmin(): Promise<UserEntity | null>;
+  hasAdmin(): Promise<boolean>;
   getByEmail(email: string, withPassword?: boolean): Promise<UserEntity | null>;
   getByEmail(email: string, withPassword?: boolean): Promise<UserEntity | null>;
   getByStorageLabel(storageLabel: string): Promise<UserEntity | null>;
   getByStorageLabel(storageLabel: string): Promise<UserEntity | null>;
   getByOAuthId(oauthId: string): Promise<UserEntity | null>;
   getByOAuthId(oauthId: string): Promise<UserEntity | null>;

+ 1 - 0
server/src/domain/server-info/server-info.dto.ts

@@ -85,6 +85,7 @@ export class ServerConfigDto {
   mapTileUrl!: string;
   mapTileUrl!: string;
   @ApiProperty({ type: 'integer' })
   @ApiProperty({ type: 'integer' })
   trashDays!: number;
   trashDays!: number;
+  isInitialized!: boolean;
 }
 }
 
 
 export class ServerFeaturesDto implements FeatureFlags {
 export class ServerFeaturesDto implements FeatureFlags {

+ 3 - 0
server/src/domain/server-info/server-info.service.ts

@@ -74,11 +74,14 @@ export class ServerInfoService {
     // TODO move to system config
     // TODO move to system config
     const loginPageMessage = process.env.PUBLIC_LOGIN_PAGE_MESSAGE || '';
     const loginPageMessage = process.env.PUBLIC_LOGIN_PAGE_MESSAGE || '';
 
 
+    const isInitialized = await this.userRepository.hasAdmin();
+
     return {
     return {
       loginPageMessage,
       loginPageMessage,
       mapTileUrl: config.map.tileUrl,
       mapTileUrl: config.map.tileUrl,
       trashDays: config.trash.days,
       trashDays: config.trash.days,
       oauthButtonText: config.oauth.buttonText,
       oauthButtonText: config.oauth.buttonText,
+      isInitialized,
     };
     };
   }
   }
 
 

+ 2 - 2
server/src/immich/controllers/user.controller.ts

@@ -26,7 +26,7 @@ import {
 } from '@nestjs/common';
 } from '@nestjs/common';
 import { ApiBody, ApiConsumes, ApiTags } from '@nestjs/swagger';
 import { ApiBody, ApiConsumes, ApiTags } from '@nestjs/swagger';
 import { Response as Res } from 'express';
 import { Response as Res } from 'express';
-import { AdminRoute, AuthUser, Authenticated, PublicRoute } from '../app.guard';
+import { AdminRoute, AuthUser, Authenticated } from '../app.guard';
 import { FileUploadInterceptor, Route } from '../app.interceptor';
 import { FileUploadInterceptor, Route } from '../app.interceptor';
 import { UseValidation } from '../app.utils';
 import { UseValidation } from '../app.utils';
 import { UUIDParamDto } from './dto/uuid-param.dto';
 import { UUIDParamDto } from './dto/uuid-param.dto';
@@ -59,7 +59,7 @@ export class UserController {
     return this.service.create(createUserDto);
     return this.service.create(createUserDto);
   }
   }
 
 
-  @PublicRoute()
+  @AdminRoute()
   @Get('count')
   @Get('count')
   getUserCount(@Query() dto: CountDto): Promise<UserCountResponseDto> {
   getUserCount(@Query() dto: CountDto): Promise<UserCountResponseDto> {
     return this.service.getCount(dto);
     return this.service.getCount(dto);

+ 4 - 0
server/src/infra/repositories/user.repository.ts

@@ -16,6 +16,10 @@ export class UserRepository implements IUserRepository {
     return this.userRepository.findOne({ where: { isAdmin: true } });
     return this.userRepository.findOne({ where: { isAdmin: true } });
   }
   }
 
 
+  async hasAdmin(): Promise<boolean> {
+    return this.userRepository.exist({ where: { isAdmin: true } });
+  }
+
   async getByEmail(email: string, withPassword?: boolean): Promise<UserEntity | null> {
   async getByEmail(email: string, withPassword?: boolean): Promise<UserEntity | null> {
     let builder = this.userRepository.createQueryBuilder('user').where({ email });
     let builder = this.userRepository.createQueryBuilder('user').where({ email });
 
 

+ 1 - 0
server/test/e2e/server-info.e2e-spec.ts

@@ -102,6 +102,7 @@ describe(`${ServerInfoController.name} (e2e)`, () => {
         oauthButtonText: 'Login with OAuth',
         oauthButtonText: 'Login with OAuth',
         mapTileUrl: 'https://tile.openstreetmap.org/{z}/{x}/{y}.png',
         mapTileUrl: 'https://tile.openstreetmap.org/{z}/{x}/{y}.png',
         trashDays: 30,
         trashDays: 30,
+        isInitialized: true,
       });
       });
     });
     });
   });
   });

+ 3 - 3
server/test/e2e/user.e2e-spec.ts

@@ -311,10 +311,10 @@ describe(`${UserController.name}`, () => {
   });
   });
 
 
   describe('GET /user/count', () => {
   describe('GET /user/count', () => {
-    it('should not require authentication', async () => {
+    it('should require authentication', async () => {
       const { status, body } = await request(server).get(`/user/count`);
       const { status, body } = await request(server).get(`/user/count`);
-      expect(status).toBe(200);
-      expect(body).toEqual({ userCount: 1 });
+      expect(status).toBe(401);
+      expect(body).toEqual(errorStub.unauthorized);
     });
     });
 
 
     it('should start with just the admin', async () => {
     it('should start with just the admin', async () => {

+ 1 - 0
server/test/repositories/user.repository.mock.ts

@@ -14,5 +14,6 @@ export const newUserRepositoryMock = (): jest.Mocked<IUserRepository> => {
     delete: jest.fn(),
     delete: jest.fn(),
     getDeletedUsers: jest.fn(),
     getDeletedUsers: jest.fn(),
     restore: jest.fn(),
     restore: jest.fn(),
+    hasAdmin: jest.fn(),
   };
   };
 };
 };

+ 15 - 0
web/src/api/open-api/api.ts

@@ -2582,6 +2582,12 @@ export interface SearchResponseDto {
  * @interface ServerConfigDto
  * @interface ServerConfigDto
  */
  */
 export interface ServerConfigDto {
 export interface ServerConfigDto {
+    /**
+     * 
+     * @type {boolean}
+     * @memberof ServerConfigDto
+     */
+    'isInitialized': boolean;
     /**
     /**
      * 
      * 
      * @type {string}
      * @type {string}
@@ -15081,6 +15087,15 @@ export const UserApiAxiosParamCreator = function (configuration?: Configuration)
             const localVarHeaderParameter = {} as any;
             const localVarHeaderParameter = {} as any;
             const localVarQueryParameter = {} as any;
             const localVarQueryParameter = {} as any;
 
 
+            // authentication cookie required
+
+            // authentication api_key required
+            await setApiKeyToObject(localVarHeaderParameter, "x-api-key", configuration)
+
+            // authentication bearer required
+            // http bearer authentication required
+            await setBearerAuthToObject(localVarHeaderParameter, configuration)
+
             if (admin !== undefined) {
             if (admin !== undefined) {
                 localVarQueryParameter['admin'] = admin;
                 localVarQueryParameter['admin'] = admin;
             }
             }

+ 1 - 0
web/src/lib/stores/server-config.store.ts

@@ -27,6 +27,7 @@ export const serverConfig = writable<ServerConfig>({
   mapTileUrl: '',
   mapTileUrl: '',
   loginPageMessage: '',
   loginPageMessage: '',
   trashDays: 30,
   trashDays: 30,
+  isInitialized: false,
 });
 });
 
 
 export const loadConfig = async () => {
 export const loadConfig = async () => {

+ 3 - 3
web/src/routes/+page.server.ts

@@ -10,10 +10,10 @@ export const load = (async ({ parent, locals: { api } }) => {
     throw redirect(302, AppRoute.PHOTOS);
     throw redirect(302, AppRoute.PHOTOS);
   }
   }
 
 
-  const { data } = await api.userApi.getUserCount({ admin: true });
+  const { data } = await api.serverInfoApi.getServerConfig();
 
 
-  if (data.userCount > 0) {
-    // Redirect to login page if an admin is already registered.
+  if (data.isInitialized) {
+    // Redirect to login page if there exists an admin account (i.e. server is initialized)
     throw redirect(302, AppRoute.AUTH_LOGIN);
     throw redirect(302, AppRoute.AUTH_LOGIN);
   }
   }
 
 

+ 2 - 2
web/src/routes/auth/login/+page.server.ts

@@ -3,8 +3,8 @@ import { redirect } from '@sveltejs/kit';
 import type { PageServerLoad } from './$types';
 import type { PageServerLoad } from './$types';
 
 
 export const load = (async ({ locals: { api } }) => {
 export const load = (async ({ locals: { api } }) => {
-  const { data } = await api.userApi.getUserCount({ admin: true });
-  if (data.userCount === 0) {
+  const { data } = await api.serverInfoApi.getServerConfig();
+  if (!data.isInitialized) {
     // Admin not registered
     // Admin not registered
     throw redirect(302, AppRoute.AUTH_REGISTER);
     throw redirect(302, AppRoute.AUTH_REGISTER);
   }
   }

+ 2 - 2
web/src/routes/auth/register/+page.server.ts

@@ -3,8 +3,8 @@ import { redirect } from '@sveltejs/kit';
 import type { PageServerLoad } from './$types';
 import type { PageServerLoad } from './$types';
 
 
 export const load = (async ({ locals: { api } }) => {
 export const load = (async ({ locals: { api } }) => {
-  const { data } = await api.userApi.getUserCount({ admin: true });
-  if (data.userCount != 0) {
+  const { data } = await api.serverInfoApi.getServerConfig();
+  if (data.isInitialized) {
     // Admin has been registered, redirect to login
     // Admin has been registered, redirect to login
     throw redirect(302, AppRoute.AUTH_LOGIN);
     throw redirect(302, AppRoute.AUTH_LOGIN);
   }
   }