diff --git a/server/src/queries/session.repository.sql b/server/src/queries/session.repository.sql index a29b6f7cc3..f68f2dbe95 100644 --- a/server/src/queries/session.repository.sql +++ b/server/src/queries/session.repository.sql @@ -4,7 +4,8 @@ select "id", "expiresAt", - "pinExpiresAt" + "pinExpiresAt", + "oauthBearerToken" from "session" where diff --git a/server/src/repositories/oauth.repository.ts b/server/src/repositories/oauth.repository.ts index c3f199d246..1a09fff70d 100644 --- a/server/src/repositories/oauth.repository.ts +++ b/server/src/repositories/oauth.repository.ts @@ -83,7 +83,7 @@ export class OAuthRepository { url: string, expectedState: string, codeVerifier: string, - ): Promise<{ profile: OAuthProfile; sid?: string }> { + ): Promise<{ profile: OAuthProfile; sid?: string; idToken?: string }> { const client = await this.getClient(config); const pkceCodeVerifier = client.serverMetadata().supportsPKCE() ? codeVerifier : undefined; @@ -111,7 +111,7 @@ export class OAuthRepository { } } - return { profile, sid }; + return { profile, sid, idToken: tokens.id_token }; } catch (error: Error | any) { if (error.message.includes('unexpected JWT alg received')) { this.logger.warn( diff --git a/server/src/repositories/session.repository.ts b/server/src/repositories/session.repository.ts index 451b2263e5..f1cb541b39 100644 --- a/server/src/repositories/session.repository.ts +++ b/server/src/repositories/session.repository.ts @@ -32,7 +32,7 @@ export class SessionRepository { get(id: string) { return this.db .selectFrom('session') - .select(['id', 'expiresAt', 'pinExpiresAt']) + .select(['id', 'expiresAt', 'pinExpiresAt', 'oauthBearerToken']) .where('id', '=', id) .executeTakeFirst(); } diff --git a/server/src/schema/migrations/1784647658615-AddOAuthBearerTokenToSession.ts b/server/src/schema/migrations/1784647658615-AddOAuthBearerTokenToSession.ts new file mode 100644 index 0000000000..01994a2ab5 --- /dev/null +++ b/server/src/schema/migrations/1784647658615-AddOAuthBearerTokenToSession.ts @@ -0,0 +1,9 @@ +import { Kysely, sql } from 'kysely'; + +export async function up(db: Kysely): Promise { + await sql`ALTER TABLE "session" ADD "oauthBearerToken" character varying;`.execute(db); +} + +export async function down(db: Kysely): Promise { + await sql`ALTER TABLE "session" DROP COLUMN "oauthBearerToken";`.execute(db); +} diff --git a/server/src/schema/tables/session.table.ts b/server/src/schema/tables/session.table.ts index 950c1eeffd..68381b7f8e 100644 --- a/server/src/schema/tables/session.table.ts +++ b/server/src/schema/tables/session.table.ts @@ -55,4 +55,7 @@ export class SessionTable { @Column({ nullable: true, index: true }) oauthSid!: string | null; + + @Column({ nullable: true }) + oauthBearerToken!: string | null; } diff --git a/server/src/services/auth.service.spec.ts b/server/src/services/auth.service.spec.ts index ed5b553a5b..5c76b38542 100644 --- a/server/src/services/auth.service.spec.ts +++ b/server/src/services/auth.service.spec.ts @@ -160,7 +160,25 @@ describe(AuthService.name, () => { await expect(sut.logout(auth, AuthType.OAuth)).resolves.toEqual({ successful: true, - redirectUri: 'http://end-session-endpoint', + redirectUri: 'http://end-session-endpoint/', + }); + }); + + it('should include the id token hint for OAuth sessions', async () => { + const auth = AuthFactory.from().session().build(); + + mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.enabled); + mocks.session.get.mockResolvedValue({ + id: auth.session!.id, + expiresAt: null, + oauthBearerToken: 'id-token', + pinExpiresAt: null, + }); + mocks.session.delete.mockResolvedValue(); + + await expect(sut.logout(auth, AuthType.OAuth)).resolves.toEqual({ + successful: true, + redirectUri: 'http://end-session-endpoint/?id_token_hint=id-token', }); }); @@ -173,7 +191,7 @@ describe(AuthService.name, () => { await expect(sut.logout(auth, AuthType.OAuth)).resolves.toEqual({ successful: true, - redirectUri: 'http://custom-logout-url', + redirectUri: 'http://custom-logout-url/', }); }); @@ -186,7 +204,7 @@ describe(AuthService.name, () => { await expect(sut.logout(auth, AuthType.OAuth)).resolves.toEqual({ successful: true, - redirectUri: 'http://end-session-endpoint', + redirectUri: 'http://end-session-endpoint/', }); }); @@ -201,6 +219,12 @@ describe(AuthService.name, () => { it('should delete the access token', async () => { const auth = { user: { id: '123' }, session: { id: 'token123' } } as AuthDto; + mocks.session.get.mockResolvedValue({ + id: auth.session!.id, + expiresAt: null, + oauthBearerToken: null, + pinExpiresAt: null, + }); mocks.session.delete.mockResolvedValue(); await expect(sut.logout(auth, AuthType.Password)).resolves.toEqual({ @@ -722,6 +746,27 @@ describe(AuthService.name, () => { expect(mocks.user.update).toHaveBeenCalledWith(user.id, { oauthId: profile.sub }); }); + it('should store the OAuth bearer token on the new session', async () => { + const user = UserFactory.create(); + const profile = OAuthProfileFactory.create(); + + mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthEnabled); + mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ profile, sid: 'oauth-sid', idToken: 'oauth-bearer-token' }); + mocks.user.getByEmail.mockResolvedValue(user); + mocks.user.update.mockResolvedValue(user); + mocks.session.create.mockResolvedValue(SessionFactory.create()); + + await sut.callback( + { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foobar' }, + {}, + loginDetails, + ); + + expect(mocks.session.create).toHaveBeenCalledWith( + expect.objectContaining({ oauthSid: 'oauth-sid', oauthBearerToken: 'oauth-bearer-token' }), + ); + }); + it('should normalize the email from the OAuth profile before linking', async () => { const user = UserFactory.create(); const profile = OAuthProfileFactory.create({ email: ' TEST@IMMICH.CLOUD ' }); @@ -1125,6 +1170,7 @@ describe(AuthService.name, () => { mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ profile: { sub: 'sub' }, sid: session.oauthSid ?? undefined, + idToken: session.oauthBearerToken ?? undefined, }); mocks.user.update.mockResolvedValue(user); mocks.session.update.mockResolvedValue(session); @@ -1135,7 +1181,10 @@ describe(AuthService.name, () => { {}, ); - expect(mocks.session.update).toHaveBeenCalledWith(session.id, { oauthSid: session.oauthSid }); + expect(mocks.session.update).toHaveBeenCalledWith(session.id, { + oauthSid: session.oauthSid, + oauthBearerToken: session.oauthBearerToken, + }); expect(mocks.user.update).toHaveBeenCalledWith(auth.user.id, { oauthId: 'sub' }); }); @@ -1169,7 +1218,7 @@ describe(AuthService.name, () => { expect(mocks.user.update).toHaveBeenCalledWith(auth.user.id, { oauthId: '' }); }); - it('should unlink an account and remove the oauthSid from the session', async () => { + it('should unlink an account and remove the OAuth data from the session', async () => { const user = UserFactory.create(); const session = SessionFactory.create(); const auth = AuthFactory.from(user).session(session).build(); @@ -1180,7 +1229,7 @@ describe(AuthService.name, () => { await sut.unlink(auth); - expect(mocks.session.update).toHaveBeenCalledWith(session.id, { oauthSid: null }); + expect(mocks.session.update).toHaveBeenCalledWith(session.id, { oauthSid: null, oauthBearerToken: null }); expect(mocks.user.update).toHaveBeenCalledWith(auth.user.id, { oauthId: '' }); }); }); diff --git a/server/src/services/auth.service.ts b/server/src/services/auth.service.ts index 133eef2cf2..d30c912194 100644 --- a/server/src/services/auth.service.ts +++ b/server/src/services/auth.service.ts @@ -76,14 +76,17 @@ export class AuthService extends BaseService { } async logout(auth: AuthDto, authType: AuthType): Promise { + let oauthBearerToken: string | undefined; if (auth.session) { + const session = await this.sessionRepository.get(auth.session.id); + oauthBearerToken = session?.oauthBearerToken ?? undefined; await this.sessionRepository.delete(auth.session.id); await this.eventRepository.emit('SessionDelete', { sessionId: auth.session.id }); } return { successful: true, - redirectUri: await this.getLogoutEndpoint(authType), + redirectUri: await this.getLogoutEndpoint(authType, oauthBearerToken), }; } @@ -306,12 +309,11 @@ export class AuthService extends BaseService { } const url = this.resolveRedirectUri(oauth, dto.url); - const { profile, sid: oauthSid } = await this.oauthRepository.getProfileAndOAuthSid( - oauth, - url, - expectedState, - codeVerifier, - ); + const { + profile, + sid: oauthSid, + idToken: oauthBearerToken, + } = await this.oauthRepository.getProfileAndOAuthSid(oauth, url, expectedState, codeVerifier); const normalizedEmail = profile.email ? profile.email.trim().toLowerCase() : undefined; const { autoRegister, defaultStorageQuota, storageLabelClaim, storageQuotaClaim, roleClaim } = oauth; this.logger.debug(`Logging in with OAuth: ${JSON.stringify(profile)}`); @@ -378,7 +380,7 @@ export class AuthService extends BaseService { await this.syncProfilePicture(user, profile.picture); } - return this.createLoginResponse(user, loginDetails, oauthSid); + return this.createLoginResponse(user, loginDetails, oauthSid, oauthBearerToken); } private async syncProfilePicture(user: UserAdmin, url: string) { @@ -419,6 +421,7 @@ export class AuthService extends BaseService { const { profile: { sub: oauthId }, sid, + idToken, } = await this.oauthRepository.getProfileAndOAuthSid(oauth, dto.url, expectedState, codeVerifier); const duplicate = await this.userRepository.getByOAuthId(oauthId); if (duplicate && duplicate.id !== auth.user.id) { @@ -426,8 +429,11 @@ export class AuthService extends BaseService { throw new BadRequestException('This OAuth account has already been linked to another user.'); } - if (auth.session && sid) { - await this.sessionRepository.update(auth.session.id, { oauthSid: sid }); + if (auth.session && (sid || idToken)) { + await this.sessionRepository.update(auth.session.id, { + oauthSid: sid, + oauthBearerToken: idToken, + }); } const user = await this.userRepository.update(auth.user.id, { oauthId }); @@ -436,14 +442,14 @@ export class AuthService extends BaseService { async unlink(auth: AuthDto): Promise { if (auth.session) { - await this.sessionRepository.update(auth.session.id, { oauthSid: null }); + await this.sessionRepository.update(auth.session.id, { oauthSid: null, oauthBearerToken: null }); } const user = await this.userRepository.update(auth.user.id, { oauthId: '' }); return mapUserAdmin(user); } - private async getLogoutEndpoint(authType: AuthType): Promise { + private async getLogoutEndpoint(authType: AuthType, oauthBearerToken?: string | null): Promise { if (authType !== AuthType.OAuth) { return LOGIN_URL; } @@ -453,11 +459,20 @@ export class AuthService extends BaseService { return LOGIN_URL; } - if (config.oauth.endSessionEndpoint) { - return config.oauth.endSessionEndpoint; + const endSessionEndpoint = + config.oauth.endSessionEndpoint || (await this.oauthRepository.getLogoutEndpoint(config.oauth)); + + if (!endSessionEndpoint) { + return LOGIN_URL; } - return (await this.oauthRepository.getLogoutEndpoint(config.oauth)) || LOGIN_URL; + const url = new URL(endSessionEndpoint); + + if (oauthBearerToken) { + url.searchParams.set('id_token_hint', oauthBearerToken); + } + + return url.href; } private getBearerToken(headers: IncomingHttpHeaders): string | null { @@ -599,7 +614,12 @@ export class AuthService extends BaseService { await this.sessionRepository.update(auth.session.id, { pinExpiresAt: null }); } - private async createLoginResponse(user: UserAdmin, loginDetails: LoginDetails, oauthSid?: string) { + private async createLoginResponse( + user: UserAdmin, + loginDetails: LoginDetails, + oauthSid?: string, + oauthBearerToken?: string, + ) { const token = this.cryptoRepository.randomBytesAsText(32); const hashed = this.cryptoRepository.hashSha256(token); @@ -610,6 +630,7 @@ export class AuthService extends BaseService { appVersion: loginDetails.appVersion, userId: user.id, oauthSid: oauthSid ?? null, + oauthBearerToken: oauthBearerToken ?? null, }); return mapLoginResponse(user, token); diff --git a/server/test/factories/session.factory.ts b/server/test/factories/session.factory.ts index 44a25edcfa..188b8b638c 100644 --- a/server/test/factories/session.factory.ts +++ b/server/test/factories/session.factory.ts @@ -26,6 +26,7 @@ export class SessionFactory { updatedAt: newDate(), userId: newUuid(), oauthSid: newUuid(), + oauthBearerToken: 'oauth-bearer-token', ...dto, }); } diff --git a/server/test/small.factory.ts b/server/test/small.factory.ts index 1c17d955a9..ae83727d70 100644 --- a/server/test/small.factory.ts +++ b/server/test/small.factory.ts @@ -22,7 +22,7 @@ const authFactory = ({ user, }: { apiKey?: Partial; - session?: { id?: string; hasElevatedPermission?: boolean }; + session?: { id?: string; hasElevatedPermission?: boolean; oauthBearerToken?: string | null }; user?: Omit< Partial, 'createdAt' | 'updatedAt' | 'deletedAt' | 'fileCreatedAt' | 'fileModifiedAt' | 'localDateTime' | 'profileChangedAt'