Skip to content

Conversation

@luca-peruzzo
Copy link
Member

@luca-peruzzo luca-peruzzo commented Sep 29, 2025

This pull request enhances the oidc-spa Angular integration by introducing new, idiomatic APIs for authentication and authorization.

A key feature is the introduction of a generic createAuthGuard static method in AbstractOidcService.
The new AdminGuard in the angular-kitchensink example demonstrates its usage.

Angular Interceptors have also been improved.
AbstractOidcService now exports createAdvancedBearerTokenInterceptor, createBasicBearerTokenInterceptor, and bearerTokenInterceptor static methods for more flexible control over which requests receive the bearer token and in which conditions.

Summary by CodeRabbit

  • New Features

    • Added bearer-token HTTP interceptor for API requests.
    • Introduced admin-only route protection with an alert and redirect when access is denied.
    • Added a configuration helper to switch between real and mock OIDC.
    • Enabled lazy loading for the Public page to improve startup.
    • Enhanced Register flow with improved redirect handling.
  • Refactor

    • Replaced legacy interceptors, consolidated OIDC setup, and streamlined templates without changing behavior.
  • Chores

    • Example apps now resolve the OIDC package from a local build for development.

@coderabbitai
Copy link

coderabbitai bot commented Sep 29, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Switches Angular examples to use a new Bearer token interceptor and an AdminGuard. Centralizes OIDC provisioning via a helper. Removes legacy per-service interceptor. Updates routes to use lazy loading and the new guard. Points examples to local package builds. Adds core Angular APIs for bearer-interceptor and guard creation.

Changes

Cohort / File(s) Summary of edits
Local package resolution
examples/angular/package.json, examples/angular-kitchensink/package.json
Changed dependency oidc-spa from "latest" to file:../../dist.
Kitchensink app wiring
examples/angular-kitchensink/src/app/app.config.ts, examples/angular-kitchensink/src/app/app.html, examples/angular-kitchensink/src/app/app.routes.ts
Replaced inline OIDC setup with provideOidc(...); switched HTTP interceptors to BearerInterceptor; updated routes to lazy-load Public and use AdminGuard; markup refactoring in template.
Kitchensink new guard/interceptor
examples/angular-kitchensink/src/app/guards/admin.guard.ts, examples/angular-kitchensink/src/app/interceptors/bearer.interceptor.ts
Added AdminGuard using Oidc.createAuthGuard; added BearerInterceptor using Oidc.createBasicBearerTokenInterceptor for JSONPlaceholder URLs.
Kitchensink service cleanup
examples/angular-kitchensink/src/app/services/todo.service.ts
Removed exported todoApiInterceptor; simplified imports; service API unchanged.
Angular example app wiring
examples/angular/src/app/app.config.ts, examples/angular/src/app/app.html
Switched to BearerInterceptor in HTTP client; small template adjustments, including register flow URL transform.
Angular example new interceptor
examples/angular/src/app/interceptors/bearer.interceptor.ts
Added exported BearerInterceptor targeting JSONPlaceholder URLs.
Angular example service cleanup
examples/angular/src/app/services/todo.service.ts
Removed exported todoApiInterceptor; trimmed OIDC/RxJS operator usage; service API unchanged.
Core Angular OIDC APIs
src/angular.ts
Added bearer token interceptor framework: createAdvancedBearerTokenInterceptor, createBasicBearerTokenInterceptor, bearerTokenInterceptor, and BearerTokenCondition type. Refactored/added auth guard helpers: private #createAuthGuard, public createAuthGuard, updated enforceLoginGuard.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant App as Angular App
  participant Http as HttpClient
  participant Int as BearerInterceptor
  participant OIDC as Oidc Service
  participant API as JSONPlaceholder API

  User->>App: Triggers HTTP request
  App->>Http: http.get(...)
  Http->>Int: Intercept(req)
  Int->>OIDC: await init + getAccessToken()
  alt URL matches pattern and method allowed
    Int->>Http: next(req + Authorization: Bearer ...)
  else No match or no token
    Int->>Http: next(req)
  end
  Http->>API: Send request
  API-->>Http: Response
  Http-->>App: Observable result
  App-->>User: Render data
