Prechádzať zdrojové kódy

fix(server/cookies): Making the cookie better (#1366)

* fix(server/cookies): Making the cookie better

Cookie should have SameSite=Stict and Secure if served via https, otherwise just SameSite=Strict set.

* feat(server): forgot to add secure to the other cookie.

* Fixed the cookies and tests for them.
Skyler Mäntysaari 2 rokov pred
rodič
commit
5262e92b9f

+ 5 - 1
server/apps/immich/src/api-v1/auth/auth.controller.ts

@@ -25,9 +25,13 @@ export class AuthController {
     @Body(new ValidationPipe({ transform: true })) loginCredential: LoginCredentialDto,
     @Ip() clientIp: string,
     @Res({ passthrough: true }) response: Response,
+    @Req() request: Request,
   ): Promise<LoginResponseDto> {
     const loginResponse = await this.authService.login(loginCredential, clientIp);
-    response.setHeader('Set-Cookie', this.immichJwtService.getCookies(loginResponse, AuthType.PASSWORD));
+    response.setHeader(
+      'Set-Cookie',
+      this.immichJwtService.getCookies(loginResponse, AuthType.PASSWORD, request.secure),
+    );
     return loginResponse;
   }
 

+ 2 - 1
server/apps/immich/src/api-v1/oauth/oauth.controller.ts

@@ -33,9 +33,10 @@ export class OAuthController {
   public async callback(
     @Res({ passthrough: true }) response: Response,
     @Body(ValidationPipe) dto: OAuthCallbackDto,
+    @Req() request: Request,
   ): Promise<LoginResponseDto> {
     const loginResponse = await this.oauthService.login(dto);
-    response.setHeader('Set-Cookie', this.immichJwtService.getCookies(loginResponse, AuthType.OAUTH));
+    response.setHeader('Set-Cookie', this.immichJwtService.getCookies(loginResponse, AuthType.OAUTH, request.secure));
     return loginResponse;
   }
 

+ 14 - 4
server/apps/immich/src/modules/immich-jwt/immich-jwt.service.spec.ts

@@ -36,13 +36,23 @@ describe('ImmichJwtService', () => {
   });
 
   describe('getCookies', () => {
-    it('should generate the cookie headers', async () => {
+    it('should generate the cookie headers (secure)', async () => {
       jwtServiceMock.sign.mockImplementation((value) => value as string);
       const dto = { accessToken: 'test-user@immich.com', userId: 'test-user' };
-      const cookies = await sut.getCookies(dto as LoginResponseDto, AuthType.PASSWORD);
+      const cookies = sut.getCookies(dto as LoginResponseDto, AuthType.PASSWORD, true);
       expect(cookies).toEqual([
-        'immich_access_token=test-user@immich.com; HttpOnly; Path=/; Max-Age=604800',
-        'immich_auth_type=password; Path=/; Max-Age=604800',
+        'immich_access_token=test-user@immich.com; Secure; Path=/; Max-Age=604800; SameSite=Strict;',
+        'immich_auth_type=password; Secure; Path=/; Max-Age=604800; SameSite=Strict;',
+      ]);
+    });
+
+    it('should generate the cookie headers (insecure)', () => {
+      jwtServiceMock.sign.mockImplementation((value) => value as string);
+      const dto = { accessToken: 'test-user@immich.com', userId: 'test-user' };
+      const cookies = sut.getCookies(dto as LoginResponseDto, AuthType.PASSWORD, false);
+      expect(cookies).toEqual([
+        'immich_access_token=test-user@immich.com; HttpOnly; Path=/; Max-Age=604800 SameSite=Strict;',
+        'immich_auth_type=password; HttpOnly; Path=/; Max-Age=604800; SameSite=Strict;',
       ]);
     });
   });

+ 11 - 3
server/apps/immich/src/modules/immich-jwt/immich-jwt.service.ts

@@ -22,11 +22,19 @@ export class ImmichJwtService {
     return [IMMICH_ACCESS_COOKIE, IMMICH_AUTH_TYPE_COOKIE];
   }
 
-  public getCookies(loginResponse: LoginResponseDto, authType: AuthType) {
+  public getCookies(loginResponse: LoginResponseDto, authType: AuthType, isSecure: boolean) {
     const maxAge = 7 * 24 * 3600; // 7 days
 
-    const accessTokenCookie = `${IMMICH_ACCESS_COOKIE}=${loginResponse.accessToken}; HttpOnly; Path=/; Max-Age=${maxAge}`;
-    const authTypeCookie = `${IMMICH_AUTH_TYPE_COOKIE}=${authType}; Path=/; Max-Age=${maxAge}`;
+    let accessTokenCookie = '';
+    let authTypeCookie = '';
+
+    if (isSecure) {
+      accessTokenCookie = `${IMMICH_ACCESS_COOKIE}=${loginResponse.accessToken}; Secure; Path=/; Max-Age=${maxAge}; SameSite=Strict;`;
+      authTypeCookie = `${IMMICH_AUTH_TYPE_COOKIE}=${authType}; Secure; Path=/; Max-Age=${maxAge}; SameSite=Strict;`;
+    } else {
+      accessTokenCookie = `${IMMICH_ACCESS_COOKIE}=${loginResponse.accessToken}; HttpOnly; Path=/; Max-Age=${maxAge} SameSite=Strict;`;
+      authTypeCookie = `${IMMICH_AUTH_TYPE_COOKIE}=${authType}; HttpOnly; Path=/; Max-Age=${maxAge}; SameSite=Strict;`;
+    }
 
     return [accessTokenCookie, authTypeCookie];
   }