Skip to content

Commit ca5e6a0

Browse files
authored
fix(#770): Stabilize user.roles reference in AuthProvider to prevent unnecessary router recreation (#771)
1 parent 192cf75 commit ca5e6a0

2 files changed

Lines changed: 108 additions & 2 deletions

File tree

frontend/src/context/auth/AuthProvider.tsx

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,28 @@ import { signOutUrl } from '@/config/fam/config';
1010
import { env } from '@/env';
1111
import { navigateTo } from '@/utils/navigation';
1212

13+
/**
14+
* Preserves the existing roles array reference when the next auth user has the
15+
* same effective role assignments, avoiding unnecessary downstream recomputation.
16+
*
17+
* @param previousUser The previously cached authenticated user.
18+
* @param nextUser The next authenticated user produced from token refresh.
19+
* @returns The next user, reusing the previous roles reference when safe.
20+
*/
21+
export const preserveRolesReference = (
22+
previousUser: FamLoginUser | undefined,
23+
nextUser: FamLoginUser | undefined,
24+
): FamLoginUser | undefined => {
25+
if (!previousUser || !nextUser) return nextUser;
26+
if (previousUser.roles === nextUser.roles) return nextUser;
27+
if (!isEqual(previousUser.roles, nextUser.roles)) return nextUser;
28+
29+
return {
30+
...nextUser,
31+
roles: previousUser.roles,
32+
};
33+
};
34+
1335
/**
1436
* Provides authenticated user state and auth actions to the application tree.
1537
*
@@ -47,7 +69,10 @@ export const AuthProvider = ({ children }: { children: ReactNode }) => {
4769
try {
4870
const idToken = await loadUserToken();
4971
const newUser = idToken ? parseToken(idToken) : undefined;
50-
setUser((prev) => (isEqual(prev, newUser) ? prev : newUser));
72+
setUser((prev) => {
73+
if (isEqual(prev, newUser)) return prev;
74+
return preserveRolesReference(prev, newUser);
75+
});
5176
} catch {
5277
setUser(undefined);
5378
if (!silent) await signOut();

frontend/src/context/auth/AuthProvider.unit.test.tsx

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import { render, waitFor, act } from '@testing-library/react';
33
import { describe, it, expect, vi, beforeAll } from 'vitest';
44

55
import { AuthContext } from './AuthContext';
6-
import { AuthProvider } from './AuthProvider';
6+
import { AuthProvider, preserveRolesReference } from './AuthProvider';
7+
import { Role, type FamLoginUser, type FamRole } from './types';
78

89
import { jwtfy } from '@/config/tests/auth.helper';
910
import { navigateTo } from '@/utils/navigation';
@@ -113,6 +114,7 @@ describe('AuthProvider (extra coverage)', () => {
113114
await waitFor(() => expect(context).toBeDefined());
114115
expect(context.userToken()).toBe('sometoken');
115116
});
117+
116118
});
117119

118120
describe('when VITE_MOCK_AUTH is true', () => {
@@ -165,3 +167,82 @@ describe('AuthProvider (extra coverage)', () => {
165167
});
166168
});
167169
});
170+
171+
describe('preserveRolesReference', () => {
172+
const mockRoles: FamRole[] = [{ role: Role.VIEWER, clients: ['100'] }];
173+
const mockRoles2: FamRole[] = [{ role: Role.ADMIN, clients: ['200'] }];
174+
const mockUser: FamLoginUser = {
175+
userName: 'testuser',
176+
displayName: 'Test User',
177+
email: 'test@example.com',
178+
roles: mockRoles,
179+
privileges: {},
180+
};
181+
182+
it('returns nextUser when previousUser is undefined', () => {
183+
const nextUser = { ...mockUser, roles: mockRoles };
184+
const result = preserveRolesReference(undefined, nextUser);
185+
expect(result).toBe(nextUser);
186+
});
187+
188+
it('returns nextUser when nextUser is undefined', () => {
189+
const result = preserveRolesReference(mockUser, undefined);
190+
expect(result).toBeUndefined();
191+
});
192+
193+
it('returns nextUser when both are undefined', () => {
194+
const result = preserveRolesReference(undefined, undefined);
195+
expect(result).toBeUndefined();
196+
});
197+
198+
it('returns nextUser when role content differs', () => {
199+
const previousUser = { ...mockUser, roles: mockRoles };
200+
const nextUser = { ...mockUser, roles: mockRoles2 };
201+
const result = preserveRolesReference(previousUser, nextUser);
202+
expect(result).toBe(nextUser);
203+
expect(result?.roles).toBe(mockRoles2);
204+
});
205+
206+
it('preserves previous roles reference when role content is equal but reference differs', () => {
207+
const previousRoles: FamRole[] = [{ role: Role.VIEWER, clients: ['100'] }];
208+
const nextRoles: FamRole[] = [{ role: Role.VIEWER, clients: ['100'] }];
209+
const previousUser = { ...mockUser, roles: previousRoles };
210+
const nextUser = { ...mockUser, roles: nextRoles };
211+
212+
const result = preserveRolesReference(previousUser, nextUser);
213+
214+
expect(result).toBeDefined();
215+
expect(result?.roles).toBe(previousRoles);
216+
expect(result?.roles).not.toBe(nextRoles);
217+
expect(result?.userName).toBe(nextUser.userName);
218+
});
219+
220+
it('returns nextUser unchanged when roles reference is already the same', () => {
221+
const sharedRoles = mockRoles;
222+
const previousUser = { ...mockUser, roles: sharedRoles };
223+
const nextUser = { ...mockUser, roles: sharedRoles };
224+
225+
const result = preserveRolesReference(previousUser, nextUser);
226+
227+
expect(result).toBe(nextUser);
228+
});
229+
230+
it('preserves roles while updating other user fields', () => {
231+
const sharedRoles: FamRole[] = [{ role: Role.SUBMITTER, clients: ['300'] }];
232+
const previousUser = {
233+
...mockUser,
234+
roles: sharedRoles,
235+
email: 'old@example.com',
236+
};
237+
const nextUser = {
238+
...mockUser,
239+
roles: sharedRoles,
240+
email: 'new@example.com',
241+
};
242+
243+
const result = preserveRolesReference(previousUser, nextUser);
244+
245+
expect(result?.roles).toBe(sharedRoles);
246+
expect(result?.email).toBe('new@example.com');
247+
});
248+
});

0 commit comments

Comments
 (0)