Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(server): mobile oauth login #13474

Merged
merged 1 commit into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 50 additions & 2 deletions e2e/src/api/specs/oauth.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,17 @@ const authServer = {
external: 'http://127.0.0.1:3000',
};

const mobileOverrideRedirectUri = 'https://photos.immich.app/oauth/mobile-redirect';

const redirect = async (url: string, cookies?: string[]) => {
const { headers } = await request(url)
.get('/')
.set('Cookie', cookies || []);
return { cookies: (headers['set-cookie'] as unknown as string[]) || [], location: headers.location };
};

const loginWithOAuth = async (sub: OAuthUser | string) => {
const { url } = await startOAuth({ oAuthConfigDto: { redirectUri: `${baseUrl}/auth/login` } });
const loginWithOAuth = async (sub: OAuthUser | string, redirectUri?: string) => {
const { url } = await startOAuth({ oAuthConfigDto: { redirectUri: redirectUri ?? `${baseUrl}/auth/login` } });

// login
const response1 = await redirect(url.replace(authServer.internal, authServer.external));
Expand Down Expand Up @@ -255,4 +257,50 @@ describe(`/oauth`, () => {
});
});
});

describe('mobile redirect override', () => {
beforeAll(async () => {
await setupOAuth(admin.accessToken, {
enabled: true,
clientId: OAuthClient.DEFAULT,
clientSecret: OAuthClient.DEFAULT,
buttonText: 'Login with Immich',
storageLabelClaim: 'immich_username',
mobileOverrideEnabled: true,
mobileRedirectUri: mobileOverrideRedirectUri,
});
});

it('should return the mobile redirect uri', async () => {
const { status, body } = await request(app)
.post('/oauth/authorize')
.send({ redirectUri: 'app.immich:///oauth-callback' });
expect(status).toBe(201);
expect(body).toEqual({ url: expect.stringContaining(`${authServer.internal}/auth?`) });

const params = new URL(body.url).searchParams;
expect(params.get('client_id')).toBe('client-default');
expect(params.get('response_type')).toBe('code');
expect(params.get('redirect_uri')).toBe(mobileOverrideRedirectUri);
expect(params.get('state')).toBeDefined();
});

it('should auto register the user by default', async () => {
const url = await loginWithOAuth('oauth-mobile-override', 'app.immich:///oauth-callback');
expect(url).toEqual(expect.stringContaining(mobileOverrideRedirectUri));

// simulate redirecting back to mobile app
const redirectUri = url.replace(mobileOverrideRedirectUri, 'app.immich:///oauth-callback');

const { status, body } = await request(app).post('/oauth/callback').send({ url: redirectUri });
expect(status).toBe(201);
expect(body).toMatchObject({
accessToken: expect.any(String),
isAdmin: false,
name: 'OAuth User',
userEmail: '[email protected]',
userId: expect.any(String),
});
});
});
});
7 changes: 4 additions & 3 deletions e2e/src/setup/auth-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const getClaims = (sub: string) => claims.find((user) => user.sub === sub) || wi
const setup = async () => {
const { privateKey, publicKey } = await generateKeyPair('RS256');

const redirectUris = ['http://127.0.0.1:2285/auth/login', 'https://photos.immich.app/oauth/mobile-redirect'];
const port = 3000;
const host = '0.0.0.0';
const oidc = new Provider(`http://${host}:${port}`, {
Expand Down Expand Up @@ -86,22 +87,22 @@ const setup = async () => {
{
client_id: OAuthClient.DEFAULT,
client_secret: OAuthClient.DEFAULT,
redirect_uris: ['http://127.0.0.1:2285/auth/login'],
redirect_uris: redirectUris,
grant_types: ['authorization_code'],
response_types: ['code'],
},
{
client_id: OAuthClient.RS256_TOKENS,
client_secret: OAuthClient.RS256_TOKENS,
redirect_uris: ['http://127.0.0.1:2285/auth/login'],
redirect_uris: redirectUris,
grant_types: ['authorization_code'],
id_token_signed_response_alg: 'RS256',
jwks: { keys: [await exportJWK(publicKey)] },
},
{
client_id: OAuthClient.RS256_PROFILE,
client_secret: OAuthClient.RS256_PROFILE,
redirect_uris: ['http://127.0.0.1:2285/auth/login'],
redirect_uris: redirectUris,
grant_types: ['authorization_code'],
userinfo_signed_response_alg: 'RS256',
jwks: { keys: [await exportJWK(publicKey)] },
Expand Down
11 changes: 6 additions & 5 deletions server/src/services/auth.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,13 @@ export class AuthService extends BaseService {
throw new BadRequestException('OAuth is not enabled');
}

const url = await this.oauthRepository.authorize(oauth, dto.redirectUri);
const url = await this.oauthRepository.authorize(oauth, this.resolveRedirectUri(oauth, dto.redirectUri));
return { url };
}

async callback(dto: OAuthCallbackDto, loginDetails: LoginDetails) {
const { oauth } = await this.getConfig({ withCache: false });
const profile = await this.oauthRepository.getProfile(oauth, dto.url, this.normalize(oauth, dto.url.split('?')[0]));
const profile = await this.oauthRepository.getProfile(oauth, dto.url, this.resolveRedirectUri(oauth, dto.url));
const { autoRegister, defaultStorageQuota, storageLabelClaim, storageQuotaClaim } = oauth;
this.logger.debug(`Logging in with OAuth: ${JSON.stringify(profile)}`);
let user = await this.userRepository.getByOAuthId(profile.sub);
Expand Down Expand Up @@ -257,7 +257,7 @@ export class AuthService extends BaseService {
const { sub: oauthId } = await this.oauthRepository.getProfile(
oauth,
dto.url,
this.normalize(oauth, dto.url.split('?')[0]),
this.resolveRedirectUri(oauth, dto.url),
);
const duplicate = await this.userRepository.getByOAuthId(oauthId);
if (duplicate && duplicate.id !== auth.user.id) {
Expand Down Expand Up @@ -369,10 +369,11 @@ export class AuthService extends BaseService {
return options.isValid(value) ? (value as T) : options.default;
}

private normalize(
private resolveRedirectUri(
{ mobileRedirectUri, mobileOverrideEnabled }: { mobileRedirectUri: string; mobileOverrideEnabled: boolean },
redirectUri: string,
url: string,
) {
const redirectUri = url.split('?')[0];
const isMobile = redirectUri.startsWith('app.immich:/');
if (isMobile && mobileOverrideEnabled && mobileRedirectUri) {
return mobileRedirectUri;
Expand Down
Loading