Loading
sequenceDiagram
  autonumber
  participant Router as Angular Router
  participant Guard as AdminGuard (createAuthGuard)
  participant OIDC as Oidc Service
  participant Dec as Token Decoder

  Router->>Guard: canActivate(route, state)
  Guard->>OIDC: await init, isUserLoggedIn()
  alt Logged in
    Guard->>Dec: decodeIdToken()
    alt roles include "admin"
      Guard-->>Router: true
    else missing role
      Guard-->>Router: RedirectCommand(parseUrl("/"))
      note right of Guard: Alerts "Only Admins can access this page"
    end
  else Not logged in
    Guard-->>Router: RedirectCommand(parseUrl("/"))
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • V7 #75 — Introduces bearer-token interceptor APIs and related guard logic, overlapping with the new core methods and example interceptors.
  • Angular adapter #89 — Modifies Angular integration and exports/DI patterns similar to the added guard/interceptor wiring in examples.
  • Angular next #93 — Updates Angular OIDC integration, including example wiring and interceptor usage aligned with this PR’s changes.

Poem

A hop, a skip, a token to bear,
I guard the routes with vigilant care.
Lazy loads whisper, “Admins only, please,”
While headers don capes on API breeze.
From local burrow, packages compile—
Carrot approved, I ship with a smile. 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately describes the primary change of adding new factory functions for guards and interceptors in the Angular integration, which aligns with the pull request’s objectives of introducing createAuthGuard and bearer token interceptor methods.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

temporary, only for testing locally

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

temporary, only for testing locally

return interceptor;
}

static get bearerTokenInterceptor() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if it could be useful for users that just want to add bearer in every request

@luca-peruzzo luca-peruzzo marked this pull request as draft September 29, 2025 16:17
@luca-peruzzo luca-peruzzo self-assigned this Sep 29, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (5)
examples/angular-kitchensink/src/app/guards/admin.guard.ts (2)

14-17: Prefer returning a UrlTree directly instead of RedirectCommand.

Simpler and widely used in guards; avoids coupling to Router internals and keeps return union small.

Apply this diff:

-    alert('Only Admins can access this page');
-
-    return new RedirectCommand(router.parseUrl('/'));
+    alert('Only Admins can access this page');
+    return router.createUrlTree(['/']);

14-15: Avoid blocking alert in guards.

Swap alert for a non-blocking UX (snackbar/toast) or emit a navigation message to the landing route and show it there. Guards should not block the event loop.

examples/angular-kitchensink/src/app/app.config.ts (1)

18-23: Add an explicit return type for provideOidc for stronger typings.

Helps tooling and prevents accidental misuse in providers arrays.

Apply this diff:

