mirror of
https://github.com/immich-app/immich.git
synced 2026-05-18 03:10:24 +03:00
fix(oauth): normalize email claim to lowercase and trim before account lookup and registration (#26841)
* fix(oauth): normalize email claim to lowercase before account lookup and registration * test(auth): add test for OAuth email case normalization * chore: clean up --------- Co-authored-by: Jason Rasmussen <jason@rasm.me>
This commit is contained in:
@@ -259,7 +259,7 @@ describe(`/oauth`, () => {
|
|||||||
accessToken: expect.any(String),
|
accessToken: expect.any(String),
|
||||||
isAdmin: false,
|
isAdmin: false,
|
||||||
name: 'OAuth User',
|
name: 'OAuth User',
|
||||||
userEmail: 'oauth-RS256-token@immich.app',
|
userEmail: 'oauth-rs256-token@immich.app',
|
||||||
userId: expect.any(String),
|
userId: expect.any(String),
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -634,6 +634,26 @@ describe(AuthService.name, () => {
|
|||||||
expect(mocks.user.update).toHaveBeenCalledWith(user.id, { oauthId: profile.sub });
|
expect(mocks.user.update).toHaveBeenCalledWith(user.id, { oauthId: profile.sub });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should normalize the email from the OAuth profile before linking', async () => {
|
||||||
|
const user = UserFactory.create();
|
||||||
|
const profile = OAuthProfileFactory.create({ email: ' TEST@IMMICH.CLOUD ' });
|
||||||
|
|
||||||
|
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthEnabled);
|
||||||
|
mocks.oauth.getProfile.mockResolvedValue(profile);
|
||||||
|
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.user.getByEmail).toHaveBeenCalledWith('test@immich.cloud');
|
||||||
|
expect(mocks.user.update).toHaveBeenCalledWith(user.id, { oauthId: profile.sub });
|
||||||
|
});
|
||||||
|
|
||||||
it('should not link to a user with a different oauth sub', async () => {
|
it('should not link to a user with a different oauth sub', async () => {
|
||||||
const user = UserFactory.create({ oauthId: 'existing-sub' });
|
const user = UserFactory.create({ oauthId: 'existing-sub' });
|
||||||
|
|
||||||
|
|||||||
@@ -277,13 +277,14 @@ export class AuthService extends BaseService {
|
|||||||
|
|
||||||
const url = this.resolveRedirectUri(oauth, dto.url);
|
const url = this.resolveRedirectUri(oauth, dto.url);
|
||||||
const profile = await this.oauthRepository.getProfile(oauth, url, expectedState, codeVerifier);
|
const profile = await this.oauthRepository.getProfile(oauth, url, expectedState, codeVerifier);
|
||||||
|
const normalizedEmail = profile.email ? profile.email.trim().toLowerCase() : undefined;
|
||||||
const { autoRegister, defaultStorageQuota, storageLabelClaim, storageQuotaClaim, roleClaim } = oauth;
|
const { autoRegister, defaultStorageQuota, storageLabelClaim, storageQuotaClaim, roleClaim } = oauth;
|
||||||
this.logger.debug(`Logging in with OAuth: ${JSON.stringify(profile)}`);
|
this.logger.debug(`Logging in with OAuth: ${JSON.stringify(profile)}`);
|
||||||
let user: UserAdmin | undefined = await this.userRepository.getByOAuthId(profile.sub);
|
let user: UserAdmin | undefined = await this.userRepository.getByOAuthId(profile.sub);
|
||||||
|
|
||||||
// link by email
|
// link by email
|
||||||
if (!user && profile.email) {
|
if (!user && normalizedEmail) {
|
||||||
const emailUser = await this.userRepository.getByEmail(profile.email);
|
const emailUser = await this.userRepository.getByEmail(normalizedEmail);
|
||||||
if (emailUser) {
|
if (emailUser) {
|
||||||
if (emailUser.oauthId) {
|
if (emailUser.oauthId) {
|
||||||
throw new BadRequestException('User already exists, but is linked to another account.');
|
throw new BadRequestException('User already exists, but is linked to another account.');
|
||||||
@@ -296,17 +297,16 @@ export class AuthService extends BaseService {
|
|||||||
if (!user) {
|
if (!user) {
|
||||||
if (!autoRegister) {
|
if (!autoRegister) {
|
||||||
this.logger.warn(
|
this.logger.warn(
|
||||||
`Unable to register ${profile.sub}/${profile.email || '(no email)'}. To enable set OAuth Auto Register to true in admin settings.`,
|
`Unable to register ${profile.sub}/${normalizedEmail || '(no email)'}. To enable set OAuth Auto Register to true in admin settings.`,
|
||||||
);
|
);
|
||||||
throw new BadRequestException(`User does not exist and auto registering is disabled.`);
|
throw new BadRequestException(`User does not exist and auto registering is disabled.`);
|
||||||
}
|
}
|
||||||
|
|
||||||
const email = profile.email;
|
if (!normalizedEmail) {
|
||||||
if (!email) {
|
|
||||||
throw new BadRequestException('OAuth profile does not have an email address');
|
throw new BadRequestException('OAuth profile does not have an email address');
|
||||||
}
|
}
|
||||||
|
|
||||||
this.logger.log(`Registering new user: ${profile.sub}/${profile.email}`);
|
this.logger.log(`Registering new user: ${profile.sub}/${normalizedEmail}`);
|
||||||
|
|
||||||
const storageLabel = this.getClaim(profile, {
|
const storageLabel = this.getClaim(profile, {
|
||||||
key: storageLabelClaim,
|
key: storageLabelClaim,
|
||||||
@@ -329,8 +329,8 @@ export class AuthService extends BaseService {
|
|||||||
profile.name ||
|
profile.name ||
|
||||||
`${profile.given_name || ''} ${profile.family_name || ''}`.trim() ||
|
`${profile.given_name || ''} ${profile.family_name || ''}`.trim() ||
|
||||||
profile.preferred_username ||
|
profile.preferred_username ||
|
||||||
email,
|
normalizedEmail,
|
||||||
email,
|
email: normalizedEmail,
|
||||||
oauthId: profile.sub,
|
oauthId: profile.sub,
|
||||||
quotaSizeInBytes: storageQuota === null ? null : storageQuota * HumanReadableSize.GiB,
|
quotaSizeInBytes: storageQuota === null ? null : storageQuota * HumanReadableSize.GiB,
|
||||||
storageLabel: storageLabel || null,
|
storageLabel: storageLabel || null,
|
||||||
|
|||||||
Reference in New Issue
Block a user