diff --git a/.env.example b/.env.example new file mode 100644 index 0000000000..af8ec52c7f --- /dev/null +++ b/.env.example @@ -0,0 +1,14 @@ +# Optional values for development through the legacy root docker-compose.yml. +# Copy this file to .env. Database authentication remains the safe default. +# Never commit an institution credential or a populated .env file. +DF_AUTH_METHOD=database + +# To exercise AAF locally, obtain a dedicated non-production registration from +# the identity owner, change DF_AUTH_METHOD to aaf, and fill every field below. +DF_AAF_ISSUER_URL= +DF_AAF_AUDIENCE_URL=http://localhost:4200 +DF_AAF_CALLBACK_URL=http://localhost:4200/api/auth/jwt +DF_AAF_IDENTITY_PROVIDER_URL= +DF_AAF_UNIQUE_URL= +DF_AAF_AUTH_SIGNOUT_URL= +DF_SECRET_KEY_AAF= diff --git a/Dockerfile b/Dockerfile index 0a0b3f0c7c..8818bb1674 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,8 +1,14 @@ -FROM node:22 +FROM node:22.23.2-bookworm-slim@sha256:83f487e0a63425e5b4d146fb5e5be574bcbe1b7b843d3ebafdd95eaf7767a7e5 -ENV DEBIAN_FRONTEND noninteractive +# The upstream image ships npm 10.9.8. Upgrade to the latest Node 22-compatible +# npm 11 release to pick up patched bundled dependencies before dropping +# privileges. +RUN npm install --global npm@11.19.1 \ + && npm cache clean --force + +ENV DEBIAN_FRONTEND=noninteractive ENV USER=node -ENV NODE_ENV docker +ENV NODE_ENV=docker # You can not use `${USER}` here, but reference `/home/node`. ENV PATH="/home/node/.npm-global/bin:${PATH}" @@ -26,5 +32,5 @@ RUN npm ci --force --include=optional EXPOSE 9876 -# Launch - install on launch so that node_modules are updated in volume -CMD /bin/bash -c 'npm install; npm start' +# Install on launch so that bind-mounted source and node_modules stay current. +CMD ["/bin/bash", "-c", "npm install && exec npm start"] diff --git a/deploy.Dockerfile b/deploy.Dockerfile index ce51c53b13..d0e431c68d 100644 --- a/deploy.Dockerfile +++ b/deploy.Dockerfile @@ -1,5 +1,10 @@ ### STAGE 1: Build ### -FROM node:22 AS build +FROM node:22.23.2-bookworm-slim@sha256:83f487e0a63425e5b4d146fb5e5be574bcbe1b7b843d3ebafdd95eaf7767a7e5 AS build + +# Keep the build package manager aligned with the development image. npm 11 +# removes the vulnerable dependencies bundled with this Node 22 base. +RUN npm install --global npm@11.19.1 \ + && npm cache clean --force RUN apt-get update && apt-get install -y --no-install-recommends \ gettext-base \ @@ -11,10 +16,9 @@ USER node # Copy in doubtfire-web code WORKDIR /doubtfire-web COPY package.json package-lock.json ./ -RUN npm ci --force --include=optional +RUN npm ci --include=optional COPY --chown=node:node . . -RUN chmod 777 src ARG SENTRY_DSN ARG SENTRY_ORG @@ -42,7 +46,11 @@ RUN --mount=type=secret,id=sentry_auth_token,uid=1000 \ ## STAGE 2: Host ### -FROM nginx:1.29.0-alpine +FROM nginx:1.30.4-alpine@sha256:97d490c12ba55b4946b01546d1c3ed324e8d41ab1c9fcb2a616aa470620e5b46 + +# The pinned Nginx image predates Alpine's OpenSSL 3.5.8 security update. +# Upgrade only the affected runtime libraries rather than the whole base. +RUN apk upgrade --no-cache libcrypto3 libssl3 # Remove the default Nginx configuration file RUN rm -v /etc/nginx/nginx.conf diff --git a/docker-compose.yml b/docker-compose.yml index ee16895e5d..e478833212 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,4 +1,3 @@ -version: '3' services: doubtfire-api: container_name: web-doubtfire-api @@ -21,15 +20,17 @@ services: DF_SECRET_KEY_ATTR: test-secret-key-test-secret-key! DF_SECRET_KEY_DEVISE: test-secret-key-test-secret-key! - # Authentication method - can set to AAF or ldap - DF_AUTH_METHOD: database - DF_AAF_ISSUER_URL: https://rapid.test.aaf.edu.au - DF_AAF_AUDIENCE_URL: http://localhost:4200 - DF_AAF_CALLBACK_URL: http://localhost:4200/api/auth/jwt - DF_AAF_IDENTITY_PROVIDER_URL: https://signon-uat.deakin.edu.au/idp/shibboleth - DF_AAF_UNIQUE_URL: https://rapid.test.aaf.edu.au/jwt/authnrequest/research/Ag4EJJhjf0zXHqlKvKZEbg - DF_AAF_AUTH_SIGNOUT_URL: https://sync-uat.deakin.edu.au/auth/logout - DF_SECRET_KEY_AAF: v4~LMFLzzwRGZdju\5QBa@FiHIN9 + # Database authentication is the safe local default. Optional AAF + # development values must come from an ignored .env file; never commit an + # institution credential or registration URL here. + DF_AUTH_METHOD: ${DF_AUTH_METHOD:-database} + DF_AAF_ISSUER_URL: ${DF_AAF_ISSUER_URL:-} + DF_AAF_AUDIENCE_URL: ${DF_AAF_AUDIENCE_URL:-http://localhost:4200} + DF_AAF_CALLBACK_URL: ${DF_AAF_CALLBACK_URL:-http://localhost:4200/api/auth/jwt} + DF_AAF_IDENTITY_PROVIDER_URL: ${DF_AAF_IDENTITY_PROVIDER_URL:-} + DF_AAF_UNIQUE_URL: ${DF_AAF_UNIQUE_URL:-} + DF_AAF_AUTH_SIGNOUT_URL: ${DF_AAF_AUTH_SIGNOUT_URL:-} + DF_SECRET_KEY_AAF: ${DF_SECRET_KEY_AAF:-} # Database settings - for development env DF_DEV_DB_ADAPTER: mysql2 diff --git a/scripts/verify-deployment-config.js b/scripts/verify-deployment-config.js new file mode 100644 index 0000000000..03866c5f20 --- /dev/null +++ b/scripts/verify-deployment-config.js @@ -0,0 +1,145 @@ +const assert = require('node:assert/strict'); +const {readFileSync} = require('node:fs'); +const {resolve} = require('node:path'); + +const root = resolve(__dirname, '..'); +const nginx = readFileSync(resolve(root, 'nginx.conf'), 'utf8'); +const dockerfile = readFileSync(resolve(root, 'deploy.Dockerfile'), 'utf8'); +const developmentCompose = readFileSync(resolve(root, 'docker-compose.yml'), 'utf8'); +const deploymentWorkflows = [ + ['default', readFileSync(resolve(root, '.github/workflows/deployment.yml'), 'utf8')], + [ + 'institution', + readFileSync(resolve(root, '.github/workflows/deployment-institution.yml'), 'utf8'), + ], +]; +const applicationBootstrap = readFileSync(resolve(root, 'src/main.ts'), 'utf8'); +const scormPlayerTemplate = readFileSync( + resolve(root, 'src/app/common/scorm-player/scorm-player.component.html'), + 'utf8', +); +const productionEnvironment = readFileSync( + resolve(root, 'src/environments/environment.prod.ts'), + 'utf8', +); + +const controlFiles = [ + '/index.html', + '/ngsw.json', + '/ngsw-worker.js', + '/safety-worker.js', + '/worker-basic.min.js', + '/manifest.webmanifest', +]; +const noStorePolicy = 'no-store, no-cache, must-revalidate, proxy-revalidate, max-age=0'; + +const escapeRegExp = (value) => value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + +for (const path of controlFiles) { + const escapedPath = escapeRegExp(path); + + assert.match( + nginx, + new RegExp(`^\\s*${escapedPath}\\s+"${noStorePolicy}";`, 'm'), + `${path} must map to the no-store cache policy`, + ); + assert.match( + nginx, + new RegExp(`location = ${escapedPath} \\{\\s*try_files \\$uri =404;\\s*\\}`, 'm'), + `${path} must use an exact location and return 404 when absent`, + ); +} + +assert.match( + nginx, + /add_header Cache-Control \$ontrack_control_file_cache_control always;/, + 'the mapped cache policy must be added to responses', +); +assert.match( + nginx, + /try_files \$uri \$uri\/ \$uri\/index\.html \/index\.html;/, + 'SPA routes must continue to fall back to /index.html', +); +for (const [name, workflow] of deploymentWorkflows) { + assert.doesNotMatch( + workflow, + /steps\.meta\.outputs\.labels/, + `${name} deployment metadata labels must reference the declared docker_meta step`, + ); + assert.match(workflow, /steps\.docker_meta\.outputs\.labels/); + + for (const line of workflow.split('\n')) { + if (/^\s*uses:/.test(line)) { + assert.match( + line, + /@[0-9a-f]{40}(?:\s+#.*)?$/, + `${name} production image workflow actions must be pinned to immutable commits`, + ); + } + } + + assert.match(workflow, /^\s*sbom:\s*true$/m); + assert.match(workflow, /^\s*provenance:\s*mode=max$/m); +} +assert.match( + dockerfile, + /^FROM node:22\.23\.2-bookworm-slim@sha256:[0-9a-f]{64} AS build$/m, + 'the release build must pin the supported Node image digest', +); +assert.match( + dockerfile, + /npm install --global npm@11\.19\.1/, + 'the release build must install the reviewed npm security update', +); +assert.match( + dockerfile, + /^FROM nginx:1\.30\.4-alpine@sha256:[0-9a-f]{64}$/m, + 'the release runtime must pin the Nginx image digest', +); +assert.match( + dockerfile, + /^RUN apk upgrade --no-cache libcrypto3 libssl3$/m, + 'the release runtime must install the reviewed Alpine OpenSSL security update', +); +assert.doesNotMatch( + dockerfile, + /npm ci[^\n]*--force/, + 'the release build must not force npm resolution', +); +assert.doesNotMatch( + dockerfile, + /chmod\s+777/, + 'the release build must not create world-writable source', +); +assert.match( + developmentCompose, + /DF_SECRET_KEY_AAF:\s*\$\{DF_SECRET_KEY_AAF:-\}/, + 'optional development AAF credentials must come from the ignored environment', +); +assert.doesNotMatch( + developmentCompose, + /https?:\/\/[^\s$]*(?:aaf\.edu\.au|deakin\.edu\.au)/i, + 'institution-specific AAF endpoints must not be committed in development Compose', +); +assert.doesNotMatch( + applicationBootstrap, + /(?:browserTracingIntegration|replayIntegration|tracesSampleRate|replaysSessionSampleRate|replaysOnErrorSampleRate)/, + 'tracing and replay must stay disabled while SCORM uses credential-bearing paths', +); +assert.match( + applicationBootstrap, + /enableLogs:\s*false/, + 'Sentry Logs must stay disabled for the errors-only telemetry policy', +); +assert.match( + scormPlayerTemplate, + /]*data-sentry-block[^>]*referrerpolicy="no-referrer"/, + 'the SCORM iframe must suppress referrers and remain blocked from any future replay integration', +); +assert.match( + productionEnvironment, + /enableDemoTools:\s*false/, + 'production must keep demo tools disabled', +); + +console.log('Deployment configuration checks passed.'); diff --git a/src/app/api/services/authentication.service.ts b/src/app/api/services/authentication.service.ts index 1f4ed2b7ff..5c045c24f1 100644 --- a/src/app/api/services/authentication.service.ts +++ b/src/app/api/services/authentication.service.ts @@ -1,12 +1,40 @@ -import {HttpClient} from '@angular/common/http'; +import {HttpClient, HttpErrorResponse} from '@angular/common/http'; import {Injectable} from '@angular/core'; import {Router} from '@angular/router'; -import {AsyncSubject, Observable, catchError, map, throwError} from 'rxjs'; +import { + AsyncSubject, + EMPTY, + Observable, + Subject, + Subscription, + TimeoutError, + catchError, + filter, + map, + of, + switchMap, + takeUntil, + tap, + throwError, + timeout, +} from 'rxjs'; import {User, UserService} from 'src/app/api/models/doubtfire-model'; +import {NotificationFeedbackRouteIntentService} from 'src/app/api/services/notification-feedback-route-intent.service'; +import {NotificationService} from 'src/app/api/services/notification.service'; +import {PushNotificationService} from 'src/app/api/services/push-notification.service'; import {AppInjector} from 'src/app/app-injector'; import {AlertService} from 'src/app/common/services/alert.service'; -import {DoubtfireConstants} from 'src/app/config/constants/doubtfire-constants'; +import {FeedbackDraftStore} from 'src/app/common/services/feedback-draft-store.service'; +import {ThemeService} from 'src/app/common/theme/theme.service'; +import { + type AuthenticatedSettingsResponseFormat, + DoubtfireConstants, +} from 'src/app/config/constants/doubtfire-constants'; +import {DemoModeStore} from 'src/app/demo/demo-mode.store'; +import {DemoScenarioRegistryService} from 'src/app/demo/demo-scenario-registry.service'; import {GlobalStateService, ViewType} from 'src/app/projects/states/index/global-state.service'; +import {buildAuthCallbackFragment} from 'src/app/security/auth-callback'; +import {AuthReturnUrlService} from 'src/app/security/auth-return-url.service'; /** * The format for the data returned from the auth api. @@ -18,6 +46,17 @@ interface AuthResponse { lti_token?: string; } +export type RefreshTokenFailureReason = + | 'not-requested' + | 'expired' + | 'offline' + | 'timeout' + | 'unavailable'; + +export const REFRESH_TOKEN_TIMEOUT_MS = 12_000; + +class RefreshTokenRejectedError extends Error {} + @Injectable() export class AuthenticationService { /** @@ -41,19 +80,45 @@ export class AuthenticationService { */ private authComplete$: AsyncSubject = new AsyncSubject(); + /** + * Cancels a protected settings request as soon as sign out starts. A plain + * Subject is intentional: requests created by a later session must not see a + * previous session's cancellation event. + */ + private authenticatedSettingsCancellation$: Subject = new Subject(); + + /** + * Changes on sign out so a response belonging to an earlier session cannot + * publish settings or complete authentication for the current session. + */ + private authenticationGeneration = 0; + + /** + * Only the newest settings response for the current session may update the + * singleton constants. Token refresh can legitimately overlap a prior read. + */ + private authenticatedSettingsRequestId = 0; + private hydrationRequestId = 0; + constructor( private httpClient: HttpClient, private userService: UserService, private alertService: AlertService, private angularRouter: Router, private doubtfireConstants: DoubtfireConstants, + private demoMode: DemoModeStore, + private demoScenarioRegistry: DemoScenarioRegistryService, + private authReturnUrl: AuthReturnUrlService, + private themeService: ThemeService, + private feedbackDraftStore: FeedbackDraftStore, ) { this.AUTH_URL = `${this.doubtfireConstants.API_URL}/auth`; // Ensure any only user data is removed from local storage - localStorage.removeItem(this.USERNAME_KEY); + this.browserStorage?.removeItem(this.USERNAME_KEY); } private actionAuthFailed() { + this.authReturnUrl.rememberCurrentUrl(); this.signOut(false); } @@ -64,34 +129,93 @@ export class AuthenticationService { * @param loginResultCallback - Callback function to indicate success or failure of login. */ public attemptLoginUsingRefreshToken( - loginResultCallback: (result: boolean) => void, + loginResultCallback: (result: boolean, failureReason?: RefreshTokenFailureReason) => void, firstTime: boolean = true, ): void { // Check we have indication of secure cookie in local storage const remember: boolean = this.rememberMe; if (!remember) { - loginResultCallback(false); + loginResultCallback(false, 'not-requested'); return; } - // Attempt to get an access token using the refresh token cookie - this.httpClient.post(this.AUTH_URL + '/access-token', {}).subscribe({ - next: (response: AuthResponse | null) => { - if (response && response.auth_token) { - this.setupUserFromResponse(response, firstTime); - loginResultCallback(true); - } else { - this.actionAuthFailed(); - loginResultCallback(false); - } - }, - error: (_error) => { - // Will occur on 404 when the refresh token cookie is not present - this.actionAuthFailed(); - loginResultCallback(false); - }, - }); + const authenticationGeneration = this.authenticationGeneration; + + // The timeout wraps both the token exchange and protected settings + // hydration. A response from /access-token alone is not a usable session. + this.httpClient + .post(this.AUTH_URL + '/access-token', {}) + .pipe( + switchMap((response) => { + if (authenticationGeneration !== this.authenticationGeneration) { + return EMPTY; + } + if (!response?.auth_token) { + return throwError(() => new RefreshTokenRejectedError()); + } + return this.setupUserFromResponse(response, firstTime, authenticationGeneration); + }), + timeout({first: REFRESH_TOKEN_TIMEOUT_MS}), + ) + .subscribe({ + next: () => loginResultCallback(true), + error: (error: unknown) => { + if (authenticationGeneration !== this.authenticationGeneration) { + return; + } + + const failureReason = + error instanceof RefreshTokenRejectedError + ? 'expired' + : this.refreshTokenFailureReason(error); + if (failureReason === 'expired') { + // A definitive rejection clears the obsolete session. Network and + // server availability failures remain retryable and do not discard a + // valid refresh cookie that the browser may still hold. + this.actionAuthFailed(); + } else if (firstTime) { + // The token exchange may have populated currentUser before protected + // settings timed out. Cold-start recovery must return to a genuinely + // signed-out in-memory state without deleting the still-valid refresh + // cookie, so Retry performs the complete hydration again. + this.clearPartialRestoredSession(authenticationGeneration); + } + loginResultCallback(false, failureReason); + }, + }); + } + + private refreshTokenFailureReason(error: unknown): RefreshTokenFailureReason { + if (error instanceof TimeoutError) { + return 'timeout'; + } + + if (error instanceof HttpErrorResponse) { + if (error.status === 0) { + return typeof navigator !== 'undefined' && navigator.onLine === false + ? 'offline' + : 'unavailable'; + } + if ([401, 403, 404, 419].includes(error.status)) { + return 'expired'; + } + } + + return 'unavailable'; + } + + private clearPartialRestoredSession(authenticationGeneration: number): void { + if (authenticationGeneration !== this.authenticationGeneration) { + return; + } + + this.authenticatedSettingsRequestId += 1; + this.authenticatedSettingsCancellation$.next(); + this.themeService.disconnectAccount(); + this.doubtfireConstants.resetAuthenticatedSettings(); + this.userService.currentUser = this.userService.anonymousUser; + this.userService.cache.clear(); } /** @@ -107,11 +231,22 @@ export class AuthenticationService { } public get rememberMe(): boolean { - return localStorage.getItem(this.REMEMBER_DOUBTFIRE_CREDENTIALS_TOKEN) !== 'false'; + return this.browserStorage?.getItem(this.REMEMBER_DOUBTFIRE_CREDENTIALS_TOKEN) !== 'false'; } public set rememberMe(remember: boolean) { - localStorage.setItem(this.REMEMBER_DOUBTFIRE_CREDENTIALS_TOKEN, remember ? 'true' : 'false'); + this.browserStorage?.setItem( + this.REMEMBER_DOUBTFIRE_CREDENTIALS_TOKEN, + remember ? 'true' : 'false', + ); + } + + private get browserStorage(): Storage | null { + try { + return globalThis.localStorage ?? null; + } catch { + return null; + } } /** @@ -160,13 +295,68 @@ export class AuthenticationService { ); } + private isCurrentAuthentication(userId: number, authenticationGeneration: number): boolean { + return ( + authenticationGeneration === this.authenticationGeneration && + userId === this.userService.currentUser.id && + !!this.userService.currentUser.authenticationToken + ); + } + + private loadAuthenticatedSettings( + userId: number, + authenticationGeneration: number, + ): Observable { + const url: string = `${this.doubtfireConstants.API_URL}/settings`; + const requestId = ++this.authenticatedSettingsRequestId; + + return this.httpClient.get(url).pipe( + takeUntil(this.authenticatedSettingsCancellation$), + switchMap((settings) => { + if ( + !this.isCurrentAuthentication(userId, authenticationGeneration) || + requestId !== this.authenticatedSettingsRequestId + ) { + return EMPTY; + } + + this.doubtfireConstants.applyAuthenticatedSettings(settings); + + return of(void 0); + }), + catchError((error) => { + if ( + !this.isCurrentAuthentication(userId, authenticationGeneration) || + requestId !== this.authenticatedSettingsRequestId + ) { + return EMPTY; + } + + // A failed settings read must not leave a previous session's feature + // flags or VAPID key active. Stale overlapping requests return above and + // cannot reset or publish authentication readiness. + this.doubtfireConstants.resetAuthenticatedSettings(); + console.error('Unable to load authenticated settings', error); + + // Settings are fail closed, but their availability must not turn valid + // credentials into a failed sign in. + return of(void 0); + }), + ); + } + /** * Use the user service to get or create a user object, and update it * from the response. Ensure that the authentication token is set. * * @param response the response from the authentication API */ - private setupUserFromResponse(response: AuthResponse, firstTime: boolean = true): void { + private setupUserFromResponse( + response: AuthResponse, + firstTime: boolean = true, + authenticationGeneration: number = this.authenticationGeneration, + ): Observable { + const hydrationRequestId = ++this.hydrationRequestId; // Extract relevant data from response and construct user object to store in cache. const user: User = this.userService.cache.getOrCreate( response.user['id'], @@ -178,19 +368,61 @@ export class AuthenticationService { user.authenticationToken = response['auth_token']; user.authenticationTokenExpiry = response['auth_token_expiry']; + // This path also runs on hourly token refresh and can receive overlapping + // sign-ins. Capture the prior account identity before replacing currentUser: + // only a refresh for the same account may keep the existing save closure. + const previousAuthenticatedUserId = this.isAuthenticated() + ? this.userService.currentUser.id + : undefined; + // Record the current user this.userService.currentUser = user; - if (firstTime) { - // Load everything! - AppInjector.get(GlobalStateService).loadGlobals(); - // Update token in one hour - setTimeout(() => this.cycleAccessToken(), 1000 * 60 * 60); + // Connect only on a real sign-in/session restore. Hourly token refreshes keep + // the existing connection and therefore cannot clobber a mid-session choice. + if (previousAuthenticatedUserId !== user.id) { + this.themeService.connectAccount( + user.id, + user.themePreference, + user.themePreferenceUpdatedAt, + (preference) => + this.userService.updateThemePreference(user, preference).pipe( + map((updatedUser) => ({ + preference: updatedUser.themePreference, + updatedAt: updatedUser.themePreferenceUpdatedAt, + })), + ), + ); } - // Inidcate that the authentication was successful - this.authComplete$.next(true); - this.authComplete$.complete(); + // Feature flags are part of authentication readiness. Consumers use their + // current values synchronously, so do not load globals, publish authComplete + // or complete signIn until this authenticated request has settled. + const isCurrentHydration = () => + hydrationRequestId === this.hydrationRequestId && + this.isCurrentAuthentication(user.id, authenticationGeneration); + + return this.loadAuthenticatedSettings(user.id, authenticationGeneration).pipe( + switchMap(() => + this.demoScenarioRegistry.loadForAuthenticatedUser(user.id, isCurrentHydration), + ), + filter(isCurrentHydration), + // The demo lookup is also protected session work. Sign-out must cancel it + // before its response can publish state or complete authentication. + takeUntil(this.authenticatedSettingsCancellation$), + tap(() => { + if (firstTime) { + // Load everything! + AppInjector.get(GlobalStateService).loadGlobals(); + // Update token in one hour + setTimeout(() => this.cycleAccessToken(), 1000 * 60 * 60); + } + + // Indicate that authentication and its protected settings are ready. + this.authComplete$.next(true); + this.authComplete$.complete(); + }), + ); } /** @@ -201,8 +433,8 @@ export class AuthenticationService { * * @param callback the callback function to call */ - public afterAuthCall(callback: (result: boolean) => void): void { - this.authComplete$.subscribe({ + public afterAuthCall(callback: (result: boolean) => void): Subscription { + return this.authComplete$.subscribe({ next: (result) => { callback(result); }, @@ -222,10 +454,14 @@ export class AuthenticationService { remember: boolean; }, ): Observable { + const authenticationGeneration = this.authenticationGeneration; + return this.httpClient.post(this.AUTH_URL, userCredentials).pipe( - map((response: AuthResponse) => { - this.setupUserFromResponse(response); - }), + switchMap((response: AuthResponse) => + authenticationGeneration === this.authenticationGeneration + ? this.setupUserFromResponse(response, true, authenticationGeneration) + : EMPTY, + ), catchError((error) => { // this.authComplete$.next(false); // this.authComplete$.complete(); @@ -238,11 +474,14 @@ export class AuthenticationService { public signInWithLti(userCredentials: {ltik: string; lti_token: string}): Observable { return this.httpClient.post(`${this.AUTH_URL}/lti`, userCredentials).pipe( map((response: AuthResponse) => { - const username = encodeURIComponent(response.user['username']); - const authToken = encodeURIComponent(response.auth_token); - const ltik = encodeURIComponent(userCredentials.ltik); + const fragment = buildAuthCallbackFragment({ + username: response.user['username'], + authToken: response.auth_token, + ltik: userCredentials.ltik, + isLtiLogin: true, + }); setTimeout(() => { - window.location.href = `/sign_in?username=${username}&authToken=${authToken}<ik=${ltik}&isLtiLogin=true`; + window.location.href = `/sign_in#${fragment}`; }); }), catchError((error) => { @@ -251,13 +490,90 @@ export class AuthenticationService { ); } + // Removes this user's unsent comment drafts from browser storage, along with + // any left over from before drafts were scoped to a user, which belong to + // nobody and would otherwise sit there for good. + // + // Keys are collected first and deleted afterwards. Calling removeItem inside a + // forward loop over localStorage.key(i) shifts the indices along and skips + // every second match. + private clearCommentDrafts(userId?: number): void { + if (userId) { + this.feedbackDraftStore.clearUser(userId); + } + const draftPrefix = 'task_comment_draft_'; + const userPrefix = userId ? `${draftPrefix}uid${userId}_` : null; + + try { + const doomed: string[] = []; + for (let i = 0; i < localStorage.length; i++) { + const key = localStorage.key(i); + if (!key || !key.startsWith(draftPrefix)) { + continue; + } + // This user's drafts, and the old unscoped keys which have a task id + // straight after the prefix rather than a user id. + if ((userPrefix && key.startsWith(userPrefix)) || !this.isUserScopedDraftKey(key)) { + doomed.push(key); + } + } + doomed.forEach((key) => localStorage.removeItem(key)); + } catch (e) { + console.error('Error clearing comment drafts:', e); + } + + try { + if (userId) { + sessionStorage.removeItem(`task_comments_submitted_${userId}`); + } + sessionStorage.removeItem('task_comments_submitted'); + } catch (e) { + console.error('Error clearing submitted comment tasks:', e); + } + } + + // A scoped key is task_comment_draft_uid_. Anything else under + // the prefix was written before drafts were scoped to a user, belongs to + // nobody, and nothing will ever read it again. A legacy key starts with a task + // or project id, or the literal 'unknown', so it can never begin with uid + // followed by digits. + private isUserScopedDraftKey(key: string): boolean { + const rest = key.substring('task_comment_draft_'.length); + return /^uid\d+_/.test(rest); + } + public signOut(ssoSignOut = true): void { + if (ssoSignOut) { + this.authReturnUrl.clear(); + } + + this.demoMode.reset(); + this.demoScenarioRegistry.clear(); + + // Cancel any debounced/in-flight account write for this session. The local + // theme keys intentionally remain so the next boot stays flash-free. + this.themeService.disconnectAccount(); + + // Invalidate authentication work before the asynchronous push and token + // teardown. This prevents a late /settings response from restoring values + // after sign out or completing an obsolete sign-in observable. + this.authenticationGeneration += 1; + this.authenticatedSettingsRequestId += 1; + this.authenticatedSettingsCancellation$.next(); + this.doubtfireConstants.resetAuthenticatedSettings(); + // This function is called after the token is deleted... const doSignOut = () => { // Setup ability to auth again this.authComplete$.complete(); this.authComplete$ = new AsyncSubject(); + // Unsent comment drafts live in browser storage and sign out routes rather + // than reloading, so nothing else drops them. Clear them before the current + // user is replaced, otherwise the prefix is built from the anonymous user + // and nothing matches. + this.clearCommentDrafts(this.userService.currentUser?.id); + // Change the current user to the anonymous user this.userService.currentUser = this.userService.anonymousUser; @@ -266,6 +582,13 @@ export class AuthenticationService { globalStateService.clearUnitsAndProjects(); this.userService.cache.clear(); + // Notifications are per user and NotificationService is a root singleton, + // so its cache and unread count outlive the session. Sign out routes + // rather than reloading, so nothing else drops them and the next person on + // a shared machine would see this user's messages. + AppInjector.get(NotificationService).reset(); + AppInjector.get(NotificationFeedbackRouteIntentService).clear(); + // Trigger the UI changes globalStateService.hideHeader(); globalStateService.setView(ViewType.OTHER); @@ -279,18 +602,43 @@ export class AuthenticationService { }; // If we have a token, delete it... + const deleteTokenAndSignOut = () => { + if (this.userService.currentUser.authenticationToken) { + this.httpClient.delete(this.AUTH_URL, {params: {remember: false}}).subscribe({ + next: (_response) => doSignOut(), + error: (_response) => doSignOut(), + }); + } else { + doSignOut(); + } + }; + + // Drop the push registration before the auth token goes. + // + // The subscription belongs to whoever enabled it, and the browser keeps it + // across sign out. On a shared machine that means the next person to sign in + // would keep receiving the previous user's notifications on this device. + // + // Order matters twice over. DELETE /push_subscriptions needs the token, so + // it has to run first, and sign out must never be blocked by it, so both + // outcomes continue. Resolved through AppInjector rather than the + // constructor to avoid a circular dependency, the same as GlobalStateService + // above. if (this.userService.currentUser.authenticationToken) { - this.httpClient.delete(this.AUTH_URL, {params: {remember: false}}).subscribe({ - next: (_response) => doSignOut(), - error: (_response) => doSignOut(), - }); + AppInjector.get(PushNotificationService) + .unsubscribeQuietly() + .subscribe({ + next: () => deleteTokenAndSignOut(), + error: () => deleteTokenAndSignOut(), + }); } else { - doSignOut(); + deleteTokenAndSignOut(); } } public timeoutAuthentication(): void { if (window.location.pathname !== '/timeout') { + this.authReturnUrl.rememberCurrentUrl(); this.alertService.error('Authentication timed out', 6000); setTimeout(() => this.angularRouter.navigateByUrl('/timeout'), 500); } diff --git a/src/app/home/states/lti-dashboard/lti-dashboard.component.ts b/src/app/home/states/lti-dashboard/lti-dashboard.component.ts index dd775f824d..eac5af42e0 100644 --- a/src/app/home/states/lti-dashboard/lti-dashboard.component.ts +++ b/src/app/home/states/lti-dashboard/lti-dashboard.component.ts @@ -1,5 +1,5 @@ -import {AfterViewInit, ChangeDetectionStrategy, Component, Input} from '@angular/core'; -import {ActivatedRoute, Router} from '@angular/router'; +import {ChangeDetectionStrategy, Component, Input, OnInit} from '@angular/core'; +import {Router} from '@angular/router'; import {ProjectService, User} from 'src/app/api/models/doubtfire-model'; import {Unit} from 'src/app/api/models/unit'; import {AuthenticationService} from 'src/app/api/services/authentication.service'; @@ -18,10 +18,9 @@ import {AlertService} from 'src/app/common/services/alert.service'; changeDetection: ChangeDetectionStrategy.Eager, standalone: false, }) -export class LtiDashboardComponent implements AfterViewInit { +export class LtiDashboardComponent implements OnInit { constructor( private router: Router, - private route: ActivatedRoute, private ltiService: LtiService, private userService: UserService, private authenticationService: AuthenticationService, @@ -41,21 +40,21 @@ export class LtiDashboardComponent implements AfterViewInit { unauthorised: boolean = false; loadingState: 'creatingUser' | 'enrollingUser' | 'fetchingUnit'; - isLoading: boolean; + // Starts true, so the page opens on its loading state rather than a blank one. + isLoading: boolean = true; isSyncingGrades: boolean; isSyncingEnrolments: boolean; - ngAfterViewInit(): void { - this.ltik = this.ltik ?? this.route.snapshot.queryParamMap.get('ltik'); + // In ngOnInit rather than ngAfterViewInit: nothing here needs the view, and state set + // after the view was checked threw ExpressionChangedAfterItHasBeenChecked in dev builds. + ngOnInit(): void { + this.ltik = this.ltik ?? this.userService.currentUser.ltik; // Scroll to the bottom of the page in case the header is visible // Ensures our action buttons are centered setTimeout(() => window.scrollTo(0, document.body.scrollHeight), 100); - this.isLoading = true; - // TODO: add a spinner or loading indicator until final loading state is complete - this.authenticationService.afterAuthCall(() => { this.userService.currentUser.ltik = this.ltik; this.currentUser = this.userService.currentUser; @@ -100,19 +99,23 @@ export class LtiDashboardComponent implements AfterViewInit { }); } + /** Convenors and admins can link, unlink and sync. Everyone else can only launch. */ + get canManageLink(): boolean { + return this.currentUser?.systemRole === 'Convenor' || this.currentUser?.systemRole === 'Admin'; + } + goToLinkUnit(): void { - this.router.navigate(['/lti/link'], {queryParams: {ltik: this.ltik}}); + this.router.navigateByUrl('/lti/link'); } removeLink(): void { this.ltiService.removeUnitLink().subscribe({ - next: (link) => { + next: () => { this.linkedUnit = null; - console.log(link); }, error: (error) => { - console.error(error); - this.alertsService.error(error.error, 6000); + // Errors reach here as the message itself, so error.error was always undefined. + this.alertsService.error(`Failed to unlink the unit: ${error?.error ?? error}`, 6000); }, }); } @@ -203,16 +206,17 @@ export class LtiDashboardComponent implements AfterViewInit { this.alertsService.success('Successfully synced grades from OnTrack', 5000); this.csvResultModalService.show('Grade sync', result); }, - error: (error) => { - console.log(error); - this.alertsService.error(`Failed to retrieve grade`); - this.isSyncingGrades = true; + error: () => { + this.alertsService.error(`Failed to sync grades`); + // This was set back to true, which left both sync buttons disabled and + // spinning after any failed grade sync until the page was reloaded. + this.isSyncingGrades = false; }, }); }, ); } public launchApplication(): void { - window.open('http://localhost:4200/home', '_blank'); + window.open('/home', '_blank'); } } diff --git a/src/app/home/states/lti-unit-link/lti-unit-link.component.ts b/src/app/home/states/lti-unit-link/lti-unit-link.component.ts index 74f4c8bbb3..bc896955d0 100644 --- a/src/app/home/states/lti-unit-link/lti-unit-link.component.ts +++ b/src/app/home/states/lti-unit-link/lti-unit-link.component.ts @@ -1,5 +1,5 @@ -import {AfterViewInit, ChangeDetectionStrategy, Component, Input} from '@angular/core'; -import {ActivatedRoute, Router} from '@angular/router'; +import {ChangeDetectionStrategy, Component, Input, OnInit} from '@angular/core'; +import {Router} from '@angular/router'; import {CreateNewUnitModal} from 'src/app/admin/modals/create-new-unit-modal/create-new-unit-modal.component'; import {Unit} from 'src/app/api/models/unit'; import {AuthenticationService} from 'src/app/api/services/authentication.service'; @@ -16,7 +16,7 @@ import {AlertService} from 'src/app/common/services/alert.service'; changeDetection: ChangeDetectionStrategy.Eager, standalone: false, }) -export class LtiUnitLinkComponent implements AfterViewInit { +export class LtiUnitLinkComponent implements OnInit { constructor( private createUnitModalService: CreateNewUnitModal, private unitService: UnitService, @@ -26,7 +26,6 @@ export class LtiUnitLinkComponent implements AfterViewInit { private ltiService: LtiService, private userService: UserService, private router: Router, - private route: ActivatedRoute, ) {} @Input() ltik: string; @@ -36,11 +35,14 @@ export class LtiUnitLinkComponent implements AfterViewInit { public unitLinked: boolean = false; - public loadingUnits: boolean; + // Starts true, so the page opens on its loading state rather than a blank one. + public loadingUnits: boolean = true; + public loadUnitsFailed = false; - ngAfterViewInit(): void { - this.ltik = this.ltik ?? this.route.snapshot.queryParamMap.get('ltik'); - this.loadingUnits = true; + // In ngOnInit rather than ngAfterViewInit: nothing here needs the view, and state set + // after the view was checked threw ExpressionChangedAfterItHasBeenChecked in dev builds. + ngOnInit(): void { + this.ltik = this.ltik ?? this.userService.currentUser.ltik; // Scroll to the bottom of the page in case the header is visible // Ensures our action buttons are centered @@ -60,7 +62,6 @@ export class LtiUnitLinkComponent implements AfterViewInit { 'Once you have linked an OnTrack unit, students who launch this app will be enrolled automatically. Unlinking a unit will not withdraw students automatically.', () => { // Trigger API call to LTI.js with our unit link request - console.log('Trigger API call to LTI.js'); this.linkUnit(this.selectedUnit); }, ); @@ -88,21 +89,21 @@ export class LtiUnitLinkComponent implements AfterViewInit { // unitName: unit.name, }) .subscribe({ - next: (link) => { - console.log(link); - + next: () => { this.alertsService.success(`Successfully linked ${unit.code}`, 5000); - this.router.navigate(['/lti'], {queryParams: {ltik: this.ltik}}); + this.router.navigateByUrl('/lti'); }, error: (error) => { - console.log(error); - this.alertsService.error(`Failed to link unit: ${error.error}`, 6000); + // Errors reach here as the message itself, so error.error was always undefined. + this.alertsService.error(`Failed to link unit: ${error?.error ?? error}`, 6000); }, }); } public loadUnits(): void { + this.loadingUnits = true; + this.loadUnitsFailed = false; // Load units that current user is a convenor/admin of this.unitService .fetchAll( @@ -122,7 +123,9 @@ export class LtiUnitLinkComponent implements AfterViewInit { this.loadingUnits = false; }, error: (_error) => { - this.alertsService.error(`Failed to fetch units`, 6000); + // Without this the page said "Loading..." for good after a failed request. + this.loadingUnits = false; + this.loadUnitsFailed = true; }, }); } diff --git a/src/app/security/auth-callback.spec.ts b/src/app/security/auth-callback.spec.ts new file mode 100644 index 0000000000..c55b1d1b33 --- /dev/null +++ b/src/app/security/auth-callback.spec.ts @@ -0,0 +1,103 @@ +import {beforeEach, describe, expect, it} from 'vitest'; +import { + buildAuthCallbackFragment, + captureAndScrubAuthCallback, + consumeAuthCallback, + redactAuthCallbackFromUrl, + resetAuthCallbackForTest, +} from './auth-callback'; + +const SENTINEL = 'SECRET_CALLBACK_SENTINEL_7f91'; + +describe('authentication callback security', () => { + beforeEach(() => resetAuthCallbackForTest()); + + it('captures a fragment callback and removes credentials before startup', () => { + let cleanUrl = ''; + + const safe = captureAndScrubAuthCallback( + `https://ontrack.example/sign_in#authToken=${SENTINEL}&username=student%40example.edu`, + (_data, _unused, url) => { + cleanUrl = String(url); + }, + ); + + expect(safe).toBe(true); + expect(cleanUrl).toBe('/sign_in'); + expect(cleanUrl).not.toContain(SENTINEL); + expect(consumeAuthCallback()).toEqual({ + username: 'student@example.edu', + authToken: SENTINEL, + ltiToken: undefined, + ltik: undefined, + isLtiLogin: false, + }); + expect(consumeAuthCallback()).toBeUndefined(); + }); + + it('supports and scrubs a legacy LTI query while preserving unrelated values', () => { + let cleanUrl = ''; + + captureAndScrubAuthCallback( + `https://ontrack.example/sign_in?theme=dark<iToken=${SENTINEL}<ik=launch-token&isLtiLogin=true#panel=help`, + (_data, _unused, url) => { + cleanUrl = String(url); + }, + ); + + expect(cleanUrl).toBe('/sign_in?theme=dark#panel=help'); + expect(consumeAuthCallback()).toMatchObject({ + ltiToken: SENTINEL, + ltik: 'launch-token', + isLtiLogin: true, + }); + }); + + it('disables telemetry and drops the callback when history cannot be scrubbed', () => { + const safe = captureAndScrubAuthCallback( + `https://ontrack.example/sign_in#authToken=${SENTINEL}`, + () => { + throw new Error('history unavailable'); + }, + ); + + expect(safe).toBe(false); + expect(consumeAuthCallback()).toBeUndefined(); + }); + + it('encodes fragment callbacks without putting values in a query', () => { + const fragment = buildAuthCallbackFragment({ + username: 'student+alias@example.edu', + authToken: `${SENTINEL}+/=`, + isLtiLogin: true, + }); + const values = new URLSearchParams(fragment); + + expect(fragment).not.toContain('?'); + expect(values.get('username')).toBe('student+alias@example.edu'); + expect(values.get('authToken')).toBe(`${SENTINEL}+/=`); + expect(values.get('isLtiLogin')).toBe('true'); + }); + + it('redacts callback secrets from telemetry URLs', () => { + const callbackUsername = 'student.person@example.edu'; + const redacted = redactAuthCallbackFromUrl( + `https://ontrack.example/sign_in?authToken=${SENTINEL}&username=${encodeURIComponent(callbackUsername)}#ltik=${SENTINEL}&user_name=${encodeURIComponent(callbackUsername)}`, + ); + + expect(redacted).not.toContain(SENTINEL); + expect(redacted).not.toContain('student.person'); + expect(redacted).not.toContain(encodeURIComponent(callbackUsername)); + expect(redacted).toContain('%5BFiltered%5D'); + }); + + it('redacts inherited SCORM path credentials from telemetry URLs', () => { + const redacted = redactAuthCallbackFromUrl( + `https://ontrack.example/api/scorm/42/student/${SENTINEL}/index.html`, + ); + + expect(redacted).not.toContain('student'); + expect(redacted).not.toContain(SENTINEL); + expect(redacted).toContain('/api/scorm/42/[Filtered]/[Filtered]/index.html'); + }); +}); diff --git a/src/app/security/auth-callback.ts b/src/app/security/auth-callback.ts new file mode 100644 index 0000000000..6cc54adab4 --- /dev/null +++ b/src/app/security/auth-callback.ts @@ -0,0 +1,151 @@ +export interface AuthCallbackParameters { + username?: string; + authToken?: string; + ltiToken?: string; + ltik?: string; + isLtiLogin?: boolean; +} + +type ReplaceState = (data: unknown, unused: string, url?: string | URL | null) => void; + +const CALLBACK_KEYS = [ + 'username', + 'authToken', + 'auth_token', + 'ltiToken', + 'lti_token', + 'ltik', + 'isLtiLogin', +] as const; +const SECRET_KEYS = ['authToken', 'auth_token', 'ltiToken', 'lti_token', 'ltik'] as const; +const TELEMETRY_KEYS = [...SECRET_KEYS, 'username', 'user_name'] as const; + +let pendingCallback: AuthCallbackParameters | undefined; + +const firstValue = ( + fragment: URLSearchParams, + query: URLSearchParams, + ...keys: string[] +): string | undefined => { + for (const key of keys) { + const value = fragment.get(key) ?? query.get(key); + if (value !== null) { + return value; + } + } + + return undefined; +}; + +const callbackFragment = (hash: string): URLSearchParams => + new URLSearchParams(hash.replace(/^#\??/, '')); + +/** + * Capture a one-time authentication callback in module memory and remove it + * from the address bar before telemetry or Angular start. No callback value is + * persisted in browser storage or exposed on window. + */ +export const captureAndScrubAuthCallback = ( + href: string = globalThis.location.href, + replaceState: ReplaceState = (data, unused, url) => + globalThis.history.replaceState(data, unused, url), +): boolean => { + try { + const url = new URL(href); + const fragment = callbackFragment(url.hash); + const containsSecret = SECRET_KEYS.some( + (key) => fragment.has(key) || url.searchParams.has(key), + ); + + if (!containsSecret) { + return true; + } + + const callback: AuthCallbackParameters = { + username: firstValue(fragment, url.searchParams, 'username'), + authToken: firstValue(fragment, url.searchParams, 'authToken', 'auth_token'), + ltiToken: firstValue(fragment, url.searchParams, 'ltiToken', 'lti_token'), + ltik: firstValue(fragment, url.searchParams, 'ltik'), + isLtiLogin: firstValue(fragment, url.searchParams, 'isLtiLogin')?.toLowerCase() === 'true', + }; + + for (const key of CALLBACK_KEYS) { + fragment.delete(key); + url.searchParams.delete(key); + } + + const remainingFragment = fragment.toString(); + url.hash = remainingFragment ? `#${remainingFragment}` : ''; + replaceState(globalThis.history?.state ?? null, '', `${url.pathname}${url.search}${url.hash}`); + pendingCallback = callback; + return true; + } catch { + // If the browser cannot remove the credential from the address bar, keep + // telemetry disabled and force the user through a fresh sign-in. + pendingCallback = undefined; + return false; + } +}; + +export const consumeAuthCallback = (): AuthCallbackParameters | undefined => { + const callback = pendingCallback; + pendingCallback = undefined; + return callback; +}; + +export const buildAuthCallbackFragment = (callback: AuthCallbackParameters): string => { + const params = new URLSearchParams(); + + if (callback.username) { + params.set('username', callback.username); + } + if (callback.authToken) { + params.set('authToken', callback.authToken); + } + if (callback.ltiToken) { + params.set('ltiToken', callback.ltiToken); + } + if (callback.ltik) { + params.set('ltik', callback.ltik); + } + if (callback.isLtiLogin) { + params.set('isLtiLogin', 'true'); + } + + return params.toString(); +}; + +/** Replace credentials defensively if an error SDK later receives a URL. */ +export const redactAuthCallbackFromUrl = (value: string): string => { + try { + const url = new URL(value, globalThis.location?.origin ?? 'https://invalid.local'); + const fragment = callbackFragment(url.hash); + + // The inherited SCORM launch endpoint embeds username/token path segments. + // Redact them from browser telemetry until that endpoint is replaced by a + // cookie/header-backed launch exchange. + url.pathname = url.pathname.replace( + /(\/scorm\/[^/]+\/)[^/]+\/[^/]+(?=\/|$)/, + '$1[Filtered]/[Filtered]', + ); + + for (const key of TELEMETRY_KEYS) { + if (url.searchParams.has(key)) { + url.searchParams.set(key, '[Filtered]'); + } + if (fragment.has(key)) { + fragment.set(key, '[Filtered]'); + } + } + + const redactedFragment = fragment.toString(); + url.hash = redactedFragment ? `#${redactedFragment}` : ''; + return url.toString(); + } catch { + return '[Filtered URL]'; + } +}; + +export const resetAuthCallbackForTest = (): void => { + pendingCallback = undefined; +}; diff --git a/src/app/security/auth-return-url.service.spec.ts b/src/app/security/auth-return-url.service.spec.ts new file mode 100644 index 0000000000..34645e3b18 --- /dev/null +++ b/src/app/security/auth-return-url.service.spec.ts @@ -0,0 +1,135 @@ +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; +import {AuthReturnUrlService, normaliseAuthReturnUrl} from './auth-return-url.service'; + +describe('AuthReturnUrlService', () => { + let service: AuthReturnUrlService; + + beforeEach(() => { + sessionStorage.clear(); + service = new AuthReturnUrlService(); + }); + + afterEach(() => { + sessionStorage.clear(); + vi.restoreAllMocks(); + }); + + it('restores a protected route exactly once after authentication', () => { + const target = '/projects/2/dashboard/1.1P/feedback?source=push#latest'; + + expect(service.remember(target)).toBe(true); + expect(service.consume()).toBe(target); + expect(service.consume()).toBeNull(); + }); + + it('survives a full-page SSO round trip in the same tab', () => { + const target = '/projects/2/dashboard/1.1P/feedback'; + service.remember(target); + + // A new root service instance represents the application booting again + // after the identity provider redirects back to /sign_in. + const serviceAfterSsoCallback = new AuthReturnUrlService(); + + expect(serviceAfterSsoCallback.consume()).toBe(target); + }); + + it.each([ + 'https://evil.example/projects/2', + '//evil.example/projects/2', + '/\\evil.example/projects/2', + '/sign_in?returnUrl=%2Fprojects%2F2', + '/timeout', + '/welcome', + '/lti', + '/lti/link', + ])('rejects unsafe or auth-loop destination %s', (target) => { + expect(service.remember(target)).toBe(false); + expect(service.consume()).toBeNull(); + }); + + it.each([ + '/projects/2?authToken=secret', + '/projects/2#auth_token=secret', + '/projects/2#ltiToken=secret<ik=launch', + ])('does not persist callback credentials from %s', (target) => { + expect(service.remember(target)).toBe(false); + }); + + it('normalises only same-origin application paths', () => { + expect(normaliseAuthReturnUrl('/notifications?filter=unread', 'https://ontrack.example')).toBe( + '/notifications?filter=unread', + ); + expect( + normaliseAuthReturnUrl('https://ontrack.example/notifications', 'https://ontrack.example'), + ).toBeNull(); + }); + + it('clears a pending destination on explicit sign out', () => { + service.remember('/projects/2/dashboard/1.1P/feedback'); + + service.clear(); + + expect(new AuthReturnUrlService().consume()).toBeNull(); + }); + + it('drops a stale destination instead of surprising a later login', () => { + const clock = vi.spyOn(Date, 'now').mockReturnValue(1_000); + service.remember('/projects/2/dashboard/1.1P/feedback'); + clock.mockReturnValue(1_000 + 30 * 60 * 1000 + 1); + + expect(service.consume()).toBeNull(); + }); + + it('drops a stored destination with a non-finite timestamp', () => { + sessionStorage.setItem( + 'doubtfire_auth_return_url', + '{"url":"/projects/2/dashboard/1.1P/feedback","capturedAt":1e999}', + ); + + expect(service.consume()).toBeNull(); + }); + + it('keeps a one-use in-memory destination when browser storage is blocked', () => { + vi.spyOn(Storage.prototype, 'setItem').mockImplementation(() => { + throw new DOMException('Blocked', 'SecurityError'); + }); + vi.spyOn(Storage.prototype, 'getItem').mockImplementation(() => { + throw new DOMException('Blocked', 'SecurityError'); + }); + + expect(service.remember('/projects/2/dashboard')).toBe(true); + expect(service.consume()).toBe('/projects/2/dashboard'); + expect(service.consume()).toBeNull(); + }); + + it.each(['not-json', 'null', '[]', '{"url":42,"capturedAt":1000}'])( + 'ignores corrupt stored state %s after a page reload', + (stored) => { + sessionStorage.setItem('doubtfire_auth_return_url', stored); + expect(new AuthReturnUrlService().consume()).toBeNull(); + expect(sessionStorage.getItem('doubtfire_auth_return_url')).toBeNull(); + }, + ); + + it('rejects a future timestamp and removes it permanently', () => { + vi.spyOn(Date, 'now').mockReturnValue(1_000); + sessionStorage.setItem( + 'doubtfire_auth_return_url', + JSON.stringify({url: '/projects/2', capturedAt: 2_000}), + ); + expect(service.consume()).toBeNull(); + expect(service.consume()).toBeNull(); + }); + + it('accepts a return at the expiry boundary but rejects it one millisecond later', () => { + const now = vi.spyOn(Date, 'now').mockReturnValue(1_000); + service.remember('/projects/2'); + now.mockReturnValue(1_000 + 30 * 60 * 1_000); + expect(service.consume()).toBe('/projects/2'); + + now.mockReturnValue(1_000); + service.remember('/projects/2'); + now.mockReturnValue(1_001 + 30 * 60 * 1_000); + expect(service.consume()).toBeNull(); + }); +}); diff --git a/src/app/security/auth-return-url.service.ts b/src/app/security/auth-return-url.service.ts new file mode 100644 index 0000000000..e6cf89f471 --- /dev/null +++ b/src/app/security/auth-return-url.service.ts @@ -0,0 +1,165 @@ +import {Injectable} from '@angular/core'; + +const STORAGE_KEY = 'doubtfire_auth_return_url'; +const MAX_AGE_MS = 30 * 60 * 1000; +const MAX_URL_LENGTH = 4096; + +const AUTH_FLOW_PATHS = new Set([ + '/sign_in', + '/timeout', + '/welcome', + '/unauthorised', + '/success-close', + '/lti', + '/lti/link', +]); + +const CALLBACK_SECRET_KEYS = new Set(['authToken', 'auth_token', 'ltiToken', 'lti_token', 'ltik']); + +interface StoredReturnUrl { + url: string; + capturedAt: number; +} + +const hasControlCharacters = (value: string): boolean => + Array.from(value).some((character) => { + const code = character.charCodeAt(0); + return code <= 0x1f || code === 0x7f; + }); + +const containsCallbackSecret = (url: URL): boolean => { + if ([...CALLBACK_SECRET_KEYS].some((key) => url.searchParams.has(key))) { + return true; + } + + const fragment = new URLSearchParams(url.hash.replace(/^#\??/, '')); + return [...CALLBACK_SECRET_KEYS].some((key) => fragment.has(key)); +}; + +/** + * Accept only a local application path. This value is eventually handed to + * Router.navigateByUrl, so rejecting absolute and protocol-relative URLs here + * prevents the saved destination becoming an open redirect in a future auth + * integration. + */ +export const normaliseAuthReturnUrl = ( + candidate: string, + origin: string = globalThis.location?.origin ?? 'https://invalid.local', +): string | null => { + if ( + !candidate || + candidate.length > MAX_URL_LENGTH || + !candidate.startsWith('/') || + candidate.startsWith('//') || + candidate.includes('\\') || + hasControlCharacters(candidate) + ) { + return null; + } + + try { + const base = new URL(origin); + const url = new URL(candidate, base); + + if (url.origin !== base.origin || AUTH_FLOW_PATHS.has(url.pathname)) { + return null; + } + + if (containsCallbackSecret(url)) { + return null; + } + + return `${url.pathname}${url.search}${url.hash}`; + } catch { + return null; + } +}; + +/** + * Keeps a protected destination across the sign-in page and a full-page SSO + * round trip. Session storage is intentionally tab-scoped; a destination from + * one user or tab must not unexpectedly redirect another browser session. + */ +@Injectable({providedIn: 'root'}) +export class AuthReturnUrlService { + private memoryFallback: StoredReturnUrl | null = null; + + public remember(url: string): boolean { + const normalised = normaliseAuthReturnUrl(url); + if (!normalised) { + return false; + } + + const entry: StoredReturnUrl = {url: normalised, capturedAt: Date.now()}; + this.memoryFallback = entry; + + try { + globalThis.sessionStorage?.setItem(STORAGE_KEY, JSON.stringify(entry)); + } catch { + // Some embedded or privacy-restricted browsers disable storage. The + // in-memory value still handles a database login without a page reload. + } + + return true; + } + + public rememberCurrentUrl(): boolean { + try { + return this.remember( + `${globalThis.location.pathname}${globalThis.location.search}${globalThis.location.hash}`, + ); + } catch { + return false; + } + } + + public consume(): string | null { + const entry = this.readAndRemove(); + if ( + !entry || + !Number.isFinite(entry.capturedAt) || + Date.now() - entry.capturedAt > MAX_AGE_MS || + entry.capturedAt > Date.now() + ) { + return null; + } + + return normaliseAuthReturnUrl(entry.url); + } + + public clear(): void { + this.memoryFallback = null; + try { + globalThis.sessionStorage?.removeItem(STORAGE_KEY); + } catch { + // Nothing else to clear when browser storage is unavailable. + } + } + + private readAndRemove(): StoredReturnUrl | null { + let entry = this.memoryFallback; + this.memoryFallback = null; + + try { + const stored = globalThis.sessionStorage?.getItem(STORAGE_KEY); + globalThis.sessionStorage?.removeItem(STORAGE_KEY); + if (stored) { + const parsed: unknown = JSON.parse(stored); + if ( + typeof parsed === 'object' && + parsed !== null && + typeof parsed['url'] === 'string' && + typeof parsed['capturedAt'] === 'number' + ) { + entry = parsed as StoredReturnUrl; + } else { + entry = null; + } + } + } catch { + // Use the in-memory copy when storage is blocked or malformed. + } + + return entry; + } +} diff --git a/src/app/sessions/states/sign-in/sign-in.component.spec.ts b/src/app/sessions/states/sign-in/sign-in.component.spec.ts index f80e7c1961..2d17545736 100644 --- a/src/app/sessions/states/sign-in/sign-in.component.spec.ts +++ b/src/app/sessions/states/sign-in/sign-in.component.spec.ts @@ -1,41 +1,79 @@ -import {beforeEach, describe, expect, it} from 'vitest'; +import {beforeEach, describe, expect, it, vi} from 'vitest'; import {HttpClient} from '@angular/common/http'; -import {NO_ERRORS_SCHEMA} from '@angular/core'; +import {Directive, NO_ERRORS_SCHEMA} from '@angular/core'; import {ComponentFixture, TestBed} from '@angular/core/testing'; -import {ActivatedRoute, Router} from '@angular/router'; +import {FormsModule} from '@angular/forms'; +import {MatButtonModule} from '@angular/material/button'; +import {MatCheckboxModule} from '@angular/material/checkbox'; +import {MatFormFieldModule} from '@angular/material/form-field'; +import {MatInputModule} from '@angular/material/input'; +import {MatProgressSpinnerModule} from '@angular/material/progress-spinner'; +import {Router} from '@angular/router'; +import {BehaviorSubject, of} from 'rxjs'; import {AuthenticationService} from 'src/app/api/services/authentication.service'; import {UserService} from 'src/app/api/services/user.service'; import {AlertService} from 'src/app/common/services/alert.service'; import {DoubtfireConstants} from 'src/app/config/constants/doubtfire-constants'; import {GlobalStateService} from 'src/app/projects/states/index/global-state.service'; +import {AuthReturnUrlService} from 'src/app/security/auth-return-url.service'; import {SignInComponent} from './sign-in.component'; -const emptyProvider = {}; - describe('SignInComponent', () => { let component: SignInComponent; let fixture: ComponentFixture; + let afterAuthCallback: ((result: boolean) => void) | undefined; + let router: {navigateByUrl: ReturnType}; + let authReturnUrl: {consume: ReturnType; clear: ReturnType}; + let userService: {currentUser: {hasRunFirstTimeSetup: boolean; ltik?: string}}; + let globalState: { + goHome: ReturnType; + hideHeader: ReturnType; + onLoad: ReturnType; + }; beforeEach(async () => { + afterAuthCallback = undefined; + router = {navigateByUrl: vi.fn().mockResolvedValue(true)}; + authReturnUrl = {consume: vi.fn().mockReturnValue(null), clear: vi.fn()}; + userService = {currentUser: {hasRunFirstTimeSetup: true}}; + globalState = { + goHome: vi.fn(), + hideHeader: vi.fn(), + onLoad: vi.fn(), + }; + await TestBed.configureTestingModule({ + imports: [ + FormsModule, + MatButtonModule, + MatCheckboxModule, + MatFormFieldModule, + MatInputModule, + MatProgressSpinnerModule, + ], declarations: [SignInComponent], providers: [ - {provide: AuthenticationService, useValue: emptyProvider}, - {provide: UserService, useValue: emptyProvider}, - {provide: Router, useValue: emptyProvider}, - {provide: ActivatedRoute, useValue: emptyProvider}, - {provide: DoubtfireConstants, useValue: emptyProvider}, - {provide: HttpClient, useValue: emptyProvider}, - {provide: GlobalStateService, useValue: emptyProvider}, - {provide: AlertService, useValue: emptyProvider}, + { + provide: AuthenticationService, + useValue: { + afterAuthCall: vi.fn((callback: (result: boolean) => void) => { + afterAuthCallback = callback; + }), + signIn: vi.fn().mockReturnValue(of(void 0)), + rememberMe: true, + }, + }, + {provide: UserService, useValue: userService}, + {provide: Router, useValue: router}, + {provide: DoubtfireConstants, useValue: {}}, + {provide: HttpClient, useValue: {}}, + {provide: GlobalStateService, useValue: globalState}, + {provide: AlertService, useValue: {error: vi.fn()}}, + {provide: AuthReturnUrlService, useValue: authReturnUrl}, ], schemas: [NO_ERRORS_SCHEMA], - }) - .overrideComponent(SignInComponent, {set: {template: ''}}) - .compileComponents(); - }); + }).compileComponents(); - beforeEach(() => { fixture = TestBed.createComponent(SignInComponent); component = fixture.componentInstance; }); @@ -43,4 +81,165 @@ describe('SignInComponent', () => { it('should create', () => { expect(component).toBeTruthy(); }); + + it('uses the approved coloured logo and centred signed-out branding', () => { + component.isLoading = false; + component.externalName = new BehaviorSubject('OnTrack'); + component.formData = { + username: '', + password: '', + remember: true, + autoLogin: false, + }; + component.showCredentials = true; + fixture.detectChanges(); + + const logo = fixture.nativeElement.querySelector('.wordmark img') as HTMLImageElement; + const wordmark = fixture.nativeElement.querySelector('.wordmark') as HTMLElement; + const heading = fixture.nativeElement.querySelector('.welcome-heading') as HTMLElement; + + expect(logo.getAttribute('src')).toBe('/assets/images/logo.svg'); + expect(logo.getAttribute('alt')).toBe('OnTrack logo'); + expect(wordmark.classList).toContain('justify-center'); + expect(heading.classList).toContain('text-center'); + }); + + it('returns a database login to the protected notification destination', () => { + const target = '/projects/2/dashboard/1.1P/feedback'; + authReturnUrl.consume.mockReturnValue(target); + + component.signIn({username: 'student', password: 'password', remember: true, autoLogin: false}); + + expect(globalState.goHome).toHaveBeenCalledOnce(); + expect(router.navigateByUrl).toHaveBeenCalledWith(target); + }); + + it('uses the same destination after an authentication callback', () => { + const target = '/projects/2/dashboard/1.1P/feedback'; + authReturnUrl.consume.mockReturnValue(target); + fixture.detectChanges(); + + afterAuthCallback?.(true); + + expect(router.navigateByUrl).toHaveBeenCalledWith(target); + }); + + it('does not let a second authentication observer replace the destination with home', () => { + const target = '/projects/2/dashboard/1.1P/feedback'; + authReturnUrl.consume.mockReturnValue(target); + fixture.detectChanges(); + + component.signIn({username: 'student', password: 'password', remember: true, autoLogin: false}); + afterAuthCallback?.(true); + + expect(authReturnUrl.consume).toHaveBeenCalledOnce(); + expect(router.navigateByUrl).toHaveBeenCalledOnce(); + expect(router.navigateByUrl).toHaveBeenCalledWith(target); + }); + + it('falls back to home when there is no pending destination', () => { + component.signIn({username: 'student', password: 'password', remember: true, autoLogin: false}); + + expect(router.navigateByUrl).toHaveBeenCalledWith('/home'); + }); + + it('keeps mandatory first-time setup ahead of a pending destination', () => { + userService.currentUser.hasRunFirstTimeSetup = false; + authReturnUrl.consume.mockReturnValue('/projects/2/dashboard/1.1P/feedback'); + + component.signIn({username: 'student', password: 'password', remember: true, autoLogin: false}); + + expect(router.navigateByUrl).toHaveBeenCalledWith('/welcome'); + expect(authReturnUrl.clear).toHaveBeenCalledOnce(); + expect(authReturnUrl.consume).not.toHaveBeenCalled(); + }); + + it('keeps LTI routing ahead of and clears a pending destination', () => { + component.isLtiLogin = true; + component.ltik = 'launch-token'; + authReturnUrl.consume.mockReturnValue('/projects/2/dashboard/1.1P/feedback'); + + component.signIn({username: 'student', password: 'password', remember: true, autoLogin: false}); + + expect(globalState.hideHeader).toHaveBeenCalledOnce(); + expect(userService.currentUser.ltik).toBe('launch-token'); + expect(router.navigateByUrl).toHaveBeenCalledWith('/lti'); + expect(authReturnUrl.clear).toHaveBeenCalledOnce(); + expect(authReturnUrl.consume).not.toHaveBeenCalled(); + }); +}); + +// A11Y-FORM06: WCAG 1.3.5 Identify Input Purpose (AA). +// Renders the real sign-in template and asserts the two credential inputs +// declare their autocomplete purpose token, so browser autofill and password +// managers reliably offer the stored credential. Standing in for the whole +// Material/Forms stack, StubNgForm satisfies `#form="ngForm"` while +// NO_ERRORS_SCHEMA lets the unknown mat-* elements render as plain markup, so +// the static autocomplete attribute is what we assert on. +@Directive({selector: 'form', exportAs: 'ngForm', standalone: false}) +class StubNgFormSignIn { + public invalid = false; +} + +describe('SignInComponent autocomplete purpose (A11Y-FORM06)', () => { + let fixture: ComponentFixture; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + declarations: [SignInComponent, StubNgFormSignIn], + providers: [ + { + provide: AuthenticationService, + useValue: { + afterAuthCall: vi.fn(), + signIn: vi.fn().mockReturnValue(of(void 0)), + rememberMe: false, + }, + }, + {provide: UserService, useValue: {currentUser: {hasRunFirstTimeSetup: true}}}, + {provide: Router, useValue: {navigateByUrl: vi.fn()}}, + {provide: DoubtfireConstants, useValue: {}}, + {provide: HttpClient, useValue: {}}, + { + provide: GlobalStateService, + useValue: {goHome: vi.fn(), hideHeader: vi.fn(), onLoad: vi.fn()}, + }, + {provide: AlertService, useValue: {error: vi.fn()}}, + {provide: AuthReturnUrlService, useValue: {consume: vi.fn(), clear: vi.fn()}}, + ], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents(); + + fixture = TestBed.createComponent(SignInComponent); + const component = fixture.componentInstance; + // Reveal the credential fields (both guarded behind @if in the template). + component.isLoading = false; + component.showCredentials = true; + component.externalName = new BehaviorSubject('OnTrack'); + component.formData = {username: '', password: '', remember: false, autoLogin: false}; + fixture.detectChanges(); + }); + + const input = (name: string): HTMLElement | null => + fixture.nativeElement.querySelector(`input[name="${name}"]`); + + it('declares autocomplete="username" on the sign-in username field', () => { + const username = input('username'); + expect(username).toBeTruthy(); + expect(username?.getAttribute('autocomplete')).toBe('username'); + }); + + it('declares autocomplete="current-password" on the sign-in password field', () => { + const password = input('password'); + expect(password).toBeTruthy(); + expect(password?.getAttribute('autocomplete')).toBe('current-password'); + }); + + // Failure path: the fix must be scoped to the credential inputs. The + // "stay logged in" control is not a fillable text field and must not be + // handed a purpose token. + it('does not put an autocomplete purpose token on the non-credential controls', () => { + const remember = fixture.nativeElement.querySelector('[name="remember"]'); + expect(remember?.getAttribute('autocomplete') ?? null).toBeNull(); + }); }); diff --git a/src/app/sessions/states/sign-in/sign-in.component.ts b/src/app/sessions/states/sign-in/sign-in.component.ts index 4e1905a484..cf696b9a99 100644 --- a/src/app/sessions/states/sign-in/sign-in.component.ts +++ b/src/app/sessions/states/sign-in/sign-in.component.ts @@ -1,24 +1,14 @@ import {HttpClient} from '@angular/common/http'; import {ChangeDetectionStrategy, Component, Input, OnInit} from '@angular/core'; -import {ActivatedRoute, Router} from '@angular/router'; +import {Router} from '@angular/router'; import {BehaviorSubject} from 'rxjs'; import {AuthenticationService} from 'src/app/api/services/authentication.service'; import {UserService} from 'src/app/api/services/user.service'; import {AlertService} from 'src/app/common/services/alert.service'; import {DoubtfireConstants} from 'src/app/config/constants/doubtfire-constants'; import {GlobalStateService} from 'src/app/projects/states/index/global-state.service'; - -// Add fallback to check url for query parameters -type IParams = Record; - -const paramReducer = (params: IParams, pair: string): IParams => { - const [key, value] = `${pair}=`.split('=').map(decodeURIComponent); - - return key.length > 0 ? {...params, [key]: value} : params; -}; - -const getUrlParams = (search: string): IParams => - `${search}?`.split('?')[1].split('&').reduce(paramReducer, {}); +import {consumeAuthCallback} from 'src/app/security/auth-callback'; +import {AuthReturnUrlService} from 'src/app/security/auth-return-url.service'; type signInData = | { @@ -56,6 +46,8 @@ export class SignInComponent implements OnInit { public redirectingSSO: boolean = false; + private postSignInNavigationStarted = false; + // Get query params from the resolve in the router state @Input() username: string; @Input() authToken: string; @@ -67,29 +59,25 @@ export class SignInComponent implements OnInit { public authService: AuthenticationService, private userService: UserService, private router: Router, - private route: ActivatedRoute, private constants: DoubtfireConstants, private http: HttpClient, private globalState: GlobalStateService, private alerts: AlertService, + private authReturnUrl: AuthReturnUrlService, ) {} ngOnInit(): void { + const callback = consumeAuthCallback(); + this.username = callback?.username ?? this.username; + this.authToken = callback?.authToken ?? this.authToken; + this.ltiToken = callback?.ltiToken ?? this.ltiToken; + this.ltik = callback?.ltik ?? this.ltik; + this.isLtiLogin = callback?.isLtiLogin ?? this.isLtiLogin; + this.authService.afterAuthCall((result) => { if (result) { - const params = getUrlParams(document.location.href); this.isLoading = false; - - if (params.isLtiLogin && params.ltik) { - this.globalState.hideHeader(); - this.userService.currentUser.ltik = params.ltik; - return this.router.navigate(['/lti'], {queryParams: {ltik: params.ltik}}); - } else if (this.userService.currentUser.hasRunFirstTimeSetup === false) { - return this.router.navigateByUrl('/welcome'); - } else { - this.globalState.goHome(); - return this.router.navigateByUrl('/home'); - } + return this.actionSignInSuccess(); } this.isLoading = true; }); @@ -114,18 +102,6 @@ export class SignInComponent implements OnInit { this.api = this.constants.API_URL; this.externalName = this.constants.ExternalName; - const queryParams = this.route.snapshot.queryParams; - const params = getUrlParams(document.location.href); - if (!this.username) { - this.username = queryParams.username || params.username; - this.authToken = queryParams.authToken || params.authToken; - } - - this.ltiToken = queryParams.ltiToken || params.ltiToken || undefined; - this.ltik = queryParams.ltik || params.ltik || undefined; - this.isLtiLogin = - (queryParams.isLtiLogin || params.isLtiLogin)?.toLowerCase() === 'true' ? true : false; - // wait 2 seconds with rxjs const wait = new Promise((resolve) => setTimeout(resolve, 3000)); this.http.get(`${this.constants.API_URL}/auth/method`).subscribe({ @@ -212,9 +188,30 @@ export class SignInComponent implements OnInit { * Perform the actions needed when the user successfully signs in. */ private actionSignInSuccess(): void { - this.router.navigateByUrl( - this.userService.currentUser.hasRunFirstTimeSetup === false ? '/welcome' : '/home', - ); + // Authentication completion is observed both by afterAuthCall and by the + // direct sign-in subscription. Only the first observer should navigate; + // otherwise the second one can replace a restored deep link with /home. + if (this.postSignInNavigationStarted) { + return; + } + this.postSignInNavigationStarted = true; + + if (this.isLtiLogin && this.ltik) { + this.authReturnUrl.clear(); + this.globalState.hideHeader(); + this.userService.currentUser.ltik = this.ltik; + void this.router.navigateByUrl('/lti'); + return; + } + + if (this.userService.currentUser.hasRunFirstTimeSetup === false) { + this.authReturnUrl.clear(); + void this.router.navigateByUrl('/welcome'); + return; + } + + this.globalState.goHome(); + void this.router.navigateByUrl(this.authReturnUrl.consume() ?? '/home'); } /** @@ -251,14 +248,7 @@ export class SignInComponent implements OnInit { this.signingIn = true; this.authService.signIn(signInCredentials).subscribe({ - next: () => { - if (this.isLtiLogin) { - const params = getUrlParams(document.location.href); - this.router.navigate(['/lti'], {queryParams: {ltik: params.ltik}}); - } else { - this.actionSignInSuccess(); - } - }, + next: () => this.actionSignInSuccess(), error: (err) => { this.signingIn = false; this.formData.password = ''; diff --git a/src/main.ts b/src/main.ts index f56c11e33f..4715c46eba 100644 --- a/src/main.ts +++ b/src/main.ts @@ -3,19 +3,49 @@ import {enableProdMode, provideZoneChangeDetection} from '@angular/core'; import {platformBrowserDynamic} from '@angular/platform-browser-dynamic'; import {environment} from 'src/environments/environment'; import {DoubtfireAngularModule} from './app/doubtfire-angular.module'; +import { + captureAndScrubAuthCallback, + redactAuthCallbackFromUrl, +} from './app/security/auth-callback'; +import { + captureAndScrubAdditionalEmailVerification, + redactAdditionalEmailVerificationFromUrl, +} from './app/security/additional-email-verification-callback'; -if (environment.sentryDsn) { +// Authentication callbacks may contain a one-time credential. Remove it from +// browser history before any telemetry SDK or application code can observe it. +const telemetrySafe = captureAndScrubAuthCallback(); +const additionalEmailTelemetrySafe = captureAndScrubAdditionalEmailVerification(); + +if (environment.sentryDsn && telemetrySafe && additionalEmailTelemetrySafe) { Sentry.init({ dsn: environment.sentryDsn, tunnel: '/api/client-reports', release: environment.sentryRelease || undefined, dist: environment.sentryDist || undefined, - integrations: [Sentry.browserTracingIntegration(), Sentry.replayIntegration()], - tracesSampleRate: 1, - replaysSessionSampleRate: 0.1, - replaysOnErrorSampleRate: 1, - enableLogs: true, + // Keep telemetry to redacted error events for this release. Browser tracing + // and Session Replay can observe iframe/resource URLs outside beforeSend; + // legacy SCORM URLs still contain a reusable credential in their path. + // They must stay disabled until that launch protocol is redesigned. + enableLogs: false, sendDefaultPii: false, + beforeSend(event) { + if (event.request?.url) { + event.request.url = redactAdditionalEmailVerificationFromUrl( + redactAuthCallbackFromUrl(event.request.url), + ); + } + return event; + }, + beforeBreadcrumb(breadcrumb) { + const url = breadcrumb.data?.['url']; + if (typeof url === 'string') { + breadcrumb.data['url'] = redactAdditionalEmailVerificationFromUrl( + redactAuthCallbackFromUrl(url), + ); + } + return breadcrumb; + }, }); }