Skip to content

Commit

Permalink
chore: migrate oauth to repo
Browse files Browse the repository at this point in the history
  • Loading branch information
danieldietzler committed Oct 5, 2024
1 parent 9d9bf1c commit a34efb6
Show file tree
Hide file tree
Showing 8 changed files with 167 additions and 121 deletions.
22 changes: 22 additions & 0 deletions server/src/interfaces/oauth.interface.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { UserinfoResponse } from 'openid-client';

export const IOAuthRepository = 'IOAuthRepository';

export type OAuthConfig = {
clientId: string;
clientSecret: string;
issuerUrl: string;
mobileOverrideEnabled: boolean;
mobileRedirectUri: string;
profileSigningAlgorithm: string;
scope: string;
signingAlgorithm: string;
};
export type OAuthProfile = UserinfoResponse;

export interface IOAuthRepository {
init(): void;
authorize(config: OAuthConfig, redirectUrl: string): Promise<string>;
getLogoutEndpoint(config: OAuthConfig): Promise<string | undefined>;
getProfile(config: OAuthConfig, url: string): Promise<OAuthProfile>;
}
3 changes: 3 additions & 0 deletions server/src/repositories/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { IMetadataRepository } from 'src/interfaces/metadata.interface';
import { IMetricRepository } from 'src/interfaces/metric.interface';
import { IMoveRepository } from 'src/interfaces/move.interface';
import { INotificationRepository } from 'src/interfaces/notification.interface';
import { IOAuthRepository } from 'src/interfaces/oauth.interface';
import { IPartnerRepository } from 'src/interfaces/partner.interface';
import { IPersonRepository } from 'src/interfaces/person.interface';
import { ISearchRepository } from 'src/interfaces/search.interface';
Expand Down Expand Up @@ -56,6 +57,7 @@ import { MetadataRepository } from 'src/repositories/metadata.repository';
import { MetricRepository } from 'src/repositories/metric.repository';
import { MoveRepository } from 'src/repositories/move.repository';
import { NotificationRepository } from 'src/repositories/notification.repository';
import { OAuthRepository } from 'src/repositories/oauth.repository';
import { PartnerRepository } from 'src/repositories/partner.repository';
import { PersonRepository } from 'src/repositories/person.repository';
import { SearchRepository } from 'src/repositories/search.repository';
Expand Down Expand Up @@ -94,6 +96,7 @@ export const repositories = [
{ provide: IMetricRepository, useClass: MetricRepository },
{ provide: IMoveRepository, useClass: MoveRepository },
{ provide: INotificationRepository, useClass: NotificationRepository },
{ provide: IOAuthRepository, useClass: OAuthRepository },
{ provide: IPartnerRepository, useClass: PartnerRepository },
{ provide: IPersonRepository, useClass: PersonRepository },
{ provide: ISearchRepository, useClass: SearchRepository },
Expand Down
84 changes: 84 additions & 0 deletions server/src/repositories/oauth.repository.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { Inject, Injectable, InternalServerErrorException } from '@nestjs/common';
import { custom, generators, Issuer } from 'openid-client';
import { ILoggerRepository } from 'src/interfaces/logger.interface';
import { IOAuthRepository, OAuthConfig, OAuthProfile } from 'src/interfaces/oauth.interface';
import { Instrumentation } from 'src/utils/instrumentation';

@Instrumentation()
@Injectable()
export class OAuthRepository implements IOAuthRepository {
constructor(@Inject(ILoggerRepository) private logger: ILoggerRepository) {
this.logger.setContext(OAuthRepository.name);
}

init() {
custom.setHttpOptionsDefaults({ timeout: 30_000 });
}

async authorize(config: OAuthConfig, redirectUrl: string) {
const client = await this.getClient(config);
return client.authorizationUrl({
redirect_uri: redirectUrl,
scope: config.scope,
state: generators.state(),
});
}

async getLogoutEndpoint(config: OAuthConfig) {
const client = await this.getClient(config);
return client.issuer.metadata.end_session_endpoint;
}

async getProfile(config: OAuthConfig, url: string): Promise<OAuthProfile> {
const client = await this.getClient(config);
const params = client.callbackParams(url);
try {
const tokens = await client.callback(this.normalize(config, url.split('?')[0]), params, { state: params.state });
return await client.userinfo<OAuthProfile>(tokens.access_token || '');
} catch (error: Error | any) {
if (error.message.includes('unexpected JWT alg received')) {
this.logger.warn(
[
'Algorithm mismatch. Make sure the signing algorithm is set correctly in the OAuth settings.',
'Or, that you have specified a signing key in your OAuth provider.',
].join(' '),
);
}

throw error;
}
}

private async getClient({
issuerUrl,
clientId,
clientSecret,
profileSigningAlgorithm,
signingAlgorithm,
}: OAuthConfig) {
try {
const issuer = await Issuer.discover(issuerUrl);
return new issuer.Client({
client_id: clientId,
client_secret: clientSecret,
response_types: ['code'],
userinfo_signed_response_alg: profileSigningAlgorithm === 'none' ? undefined : profileSigningAlgorithm,
id_token_signed_response_alg: signingAlgorithm,
});
} catch (error: any | AggregateError) {
this.logger.error(`Error in OAuth discovery: ${error}`, error?.stack, error?.errors);
throw new InternalServerErrorException(`Error in OAuth discovery: ${error}`, { cause: error });
}
}

private normalize(
{ mobileRedirectUri, mobileOverrideEnabled }: { mobileRedirectUri: string; mobileOverrideEnabled: boolean },
redirectUri: string,
) {
const isMobile = redirectUri.startsWith('app.immich:/');
if (isMobile && mobileOverrideEnabled && mobileRedirectUri) {
return mobileRedirectUri;
}
return redirectUri;
}
}
69 changes: 26 additions & 43 deletions server/src/services/auth.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { BadRequestException, ForbiddenException, UnauthorizedException } from '@nestjs/common';
import { Issuer, generators } from 'openid-client';
import { AuthDto, SignUpDto } from 'src/dtos/auth.dto';
import { UserMetadataEntity } from 'src/entities/user-metadata.entity';
import { UserEntity } from 'src/entities/user.entity';
import { AuthType, Permission } from 'src/enum';
import { IKeyRepository } from 'src/interfaces/api-key.interface';
import { ICryptoRepository } from 'src/interfaces/crypto.interface';
import { IEventRepository } from 'src/interfaces/event.interface';
import { IOAuthRepository } from 'src/interfaces/oauth.interface';
import { ISessionRepository } from 'src/interfaces/session.interface';
import { ISharedLinkRepository } from 'src/interfaces/shared-link.interface';
import { ISystemMetadataRepository } from 'src/interfaces/system-metadata.interface';
Expand All @@ -19,7 +19,7 @@ import { sharedLinkStub } from 'test/fixtures/shared-link.stub';
import { systemConfigStub } from 'test/fixtures/system-config.stub';
import { userStub } from 'test/fixtures/user.stub';
import { newTestService } from 'test/utils';
import { Mock, Mocked, vitest } from 'vitest';
import { Mocked } from 'vitest';

// const token = Buffer.from('my-api-key', 'utf8').toString('base64');

Expand Down Expand Up @@ -53,36 +53,19 @@ describe('AuthService', () => {
let cryptoMock: Mocked<ICryptoRepository>;
let eventMock: Mocked<IEventRepository>;
let keyMock: Mocked<IKeyRepository>;
let oauthMock: Mocked<IOAuthRepository>;
let sessionMock: Mocked<ISessionRepository>;
let sharedLinkMock: Mocked<ISharedLinkRepository>;
let systemMock: Mocked<ISystemMetadataRepository>;
let userMock: Mocked<IUserRepository>;

let callbackMock: Mock;
let userinfoMock: Mock;

beforeEach(() => {
callbackMock = vitest.fn().mockReturnValue({ access_token: 'access-token' });
userinfoMock = vitest.fn().mockResolvedValue({ sub, email });

vitest.spyOn(generators, 'state').mockReturnValue('state');
vitest.spyOn(Issuer, 'discover').mockResolvedValue({
id_token_signing_alg_values_supported: ['RS256'],
Client: vitest.fn().mockResolvedValue({
issuer: {
metadata: {
end_session_endpoint: 'http://end-session-endpoint',
},
},
authorizationUrl: vitest.fn().mockReturnValue('http://authorization-url'),
callbackParams: vitest.fn().mockReturnValue({ state: 'state' }),
callback: callbackMock,
userinfo: userinfoMock,
}),
} as any);

({ sut, cryptoMock, eventMock, keyMock, sessionMock, sharedLinkMock, systemMock, userMock } =
({ sut, cryptoMock, eventMock, keyMock, oauthMock, sessionMock, sharedLinkMock, systemMock, userMock } =
newTestService(AuthService));

oauthMock.authorize.mockResolvedValue('access-token');
oauthMock.getProfile.mockResolvedValue({ sub, email });
oauthMock.getLogoutEndpoint.mockResolvedValue('http://end-session-endpoint');
});

it('should be defined', () => {
Expand Down Expand Up @@ -515,21 +498,21 @@ describe('AuthService', () => {
expect(userMock.create).toHaveBeenCalledTimes(1);
});

// TODO write once oidc has been moved to a repo and can be mocked.
// it('should throw an error if user should be auto registered but the email claim does not exist', async () => {
// systemMock.get.mockResolvedValue(systemConfigStub.enabled);
// userMock.getByEmail.mockResolvedValue(null);
// userMock.getAdmin.mockResolvedValue(userStub.user1);
// userMock.create.mockResolvedValue(userStub.user1);
// sessionMock.create.mockResolvedValue(sessionStub.valid);
it('should throw an error if user should be auto registered but the email claim does not exist', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.enabled);
userMock.getByEmail.mockResolvedValue(null);
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);
sessionMock.create.mockResolvedValue(sessionStub.valid);
oauthMock.getProfile.mockResolvedValue({ sub, email: undefined });

// await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' }, loginDetails)).rejects.toBeInstanceOf(
// BadRequestException,
// );
await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' }, loginDetails)).rejects.toBeInstanceOf(
BadRequestException,
);

// expect(userMock.getByEmail).toHaveBeenCalledTimes(1);
// expect(userMock.create).toHaveBeenCalledTimes(1);
// });
expect(userMock.getByEmail).not.toHaveBeenCalled();
expect(userMock.create).not.toHaveBeenCalled();
});

for (const url of [
'app.immich:/',
Expand All @@ -545,7 +528,7 @@ describe('AuthService', () => {
sessionMock.create.mockResolvedValue(sessionStub.valid);

await sut.callback({ url }, loginDetails);
expect(callbackMock).toHaveBeenCalledWith('http://mobile-redirect', { state: 'state' }, { state: 'state' });
expect(oauthMock.getProfile).toHaveBeenCalledWith(expect.objectContaining({}), 'http://mobile-redirect');

Check failure on line 531 in server/src/services/auth.service.spec.ts

View workflow job for this annotation

GitHub Actions / Test & Lint Server

src/services/auth.service.spec.ts > AuthService > callback > should use the mobile redirect override for a url of app.immich:/

AssertionError: expected "spy" to be called with arguments: [ ObjectContaining {}, …(1) ] Received: 1st spy call: Array [ - ObjectContaining {}, - "http://mobile-redirect", + Object { + "autoLaunch": false, + "autoRegister": true, + "buttonText": "OAuth", + "clientId": "", + "clientSecret": "", + "defaultStorageQuota": 0, + "enabled": true, + "issuerUrl": "", + "mobileOverrideEnabled": true, + "mobileRedirectUri": "http://mobile-redirect", + "profileSigningAlgorithm": "none", + "scope": "openid email profile", + "signingAlgorithm": "RS256", + "storageLabelClaim": "preferred_username", + "storageQuotaClaim": "immich_quota", + }, + "app.immich:/", ] Number of calls: 1 ❯ src/services/auth.service.spec.ts:531:38

Check failure on line 531 in server/src/services/auth.service.spec.ts

View workflow job for this annotation

GitHub Actions / Test & Lint Server

src/services/auth.service.spec.ts > AuthService > callback > should use the mobile redirect override for a url of app.immich://

AssertionError: expected "spy" to be called with arguments: [ ObjectContaining {}, …(1) ] Received: 1st spy call: Array [ - ObjectContaining {}, - "http://mobile-redirect", + Object { + "autoLaunch": false, + "autoRegister": true, + "buttonText": "OAuth", + "clientId": "", + "clientSecret": "", + "defaultStorageQuota": 0, + "enabled": true, + "issuerUrl": "", + "mobileOverrideEnabled": true, + "mobileRedirectUri": "http://mobile-redirect", + "profileSigningAlgorithm": "none", + "scope": "openid email profile", + "signingAlgorithm": "RS256", + "storageLabelClaim": "preferred_username", + "storageQuotaClaim": "immich_quota", + }, + "app.immich://", ] Number of calls: 1 ❯ src/services/auth.service.spec.ts:531:38

Check failure on line 531 in server/src/services/auth.service.spec.ts

View workflow job for this annotation

GitHub Actions / Test & Lint Server

src/services/auth.service.spec.ts > AuthService > callback > should use the mobile redirect override for a url of app.immich:///

AssertionError: expected "spy" to be called with arguments: [ ObjectContaining {}, …(1) ] Received: 1st spy call: Array [ - ObjectContaining {}, - "http://mobile-redirect", + Object { + "autoLaunch": false, + "autoRegister": true, + "buttonText": "OAuth", + "clientId": "", + "clientSecret": "", + "defaultStorageQuota": 0, + "enabled": true, + "issuerUrl": "", + "mobileOverrideEnabled": true, + "mobileRedirectUri": "http://mobile-redirect", + "profileSigningAlgorithm": "none", + "scope": "openid email profile", + "signingAlgorithm": "RS256", + "storageLabelClaim": "preferred_username", + "storageQuotaClaim": "immich_quota", + }, + "app.immich:///", ] Number of calls: 1 ❯ src/services/auth.service.spec.ts:531:38

Check failure on line 531 in server/src/services/auth.service.spec.ts

View workflow job for this annotation

GitHub Actions / Test & Lint Server

src/services/auth.service.spec.ts > AuthService > callback > should use the mobile redirect override for a url of app.immich:/oauth-callback?code=abc123

AssertionError: expected "spy" to be called with arguments: [ ObjectContaining {}, …(1) ] Received: 1st spy call: Array [ - ObjectContaining {}, - "http://mobile-redirect", + Object { + "autoLaunch": false, + "autoRegister": true, + "buttonText": "OAuth", + "clientId": "", + "clientSecret": "", + "defaultStorageQuota": 0, + "enabled": true, + "issuerUrl": "", + "mobileOverrideEnabled": true, + "mobileRedirectUri": "http://mobile-redirect", + "profileSigningAlgorithm": "none", + "scope": "openid email profile", + "signingAlgorithm": "RS256", + "storageLabelClaim": "preferred_username", + "storageQuotaClaim": "immich_quota", + }, + "app.immich:/oauth-callback?code=abc123", ] Number of calls: 1 ❯ src/services/auth.service.spec.ts:531:38

Check failure on line 531 in server/src/services/auth.service.spec.ts

View workflow job for this annotation

GitHub Actions / Test & Lint Server

src/services/auth.service.spec.ts > AuthService > callback > should use the mobile redirect override for a url of app.immich://oauth-callback?code=abc123

AssertionError: expected "spy" to be called with arguments: [ ObjectContaining {}, …(1) ] Received: 1st spy call: Array [ - ObjectContaining {}, - "http://mobile-redirect", + Object { + "autoLaunch": false, + "autoRegister": true, + "buttonText": "OAuth", + "clientId": "", + "clientSecret": "", + "defaultStorageQuota": 0, + "enabled": true, + "issuerUrl": "", + "mobileOverrideEnabled": true, + "mobileRedirectUri": "http://mobile-redirect", + "profileSigningAlgorithm": "none", + "scope": "openid email profile", + "signingAlgorithm": "RS256", + "storageLabelClaim": "preferred_username", + "storageQuotaClaim": "immich_quota", + }, + "app.immich://oauth-callback?code=abc123", ] Number of calls: 1 ❯ src/services/auth.service.spec.ts:531:38

Check failure on line 531 in server/src/services/auth.service.spec.ts

View workflow job for this annotation

GitHub Actions / Test & Lint Server

src/services/auth.service.spec.ts > AuthService > callback > should use the mobile redirect override for a url of app.immich:///oauth-callback?code=abc123

AssertionError: expected "spy" to be called with arguments: [ ObjectContaining {}, …(1) ] Received: 1st spy call: Array [ - ObjectContaining {}, - "http://mobile-redirect", + Object { + "autoLaunch": false, + "autoRegister": true, + "buttonText": "OAuth", + "clientId": "", + "clientSecret": "", + "defaultStorageQuota": 0, + "enabled": true, + "issuerUrl": "", + "mobileOverrideEnabled": true, + "mobileRedirectUri": "http://mobile-redirect", + "profileSigningAlgorithm": "none", + "scope": "openid email profile", + "signingAlgorithm": "RS256", + "storageLabelClaim": "preferred_username", + "storageQuotaClaim": "immich_quota", + }, + "app.immich:///oauth-callback?code=abc123", ] Number of calls: 1 ❯ src/services/auth.service.spec.ts:531:38
});
}

Expand All @@ -567,7 +550,7 @@ describe('AuthService', () => {
userMock.getByEmail.mockResolvedValue(null);
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);
userinfoMock.mockResolvedValue({ sub, email, immich_quota: 'abc' });
oauthMock.getProfile.mockResolvedValue({ sub, email, immich_quota: 'abc' });

await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' }, loginDetails)).resolves.toEqual(
loginResponseStub.user1oauth,
Expand All @@ -581,7 +564,7 @@ describe('AuthService', () => {
userMock.getByEmail.mockResolvedValue(null);
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);
userinfoMock.mockResolvedValue({ sub, email, immich_quota: -5 });
oauthMock.getProfile.mockResolvedValue({ sub, email, immich_quota: -5 });

await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' }, loginDetails)).resolves.toEqual(
loginResponseStub.user1oauth,
Expand All @@ -595,7 +578,7 @@ describe('AuthService', () => {
userMock.getByEmail.mockResolvedValue(null);
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);
userinfoMock.mockResolvedValue({ sub, email, immich_quota: 0 });
oauthMock.getProfile.mockResolvedValue({ sub, email, immich_quota: 0 });

await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' }, loginDetails)).resolves.toEqual(
loginResponseStub.user1oauth,
Expand All @@ -615,7 +598,7 @@ describe('AuthService', () => {
userMock.getByEmail.mockResolvedValue(null);
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);
userinfoMock.mockResolvedValue({ sub, email, immich_quota: 5 });
oauthMock.getProfile.mockResolvedValue({ sub, email, immich_quota: 5 });

await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' }, loginDetails)).resolves.toEqual(
loginResponseStub.user1oauth,
Expand Down
Loading

0 comments on commit a34efb6

Please sign in to comment.