PD-12904

src/app/authorize/pages/authorize/authorize.component.spec.ts

@@ -165,6 +165,12 @@ describe('AuthorizeComponent', () => {
 
     expect(loginInterstitialsSpy.isUserFullyLoaded).toHaveBeenCalled()
     expect(loginInterstitialsSpy.checkLoginInterstitials).toHaveBeenCalled()
+    // The manager only consults the post-registration OAuth flag when it is
+    // told it is on the OAuth surface, so this prefix is load bearing (PD-12904)
+    expect(loginInterstitialsSpy.checkLoginInterstitials).toHaveBeenCalledWith(
+      jasmine.anything(),
+      { returnType: 'component', togglzPrefix: 'OAUTH' }
+    )
     expect((component as any).interstitialComponent).toBe(
       DummyInterstitialComponent as any
     )

src/app/core/login-interstitials-manager/login-main-interstitials-manager.service.ts

@@ -20,6 +20,7 @@ import {
 } from './abstractions/dialog-interface'
 import { ComponentType } from '@angular/cdk/overlay'
 import { PlatformInfoService } from 'src/app/cdk/platform-info'
+import { OauthURLSessionManagerService } from '../oauth-urlsession-manager/oauth-urlsession-manager.service'
 
 @Injectable({
   providedIn: 'root',
@@ -36,6 +37,7 @@ export class LoginMainInterstitialsManagerService {
   constructor(
     private interstitialsService: InterstitialsService,
     private _platform: PlatformInfoService,
+    private _oauthUrlSession: OauthURLSessionManagerService,
     LoginDomainInterstitialManagerService: LoginDomainInterstitialManagerService,
     LoginAffiliationInterstitialManagerService: LoginAffiliationInterstitialManagerService,
     LoginBackupEmailInterstitialManagerService: LoginBackupEmailInterstitialManagerService
@@ -86,7 +88,7 @@ export class LoginMainInterstitialsManagerService {
       return EMPTY
     }
 
-    if (this.userJustRegistered()) {
+    if (this.userJustRegistered(opts.togglzPrefix)) {
       if (runtimeEnvironment.debugger) {
         console.info(
           '[Interstitial Manager] Just registered, not checking interstitials'
@@ -186,12 +188,24 @@ export class LoginMainInterstitialsManagerService {
 
   /**
    * A user who has just finished registering has already been through a long
-   * form and is being shown the verify your email banner, so no interstitial
-   * should interrupt that. Registration lands here with `justRegistered` on the
-   * URL; the OAuth branch goes to the authorize page instead and never reaches
-   * my-orcid carrying the parameter.
+   * form, so no interstitial should interrupt them — in either flow.
+   *
+   * Registration signals this two different ways, because the backend only
+   * appends the query parameter when there is no saved request target:
+   *  - direct registration lands on my-orcid with `justRegistered` on the URL
+   *  - registering inside an OAuth request goes to the authorize page instead,
+   *    carrying the `oauthJustRegistered` localStorage flag
+   *
+   * The localStorage flag is only consulted on the OAuth surface. On my-orcid
+   * the query parameter already covers the case and that branch never sets the
+   * flag, so reading it there could only ever act on one left behind by an
+   * unrelated OAuth flow — suppressing an interstitial that should have shown.
+   *
+   * The flag is read non-destructively: `consumeJustRegistered()` is the
+   * one-time read owned by the RUM journey callers, and the authorize page
+   * mounts those only after this check, so consuming here would race them.
    */
-  private userJustRegistered(): boolean {
+  private userJustRegistered(toggglzPrefix: 'OAUTH' | 'LOGIN'): boolean {
     let justRegistered = false
     this._platform
       .get()
@@ -200,7 +214,10 @@ export class LoginMainInterstitialsManagerService {
         justRegistered =
           platform.queryParameters.hasOwnProperty('justRegistered')
       })
-    return justRegistered
+    if (justRegistered) {
+      return true
+    }
+    return toggglzPrefix === 'OAUTH' && this._oauthUrlSession.isJustRegistered()
   }
 
   isAccountOwner(userRecord: UserRecord): boolean {

src/app/core/login-interstitials-manager/test/login-main-interstitials-manager.service.spec.ts

@@ -12,6 +12,7 @@ import { ShareEmailsDomainsComponentDialogOutput } from 'src/app/cdk/interstitia
 import { AffilationsComponentDialogOutput } from 'src/app/cdk/interstitials/affiliations-interstitial/interstitial-dialog-extend/affiliations-interstitial-dialog.component'
 import { BackupEmailComponentDialogOutput } from 'src/app/cdk/interstitials/backup-email/interstitial-dialog-extend/backup-email-dialog.component'
 import { PlatformInfoService } from 'src/app/cdk/platform-info'
+import { OauthURLSessionManagerService } from '../../oauth-urlsession-manager/oauth-urlsession-manager.service'
 import { PlatformInfo } from 'src/app/cdk/platform-info/platform-info.type'
 
 // Mock runtime environment for debugging logs if needed
@@ -28,6 +29,7 @@ describe('LoginMainInterstitialsManagerService', () => {
   let mockLoginAffiliationInterstitialManagerService: jasmine.SpyObj<LoginAffiliationInterstitialManagerService>
   let mockLoginBackupEmailInterstitialManagerService: jasmine.SpyObj<LoginBackupEmailInterstitialManagerService>
   let mockPlatformInfoService: jasmine.SpyObj<PlatformInfoService>
+  let oauthUrlSession: OauthURLSessionManagerService
 
   // Example valid user
   const validUserRecord: UserRecord = {
@@ -60,6 +62,7 @@ describe('LoginMainInterstitialsManagerService', () => {
   } as UserRecord
 
   beforeEach(() => {
+    localStorage.removeItem('oauthJustRegistered')
     // Create spy objects for each dependency
     mockInterstitialsService = jasmine.createSpyObj<InterstitialsService>(
       'InterstitialsService',
@@ -106,6 +109,7 @@ describe('LoginMainInterstitialsManagerService', () => {
           'getInterstitialTogglz',
           'getInterstitialViewed',
           'showInterstitialAsDialog',
+          'showInterstitialAsComponent',
         ],
         {
           INTERSTITIAL_NAME: 'BACKUP_EMAIL_INTERSTITIAL',
@@ -132,6 +136,10 @@ describe('LoginMainInterstitialsManagerService', () => {
         LoginMainInterstitialsManagerService,
         { provide: InterstitialsService, useValue: mockInterstitialsService },
         { provide: PlatformInfoService, useValue: mockPlatformInfoService },
+        // Real service on purpose: it has no constructor dependencies, so the
+        // OAuth just-registered flag is exercised through actual localStorage
+        // rather than a spy that could drift from the real read.
+        OauthURLSessionManagerService,
         {
           provide: LoginDomainInterstitialManagerService,
           useValue: mockLoginDomainInterstitialManagerService,
@@ -147,10 +155,12 @@ describe('LoginMainInterstitialsManagerService', () => {
       ],
     })
 
+    oauthUrlSession = TestBed.inject(OauthURLSessionManagerService)
     service = TestBed.inject(LoginMainInterstitialsManagerService)
   })
 
   afterEach(() => {
+    localStorage.removeItem('oauthJustRegistered')
     // Reset calls so each test starts fresh
     jasmine.clock().uninstall()
     mockInterstitialsService.checkIfSessionAlreadyCheckedInterstitialsLogic.calls.reset()
@@ -628,6 +638,147 @@ describe('LoginMainInterstitialsManagerService', () => {
         })
     })
 
+    it('shows no interstitial in the OAuth flow, where a localStorage flag replaces the query parameter', (done) => {
+      // Registering inside an OAuth request never reaches my-orcid, so the
+      // backend never appends `justRegistered`; register.component sets this
+      // flag instead and the user lands on the authorize page. PD-12904.
+      mockInterstitialsService.checkIfSessionAlreadyCheckedInterstitialsLogic.and.returnValue(
+        false
+      )
+      oauthUrlSession.setJustRegistered(true)
+      // Would otherwise qualify
+      mockLoginBackupEmailInterstitialManagerService.userIsElegibleForInterstitial.and.returnValue(
+        of(true)
+      )
+
+      service
+        .checkLoginInterstitials(validUserRecord, {
+          returnType: 'component',
+          togglzPrefix: 'OAUTH',
+        })
+        .subscribe({
+          next: () => fail('Should not emit any value'),
+          complete: () => {
+            expect(
+              mockLoginBackupEmailInterstitialManagerService.userIsElegibleForInterstitial
+            ).not.toHaveBeenCalled()
+            expect(
+              mockLoginDomainInterstitialManagerService.userIsElegibleForInterstitial
+            ).not.toHaveBeenCalled()
+            expect(
+              mockLoginAffiliationInterstitialManagerService.userIsElegibleForInterstitial
+            ).not.toHaveBeenCalled()
+            expect(
+              mockInterstitialsService.markCurrentSessionToNoCheckInterstitialsLogic
+            ).toHaveBeenCalled()
+            done()
+          },
+        })
+    })
+
+    it('still suppresses after the flag was consumed destructively by another reader', (done) => {
+      // form-authorize and oauth-error consume this flag for RUM context. The
+      // authorize page renders those only after the interstitial check, but the
+      // gate must not depend on that ordering.
+      mockInterstitialsService.checkIfSessionAlreadyCheckedInterstitialsLogic.and.returnValue(
+        false
+      )
+      oauthUrlSession.setJustRegistered(true)
+      expect(oauthUrlSession.consumeJustRegistered()).toBeTrue()
+      expect(localStorage.getItem('oauthJustRegistered')).toBeNull()
+
+      mockLoginBackupEmailInterstitialManagerService.userIsElegibleForInterstitial.and.returnValue(
+        of(true)
+      )
+
+      service
+        .checkLoginInterstitials(validUserRecord, {
+          returnType: 'component',
+          togglzPrefix: 'OAUTH',
+        })
+        .subscribe({
+          next: () => fail('Should not emit any value'),
+          complete: () => {
+            expect(
+              mockLoginBackupEmailInterstitialManagerService.userIsElegibleForInterstitial
+            ).not.toHaveBeenCalled()
+            done()
+          },
+        })
+    })
+
+    it('does not suppress when the OAuth flag has expired', fakeAsync(() => {
+      mockInterstitialsService.checkIfSessionAlreadyCheckedInterstitialsLogic.and.returnValue(
+        false
+      )
+      localStorage.setItem(
+        'oauthJustRegistered',
+        JSON.stringify({ value: true, expiresAt: Date.now() - 1000 })
+      )
+
+      mockLoginBackupEmailInterstitialManagerService.userIsElegibleForInterstitial.and.returnValue(
+        of(true)
+      )
+      mockLoginBackupEmailInterstitialManagerService.getInterstitialTogglz.and.returnValue(
+        of(true)
+      )
+      mockLoginBackupEmailInterstitialManagerService.getInterstitialViewed.and.returnValue(
+        of(false)
+      )
+      mockLoginBackupEmailInterstitialManagerService.showInterstitialAsComponent.and.returnValue(
+        of({} as any)
+      )
+
+      service
+        .checkLoginInterstitials(validUserRecord, {
+          returnType: 'component',
+          togglzPrefix: 'OAUTH',
+        })
+        .subscribe()
+      tick(1)
+
+      expect(
+        mockLoginBackupEmailInterstitialManagerService.showInterstitialAsComponent
+      ).toHaveBeenCalled()
+    }))
+
+    it('ignores a stale OAuth flag on the my-orcid surface', fakeAsync(() => {
+      // The my-orcid branch never sets this flag — it carries `justRegistered`
+      // on the URL instead. A flag seen here can only be left over from an
+      // unrelated OAuth flow, so it must not suppress anything.
+      mockInterstitialsService.checkIfSessionAlreadyCheckedInterstitialsLogic.and.returnValue(
+        false
+      )
+      oauthUrlSession.setJustRegistered(true)
+
+      mockLoginBackupEmailInterstitialManagerService.userIsElegibleForInterstitial.and.returnValue(
+        of(true)
+      )
+      mockLoginBackupEmailInterstitialManagerService.getInterstitialTogglz.and.returnValue(
+        of(true)
+      )
+      mockLoginBackupEmailInterstitialManagerService.getInterstitialViewed.and.returnValue(
+        of(false)
+      )
+      mockLoginBackupEmailInterstitialManagerService.showInterstitialAsDialog.and.returnValue(
+        of({
+          type: 'backup-email-interstitial',
+        } as BackupEmailComponentDialogOutput)
+      )
+
+      service
+        .checkLoginInterstitials(validUserRecord, {
+          returnType: 'dialog',
+          togglzPrefix: 'LOGIN',
+        })
+        .subscribe()
+      tick(1)
+
+      expect(
+        mockLoginBackupEmailInterstitialManagerService.showInterstitialAsDialog
+      ).toHaveBeenCalled()
+    }))
+
     it('still shows the interstitial without the query parameter', fakeAsync(() => {
       mockInterstitialsService.checkIfSessionAlreadyCheckedInterstitialsLogic.and.returnValue(
         false

src/app/core/oauth-urlsession-manager/oauth-urlsession-manager.service.spec.ts

@@ -2,15 +2,104 @@ import { TestBed } from '@angular/core/testing'
 
 import { OauthURLSessionManagerService } from './oauth-urlsession-manager.service'
 
+const JUST_REGISTERED_KEY = 'oauthJustRegistered'
+
 describe('OauthURLSessionManagerService', () => {
   let service: OauthURLSessionManagerService
 
   beforeEach(() => {
+    localStorage.removeItem(JUST_REGISTERED_KEY)
     TestBed.configureTestingModule({})
     service = TestBed.inject(OauthURLSessionManagerService)
   })
 
+  afterEach(() => {
+    localStorage.removeItem(JUST_REGISTERED_KEY)
+  })
+
   it('should be created', () => {
     expect(service).toBeTruthy()
   })
+
+  describe('consumeJustRegistered', () => {
+    it('returns the flag and clears storage', () => {
+      service.setJustRegistered(true)
+
+      expect(service.consumeJustRegistered()).toBeTrue()
+      expect(localStorage.getItem(JUST_REGISTERED_KEY)).toBeNull()
+      // One-time by contract: the stored flag is gone for the next caller
+      expect(service.consumeJustRegistered()).toBeFalse()
+    })
+
+    it('returns false and still clears an expired flag', () => {
+      localStorage.setItem(
+        JUST_REGISTERED_KEY,
+        JSON.stringify({ value: true, expiresAt: Date.now() - 1000 })
+      )
+
+      expect(service.consumeJustRegistered()).toBeFalse()
+      expect(localStorage.getItem(JUST_REGISTERED_KEY)).toBeNull()
+    })
+  })
+
+  describe('isJustRegistered', () => {
+    it('reads the stored flag without removing it', () => {
+      service.setJustRegistered(true)
+
+      expect(service.isJustRegistered()).toBeTrue()
+      expect(localStorage.getItem(JUST_REGISTERED_KEY)).not.toBeNull()
+      // Repeatable, unlike consumeJustRegistered
+      expect(service.isJustRegistered()).toBeTrue()
+    })
+
+    it('stays true after consumeJustRegistered has cleared storage', () => {
+      service.setJustRegistered(true)
+
+      expect(service.consumeJustRegistered()).toBeTrue()
+      expect(localStorage.getItem(JUST_REGISTERED_KEY)).toBeNull()
+      expect(service.isJustRegistered()).toBeTrue()
+    })
+
+    it('is false when nothing was ever stored', () => {
+      expect(service.isJustRegistered()).toBeFalse()
+    })
+
+    it('is false for an expired flag', () => {
+      localStorage.setItem(
+        JUST_REGISTERED_KEY,
+        JSON.stringify({ value: true, expiresAt: Date.now() - 1000 })
+      )
+
+      expect(service.isJustRegistered()).toBeFalse()
+    })
+
+    it('is false for a malformed payload', () => {
+      localStorage.setItem(JUST_REGISTERED_KEY, 'not json')
+
+      expect(service.isJustRegistered()).toBeFalse()
+    })
+
+    it('is false when the flag was stored as false', () => {
+      service.setJustRegistered(false)
+
+      expect(service.isJustRegistered()).toBeFalse()
+    })
+
+    it('is false once an already-expired flag was consumed', () => {
+      localStorage.setItem(
+        JUST_REGISTERED_KEY,
+        JSON.stringify({ value: true, expiresAt: Date.now() - 1000 })
+      )
+
+      expect(service.consumeJustRegistered()).toBeFalse()
+      expect(service.isJustRegistered()).toBeFalse()
+    })
+
+    it('is not resurrected by clear()', () => {
+      service.setJustRegistered(true)
+      service.clear()
+
+      expect(service.isJustRegistered()).toBeFalse()
+    })
+  })
 })