c-l/immich
1
0
Fork 0
mirror of https://github.com/immich-app/immich.git synced 2024-12-28 06:31:58 +00:00
Code Releases Activity

fix: auth sub override (#9635)

Browse source
This commit is contained in:
Jason Rasmussen 2024-05-21 09:07:34 -04:00 committed by GitHub
parent bb79df655d
commit 91b835cfeb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 35 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

32
server/src/services/auth.service.spec.ts
View file

@ -377,7 +377,7 @@ describe('AuthService', () => {
});
it('should not allow auto registering', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.noAutoRegister);
systemMock.get.mockResolvedValue(systemConfigStub.oauthEnabled);
userMock.getByEmail.mockResolvedValue(null);
await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' }, loginDetails)).rejects.toBeInstanceOf(
BadRequestException,
@ -386,7 +386,7 @@ describe('AuthService', () => {
});
it('should link an existing user', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.noAutoRegister);
systemMock.get.mockResolvedValue(systemConfigStub.oauthEnabled);
userMock.getByEmail.mockResolvedValue(userStub.user1);
userMock.update.mockResolvedValue(userStub.user1);
sessionMock.create.mockResolvedValue(sessionStub.valid);
@ -399,6 +399,20 @@ describe('AuthService', () => {
expect(userMock.update).toHaveBeenCalledWith(userStub.user1.id, { oauthId: sub });
});
it('should not link to a user with a different oauth sub', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.oauthWithAutoRegister);
userMock.getByEmail.mockResolvedValueOnce({ ...userStub.user1, oauthId: 'existing-sub' });
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);
await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' }, loginDetails)).resolves.toEqual(
loginResponseStub.user1oauth,
);
expect(userMock.update).not.toHaveBeenCalled();
expect(userMock.create).toHaveBeenCalled();
});
it('should allow auto registering by default', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.enabled);
userMock.getByEmail.mockResolvedValue(null);
@ -415,7 +429,7 @@ describe('AuthService', () => {
});
it('should use the mobile redirect override', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.override);
systemMock.get.mockResolvedValue(systemConfigStub.oauthWithMobileOverride);
userMock.getByOAuthId.mockResolvedValue(userStub.user1);
sessionMock.create.mockResolvedValue(sessionStub.valid);
@ -425,7 +439,7 @@ describe('AuthService', () => {
});
it('should use the mobile redirect override for ios urls with multiple slashes', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.override);
systemMock.get.mockResolvedValue(systemConfigStub.oauthWithMobileOverride);
userMock.getByOAuthId.mockResolvedValue(userStub.user1);
sessionMock.create.mockResolvedValue(sessionStub.valid);
@ -435,7 +449,7 @@ describe('AuthService', () => {
});
it('should use the default quota', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.withDefaultStorageQuota);
systemMock.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota);
userMock.getByEmail.mockResolvedValue(null);
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);
@ -448,7 +462,7 @@ describe('AuthService', () => {
});
it('should ignore an invalid storage quota', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.withDefaultStorageQuota);
systemMock.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota);
userMock.getByEmail.mockResolvedValue(null);
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);
@ -462,7 +476,7 @@ describe('AuthService', () => {
});
it('should ignore a negative quota', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.withDefaultStorageQuota);
systemMock.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota);
userMock.getByEmail.mockResolvedValue(null);
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);
@ -476,7 +490,7 @@ describe('AuthService', () => {
});
it('should not set quota for 0 quota', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.withDefaultStorageQuota);
systemMock.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota);
userMock.getByEmail.mockResolvedValue(null);
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);
@ -496,7 +510,7 @@ describe('AuthService', () => {
});
it('should use a valid storage quota', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.withDefaultStorageQuota);
systemMock.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota);
userMock.getByEmail.mockResolvedValue(null);
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);

2
server/src/services/auth.service.ts
View file

@ -201,7 +201,7 @@ export class AuthService {
// link existing user
if (!user) {
const emailUser = await this.userRepository.getByEmail(profile.email);
if (emailUser) {
if (emailUser && !emailUser.oauthId) {
user = await this.userRepository.update(emailUser.id, { oauthId: profile.sub });
}
}

14
server/test/fixtures/system-config.stub.ts vendored
View file

@ -15,7 +15,7 @@ export const systemConfigStub = {
enabled: false,
},
},
noAutoRegister: {
oauthEnabled: {
oauth: {
enabled: true,
autoRegister: false,
@ -23,7 +23,15 @@ export const systemConfigStub = {
buttonText: 'OAuth',
},
},
override: {
oauthWithAutoRegister: {
oauth: {
enabled: true,
autoRegister: true,
autoLaunch: false,
buttonText: 'OAuth',
},
},
oauthWithMobileOverride: {
oauth: {
enabled: true,
autoRegister: true,
@ -32,7 +40,7 @@ export const systemConfigStub = {
buttonText: 'OAuth',
},
},
withDefaultStorageQuota: {
oauthWithStorageQuota: {
oauth: {
enabled: true,
autoRegister: true,