Drop cached projects/flows on login or logout, not just app reload
ProjectsService and FlowsService are root singletons that load their list once and cache it for the app's lifetime. Logout never cleared that cache, so a second person signing in on the same tab without a hard reload still saw whoever's projects and flows were loaded before them - no backend request was even made. Authorization now emits userChanged whenever the signed-in identity actually changes (never on the initial hydration from storage), and both services reset their cache on it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
d09ec47be6
commit
87f6e2564b
|
|
@ -3,6 +3,8 @@
|
|||
// Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM.
|
||||
|
||||
import { TestBed } from '@angular/core/testing';
|
||||
import { of, throwError } from 'rxjs';
|
||||
import { vi } from 'vitest';
|
||||
|
||||
import { Authorization } from './authorization';
|
||||
|
||||
|
|
@ -17,4 +19,68 @@ describe('Authorization', () => {
|
|||
it('should be created', () => {
|
||||
expect(service).toBeTruthy();
|
||||
});
|
||||
|
||||
describe('userChanged', () => {
|
||||
/**
|
||||
* A per-user cache (ProjectsService, FlowsService) subscribes to this to know when to drop
|
||||
* what it is holding. If it fired on every persisted state, or missed a real transition, that
|
||||
* cache would either thrash on every session check or leak one user's data into another's.
|
||||
*/
|
||||
it('fires with the new username on login', () => {
|
||||
service.authCall.login = vi.fn(() => of({ username: 'alice', email: null, role: 'USER' })) as any;
|
||||
const seen: (string | null)[] = [];
|
||||
service.userChanged.subscribe((username) => seen.push(username));
|
||||
|
||||
service.login('alice', 'secret').subscribe();
|
||||
|
||||
expect(seen).toEqual(['alice']);
|
||||
});
|
||||
|
||||
it('fires with null on logout', () => {
|
||||
service.authCall.login = vi.fn(() => of({ username: 'alice', email: null, role: 'USER' })) as any;
|
||||
service.authCall.logout = vi.fn(() => of(void 0)) as any;
|
||||
service.login('alice', 'secret').subscribe();
|
||||
const seen: (string | null)[] = [];
|
||||
service.userChanged.subscribe((username) => seen.push(username));
|
||||
|
||||
service.logout().subscribe();
|
||||
|
||||
expect(seen).toEqual([null]);
|
||||
});
|
||||
|
||||
it('fires again when a different account logs in over the current one', () => {
|
||||
service.authCall.login = vi.fn()
|
||||
.mockReturnValueOnce(of({ username: 'alice', email: null, role: 'USER' }))
|
||||
.mockReturnValueOnce(of({ username: 'bob', email: null, role: 'USER' })) as any;
|
||||
service.login('alice', 'secret').subscribe();
|
||||
const seen: (string | null)[] = [];
|
||||
service.userChanged.subscribe((username) => seen.push(username));
|
||||
|
||||
service.login('bob', 'secret').subscribe();
|
||||
|
||||
expect(seen).toEqual(['bob']);
|
||||
});
|
||||
|
||||
it('does not fire when a session check confirms the same user is still signed in', () => {
|
||||
service.authCall.login = vi.fn(() => of({ username: 'alice', email: null, role: 'USER' })) as any;
|
||||
service.authCall.currentUser = vi.fn(() => of({ username: 'alice', email: null, role: 'USER' })) as any;
|
||||
service.login('alice', 'secret').subscribe();
|
||||
const seen: (string | null)[] = [];
|
||||
service.userChanged.subscribe((username) => seen.push(username));
|
||||
|
||||
service.validateSession().subscribe();
|
||||
|
||||
expect(seen).toEqual([]);
|
||||
});
|
||||
|
||||
it('does not fire for a failed login attempt', () => {
|
||||
service.authCall.login = vi.fn(() => throwError(() => new Error('bad credentials'))) as any;
|
||||
const seen: (string | null)[] = [];
|
||||
service.userChanged.subscribe((username) => seen.push(username));
|
||||
|
||||
service.login('alice', 'wrong').subscribe({ error: () => {} });
|
||||
|
||||
expect(seen).toEqual([]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ import {
|
|||
} from '@models/user';
|
||||
import { AuthorizationCallServiceBase } from './authorization-call.base';
|
||||
import { environment } from '@environment';
|
||||
import { Observable, catchError, finalize, map, of, shareReplay, take, tap, throwError } from 'rxjs';
|
||||
import { Observable, Subject, catchError, finalize, map, of, shareReplay, take, tap, throwError } from 'rxjs';
|
||||
|
||||
@Injectable({
|
||||
providedIn: 'root',
|
||||
|
|
@ -25,6 +25,17 @@ export class Authorization {
|
|||
|
||||
loggedInUser = this.user.asReadonly();
|
||||
|
||||
private readonly userChanged$ = new Subject<string | null>();
|
||||
/**
|
||||
* Emits the new username (or null) whenever the signed-in identity actually changes - login,
|
||||
* logout, or one account replacing another - but never for the initial hydration from storage on
|
||||
* app boot, since that is a page load continuing a session, not a change. A per-user client cache
|
||||
* (`ProjectsService`, `FlowsService`, ...) subscribes to invalidate itself: those services are
|
||||
* `providedIn: 'root'` and outlive any one login, so without this a second person signing in on
|
||||
* the same tab without a hard reload would still see whoever was logged in before them.
|
||||
*/
|
||||
readonly userChanged: Observable<string | null> = this.userChanged$.asObservable();
|
||||
|
||||
constructor() {
|
||||
this.restoreUserFromStorage();
|
||||
}
|
||||
|
|
@ -114,8 +125,14 @@ export class Authorization {
|
|||
|
||||
private persistUserState(user: User | null) {
|
||||
const normalizedUser = this.normalizeUser(user);
|
||||
const previousUsername = this.user()?.username ?? null;
|
||||
this.user.set(normalizedUser);
|
||||
|
||||
const nextUsername = normalizedUser?.username ?? null;
|
||||
if (previousUsername !== nextUsername) {
|
||||
this.userChanged$.next(nextUsername);
|
||||
}
|
||||
|
||||
try {
|
||||
if (typeof localStorage === 'undefined') return;
|
||||
if (normalizedUser) {
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@
|
|||
|
||||
import { TestBed } from '@angular/core/testing';
|
||||
import { Flow, FlowData } from '@models/flow';
|
||||
import { Authorization } from '@services/authorization/authorization';
|
||||
import { firstValueFrom, of, throwError } from 'rxjs';
|
||||
import { vi } from 'vitest';
|
||||
import { FlowsCallServiceBase } from './flows-call.base';
|
||||
|
|
@ -97,6 +98,25 @@ describe('FlowsService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('drops the cached list when a different user signs in, so the next read refetches', async () => {
|
||||
// Regression: this service is providedIn 'root' and outlives any one login. Without this, a
|
||||
// second person signing in on the same tab kept seeing whoever's flows were loaded before.
|
||||
callServiceSpy.retrieveAllFlows
|
||||
.mockReturnValueOnce(of([makeFlow('alice-1')]))
|
||||
.mockReturnValueOnce(of([makeFlow('bob-1')]));
|
||||
await service.getAllFlows();
|
||||
expect(service.flows().map((flow) => flow.id)).toEqual(['alice-1']);
|
||||
|
||||
const authorization = TestBed.inject(Authorization);
|
||||
authorization.authCall.login = vi.fn(() => of({ username: 'bob', email: null, role: 'USER' })) as any;
|
||||
authorization.login('bob', 'secret').subscribe();
|
||||
|
||||
expect(service.flows()).toEqual([]);
|
||||
|
||||
await service.getAllFlows();
|
||||
expect(service.flows().map((flow) => flow.id)).toEqual(['bob-1']);
|
||||
});
|
||||
|
||||
describe('refresh', () => {
|
||||
it('should update flows signal', async () => {
|
||||
const flows = [makeFlow('r1')];
|
||||
|
|
|
|||
|
|
@ -2,9 +2,10 @@
|
|||
// SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
// Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM.
|
||||
|
||||
import { Injectable, signal } from '@angular/core';
|
||||
import { inject, Injectable, signal } from '@angular/core';
|
||||
import { environment } from '@environment';
|
||||
import { Flow, FlowData } from '@models/flow';
|
||||
import { Authorization } from '@services/authorization/authorization';
|
||||
import { FlowsCallServiceBase } from './flows-call.base';
|
||||
import { catchError, firstValueFrom, Observable, of, switchMap, tap, throwError } from 'rxjs';
|
||||
|
||||
|
|
@ -21,6 +22,17 @@ export class FlowsService {
|
|||
private _flows = signal<Flow[]>([]);
|
||||
readonly flows = this._flows.asReadonly();
|
||||
|
||||
constructor() {
|
||||
// See Authorization.userChanged / ProjectsService: same cache-outlives-login leak, same fix.
|
||||
inject(Authorization).userChanged.subscribe(() => this.reset());
|
||||
}
|
||||
|
||||
reset() {
|
||||
this._flows.set([]);
|
||||
this.toInit = true;
|
||||
this.loadingPromise = null;
|
||||
}
|
||||
|
||||
hasLoadedFlows() {
|
||||
return this._flows().length > 0 || !this.toInit;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@
|
|||
|
||||
import { TestBed } from '@angular/core/testing';
|
||||
import { Project } from '@models/project';
|
||||
import { Authorization } from '@services/authorization/authorization';
|
||||
import { of, throwError } from 'rxjs';
|
||||
import { vi } from 'vitest';
|
||||
|
||||
|
|
@ -73,6 +74,25 @@ describe('ProjectsService', () => {
|
|||
expect(service.hasLoadedProjects()).toBe(false);
|
||||
});
|
||||
|
||||
it('drops the cached list when a different user signs in, so the next read refetches', async () => {
|
||||
// Regression: this service is providedIn 'root' and outlives any one login. Without this, a
|
||||
// second person signing in on the same tab kept seeing whoever's projects were loaded before.
|
||||
callServiceSpy.retrieveAllProjects
|
||||
.mockReturnValueOnce(of([makeProject('p1', "Alice's project")]))
|
||||
.mockReturnValueOnce(of([makeProject('p2', "Bob's project")]));
|
||||
await service.getAllProjects();
|
||||
expect(service.projects().map((project) => project.id)).toEqual(['p1']);
|
||||
|
||||
const authorization = TestBed.inject(Authorization);
|
||||
authorization.authCall.login = vi.fn(() => of({ username: 'bob', email: null, role: 'USER' })) as any;
|
||||
authorization.login('bob', 'secret').subscribe();
|
||||
|
||||
expect(service.projects()).toEqual([]);
|
||||
|
||||
await service.getAllProjects();
|
||||
expect(service.projects().map((project) => project.id)).toEqual(['p2']);
|
||||
});
|
||||
|
||||
it('indexes projects by id for O(1) lookup from a flow', async () => {
|
||||
callServiceSpy.retrieveAllProjects.mockReturnValue(of([makeProject('p1', 'Recruiting')]));
|
||||
await service.getAllProjects();
|
||||
|
|
|
|||
|
|
@ -2,9 +2,10 @@
|
|||
// SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
// Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM.
|
||||
|
||||
import { computed, Injectable, signal } from '@angular/core';
|
||||
import { computed, inject, Injectable, signal } from '@angular/core';
|
||||
import { environment } from '@environment';
|
||||
import { Project, ProjectContext, ProjectDraft } from '@models/project';
|
||||
import { Authorization } from '@services/authorization/authorization';
|
||||
import { catchError, firstValueFrom, Observable, of, tap, throwError } from 'rxjs';
|
||||
import { ProjectsCallServiceBase } from './projects-call.base';
|
||||
|
||||
|
|
@ -24,6 +25,19 @@ export class ProjectsService {
|
|||
/** Lets a flow resolve its project in O(1) without threading inputs through the tree. */
|
||||
readonly projectById = computed(() => new Map(this._projects().map((project) => [project.id, project])));
|
||||
|
||||
constructor() {
|
||||
// See Authorization.userChanged: without this, a second person signing in on the same tab
|
||||
// would still see whoever loaded the page before them, since this service outlives any one login.
|
||||
inject(Authorization).userChanged.subscribe(() => this.reset());
|
||||
}
|
||||
|
||||
/** Drops the cached list so the next read has to ask the server again, as who is asking has changed. */
|
||||
reset() {
|
||||
this._projects.set([]);
|
||||
this.toInit = true;
|
||||
this.loadingPromise = null;
|
||||
}
|
||||
|
||||
hasLoadedProjects() {
|
||||
return this._projects().length > 0 || !this.toInit;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -51,7 +51,8 @@ describe('FlowItem', () => {
|
|||
{
|
||||
provide: Authorization,
|
||||
useValue: {
|
||||
loggedInUser: vi.fn().mockReturnValue({ username: 'author' })
|
||||
loggedInUser: vi.fn().mockReturnValue({ username: 'author' }),
|
||||
userChanged: of(null)
|
||||
}
|
||||
},
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in New Issue