-import { provideHttpClient, withInterceptors } from '@angular/common/http';
+import { provideHttpClient, withInterceptors } from '@angular/common/http';
+import type { Provider } from '@angular/core';
@@
-const provideOidc = (useMockOidc: boolean) =>
+const provideOidc = (useMockOidc: boolean): Provider =>
   useMockOidc
     ? Oidc.provideMock({
       isUserInitiallyLoggedIn: true,
     })
     : Oidc.provide(async () => {
examples/angular-kitchensink/src/app/app.html (2)

53-58: Avoid calling functions in template for frequently changing values.

oidc.$decodedIdToken().name is a function call; prefer a computed/signal (e.g., userName = computed(() => oidc.$decodedIdToken()?.name ?? '' )) and bind {{ userName() }} to reduce re-computation.


81-101: Compute seconds once to avoid duplicate calls in the same block.

oidc.$secondsLeftBeforeAutoLogout() is invoked in the @if condition and again in text. Cache it in the component (e.g., secondsLeft = computed(() => oidc.$secondsLeftBeforeAutoLogout())) or expose a signal from the service.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a1aae19 and e8a4152.

📒 Files selected for processing (13)
  • examples/angular-kitchensink/package.json (1 hunks)
  • examples/angular-kitchensink/src/app/app.config.ts (1 hunks)
  • examples/angular-kitchensink/src/app/app.html (3 hunks)
  • examples/angular-kitchensink/src/app/app.routes.ts (2 hunks)
  • examples/angular-kitchensink/src/app/guards/admin.guard.ts (1 hunks)
  • examples/angular-kitchensink/src/app/interceptors/bearer.interceptor.ts (1 hunks)
  • examples/angular-kitchensink/src/app/services/todo.service.ts (1 hunks)
  • examples/angular/package.json (1 hunks)
  • examples/angular/src/app/app.config.ts (1 hunks)
  • examples/angular/src/app/app.html (3 hunks)
  • examples/angular/src/app/interceptors/bearer.interceptor.ts (1 hunks)
  • examples/angular/src/app/services/todo.service.ts (1 hunks)
  • src/angular.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
examples/angular/src/app/app.config.ts (3)
examples/angular-kitchensink/src/app/app.config.ts (1)
  • appConfig (36-44)
examples/angular-kitchensink/src/app/interceptors/bearer.interceptor.ts (1)
  • BearerInterceptor (4-6)
examples/angular/src/app/interceptors/bearer.interceptor.ts (1)
  • BearerInterceptor (4-6)
examples/angular-kitchensink/src/app/guards/admin.guard.ts (1)
src/keycloak/keycloak-js/Keycloak.ts (1)
  • oidc (1051-1062)
examples/angular-kitchensink/src/app/interceptors/bearer.interceptor.ts (1)
examples/angular/src/app/interceptors/bearer.interceptor.ts (1)
  • BearerInterceptor (4-6)
examples/angular-kitchensink/src/app/app.routes.ts (2)
examples/angular/src/app/app.routes.ts (1)
  • routes (4-12)
examples/angular-kitchensink/src/app/guards/admin.guard.ts (1)
  • AdminGuard (5-18)
examples/angular-kitchensink/src/app/app.config.ts (5)
examples/angular/src/app/app.config.ts (1)
  • appConfig (12-24)
examples/angular-kitchensink/src/app/interceptors/bearer.interceptor.ts (1)
  • BearerInterceptor (4-6)
examples/angular-kitchensink/src/app/app.routes.ts (1)
  • routes (5-18)
examples/angular-kitchensink/src/environments/environment.mock.ts (1)
  • environment (1-3)
examples/angular-kitchensink/src/environments/environment.ts (1)
  • environment (1-3)
src/angular.ts (1)
src/core/Oidc.ts (1)
  • DecodedIdToken_base (135-142)
🔇 Additional comments (3)
examples/angular/package.json (1)

25-25: Same dist-path verification applies here

This package.json has the identical file:../../dist reference; please apply the outcome of the earlier verification here as well.

examples/angular-kitchensink/src/app/guards/admin.guard.ts (1)

5-18: Guard logic looks solid and idiomatic.

Functional guard + Oidc.createAuthGuard composition is clean; role check is safe with optional chaining.

examples/angular-kitchensink/src/app/app.config.ts (1)

40-43: HTTP interceptor wiring change looks good.

provideHttpClient(withInterceptors([BearerInterceptor])) precedes router and OIDC providers; ordering is sane and non-problematic here.

If other interceptors exist in the app, please confirm intended order (auth/token first or last) to avoid header overrides.

Comment on lines +35 to +49
@defer (when oidc.prInitialized | async) {    
<a
routerLink="/admin-only"
routerLinkActive
#rla3="routerLinkActive"
[style]="{
fontWeight: rla3.isActive ? 'bold' : 'normal',
cursor: canShowAdminLink ? 'pointer' : 'not-allowed',
}"
[attr.aria-disabled]="canShowAdminLink ? null : true"
>Admin only</a>
} @placeholder {
&nbsp; &nbsp;
<a routerLink="/admin-only" routerLinkActive>Admin only</a>
}
</span>
</div>
[attr.aria-disabled]="canShowAdminLink ? null : true"
>Admin only</a
>
} @placeholder { &nbsp; &nbsp;
<a routerLink="/admin-only" routerLinkActive>Admin only</a>
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify canShowAdminLink is defined and reflects the guard logic.

It’s referenced for styling/aria but not shown here. Ensure it’s a signal/computed that mirrors the admin role check to keep UX consistent with the guard.

Run to confirm definition and usages:


🏁 Script executed:

#!/bin/bash
rg -nC2 -S '\bcanShowAdminLink\b'

Length of output: 969


🏁 Script executed:

#!/bin/bash
sed -n '1,200p' examples/angular-kitchensink/src/app/app.ts

Length of output: 812


Fix canShowAdminLink logic to match the guard
In examples/angular-kitchensink/src/app/app.ts (lines 21–27), the getter returns true when the user isn’t logged in, but the route guard blocks access until login. Change the early return to false so the link is disabled unless the user is both logged in and an admin:

get canShowAdminLink(): boolean {
  if (!this.oidc.isUserLoggedIn) {
    return false;
  }
  const roles = this.oidc.$decodedIdToken().realm_access?.roles ?? [];
  return roles.includes('admin');
}
🤖 Prompt for AI Agents
In examples/angular-kitchensink/src/app/app.ts around lines 21 to 27, the
canShowAdminLink getter currently returns true when the user is not logged in
which contradicts the route guard; change the early return to false so
unauthenticated users cannot see the admin link, then ensure the getter
retrieves roles from this.oidc.$decodedIdToken().realm_access?.roles (defaulting
to empty array) and returns true only if roles.includes('admin').

@luca-peruzzo luca-peruzzo force-pushed the feature/enhance-angular branch from 537e97c to f827e81 Compare October 2, 2025 10:07
@luca-peruzzo luca-peruzzo force-pushed the feature/enhance-angular branch from f827e81 to c80d811 Compare October 2, 2025 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants