Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/SIL.XForge.Scripture/ClientApp/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,17 @@

ExceptionHandlingService.initBugsnag();

// The browser may snapshot a page into the back/forward cache when navigating away (Chrome 149+
// does this even while the realtime WebSocket is open), and pressing Back restores the snapshot —
// including auth state and user data that logging out was meant to destroy (SF-3855). No browser
// API can prevent or destroy the snapshot (the browser evicts it after a bounded time), so make
// restoring it a dead end: reload, so authentication is re-evaluated from scratch.
window.addEventListener('pageshow', (event: PageTransitionEvent) => {
if (event.persisted) {
window.location.reload();
}
});

bootstrapApplication(AppComponent, {
providers: [
{ provide: 'BASE_URL', useFactory: getBaseUrl, deps: [] as any[] },
Expand Down Expand Up @@ -78,7 +89,7 @@
),
{ provide: APP_ID, useValue: 'ng-cli-universal' },
CookieService,
provideAnimations(),

Check warning on line 92 in src/SIL.XForge.Scripture/ClientApp/src/main.ts

View workflow job for this annotation

GitHub Actions / Lint and Prettier (22.13.0, 11.11.0, 11.10.0)

`provideAnimations` is deprecated. 20.2 Use `animate.enter` or `animate.leave` instead. Intent to remove in v23
provideUICommon(),
provideTranslationMarkupTranspiler(EmTextTranspiler),
translocoMarkupRouterLinkRenderer(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@
});
env.localSettings.clear();
expect(env.service.currentUserRoles.length).toBe(0);
env.localSettings.set(ROLE_SETTING, SystemRole.SystemAdmin);

Check warning on line 203 in src/SIL.XForge.Scripture/ClientApp/src/xforge-common/auth.service.spec.ts

View workflow job for this annotation

GitHub Actions / Lint and Prettier (22.13.0, 11.11.0, 11.10.0)

`ROLE_SETTING` is deprecated. This value is deprecated, but maintained to ensure compatibility with older login sessions
expect(env.service.currentUserRoles.length).toBe(1);
env.discardTokenExpiryTimer();
}));
Expand Down Expand Up @@ -632,6 +632,18 @@
env.discardTokenExpiryTimer();
}));

it('should disable the offline store on log out so in-flight writes cannot re-create it', fakeAsync(() => {
const env = new TestEnvironment({ isOnline: true, isLoggedIn: true });
expect(env.isAuthenticated).toBe(true);
const offlineStore = TestBed.inject(OfflineStore);
expect(offlineStore.disabled).toBe(false);

env.logOut();
tick();
expect(offlineStore.disabled).toBe(true);
env.discardTokenExpiryTimer();
}));

