-
Notifications
You must be signed in to change notification settings - Fork 18
Add guards and interceptors create functions #96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughSwitches 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
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
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
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. Comment |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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() { |
There was a problem hiding this comment.
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
There was a problem hiding this 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 blockingalertin guards.Swap
alertfor 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 forprovideOidcfor 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().nameis 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@ifcondition 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
📒 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 hereThis package.json has the identical
file:../../distreference; 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.
| @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 { | ||
| | ||
| <a routerLink="/admin-only" routerLinkActive>Admin only</a> | ||
| } | ||
| </span> | ||
| </div> | ||
| [attr.aria-disabled]="canShowAdminLink ? null : true" | ||
| >Admin only</a | ||
| > | ||
| } @placeholder { | ||
| <a routerLink="/admin-only" routerLinkActive>Admin only</a> | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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.tsLength 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').
537e97c to
f827e81
Compare
f827e81 to
c80d811
Compare
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
createAuthGuardstatic method in AbstractOidcService.The new
AdminGuardin the angular-kitchensink example demonstrates its usage.Angular Interceptors have also been improved.
AbstractOidcService now exports
createAdvancedBearerTokenInterceptor,createBasicBearerTokenInterceptor, andbearerTokenInterceptorstatic methods for more flexible control over which requests receive the bearer token and in which conditions.Summary by CodeRabbit
New Features
Refactor
Chores