From 762fa3d4f97e89f0a09bb64550ddfa5ec5324815 Mon Sep 17 00:00:00 2001 From: nsemets Date: Thu, 15 Oct 2026 14:24:55 +0300 Subject: [PATCH 1/2] feat(cookie-consent): updated cookie consent for log out --- .../cookie-consent-banner.component.spec.ts | 46 ++++++++----- .../cookie-consent-banner.component.ts | 25 ++----- .../tos-consent-banner.component.ts | 3 +- src/app/core/services/auth.service.spec.ts | 58 +++++++++++++++- src/app/core/services/auth.service.ts | 19 ++++-- .../services/cookie-consent.service.spec.ts | 67 +++++++++++++++++++ .../core/services/cookie-consent.service.ts | 27 ++++++++ .../providers/cookie-consent.service.mock.ts | 17 +++++ 8 files changed, 217 insertions(+), 45 deletions(-) create mode 100644 src/app/core/services/cookie-consent.service.spec.ts create mode 100644 src/app/core/services/cookie-consent.service.ts create mode 100644 src/testing/providers/cookie-consent.service.mock.ts diff --git a/src/app/core/components/osf-banners/cookie-consent-banner/cookie-consent-banner.component.spec.ts b/src/app/core/components/osf-banners/cookie-consent-banner/cookie-consent-banner.component.spec.ts index f44b44d5e..53e3c2c95 100644 --- a/src/app/core/components/osf-banners/cookie-consent-banner/cookie-consent-banner.component.spec.ts +++ b/src/app/core/components/osf-banners/cookie-consent-banner/cookie-consent-banner.component.spec.ts @@ -1,53 +1,63 @@ -import { CookieService } from 'ngx-cookie-service'; import { MockComponent, MockProvider } from 'ng-mocks'; +import { PLATFORM_ID } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { CookieConsentService } from '@core/services/cookie-consent.service'; import { IconComponent } from '@osf/shared/components/icon/icon.component'; import { provideOSFCore } from '@testing/osf.testing.provider'; -import { CookieServiceMock } from '@testing/providers/cookie-service.mock'; +import { CookieConsentServiceMock, CookieConsentServiceMockType } from '@testing/providers/cookie-consent.service.mock'; import { CookieConsentBannerComponent } from './cookie-consent-banner.component'; describe('Component: Cookie Consent Banner', () => { let fixture: ComponentFixture; let component: CookieConsentBannerComponent; + let cookieConsentMock: CookieConsentServiceMockType; - const cookieServiceMock = CookieServiceMock.simple(); + function setup(overrides: { hasConsent?: boolean; isBrowser?: boolean } = {}) { + cookieConsentMock = CookieConsentServiceMock.simple(); + cookieConsentMock.hasConsent.mockReturnValue(overrides.hasConsent ?? false); - beforeEach(() => { TestBed.configureTestingModule({ imports: [CookieConsentBannerComponent, MockComponent(IconComponent)], - providers: [provideOSFCore(), MockProvider(CookieService, cookieServiceMock)], + providers: [ + provideOSFCore(), + MockProvider(CookieConsentService, cookieConsentMock), + MockProvider(PLATFORM_ID, overrides.isBrowser === false ? 'server' : 'browser'), + ], }); - }); - it('should show the banner if cookie is not set', () => { - cookieServiceMock.check.mockReturnValue(false); fixture = TestBed.createComponent(CookieConsentBannerComponent); component = fixture.componentInstance; + } + + it('should show the banner when consent is missing', () => { + setup(); expect(component.displayBanner()).toBe(true); }); - it('should hide the banner if cookie is set', () => { - cookieServiceMock.check.mockReturnValue(true); - fixture = TestBed.createComponent(CookieConsentBannerComponent); - component = fixture.componentInstance; + it('should hide the banner when consent is already recorded', () => { + setup({ hasConsent: true }); expect(component.displayBanner()).toBe(false); }); - it('should set cookie and hide banner on acceptCookies()', () => { - cookieServiceMock.check.mockReturnValue(false); - fixture = TestBed.createComponent(CookieConsentBannerComponent); - component = fixture.componentInstance; + it('should keep the banner hidden outside the browser', () => { + setup({ isBrowser: false }); - component.acceptCookies(); + expect(component.displayBanner()).toBe(false); + expect(cookieConsentMock.hasConsent).not.toHaveBeenCalled(); + }); - expect(cookieServiceMock.set).toHaveBeenCalledWith('cookie-consent', 'true', new Date('9999-12-31T23:59:59Z'), '/'); + it('should accept cookies and hide the banner', () => { + setup(); + + component.acceptCookies(); + expect(cookieConsentMock.accept).toHaveBeenCalled(); expect(component.displayBanner()).toBe(false); }); }); diff --git a/src/app/core/components/osf-banners/cookie-consent-banner/cookie-consent-banner.component.ts b/src/app/core/components/osf-banners/cookie-consent-banner/cookie-consent-banner.component.ts index c593da853..c2d207af9 100644 --- a/src/app/core/components/osf-banners/cookie-consent-banner/cookie-consent-banner.component.ts +++ b/src/app/core/components/osf-banners/cookie-consent-banner/cookie-consent-banner.component.ts @@ -1,4 +1,3 @@ -import { CookieService } from 'ngx-cookie-service'; import { TranslatePipe } from '@ngx-translate/core'; import { Button } from 'primeng/button'; @@ -7,12 +6,13 @@ import { Message } from 'primeng/message'; import { isPlatformBrowser } from '@angular/common'; import { ChangeDetectionStrategy, Component, inject, PLATFORM_ID, signal } from '@angular/core'; +import { CookieConsentService } from '@core/services/cookie-consent.service'; import { IconComponent } from '@osf/shared/components/icon/icon.component'; /** * Displays a cookie consent banner until the user accepts. * - * - Uses `ngx-cookie-service` to persist acceptance across sessions. + * - Persists acceptance through `CookieConsentService`. * - Automatically hides the banner if consent is already recorded. * - Supports translation via `TranslatePipe`. */ @@ -24,30 +24,22 @@ import { IconComponent } from '@osf/shared/components/icon/icon.component'; changeDetection: ChangeDetectionStrategy.OnPush, }) export class CookieConsentBannerComponent { - /** - * The name of the cookie used to track whether the user accepted cookies. - */ - private readonly cookieName = 'cookie-consent'; - /** * Signal controlling the visibility of the cookie banner. * Set to `true` if the user has not accepted cookies yet. */ readonly displayBanner = signal(false); - /** - * Cookie service used to persist dismissal state in the browser. - */ - private readonly cookies = inject(CookieService); - private readonly platformId = inject(PLATFORM_ID); + private readonly cookieConsent = inject(CookieConsentService); + private readonly isBrowser = isPlatformBrowser(inject(PLATFORM_ID)); /** * Initializes the component and sets the banner display * based on the existence of the cookie. */ constructor() { - if (isPlatformBrowser(this.platformId)) { - this.displayBanner.set(!this.cookies.check(this.cookieName)); + if (this.isBrowser) { + this.displayBanner.set(!this.cookieConsent.hasConsent()); } } @@ -57,10 +49,7 @@ export class CookieConsentBannerComponent { * - Hides the banner immediately. */ acceptCookies() { - if (isPlatformBrowser(this.platformId)) { - const expireDate = new Date('9999-12-31T23:59:59Z'); - this.cookies.set(this.cookieName, 'true', expireDate, '/'); - } + this.cookieConsent.accept(); this.displayBanner.set(false); } } diff --git a/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts b/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts index 7d6024103..abbc247c3 100644 --- a/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts +++ b/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts @@ -6,7 +6,7 @@ import { Button } from 'primeng/button'; import { Checkbox } from 'primeng/checkbox'; import { Message } from 'primeng/message'; -import { Component, computed, signal } from '@angular/core'; +import { ChangeDetectionStrategy, Component, computed, signal } from '@angular/core'; import { FormsModule } from '@angular/forms'; import { RouterLink } from '@angular/router'; @@ -32,6 +32,7 @@ import { IconComponent } from '@osf/shared/components/icon/icon.component'; selector: 'osf-tos-consent-banner', imports: [FormsModule, Checkbox, Button, Message, TranslatePipe, IconComponent, RouterLink], templateUrl: './tos-consent-banner.component.html', + changeDetection: ChangeDetectionStrategy.OnPush, }) export class TosConsentBannerComponent { /** diff --git a/src/app/core/services/auth.service.spec.ts b/src/app/core/services/auth.service.spec.ts index 642191d49..219c6ccd2 100644 --- a/src/app/core/services/auth.service.spec.ts +++ b/src/app/core/services/auth.service.spec.ts @@ -1,3 +1,5 @@ +import { Store } from '@ngxs/store'; + import { CookieService } from 'ngx-cookie-service'; import { MockProvider } from 'ng-mocks'; @@ -6,10 +8,13 @@ import { TestBed } from '@angular/core/testing'; import { ENVIRONMENT } from '@core/provider/environment.provider'; import { WINDOW } from '@core/provider/window.provider'; +import { CookieConsentService } from '@core/services/cookie-consent.service'; +import { ClearCurrentUser } from '@osf/core/store/user'; import { JsonApiService } from '@osf/shared/services/json-api.service'; import { LoaderService } from '@osf/shared/services/loader.service'; -import { CookieServiceMock } from '@testing/providers/cookie-service.mock'; +import { CookieConsentServiceMock, CookieConsentServiceMockType } from '@testing/providers/cookie-consent.service.mock'; +import { CookieServiceMock, CookieServiceMockType } from '@testing/providers/cookie-service.mock'; import { JsonApiServiceMock } from '@testing/providers/json-api.service.mock'; import { LoaderServiceMock } from '@testing/providers/loader-service.mock'; import { provideMockStore } from '@testing/providers/store-provider.mock'; @@ -22,10 +27,15 @@ describe('AuthService', () => { const origin = 'https://osf.test'; let service: AuthService; + let store: Store; let loaderService: LoaderServiceMock; + let cookieServiceMock: CookieServiceMockType; + let cookieConsentMock: CookieConsentServiceMockType; let locationMock: { href: string; pathname: string; origin: string }; - function setup(overrides: { pathname?: string; href?: string; isBrowser?: boolean } = {}) { + function setup( + overrides: { pathname?: string; href?: string; isBrowser?: boolean; hasCookieConsent?: boolean } = {} + ) { const pathname = overrides.pathname ?? '/dashboard'; locationMock = { href: overrides.href ?? `${origin}${pathname}`, @@ -34,13 +44,17 @@ describe('AuthService', () => { }; loaderService = new LoaderServiceMock(); + cookieServiceMock = CookieServiceMock.simple(); + cookieConsentMock = CookieConsentServiceMock.simple(); + cookieConsentMock.hasConsent.mockReturnValue(overrides.hasCookieConsent ?? false); TestBed.configureTestingModule({ providers: [ AuthService, provideMockStore(), MockProvider(JsonApiService, JsonApiServiceMock.simple()), - MockProvider(CookieService, CookieServiceMock.simple()), + MockProvider(CookieService, cookieServiceMock), + MockProvider(CookieConsentService, cookieConsentMock), MockProvider(LoaderService, loaderService), MockProvider(WINDOW, { location: locationMock } as Window), MockProvider(ENVIRONMENT, { webUrl, casUrl, apiDomainUrl: 'https://api.test' }), @@ -49,6 +63,7 @@ describe('AuthService', () => { }); service = TestBed.inject(AuthService); + store = TestBed.inject(Store); } function nextFromServiceParam(href: string): string | null { @@ -101,4 +116,41 @@ describe('AuthService', () => { expect(loaderService.show).not.toHaveBeenCalled(); expect(locationMock.href).toBe(`${origin}/dashboard`); }); + + it('should clear session cookies and restore cookie consent on logout', () => { + setup({ hasCookieConsent: true }); + + service.logout(); + + expect(loaderService.show).toHaveBeenCalled(); + expect(store.dispatch).toHaveBeenCalledWith(new ClearCurrentUser()); + expect(cookieConsentMock.hasConsent.mock.invocationCallOrder[0]).toBeLessThan( + cookieServiceMock.deleteAll.mock.invocationCallOrder[0] + ); + expect(cookieServiceMock.deleteAll.mock.invocationCallOrder[0]).toBeLessThan( + cookieConsentMock.accept.mock.invocationCallOrder[0] + ); + expect(locationMock.href).toBe(`${webUrl}/logout/?next=${encodeURIComponent(`${origin}/`)}`); + }); + + it('should not restore cookie consent on logout when the user has not accepted', () => { + setup(); + + service.logout(); + + expect(cookieServiceMock.deleteAll).toHaveBeenCalled(); + expect(cookieConsentMock.accept).not.toHaveBeenCalled(); + }); + + it('should not clear cookies when logging out outside the browser', () => { + setup({ isBrowser: false, hasCookieConsent: true, href: `${origin}/dashboard` }); + + service.logout(); + + expect(loaderService.show).toHaveBeenCalled(); + expect(store.dispatch).toHaveBeenCalledWith(new ClearCurrentUser()); + expect(cookieServiceMock.deleteAll).not.toHaveBeenCalled(); + expect(cookieConsentMock.hasConsent).not.toHaveBeenCalled(); + expect(locationMock.href).toBe(`${origin}/dashboard`); + }); }); diff --git a/src/app/core/services/auth.service.ts b/src/app/core/services/auth.service.ts index 70a3328e6..06ad1a995 100644 --- a/src/app/core/services/auth.service.ts +++ b/src/app/core/services/auth.service.ts @@ -8,6 +8,7 @@ import { inject, Injectable, PLATFORM_ID } from '@angular/core'; import { SignUpModel } from '@core/models/sign-up.model'; import { ENVIRONMENT } from '@core/provider/environment.provider'; import { WINDOW } from '@core/provider/window.provider'; +import { CookieConsentService } from '@core/services/cookie-consent.service'; import { ClearCurrentUser } from '@osf/core/store/user'; import { urlParam } from '@osf/shared/helpers/url-param.helper'; import { JsonApiService } from '@osf/shared/services/json-api.service'; @@ -19,10 +20,11 @@ import { LoaderService } from '@osf/shared/services/loader.service'; export class AuthService { private readonly jsonApiService = inject(JsonApiService); private readonly cookieService = inject(CookieService); + private readonly cookieConsent = inject(CookieConsentService); private readonly loaderService = inject(LoaderService); private readonly environment = inject(ENVIRONMENT); private readonly window = inject(WINDOW); - private readonly platformId = inject(PLATFORM_ID); + private readonly isBrowser = isPlatformBrowser(inject(PLATFORM_ID)); private readonly actions = createDispatchMap({ clearCurrentUser: ClearCurrentUser }); get apiUrl() { @@ -38,7 +40,7 @@ export class AuthService { } navigateToSignIn(): void { - if (!isPlatformBrowser(this.platformId)) { + if (!this.isBrowser) { return; } @@ -54,7 +56,7 @@ export class AuthService { } navigateToOrcidSignIn(): void { - if (!isPlatformBrowser(this.platformId)) { + if (!this.isBrowser) { return; } @@ -68,7 +70,7 @@ export class AuthService { } navigateToInstitutionSignIn(): void { - if (!isPlatformBrowser(this.platformId)) { + if (!this.isBrowser) { return; } @@ -85,8 +87,15 @@ export class AuthService { this.loaderService.show(); this.actions.clearCurrentUser(); - if (isPlatformBrowser(this.platformId)) { + if (this.isBrowser) { + const hasCookieConsent = this.cookieConsent.hasConsent(); + this.cookieService.deleteAll(); + + if (hasCookieConsent) { + this.cookieConsent.accept(); + } + this.window.location.href = `${this.webUrl}/logout/?next=${encodeURIComponent(nextUrl || `${this.window.location.origin}/`)}`; } } diff --git a/src/app/core/services/cookie-consent.service.spec.ts b/src/app/core/services/cookie-consent.service.spec.ts new file mode 100644 index 000000000..873a18685 --- /dev/null +++ b/src/app/core/services/cookie-consent.service.spec.ts @@ -0,0 +1,67 @@ +import { CookieService } from 'ngx-cookie-service'; +import { MockProvider } from 'ng-mocks'; + +import { PLATFORM_ID } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; + +import { CookieServiceMock, CookieServiceMockType } from '@testing/providers/cookie-service.mock'; + +import { CookieConsentService } from './cookie-consent.service'; + +describe('CookieConsentService', () => { + let service: CookieConsentService; + let cookieServiceMock: CookieServiceMockType; + + function setup(isBrowser = true) { + cookieServiceMock = CookieServiceMock.simple(); + + TestBed.configureTestingModule({ + providers: [ + CookieConsentService, + MockProvider(CookieService, cookieServiceMock), + MockProvider(PLATFORM_ID, isBrowser ? 'browser' : 'server'), + ], + }); + + service = TestBed.inject(CookieConsentService); + } + + it('should report consent when the cookie exists', () => { + setup(); + cookieServiceMock.check.mockReturnValue(true); + + expect(service.hasConsent()).toBe(true); + expect(cookieServiceMock.check).toHaveBeenCalledWith('cookie-consent'); + }); + + it('should report no consent when the cookie is missing', () => { + setup(); + + expect(service.hasConsent()).toBe(false); + expect(cookieServiceMock.check).toHaveBeenCalledWith('cookie-consent'); + }); + + it('should report no consent outside the browser', () => { + setup(false); + cookieServiceMock.check.mockReturnValue(true); + + expect(service.hasConsent()).toBe(false); + expect(cookieServiceMock.check).not.toHaveBeenCalled(); + }); + + it('should store consent until year 9999', () => { + setup(); + + service.accept(); + + expect(cookieServiceMock.set).toHaveBeenCalledWith('cookie-consent', 'true', new Date('9999-12-31T23:59:59Z'), '/'); + }); + + it('should not store consent outside the browser', () => { + setup(false); + + service.accept(); + + expect(cookieServiceMock.set).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/core/services/cookie-consent.service.ts b/src/app/core/services/cookie-consent.service.ts new file mode 100644 index 000000000..0a7715271 --- /dev/null +++ b/src/app/core/services/cookie-consent.service.ts @@ -0,0 +1,27 @@ +import { CookieService } from 'ngx-cookie-service'; + +import { isPlatformBrowser } from '@angular/common'; +import { inject, Injectable, PLATFORM_ID } from '@angular/core'; + +@Injectable({ providedIn: 'root' }) +export class CookieConsentService { + private readonly cookies = inject(CookieService); + private readonly isBrowser = isPlatformBrowser(inject(PLATFORM_ID)); + private readonly cookieName = 'cookie-consent'; + + hasConsent(): boolean { + if (!this.isBrowser) { + return false; + } + + return this.cookies.check(this.cookieName); + } + + accept(): void { + if (!this.isBrowser) { + return; + } + + this.cookies.set(this.cookieName, 'true', new Date('9999-12-31T23:59:59Z'), '/'); + } +} diff --git a/src/testing/providers/cookie-consent.service.mock.ts b/src/testing/providers/cookie-consent.service.mock.ts new file mode 100644 index 000000000..67ef9d9e3 --- /dev/null +++ b/src/testing/providers/cookie-consent.service.mock.ts @@ -0,0 +1,17 @@ +import { Mock } from 'vitest'; + +import { CookieConsentService } from '@core/services/cookie-consent.service'; + +export type CookieConsentServiceMockType = Pick & { + hasConsent: Mock; + accept: Mock; +}; + +export const CookieConsentServiceMock = { + simple(): CookieConsentServiceMockType { + return { + hasConsent: vi.fn().mockReturnValue(false), + accept: vi.fn(), + }; + }, +}; From 95074483ef848878834f202ca4d41be236d32b62 Mon Sep 17 00:00:00 2001 From: nsemets Date: Thu, 15 Oct 2026 17:51:11 +0300 Subject: [PATCH 2/2] fix(cookie-consent): removed extra check and updated tests --- .../services/cookie-consent.service.spec.ts | 32 ++----------------- .../core/services/cookie-consent.service.ts | 12 +------ 2 files changed, 4 insertions(+), 40 deletions(-) diff --git a/src/app/core/services/cookie-consent.service.spec.ts b/src/app/core/services/cookie-consent.service.spec.ts index 873a18685..8625975ce 100644 --- a/src/app/core/services/cookie-consent.service.spec.ts +++ b/src/app/core/services/cookie-consent.service.spec.ts @@ -1,7 +1,6 @@ import { CookieService } from 'ngx-cookie-service'; import { MockProvider } from 'ng-mocks'; -import { PLATFORM_ID } from '@angular/core'; import { TestBed } from '@angular/core/testing'; import { CookieServiceMock, CookieServiceMockType } from '@testing/providers/cookie-service.mock'; @@ -12,22 +11,17 @@ describe('CookieConsentService', () => { let service: CookieConsentService; let cookieServiceMock: CookieServiceMockType; - function setup(isBrowser = true) { + beforeEach(() => { cookieServiceMock = CookieServiceMock.simple(); TestBed.configureTestingModule({ - providers: [ - CookieConsentService, - MockProvider(CookieService, cookieServiceMock), - MockProvider(PLATFORM_ID, isBrowser ? 'browser' : 'server'), - ], + providers: [CookieConsentService, MockProvider(CookieService, cookieServiceMock)], }); service = TestBed.inject(CookieConsentService); - } + }); it('should report consent when the cookie exists', () => { - setup(); cookieServiceMock.check.mockReturnValue(true); expect(service.hasConsent()).toBe(true); @@ -35,33 +29,13 @@ describe('CookieConsentService', () => { }); it('should report no consent when the cookie is missing', () => { - setup(); - expect(service.hasConsent()).toBe(false); expect(cookieServiceMock.check).toHaveBeenCalledWith('cookie-consent'); }); - it('should report no consent outside the browser', () => { - setup(false); - cookieServiceMock.check.mockReturnValue(true); - - expect(service.hasConsent()).toBe(false); - expect(cookieServiceMock.check).not.toHaveBeenCalled(); - }); - it('should store consent until year 9999', () => { - setup(); - service.accept(); expect(cookieServiceMock.set).toHaveBeenCalledWith('cookie-consent', 'true', new Date('9999-12-31T23:59:59Z'), '/'); }); - - it('should not store consent outside the browser', () => { - setup(false); - - service.accept(); - - expect(cookieServiceMock.set).not.toHaveBeenCalled(); - }); }); diff --git a/src/app/core/services/cookie-consent.service.ts b/src/app/core/services/cookie-consent.service.ts index 0a7715271..bae74b5dd 100644 --- a/src/app/core/services/cookie-consent.service.ts +++ b/src/app/core/services/cookie-consent.service.ts @@ -1,27 +1,17 @@ import { CookieService } from 'ngx-cookie-service'; -import { isPlatformBrowser } from '@angular/common'; -import { inject, Injectable, PLATFORM_ID } from '@angular/core'; +import { inject, Injectable } from '@angular/core'; @Injectable({ providedIn: 'root' }) export class CookieConsentService { private readonly cookies = inject(CookieService); - private readonly isBrowser = isPlatformBrowser(inject(PLATFORM_ID)); private readonly cookieName = 'cookie-consent'; hasConsent(): boolean { - if (!this.isBrowser) { - return false; - } - return this.cookies.check(this.cookieName); } accept(): void { - if (!this.isBrowser) { - return; - } - this.cookies.set(this.cookieName, 'true', new Date('9999-12-31T23:59:59Z'), '/'); } }