it('prompt on log out if transparent authentication cookie is set', fakeAsync(() => {
const env = new TestEnvironment({ isOnline: true, isLoggedIn: true, setTransparentAuthenticationCookie: true });
expect(env.isAuthenticated).toBe(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@

get currentUserRoles(): SystemRole[] {
// Provide compatibility with login session predating role array support
const role = this.localSettings.get<SystemRole | undefined>(ROLE_SETTING);

Check warning on line 142 in src/SIL.XForge.Scripture/ClientApp/src/xforge-common/auth.service.ts

View workflow job for this annotation

GitHub Actions / Lint and Prettier (22.13.0, 11.11.0, 11.10.0)

`ROLE_SETTING` is deprecated. This value is deprecated, but maintained to ensure compatibility with older login sessions
if (role != null) {
return [role];
} else {
Expand Down Expand Up @@ -276,6 +276,9 @@
}
if (proceedWithLogout) {
this.cookieService.deleteAll('/');
// Disable before deleting so realtime persistence still in flight cannot re-create the
// database with the logged-out user's data (SF-3855)
this.offlineStore.disable();
await this.offlineStore.deleteDB();
this.localSettings.clear();
this.unscheduleRenewal();
Expand Down Expand Up @@ -632,7 +635,7 @@
this.localSettings.set(ID_TOKEN_SETTING, idToken);
this.localSettings.set(EXPIRES_AT_SETTING, expiresAt);
this.localSettings.set(USER_ID_SETTING, userId);
this.localSettings.remove(ROLE_SETTING);

Check warning on line 638 in src/SIL.XForge.Scripture/ClientApp/src/xforge-common/auth.service.ts

View workflow job for this annotation

GitHub Actions / Lint and Prettier (22.13.0, 11.11.0, 11.10.0)

`ROLE_SETTING` is deprecated. This value is deprecated, but maintained to ensure compatibility with older login sessions
this.localSettings.set(ROLES_SETTING, typeof role === 'string' ? [role] : role || []);
this.scheduleRenewal();
this.bugsnagService.leaveBreadcrumb(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import { TestBed } from '@angular/core/testing';
import { IndexeddbOfflineStore } from './indexeddb-offline-store';
import { RealtimeDocConstructor } from './models/realtime-doc';
import { TypeRegistry } from './type-registry';

const TEST_COLLECTION = 'users';

describe('IndexeddbOfflineStore', () => {
beforeEach(() => {
TestBed.configureTestingModule({
providers: [
IndexeddbOfflineStore,
{
provide: TypeRegistry,
useValue: new TypeRegistry(
[{ COLLECTION: TEST_COLLECTION, INDEX_PATHS: [] } as unknown as RealtimeDocConstructor],
[],
[]
)
}
]
});
});

afterEach(async () => {
await deleteDatabase();
});

it('should store and retrieve data', async () => {
const store = TestBed.inject(IndexeddbOfflineStore);
await store.put(TEST_COLLECTION, { id: 'user01' });
expect(await store.getAllIds(TEST_COLLECTION)).toEqual(['user01']);
});

it('should not re-create the database when written to after deleteDB and disable', async () => {
const store = TestBed.inject(IndexeddbOfflineStore);
await store.put(TEST_COLLECTION, { id: 'user01' });

store.disable();
await store.deleteDB();

// Simulates realtime doc persistence that is still in flight during logout (SF-3855)
await expectNeverSettles(store.put(TEST_COLLECTION, { id: 'user01' }));
expect(await databaseExists()).toBe(false);
});

it('should not settle reads or writes once disabled', async () => {
const store = TestBed.inject(IndexeddbOfflineStore);
await store.put(TEST_COLLECTION, { id: 'user01' });

store.disable();
expect(store.disabled).toBe(true);
await expectNeverSettles(store.put(TEST_COLLECTION, { id: 'user02' }));
await expectNeverSettles(store.getAllIds(TEST_COLLECTION));
await expectNeverSettles(store.getAll(TEST_COLLECTION));
await expectNeverSettles(store.get(TEST_COLLECTION, 'user01'));
await expectNeverSettles(store.query(TEST_COLLECTION, {}));
await expectNeverSettles(store.delete(TEST_COLLECTION, 'user01'));
});
});

const PENDING = 'pending';

/** Expects the promise to be still pending (i.e. to lose a race against a short timer). */
async function expectNeverSettles(promise: Promise<unknown>): Promise<void> {
const result = await Promise.race([
promise.then(
() => 'resolved',
() => 'rejected'
),
new Promise(resolve => setTimeout(() => resolve(PENDING), 25))
]);
expect(result).toBe(PENDING);
}

function databaseExists(): Promise<boolean> {
return indexedDB.databases().then(dbs => dbs.some(db => db.name === 'xforge'));
}

function deleteDatabase(): Promise<void> {
return new Promise<void>((resolve, reject) => {
const request = indexedDB.deleteDatabase('xforge');
request.onsuccess = () => resolve();
request.onerror = () => reject(request.error);
request.onblocked = () => resolve();
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,13 @@ export class IndexeddbOfflineStore extends OfflineStore {
}

private openDB(): Promise<IDBDatabase> {
if (this.disabled) {
// Never settles, so that reads and writes still in flight at logout halt rather than
// re-create the deleted database or act on fabricated empty results (SF-3855). The page is
// about to unload; this is the same graceful waiting used in AuthHttpInterceptor while a
// redirect is pending. Do not store this promise in openDBPromise, or closeDB would hang.
return new Promise<never>(() => {});
}
if (this.openDBPromise != null) {
return this.openDBPromise;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,23 @@ export interface OfflineData {
* retrieving offline data in the browser.
*/
export abstract class OfflineStore {
private _disabled = false;

/** Whether the store has been permanently disabled by {@link disable}. */
get disabled(): boolean {
return this._disabled;
}

/**
* Permanently prevents this store from reading or writing data; calls made after this may never
* settle. Called on logout (which is followed by a redirect away from the app) before the data is
* deleted, because reads and writes that are still in flight would otherwise re-create the
* deleted database with the logged-out user's data (SF-3855).
*/
disable(): void {
this._disabled = true;
}

abstract getAllIds(collection: string): Promise<string[]>;
abstract getAll<T extends OfflineData>(collection: string): Promise<T[]>;
abstract query<T extends OfflineData>(collection: string, parameters: QueryParameters): Promise<QueryResults<T>>;
Expand Down
Loading