From 99fd56114444d06c0c445cf45e5410908e2cfded Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Tue, 15 Sep 2026 09:33:15 -0400 Subject: [PATCH] chore: transition to nullable user.oauthId (#31560) --- .../models/server_capability.model.dart | 9 +- .../domain/services/sync_stream.service.dart | 2 + .../repositories/sync_api.repository.dart | 9 +- .../repositories/sync_stream.repository.dart | 29 ++++++ .../actions/edit_asset.action.dart | 2 +- open-api/immich-openapi-specs.json | 94 +++++++++++++++++++ packages/sdk/src/fetch-client.ts | 29 ++++++ server/src/database.ts | 2 +- server/src/dtos/sync.dto.ts | 7 ++ server/src/dtos/user.dto.ts | 3 +- server/src/enum.ts | 2 + server/src/repositories/event.repository.ts | 2 +- ...196-ConvertUserOAuthIdEmptyStringToNull.ts | 13 +++ server/src/schema/migrations/ORDER | 1 + server/src/schema/tables/user.table.ts | 4 +- server/src/services/auth-admin.service.ts | 3 +- server/src/services/auth.service.spec.ts | 24 +++-- server/src/services/auth.service.ts | 2 +- server/src/services/sync.service.ts | 14 +++ server/test/factories/user.factory.ts | 2 +- server/test/fixtures/user.stub.ts | 4 +- server/test/medium.factory.ts | 2 +- .../specs/services/auth-admin.service.spec.ts | 8 +- .../medium/specs/sync/sync-auth-user.spec.ts | 20 ++-- server/test/small.factory.ts | 2 +- 25 files changed, 244 insertions(+), 45 deletions(-) create mode 100644 server/src/schema/migrations/1789419229196-ConvertUserOAuthIdEmptyStringToNull.ts diff --git a/mobile/lib/domain/models/server_capability.model.dart b/mobile/lib/domain/models/server_capability.model.dart index ece985059..471c8e186 100644 --- a/mobile/lib/domain/models/server_capability.model.dart +++ b/mobile/lib/domain/models/server_capability.model.dart @@ -4,10 +4,13 @@ enum ServerCapability { // Feature Support cloudIdMetadata(SemVer(major: 2, minor: 4, patch: 0)), bulkCloudIdMetadata(SemVer(major: 2, minor: 5, patch: 0)), - assetEdits(SemVer(major: 2, minor: 6, patch: 0)), - assetFacesV2(SemVer(major: 2, minor: 6, patch: 0)), + + // Sync syncV2(SemVer(major: 3, minor: 0, patch: 0)), - assetOcr(SemVer(major: 3, minor: 0, patch: 0)), + syncAssetEditsV1(SemVer(major: 2, minor: 6, patch: 0)), + syncAssetFacesV2(SemVer(major: 2, minor: 6, patch: 0)), + syncAssetOcrV1(SemVer(major: 3, minor: 0, patch: 0)), + syncAuthUsersV2(SemVer(major: 3, minor: 3, patch: 0)), // Migrations assetPayloadChange20260128(SemVer(major: 2, minor: 5, patch: 0)), diff --git a/mobile/lib/domain/services/sync_stream.service.dart b/mobile/lib/domain/services/sync_stream.service.dart index 2f6ad3f04..b168a0be6 100644 --- a/mobile/lib/domain/services/sync_stream.service.dart +++ b/mobile/lib/domain/services/sync_stream.service.dart @@ -194,6 +194,8 @@ class SyncStreamService { switch (type) { case SyncEntityType.authUserV1: return _syncStreamRepository.updateAuthUsersV1(data.cast()); + case SyncEntityType.authUserV2: + return _syncStreamRepository.updateAuthUsersV2(data.cast()); case SyncEntityType.userV1: return _syncStreamRepository.updateUsersV1(data.cast()); case SyncEntityType.userDeleteV1: diff --git a/mobile/lib/infrastructure/repositories/sync_api.repository.dart b/mobile/lib/infrastructure/repositories/sync_api.repository.dart index 4e27da1a3..8962dafcc 100644 --- a/mobile/lib/infrastructure/repositories/sync_api.repository.dart +++ b/mobile/lib/infrastructure/repositories/sync_api.repository.dart @@ -43,11 +43,11 @@ class SyncApiRepository { request.body = jsonEncode( SyncStreamDto( types: [ - SyncRequestType.authUsersV1, + serverVersion.supports(.syncAuthUsersV2) ? SyncRequestType.authUsersV2 : SyncRequestType.authUsersV1, SyncRequestType.usersV1, serverVersion.supports(.syncV2) ? SyncRequestType.assetsV2 : SyncRequestType.assetsV1, SyncRequestType.assetExifsV1, - if (serverVersion.supports(.assetEdits)) SyncRequestType.assetEditsV1, + if (serverVersion.supports(.syncAssetEditsV1)) SyncRequestType.assetEditsV1, SyncRequestType.assetMetadataV1, SyncRequestType.partnersV1, serverVersion.supports(.syncV2) ? SyncRequestType.partnerAssetsV2 : SyncRequestType.partnerAssetsV1, @@ -63,8 +63,8 @@ class SyncApiRepository { SyncRequestType.partnerStacksV1, SyncRequestType.userMetadataV1, SyncRequestType.peopleV1, - serverVersion.supports(.assetFacesV2) ? SyncRequestType.assetFacesV2 : SyncRequestType.assetFacesV1, - if (serverVersion.supports(.assetOcr)) SyncRequestType.assetOcrV1, + serverVersion.supports(.syncAssetFacesV2) ? SyncRequestType.assetFacesV2 : SyncRequestType.assetFacesV1, + if (serverVersion.supports(.syncAssetOcrV1)) SyncRequestType.assetOcrV1, ], ).toJson(), ); @@ -140,6 +140,7 @@ class SyncApiRepository { const _kResponseMap = { SyncEntityType.authUserV1: SyncAuthUserV1.fromJson, + SyncEntityType.authUserV2: SyncAuthUserV2.fromJson, SyncEntityType.userV1: SyncUserV1.fromJson, SyncEntityType.userDeleteV1: SyncUserDeleteV1.fromJson, SyncEntityType.partnerV1: SyncPartnerV1.fromJson, diff --git a/mobile/lib/infrastructure/repositories/sync_stream.repository.dart b/mobile/lib/infrastructure/repositories/sync_stream.repository.dart index 7c0e16c8e..b7251d17f 100644 --- a/mobile/lib/infrastructure/repositories/sync_stream.repository.dart +++ b/mobile/lib/infrastructure/repositories/sync_stream.repository.dart @@ -116,6 +116,35 @@ class SyncStreamRepository extends DatabaseAccessor with $SyncStreamRepos } } + Future updateAuthUsersV2(Iterable data) async { + try { + await _db.batch((batch) { + for (final user in data) { + final companion = AuthUserEntityCompanion( + name: Value(user.name), + email: Value(user.email), + hasProfileImage: Value(user.hasProfileImage), + profileChangedAt: Value(user.profileChangedAt), + avatarColor: Value(user.avatarColor.orElse(null)?.toAvatarColor() ?? AvatarColor.primary), + isAdmin: Value(user.isAdmin), + pinCode: Value(user.pinCode), + quotaSizeInBytes: Value(user.quotaSizeInBytes ?? 0), + quotaUsageInBytes: Value(user.quotaUsageInBytes), + ); + + batch.insert( + _db.authUserEntity, + companion.copyWith(id: Value(user.id)), + onConflict: DoUpdate((_) => companion), + ); + } + }); + } catch (error, stack) { + _logger.severe('Error: SyncAuthUserV2', error, stack); + rethrow; + } + } + Future deleteUsersV1(Iterable data) async { try { await _db.batch((batch) { diff --git a/mobile/lib/presentation/actions/edit_asset.action.dart b/mobile/lib/presentation/actions/edit_asset.action.dart index 08cf05517..9da7fed1f 100644 --- a/mobile/lib/presentation/actions/edit_asset.action.dart +++ b/mobile/lib/presentation/actions/edit_asset.action.dart @@ -20,7 +20,7 @@ import 'package:immich_mobile/routing/router.dart'; import 'package:immich_mobile/utils/error_handler.dart'; final _stateProvider = Provider.family.autoDispose((ref, source) { - final isSupported = ref.watch(serverInfoProvider.select((state) => state.serverVersion.supports(.assetEdits))); + final isSupported = ref.watch(serverInfoProvider.select((state) => state.serverVersion.supports(.syncAssetEditsV1))); if (!isSupported) { return null; } diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index d15a8b019..7b3389c0f 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -30710,6 +30710,98 @@ ], "type": "object" }, + "SyncAuthUserV2": { + "properties": { + "avatarColor": { + "allOf": [ + { + "$ref": "#/components/schemas/UserAvatarColor" + } + ], + "nullable": true + }, + "deletedAt": { + "description": "User deleted at", + "example": "2024-01-01T00:00:00.000Z", + "format": "date-time", + "nullable": true, + "pattern": "^(?:(?:\\d\\d[2468][048]|\\d\\d[13579][26]|\\d\\d0[48]|[02468][048]00|[13579][26]00)-02-29|\\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\\d|3[01])|(?:0[469]|11)-(?:0[1-9]|[12]\\d|30)|(?:02)-(?:0[1-9]|1\\d|2[0-8])))T(?:(?:[01]\\d|2[0-3]):[0-5]\\d(?::[0-5]\\d(?:\\.\\d+)?)?(?:Z|([+-](?:[01]\\d|2[0-3]):[0-5]\\d)))$", + "type": "string" + }, + "email": { + "description": "User email", + "type": "string" + }, + "hasProfileImage": { + "description": "User has profile image", + "type": "boolean" + }, + "id": { + "description": "User ID", + "format": "uuid", + "pattern": "^([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-4[0-9a-fA-F]{3}-[89abAB][0-9a-fA-F]{3}-[0-9a-fA-F]{12})$", + "type": "string" + }, + "isAdmin": { + "description": "User is admin", + "type": "boolean" + }, + "name": { + "description": "User name", + "type": "string" + }, + "oauthId": { + "description": "User OAuth ID", + "nullable": true, + "type": "string" + }, + "pinCode": { + "description": "User pin code", + "nullable": true, + "type": "string" + }, + "profileChangedAt": { + "description": "User profile changed at", + "example": "2024-01-01T00:00:00.000Z", + "format": "date-time", + "pattern": "^(?:(?:\\d\\d[2468][048]|\\d\\d[13579][26]|\\d\\d0[48]|[02468][048]00|[13579][26]00)-02-29|\\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\\d|3[01])|(?:0[469]|11)-(?:0[1-9]|[12]\\d|30)|(?:02)-(?:0[1-9]|1\\d|2[0-8])))T(?:(?:[01]\\d|2[0-3]):[0-5]\\d(?::[0-5]\\d(?:\\.\\d+)?)?(?:Z|([+-](?:[01]\\d|2[0-3]):[0-5]\\d)))$", + "type": "string" + }, + "quotaSizeInBytes": { + "description": "Quota size in bytes", + "maximum": 9007199254740991, + "minimum": -9007199254740991, + "nullable": true, + "type": "integer" + }, + "quotaUsageInBytes": { + "description": "Quota usage in bytes", + "maximum": 9007199254740991, + "minimum": -9007199254740991, + "type": "integer" + }, + "storageLabel": { + "description": "User storage label", + "nullable": true, + "type": "string" + } + }, + "required": [ + "deletedAt", + "email", + "hasProfileImage", + "id", + "isAdmin", + "name", + "oauthId", + "pinCode", + "profileChangedAt", + "quotaSizeInBytes", + "quotaUsageInBytes", + "storageLabel" + ], + "type": "object" + }, "SyncCompleteV1": { "properties": {}, "type": "object" @@ -30718,6 +30810,7 @@ "description": "Sync entity type", "enum": [ "AuthUserV1", + "AuthUserV2", "UserV1", "UserDeleteV1", "AssetV1", @@ -31082,6 +31175,7 @@ "AssetMetadataV1", "AssetOcrV1", "AuthUsersV1", + "AuthUsersV2", "MemoriesV1", "MemoryToAssetsV1", "PartnersV1", diff --git a/packages/sdk/src/fetch-client.ts b/packages/sdk/src/fetch-client.ts index 11c041ab6..645c398bc 100644 --- a/packages/sdk/src/fetch-client.ts +++ b/packages/sdk/src/fetch-client.ts @@ -3594,6 +3594,33 @@ export type SyncAuthUserV1 = { /** User storage label */ storageLabel: string | null; }; +export type SyncAuthUserV2 = { + avatarColor?: (UserAvatarColor) | null; + /** User deleted at */ + deletedAt: string | null; + /** User email */ + email: string; + /** User has profile image */ + hasProfileImage: boolean; + /** User ID */ + id: string; + /** User is admin */ + isAdmin: boolean; + /** User name */ + name: string; + /** User OAuth ID */ + oauthId: string | null; + /** User pin code */ + pinCode: string | null; + /** User profile changed at */ + profileChangedAt: string; + /** Quota size in bytes */ + quotaSizeInBytes: number | null; + /** Quota usage in bytes */ + quotaUsageInBytes: number; + /** User storage label */ + storageLabel: string | null; +}; export type SyncCompleteV1 = {}; export type SyncMemoryAssetDeleteV1 = { /** Asset ID */ @@ -8306,6 +8333,7 @@ export enum AssetIdErrorReason { } export enum SyncEntityType { AuthUserV1 = "AuthUserV1", + AuthUserV2 = "AuthUserV2", UserV1 = "UserV1", UserDeleteV1 = "UserDeleteV1", AssetV1 = "AssetV1", @@ -8380,6 +8408,7 @@ export enum SyncRequestType { AssetMetadataV1 = "AssetMetadataV1", AssetOcrV1 = "AssetOcrV1", AuthUsersV1 = "AuthUsersV1", + AuthUsersV2 = "AuthUsersV2", MemoriesV1 = "MemoriesV1", MemoryToAssetsV1 = "MemoryToAssetsV1", PartnersV1 = "PartnersV1", diff --git a/server/src/database.ts b/server/src/database.ts index 47a713cc3..7883b049d 100644 --- a/server/src/database.ts +++ b/server/src/database.ts @@ -140,7 +140,7 @@ export type UserAdmin = User & { createdAt: Date; updatedAt: Date; deletedAt: Date | null; - oauthId: string; + oauthId: string | null; quotaSizeInBytes: number | null; quotaUsageInBytes: number; status: UserStatus; diff --git a/server/src/dtos/sync.dto.ts b/server/src/dtos/sync.dto.ts index 3f3f65e86..6659fe163 100644 --- a/server/src/dtos/sync.dto.ts +++ b/server/src/dtos/sync.dto.ts @@ -40,6 +40,10 @@ const SyncAuthUserV1Schema = SyncUserV1Schema.merge( }), ).meta({ id: 'SyncAuthUserV1' }); +const SyncAuthUserV2Schema = SyncAuthUserV1Schema.extend({ + oauthId: z.string().nullable().describe('User OAuth ID'), +}).meta({ id: 'SyncAuthUserV2' }); + const SyncUserDeleteV1Schema = z.object({ userId: z.uuidv4().describe('User ID') }).meta({ id: 'SyncUserDeleteV1' }); const SyncPartnerV1Schema = z @@ -112,6 +116,8 @@ class SyncUserV1 extends createZodDto(SyncUserV1Schema) {} @ExtraModel() class SyncAuthUserV1 extends createZodDto(SyncAuthUserV1Schema) {} @ExtraModel() +class SyncAuthUserV2 extends createZodDto(SyncAuthUserV2Schema) {} +@ExtraModel() class SyncUserDeleteV1 extends createZodDto(SyncUserDeleteV1Schema) {} @ExtraModel() class SyncPartnerV1 extends createZodDto(SyncPartnerV1Schema) {} @@ -463,6 +469,7 @@ class SyncCompleteV1 extends createZodDto(SyncCompleteV1Schema) {} export type SyncItem = { [SyncEntityType.AuthUserV1]: SyncAuthUserV1; + [SyncEntityType.AuthUserV2]: SyncAuthUserV2; [SyncEntityType.UserV1]: SyncUserV1; [SyncEntityType.UserDeleteV1]: SyncUserDeleteV1; [SyncEntityType.PartnerV1]: SyncPartnerV1; diff --git a/server/src/dtos/user.dto.ts b/server/src/dtos/user.dto.ts index e8af4da80..fa237e8eb 100644 --- a/server/src/dtos/user.dto.ts +++ b/server/src/dtos/user.dto.ts @@ -151,7 +151,8 @@ export function mapUserAdmin(entity: UserAdmin): UserAdminResponseDto { createdAt: entity.createdAt, deletedAt: entity.deletedAt, updatedAt: entity.updatedAt, - oauthId: entity.oauthId, + // TODO(v4): remove the mapping and make `oauthId` nullable + oauthId: entity.oauthId ?? '', quotaSizeInBytes: entity.quotaSizeInBytes, quotaUsageInBytes: entity.quotaUsageInBytes, status: entity.status, diff --git a/server/src/enum.ts b/server/src/enum.ts index 8cb6582c9..232f7b22b 100644 --- a/server/src/enum.ts +++ b/server/src/enum.ts @@ -1024,6 +1024,7 @@ export enum SyncRequestType { AssetMetadataV1 = 'AssetMetadataV1', AssetOcrV1 = 'AssetOcrV1', AuthUsersV1 = 'AuthUsersV1', + AuthUsersV2 = 'AuthUsersV2', MemoriesV1 = 'MemoriesV1', MemoryToAssetsV1 = 'MemoryToAssetsV1', PartnersV1 = 'PartnersV1', @@ -1048,6 +1049,7 @@ export const SyncRequestTypeSchema = z export enum SyncEntityType { AuthUserV1 = 'AuthUserV1', + AuthUserV2 = 'AuthUserV2', UserV1 = 'UserV1', UserDeleteV1 = 'UserDeleteV1', diff --git a/server/src/repositories/event.repository.ts b/server/src/repositories/event.repository.ts index 302f6ca4d..a1c6a4261 100644 --- a/server/src/repositories/event.repository.ts +++ b/server/src/repositories/event.repository.ts @@ -130,7 +130,7 @@ type UserEvent = { isAdmin: boolean; shouldChangePassword: boolean; avatarColor: UserAvatarColor | null; - oauthId: string; + oauthId: string | null; storageLabel: string | null; quotaSizeInBytes: number | null; quotaUsageInBytes: number; diff --git a/server/src/schema/migrations/1789419229196-ConvertUserOAuthIdEmptyStringToNull.ts b/server/src/schema/migrations/1789419229196-ConvertUserOAuthIdEmptyStringToNull.ts new file mode 100644 index 000000000..06390afaa --- /dev/null +++ b/server/src/schema/migrations/1789419229196-ConvertUserOAuthIdEmptyStringToNull.ts @@ -0,0 +1,13 @@ +import { Kysely, sql } from 'kysely'; + +export async function up(db: Kysely): Promise { + await sql`ALTER TABLE "user" ALTER COLUMN "oauthId" DROP NOT NULL;`.execute(db); + await sql`ALTER TABLE "user" ALTER COLUMN "oauthId" SET DEFAULT NULL;`.execute(db); + await sql`UPDATE "user" SET "oauthId" = NULL WHERE "oauthId" = '';`.execute(db); +} + +export async function down(db: Kysely): Promise { + await sql`UPDATE "user" SET "oauthId" = '' WHERE "oauthId" IS NULL;`.execute(db); + await sql`ALTER TABLE "user" ALTER COLUMN "oauthId" SET DEFAULT '';`.execute(db); + await sql`ALTER TABLE "user" ALTER COLUMN "oauthId" SET NOT NULL;`.execute(db); +} diff --git a/server/src/schema/migrations/ORDER b/server/src/schema/migrations/ORDER index 7d049296a..f4bd7df46 100644 --- a/server/src/schema/migrations/ORDER +++ b/server/src/schema/migrations/ORDER @@ -94,3 +94,4 @@ 1786972746372-AssetOcrSyncReset 1787148183729-ClusterGroups 1787148183730-DeleteMismatchedMemoryAssets +1789419229196-ConvertUserOAuthIdEmptyStringToNull diff --git a/server/src/schema/tables/user.table.ts b/server/src/schema/tables/user.table.ts index 5851f7ad7..e2235e2bc 100644 --- a/server/src/schema/tables/user.table.ts +++ b/server/src/schema/tables/user.table.ts @@ -57,8 +57,8 @@ export class UserTable { @DeleteDateColumn() deletedAt!: Timestamp | null; - @Column({ default: '' }) - oauthId!: Generated; + @Column({ nullable: true }) + oauthId!: string | null; @UpdateDateColumn() updatedAt!: Generated; diff --git a/server/src/services/auth-admin.service.ts b/server/src/services/auth-admin.service.ts index 0a42ec41c..a5a3ffae0 100644 --- a/server/src/services/auth-admin.service.ts +++ b/server/src/services/auth-admin.service.ts @@ -5,7 +5,6 @@ import { BaseService } from 'src/services/base.service.js'; @Injectable() export class AuthAdminService extends BaseService { async unlinkAll(_auth: AuthDto) { - // TODO replace '' with null - await this.userRepository.updateAll({ oauthId: '' }); + await this.userRepository.updateAll({ oauthId: null }); } } diff --git a/server/src/services/auth.service.spec.ts b/server/src/services/auth.service.spec.ts index be2d7fd3c..cc85a7902 100644 --- a/server/src/services/auth.service.spec.ts +++ b/server/src/services/auth.service.spec.ts @@ -1057,12 +1057,13 @@ describe(AuthService.name, () => { }); it('should not sync the profile picture if the user already has one', async () => { - const user = UserFactory.create({ oauthId: 'oauth-id', profileImagePath: 'not-empty' }); + const oauthId = 'oauth-id'; + const user = UserFactory.create({ oauthId, profileImagePath: 'not-empty' }); mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthEnabled); mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ profile: OAuthProfileFactory.create({ - sub: user.oauthId, + sub: oauthId, email: user.email, picture: 'https://auth.immich.cloud/profiles/1.jpg', }), @@ -1181,11 +1182,12 @@ describe(AuthService.name, () => { }); it('should promote an existing user to admin if the role claim contains admin on login', async () => { - const user = UserFactory.create({ isAdmin: false, oauthId: 'oauth-id' }); + const oauthId = 'oauth-id'; + const user = UserFactory.create({ isAdmin: false, oauthId }); mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthEnabled); mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ - profile: OAuthProfileFactory.create({ sub: user.oauthId, immich_role: 'admin' }), + profile: OAuthProfileFactory.create({ sub: oauthId, immich_role: 'admin' }), }); mocks.user.getByOAuthId.mockResolvedValue(user); mocks.user.update.mockResolvedValue({ ...user, isAdmin: true }); @@ -1201,11 +1203,12 @@ describe(AuthService.name, () => { }); it('should demote an existing admin if the role claim only contains user on login', async () => { - const user = UserFactory.create({ isAdmin: true, oauthId: 'oauth-id' }); + const oauthId = 'oauth-id'; + const user = UserFactory.create({ isAdmin: true, oauthId }); mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthEnabled); mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ - profile: OAuthProfileFactory.create({ sub: user.oauthId, immich_role: ['user'] }), + profile: OAuthProfileFactory.create({ sub: oauthId, immich_role: ['user'] }), }); mocks.user.getByOAuthId.mockResolvedValue(user); mocks.user.update.mockResolvedValue({ ...user, isAdmin: false }); @@ -1221,11 +1224,12 @@ describe(AuthService.name, () => { }); it('should not change isAdmin for an existing user if the role claim is blank', async () => { - const user = UserFactory.create({ isAdmin: true, oauthId: 'oauth-id' }); + const oauthId = 'oauth-id'; + const user = UserFactory.create({ isAdmin: true, oauthId }); mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthEnabled); mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ - profile: OAuthProfileFactory.create({ sub: user.oauthId }), + profile: OAuthProfileFactory.create({ sub: oauthId }), }); mocks.user.getByOAuthId.mockResolvedValue(user); mocks.session.create.mockResolvedValue(SessionFactory.create()); @@ -1334,7 +1338,7 @@ describe(AuthService.name, () => { await sut.unlink(auth); - expect(mocks.user.update).toHaveBeenCalledWith(auth.user.id, { oauthId: '' }); + expect(mocks.user.update).toHaveBeenCalledWith(auth.user.id, { oauthId: null }); }); it('should unlink an account and remove the OAuth data from the session', async () => { @@ -1349,7 +1353,7 @@ describe(AuthService.name, () => { await sut.unlink(auth); expect(mocks.session.update).toHaveBeenCalledWith(session.id, { oauthSid: null, oauthBearerToken: null }); - expect(mocks.user.update).toHaveBeenCalledWith(auth.user.id, { oauthId: '' }); + expect(mocks.user.update).toHaveBeenCalledWith(auth.user.id, { oauthId: null }); }); }); diff --git a/server/src/services/auth.service.ts b/server/src/services/auth.service.ts index f257bc115..dcfb60fc9 100644 --- a/server/src/services/auth.service.ts +++ b/server/src/services/auth.service.ts @@ -441,7 +441,7 @@ export class AuthService extends BaseService { await this.sessionRepository.update(auth.session.id, { oauthSid: null, oauthBearerToken: null }); } - const user = await this.userRepository.update(auth.user.id, { oauthId: '' }); + const user = await this.userRepository.update(auth.user.id, { oauthId: null }); return mapUserAdmin(user); } diff --git a/server/src/services/sync.service.ts b/server/src/services/sync.service.ts index d72d79b7d..9f694974a 100644 --- a/server/src/services/sync.service.ts +++ b/server/src/services/sync.service.ts @@ -63,6 +63,7 @@ const sendEntityBackfillCompleteAck = async (response: Writable, ackType: SyncEn export const SYNC_TYPES_ORDER = [ SyncRequestType.AuthUsersV1, + SyncRequestType.AuthUsersV2, SyncRequestType.UsersV1, SyncRequestType.PartnersV1, SyncRequestType.AssetsV1, @@ -190,6 +191,7 @@ export class SyncService extends BaseService { [SyncRequestType.AlbumAssetsV1]: () => this.syncAlbumAssetsV1(), [SyncRequestType.AuthUsersV1]: () => this.syncAuthUsersV1(options, response, checkpointMap), + [SyncRequestType.AuthUsersV2]: () => this.syncAuthUsersV2(options, response, checkpointMap), [SyncRequestType.UsersV1]: () => this.syncUsersV1(options, response, checkpointMap), [SyncRequestType.PartnersV1]: () => this.syncPartnersV1(options, response, checkpointMap), [SyncRequestType.AssetsV2]: () => this.syncAssetsV2(options, response, checkpointMap), @@ -266,6 +268,18 @@ export class SyncService extends BaseService { private async syncAuthUsersV1(options: SyncQueryOptions, response: Writable, checkpointMap: CheckpointMap) { const upsertType = SyncEntityType.AuthUserV1; const upserts = this.syncRepository.authUser.getUpserts({ ...options, ack: checkpointMap[upsertType] }); + for await (const { updateId, profileImagePath, ...data } of upserts) { + await send(response, { + type: upsertType, + ids: [updateId], + data: { ...data, oauthId: data.oauthId ?? '', hasProfileImage: !!profileImagePath }, + }); + } + } + + private async syncAuthUsersV2(options: SyncQueryOptions, response: Writable, checkpointMap: CheckpointMap) { + const upsertType = SyncEntityType.AuthUserV2; + const upserts = this.syncRepository.authUser.getUpserts({ ...options, ack: checkpointMap[upsertType] }); for await (const { updateId, profileImagePath, ...data } of upserts) { await send(response, { type: upsertType, diff --git a/server/test/factories/user.factory.ts b/server/test/factories/user.factory.ts index 8569177fc..64d562b6d 100644 --- a/server/test/factories/user.factory.ts +++ b/server/test/factories/user.factory.ts @@ -27,7 +27,7 @@ export class UserFactory { shouldChangePassword: false, avatarColor: null, deletedAt: null, - oauthId: '', + oauthId: null, updatedAt: newDate(), storageLabel: null, name: 'Test User', diff --git a/server/test/fixtures/user.stub.ts b/server/test/fixtures/user.stub.ts index 9834ea31e..c48418dd7 100644 --- a/server/test/fixtures/user.stub.ts +++ b/server/test/fixtures/user.stub.ts @@ -11,7 +11,7 @@ export const userStub = { name: 'admin_name', id: 'admin_id', storageLabel: 'admin', - oauthId: '', + oauthId: null, shouldChangePassword: false, avatarColor: null, profileImagePath: '', @@ -29,7 +29,7 @@ export const userStub = { profileChangedAt: new Date('2021-01-01'), name: 'immich_name', storageLabel: null, - oauthId: '', + oauthId: null, shouldChangePassword: false, avatarColor: null, profileImagePath: '', diff --git a/server/test/medium.factory.ts b/server/test/medium.factory.ts index 159af1a45..6913a41fb 100644 --- a/server/test/medium.factory.ts +++ b/server/test/medium.factory.ts @@ -761,7 +761,7 @@ const userInsert = (user: Partial> & { clusterGroupId: str shouldChangePassword: true, storageLabel: null, pinCode: null, - oauthId: '', + oauthId: null, avatarColor: null, quotaSizeInBytes: null, quotaUsageInBytes: 0, diff --git a/server/test/medium/specs/services/auth-admin.service.spec.ts b/server/test/medium/specs/services/auth-admin.service.spec.ts index 1a3e0023a..623d536b1 100644 --- a/server/test/medium/specs/services/auth-admin.service.spec.ts +++ b/server/test/medium/specs/services/auth-admin.service.spec.ts @@ -31,7 +31,7 @@ describe(AuthAdminService.name, () => { await expect(sut.unlinkAll(auth)).resolves.toBeUndefined(); await expect(userRepo.get(user.id, { withDeleted: true })).resolves.toEqual( - expect.objectContaining({ oauthId: '' }), + expect.objectContaining({ oauthId: null }), ); }); @@ -43,7 +43,7 @@ describe(AuthAdminService.name, () => { await expect(sut.unlinkAll(auth)).resolves.toBeUndefined(); await expect(userRepo.get(user.id, { withDeleted: true })).resolves.toEqual( - expect.objectContaining({ oauthId: '' }), + expect.objectContaining({ oauthId: null }), ); }); @@ -56,10 +56,10 @@ describe(AuthAdminService.name, () => { await expect(sut.unlinkAll(auth)).resolves.toBeUndefined(); await expect(userRepo.get(user1.id, { withDeleted: true })).resolves.toEqual( - expect.objectContaining({ oauthId: '' }), + expect.objectContaining({ oauthId: null }), ); await expect(userRepo.get(user2.id, { withDeleted: true })).resolves.toEqual( - expect.objectContaining({ oauthId: '' }), + expect.objectContaining({ oauthId: null }), ); }); }); diff --git a/server/test/medium/specs/sync/sync-auth-user.spec.ts b/server/test/medium/specs/sync/sync-auth-user.spec.ts index 255255eda..175a7ff22 100644 --- a/server/test/medium/specs/sync/sync-auth-user.spec.ts +++ b/server/test/medium/specs/sync/sync-auth-user.spec.ts @@ -17,11 +17,11 @@ beforeAll(async () => { defaultDatabase = await getKyselyDB(); }); -describe(SyncEntityType.AuthUserV1, () => { +describe(SyncEntityType.AuthUserV2, () => { it('should detect and sync the first user', async () => { const { auth, user, ctx } = await setup(await getKyselyDB()); - const response = await ctx.syncStream(auth, [SyncRequestType.AuthUsersV1]); + const response = await ctx.syncStream(auth, [SyncRequestType.AuthUsersV2]); expect(response).toEqual([ { ack: expect.any(String), @@ -40,13 +40,13 @@ describe(SyncEntityType.AuthUserV1, () => { quotaUsageInBytes: user.quotaUsageInBytes, storageLabel: user.storageLabel, }, - type: 'AuthUserV1', + type: 'AuthUserV2', }, expect.objectContaining({ type: SyncEntityType.SyncCompleteV1 }), ]); await ctx.syncAckAll(auth, response); - await ctx.assertSyncIsComplete(auth, [SyncRequestType.AuthUsersV1]); + await ctx.assertSyncIsComplete(auth, [SyncRequestType.AuthUsersV2]); }); it('should sync a change and then another change to that same user', async () => { @@ -54,7 +54,7 @@ describe(SyncEntityType.AuthUserV1, () => { const userRepo = ctx.get(UserRepository); - const response = await ctx.syncStream(auth, [SyncRequestType.AuthUsersV1]); + const response = await ctx.syncStream(auth, [SyncRequestType.AuthUsersV2]); expect(response).toEqual([ { ack: expect.any(String), @@ -62,7 +62,7 @@ describe(SyncEntityType.AuthUserV1, () => { id: user.id, isAdmin: false, }), - type: 'AuthUserV1', + type: 'AuthUserV2', }, expect.objectContaining({ type: SyncEntityType.SyncCompleteV1 }), ]); @@ -71,7 +71,7 @@ describe(SyncEntityType.AuthUserV1, () => { await userRepo.update(user.id, { isAdmin: true }); - const newResponse = await ctx.syncStream(auth, [SyncRequestType.AuthUsersV1]); + const newResponse = await ctx.syncStream(auth, [SyncRequestType.AuthUsersV2]); expect(newResponse).toEqual([ { ack: expect.any(String), @@ -79,7 +79,7 @@ describe(SyncEntityType.AuthUserV1, () => { id: user.id, isAdmin: true, }), - type: 'AuthUserV1', + type: 'AuthUserV2', }, expect.objectContaining({ type: SyncEntityType.SyncCompleteV1 }), ]); @@ -90,7 +90,7 @@ describe(SyncEntityType.AuthUserV1, () => { await ctx.newUser(); - const response = await ctx.syncStream(auth, [SyncRequestType.AuthUsersV1]); + const response = await ctx.syncStream(auth, [SyncRequestType.AuthUsersV2]); expect(response).toEqual([ { ack: expect.any(String), @@ -98,7 +98,7 @@ describe(SyncEntityType.AuthUserV1, () => { id: user.id, isAdmin: false, }), - type: 'AuthUserV1', + type: 'AuthUserV2', }, expect.objectContaining({ type: SyncEntityType.SyncCompleteV1 }), ]); diff --git a/server/test/small.factory.ts b/server/test/small.factory.ts index d81c620b9..14a9f1273 100644 --- a/server/test/small.factory.ts +++ b/server/test/small.factory.ts @@ -120,7 +120,7 @@ const userAdminFactory = (user: Partial = {}) => { createdAt = newDate(), updatedAt = newDate(), deletedAt = null, - oauthId = '', + oauthId = null, quotaSizeInBytes = null, quotaUsageInBytes = 0, status = UserStatus.Active,