Skip to content

Commit 6dff277

Browse files
author
mdatelle
committed
chore: combine cookie with csrf validation and update cookie type
1 parent 2bddf0a commit 6dff277

File tree

5 files changed

+56
-17
lines changed

5 files changed

+56
-17
lines changed

‎api/src/types/fastify.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,6 @@ import type {
66

77
export type FastifyInstance = BaseFastifyInstance;
88
export interface FastifyRequest extends BaseFastifyRequest {
9-
cookies: { [cookieName: string]: string | undefined };
9+
cookies: Record<string, string | undefined>;
1010
}
1111
export type FastifyReply = BaseFastifyReply;

‎api/src/unraid-api/auth/auth.service.spec.ts‎

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { AuthZService } from 'nest-authz';
55
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
66

77
import type { ApiKey, ApiKeyWithSecret, UserAccount } from '@app/graphql/generated/api/types.js';
8+
import type { FastifyRequest } from '@app/types/fastify.js';
89
import { Resource, Role } from '@app/graphql/generated/api/types.js';
910
import { ApiKeyService } from '@app/unraid-api/auth/api-key.service.js';
1011
import { AuthService } from '@app/unraid-api/auth/auth.service.js';
@@ -48,6 +49,19 @@ describe('AuthService', () => {
4849
roles: [Role.GUEST, Role.CONNECT],
4950
};
5051

52+
// Mock FastifyRequest object for tests
53+
const createMockRequest = (overrides = {}): FastifyRequest => {
54+
return {
55+
headers: {},
56+
query: {},
57+
cookies: {},
58+
id: 'test-id',
59+
params: {},
60+
raw: {} as any,
61+
...overrides,
62+
} as FastifyRequest;
63+
};
64+
5165
beforeEach(async () => {
5266
const enforcer = await newEnforcer();
5367

@@ -66,36 +80,57 @@ describe('AuthService', () => {
6680
vi.spyOn(cookieService, 'hasValidAuthCookie').mockResolvedValue(true);
6781
vi.spyOn(authService, 'getSessionUser').mockResolvedValue(mockUser);
6882
vi.spyOn(authzService, 'getRolesForUser').mockResolvedValue([Role.ADMIN]);
83+
vi.spyOn(authService, 'validateCsrfToken').mockReturnValue(true);
6984

70-
const result = await authService.validateCookiesCasbin({});
85+
const mockRequest = createMockRequest();
86+
const result = await authService.validateCookiesCasbin(mockRequest);
7187

7288
expect(result).toEqual(mockUser);
7389
});
7490

7591
it('should throw UnauthorizedException when auth cookie is invalid', async () => {
7692
vi.spyOn(cookieService, 'hasValidAuthCookie').mockResolvedValue(false);
93+
vi.spyOn(authService, 'validateCsrfToken').mockReturnValue(true);
7794

78-
await expect(authService.validateCookiesCasbin({})).rejects.toThrow(UnauthorizedException);
95+
const mockRequest = createMockRequest();
96+
await expect(authService.validateCookiesCasbin(mockRequest)).rejects.toThrow(
97+
UnauthorizedException
98+
);
7999
});
80100

81101
it('should throw UnauthorizedException when session user is missing', async () => {
82102
vi.spyOn(cookieService, 'hasValidAuthCookie').mockResolvedValue(true);
83103
vi.spyOn(authService, 'getSessionUser').mockResolvedValue(null as unknown as UserAccount);
104+
vi.spyOn(authService, 'validateCsrfToken').mockReturnValue(true);
84105

85-
await expect(authService.validateCookiesCasbin({})).rejects.toThrow(UnauthorizedException);
106+
const mockRequest = createMockRequest();
107+
await expect(authService.validateCookiesCasbin(mockRequest)).rejects.toThrow(
108+
UnauthorizedException
109+
);
86110
});
87111

88112
it('should add guest role when user has no roles', async () => {
89113
vi.spyOn(cookieService, 'hasValidAuthCookie').mockResolvedValue(true);
90114
vi.spyOn(authService, 'getSessionUser').mockResolvedValue(mockUser);
91115
vi.spyOn(authzService, 'getRolesForUser').mockResolvedValue([]);
116+
vi.spyOn(authService, 'validateCsrfToken').mockReturnValue(true);
92117

93118
const addRoleSpy = vi.spyOn(authzService, 'addRoleForUser');
94-
const result = await authService.validateCookiesCasbin({});
119+
const mockRequest = createMockRequest();
120+
const result = await authService.validateCookiesCasbin(mockRequest);
95121

96122
expect(result).toEqual(mockUser);
97123
expect(addRoleSpy).toHaveBeenCalledWith(mockUser.id, 'guest');
98124
});
125+
126+
it('should throw UnauthorizedException when CSRF token is invalid', async () => {
127+
vi.spyOn(authService, 'validateCsrfToken').mockReturnValue(false);
128+
129+
const mockRequest = createMockRequest();
130+
await expect(authService.validateCookiesCasbin(mockRequest)).rejects.toThrow(
131+
new UnauthorizedException('Invalid CSRF token')
132+
);
133+
});
99134
});
100135

101136
describe('syncApiKeyRoles', () => {

‎api/src/unraid-api/auth/auth.service.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { AuthZService } from 'nest-authz';
55
import type { Permission, UserAccount } from '@app/graphql/generated/api/types.js';
66
import { Role } from '@app/graphql/generated/api/types.js';
77
import { getters } from '@app/store/index.js';
8+
import { FastifyRequest } from '@app/types/fastify.js';
89
import { ApiKeyService } from '@app/unraid-api/auth/api-key.service.js';
910
import { CookieService } from '@app/unraid-api/auth/cookie.service.js';
1011
import { batchProcess, handleAuthError } from '@app/utils.js';
@@ -48,11 +49,18 @@ export class AuthService {
4849
}
4950
}
5051

51-
async validateCookiesCasbin(cookies: {
52-
[cookieName: string]: string | undefined;
53-
}): Promise<UserAccount> {
52+
async validateCookiesCasbin(request: FastifyRequest): Promise<UserAccount> {
5453
try {
55-
if (!(await this.cookieService.hasValidAuthCookie(cookies))) {
54+
if (
55+
!this.validateCsrfToken(
56+
(request.headers['x-csrf-token'] as string | undefined) ||
57+
((request.query as { csrf_token?: string })?.csrf_token as string | undefined)
58+
)
59+
) {
60+
throw new UnauthorizedException('Invalid CSRF token');
61+
}
62+
63+
if (!(await this.cookieService.hasValidAuthCookie(request.cookies))) {
5664
throw new UnauthorizedException('No user session found');
5765
}
5866

‎api/src/unraid-api/auth/cookie.service.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export class CookieService {
4343
* @param opts optional overrides for the session directory & prefix of the session cookie to look for
4444
* @returns true if any of the cookies are a valid unraid session cookie, false otherwise
4545
*/
46-
async hasValidAuthCookie(cookies: { [cookieName: string]: string | undefined }): Promise<boolean> {
46+
async hasValidAuthCookie(cookies: Record<string, string | undefined>): Promise<boolean> {
4747
const { data } = await batchProcess(Object.entries(cookies), ([cookieName, cookieValue]) =>
4848
this.isValidAuthCookie(String(cookieName), String(cookieValue ?? ''))
4949
);

‎api/src/unraid-api/auth/cookie.strategy.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { PassportStrategy } from '@nestjs/passport';
33

44
import { Strategy } from 'passport-custom';
55

6-
import type { CustomRequest } from '@app/unraid-api/types/request.js';
6+
import { FastifyRequest } from '@app/types/fastify.js';
77
import { AuthService } from '@app/unraid-api/auth/auth.service.js';
88

99
const strategyName = 'user-cookie';
@@ -17,11 +17,7 @@ export class UserCookieStrategy extends PassportStrategy(Strategy, strategyName)
1717
super();
1818
}
1919

20-
public validate = async (req: CustomRequest): Promise<any> => {
21-
return (
22-
this.authService.validateCsrfToken(
23-
req.headers['x-csrf-token'] || (req.query as { csrf_token?: string })?.csrf_token
24-
) && this.authService.validateCookiesCasbin(req.cookies || {})
25-
);
20+
public validate = async (req: FastifyRequest): Promise<any> => {
21+
return this.authService.validateCookiesCasbin(req);
2622
};
2723
}

0 commit comments

Comments
 (0)