feat(meetings): add join URL endpoint and improve authentication flow #80
Conversation
LFXV2-458: Implement meeting join URL functionality with selective authentication - Add /public/api/meetings/:id/join-url endpoint - Enhance auth middleware for public route handling - Improve meeting components for join URL management - Add M2M token support for server-side API calls - Create custom authentication error handling Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Caution Review failedThe pull request is closed. WalkthroughAdds recurring-meeting occurrence support, a public join-URL flow (optional password + restricted-email check), client/server APIs/routes for public meetings and join URLs, auth middleware logout on token-refresh failure, and UI/template/form updates for meeting pages, cards, modal, dashboard, and delete confirmation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant WC as Web Client (MeetingComponent)
participant MS as Client MeetingService
participant S as Server (PublicMeetingController)
participant OS as Upstream (M2M)
Note over WC: Page load (public meeting)
U->>WC: Navigate to /meeting/:id?password=...
WC->>MS: getPublicMeeting(id, password)
MS->>S: GET /public/api/meetings/:id?password=...
S->>S: validate password
S->>OS: fetch meeting via M2M
OS-->>S: meeting (+join_url if applicable)
S-->>MS: { meeting, project }
MS-->>WC: meeting, project
WC->>WC: set project signal, render UI
sequenceDiagram
autonumber
participant U as User
participant WC as Web Client (MeetingComponent)
participant MS as Client MeetingService
participant S as Server (PublicMeetingController)
participant SV as Server MeetingService
participant OS as Upstream (M2M)
Note over WC: Join flow
U->>WC: Click "Join"
WC->>MS: getPublicMeetingJoinUrl(id, password, {email,name})
MS->>S: POST /public/api/meetings/:id/join-url?password=...
S->>S: validate password
alt restricted meeting
S->>SV: getMeetingRegistrantsByEmail(req,id,email)
SV-->>S: registrant list
S->>S: authorize or throw AuthorizationError
end
S->>OS: fetch join URL via M2M
OS-->>S: { join_url }
S-->>MS: { join_url }
MS-->>WC: { join_url }
WC->>WC: open join_url in new tab
sequenceDiagram
autonumber
participant M as Auth Middleware
participant O as OIDC/Token Source
participant R as Route
M->>O: extract/refresh token
O-->>M: TokenExtractionResult { success, needsLogout }
alt needsLogout == true
M-->>R: decision=logout
R->>O: res.oidc.logout()
else
M-->>R: decision=allow/redirect/error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
Comment |
github-actions
bot
requested review from
andrest50,
audigregorie,
dealako and
mauriciozanettisalomao
There was a problem hiding this comment.
Pull Request Overview
This PR implements a meeting join URL endpoint and improves the authentication flow for public meeting access. The changes focus on enabling users to retrieve join URLs through a new public API endpoint while maintaining security through selective authentication and proper access controls.
Key Changes
- Added new
/public/api/meetings/:id/join-urlPOST endpoint for retrieving meeting join URLs - Enhanced authentication middleware with token refresh failure handling and selective auth patterns
- Improved public meeting controller with M2M token support and better error handling
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/meeting.interface.ts | Added MeetingOccurrence interface and occurrences field to Meeting |
| packages/shared/src/interfaces/auth.interface.ts | Extended auth interfaces with logout action and token extraction result |
| apps/lfx-pcc/src/server/utils/m2m-token.util.ts | Added TODO comment for token caching |
| apps/lfx-pcc/src/server/services/meeting.service.ts | Added getMeetingRegistrantsByEmail method |
| apps/lfx-pcc/src/server/server.ts | Enhanced SSR authentication with token expiration check |
| apps/lfx-pcc/src/server/routes/public-meetings.route.ts | Added POST endpoint for join URL retrieval |
| apps/lfx-pcc/src/server/middleware/auth.middleware.ts | Enhanced with token refresh failure handling and logout action |
| apps/lfx-pcc/src/server/errors/authentication.error.ts | Added AuthorizationError class |
| apps/lfx-pcc/src/server/controllers/public-meeting.controller.ts | Refactored with M2M token support and improved validation |
| apps/lfx-pcc/src/app/shared/services/meeting.service.ts | Added getPublicMeetingJoinUrl method and password parameter support |
| apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts | Enhanced with meeting occurrence support and calendar event handling |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-modal/meeting-modal.component.ts | Added occurrence input parameter |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-modal/meeting-modal.component.html | Updated to pass occurrence to meeting card |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.ts | Removed unused RadioButtonComponent import |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.html | Commented out recurring meeting deletion options |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts | Enhanced to support meeting occurrences with fallback to meeting data |
| apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html | Updated template to use occurrence data with fallback to meeting data |
| apps/lfx-pcc/src/app/modules/meeting/meeting.component.ts | Enhanced with join URL functionality and form improvements |
| apps/lfx-pcc/src/app/modules/meeting/meeting.component.html | Updated UI with join meeting functionality and responsive improvements |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...ect/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.ts
Show resolved
Hide resolved
✅ E2E Tests PassedBrowser: chromium All E2E tests passed successfully. Test Configuration
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/lfx-pcc/src/server/utils/m2m-token.util.ts (1)
18-25: Fail fast on missing configuration.Validate issuer, audience, client_id, and client_secret before building the request; otherwise we construct an invalid URL and POST to “undefined…”.
export async function generateM2MToken(req: Request): Promise<string> { - // TODO: Cache the token - const issuerBaseUrl = process.env['M2M_AUTH_ISSUER_BASE_URL']; + const issuerBaseUrl = process.env['M2M_AUTH_ISSUER_BASE_URL']; + const audience = process.env['M2M_AUTH_AUDIENCE']; + const clientId = process.env['M2M_AUTH_CLIENT_ID']; + const clientSecret = process.env['M2M_AUTH_CLIENT_SECRET']; + if (!issuerBaseUrl || !audience || !clientId || !clientSecret) { + throw new MicroserviceError('M2M auth not configured', 500, 'M2M_CONFIG_MISSING', { + missing: { issuerBaseUrl: !!issuerBaseUrl, audience: !!audience, clientId: !!clientId, clientSecret: !!clientSecret }, + }); + } const isAuthelia = issuerBaseUrl?.includes('auth.k8s.orb.local');apps/lfx-pcc/src/server/server.ts (2)
218-239: Bug: missing accessToken passes the “not expired” check
!req.oidc?.accessToken?.isExpired()is true whenaccessTokenis undefined, falsely treating sessions as authenticated. Require token presence explicitly.- if (req.oidc?.isAuthenticated() && !req.oidc?.accessToken?.isExpired()) { + const token = req.oidc?.accessToken; + if (req.oidc?.isAuthenticated() && token && !token.isExpired()) {
205-207: Handle errors for public API routes tooEnsure
/public/api/*responses are normalized by the same error handler.app.use('/api/*', apiErrorHandler); +app.use('/public/api/*', apiErrorHandler);
🧹 Nitpick comments (27)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.ts (2)
34-34: HardenregistrantCountcalculation.
Use nullish coalescing to avoidNaNwhen counts are undefined.- public readonly registrantCount: number = this.meeting.individual_registrants_count + this.meeting.committee_members_count; + public readonly registrantCount: number = + (this.meeting?.individual_registrants_count ?? 0) + (this.meeting?.committee_members_count ?? 0);
40-51: ResetisDeletingviafinalizeto de-duplicate state handling.
Current success/error branches both toggle the flag.- this.isDeleting.set(true); + this.isDeleting.set(true); const deleteType = this.isRecurring && !this.isPastMeeting ? this.deleteForm.get('deleteType')?.value : undefined; - this.meetingService - .deleteMeeting(this.meeting.uid, deleteType) - .pipe(take(1)) + this.meetingService + .deleteMeeting(this.meeting.uid, deleteType) + .pipe(take(1), finalize(() => this.isDeleting.set(false))) .subscribe({ next: () => { - this.isDeleting.set(false); + // handled by finalize() this.messageService.add({Add import:
- import { take } from 'rxjs'; + import { take } from 'rxjs'; + import { finalize } from 'rxjs/operators';apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.html (2)
44-75: Temporarily disabling recurrence options is fine; confirm UX/API expectations.
With this block commented,deleteTypestays "single" (default). Verify this matches backend behavior and product intent until series/future deletion is available.Optionally remove the commented HTML and gate it behind a feature flag to reduce template noise. I can provide a flagged version if helpful.
84-90: Simplify the warning copy while options are disabled.
Since users can’t choose series/future, the conditional branches won’t change. Consider a single message.- @if (isRecurring && !isPastMeeting && deleteForm.get('deleteType')?.value === 'series') { - This will permanently delete all meetings in this series. This action cannot be undone. - } @else if (isRecurring && !isPastMeeting && deleteForm.get('deleteType')?.value === 'future') { - This will permanently delete this meeting and all future occurrences in the series. This action cannot be undone. - } @else { - This will permanently delete this meeting. This action cannot be undone. - } + This will permanently delete this meeting. This action cannot be undone.apps/lfx-pcc/src/server/utils/m2m-token.util.ts (2)
41-41: Add request timeout and limited retries for transient failures.Wrap fetch with AbortController (e.g., 5s) and add bounded retries w/ backoff on 5xx/ENOTFOUND to harden this hot path.
52-65: Sanitize sensitive fields in error logs.Mask client_id/client_secret if they accidentally appear in errorBody returned by the IdP.
packages/shared/src/interfaces/meeting.interface.ts (2)
150-160: Relax occurrence fields that may not be occurrence-specific.If backend doesn’t provide per-occurrence
title/description, mark them optional and let UI fall back to parent meeting fields.-export interface MeetingOccurrence { +export interface MeetingOccurrence { occurrence_id: string; - title: string; - description: string; + title?: string; + description?: string; start_time: string; duration: number; }
142-145: Makeoccurrencesoptional and coalesce at usage sites
- In
packages/shared/src/interfaces/meeting.interface.ts, changetooccurrences: MeetingOccurrence[];or document/ensure the API always returns an empty array.occurrences?: MeetingOccurrence[];- In
meeting-dashboard.component.ts(around lines 186 and 320), updatetom.occurrences.map(...)(m.occurrences ?? []).map(...)packages/shared/src/interfaces/auth.interface.ts (2)
126-136: Consider including token/error context.
TokenExtractionResultmight benefit fromtoken?: stringanderrorCode?: stringto reduce re-parsing upstream.
148-150: Avoid tri-state for needsLogout.Prefer
needsLogout: boolean(non-optional) to avoid ambiguity.apps/lfx-pcc/src/server/services/meeting.service.ts (1)
249-259: Mirror logging/error handling from sibling method.Add info/error logs like
getMeetingRegistrantsfor observability and parity.apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts (1)
315-318: Update comment to reflect actual data source.Occurrences are read from
m.occurrences, not a “recurrence object”.apps/lfx-pcc/src/server/middleware/auth.middleware.ts (3)
296-307: Include returnTo on logout for smoother UX (SSR only)Preserves navigation context after forced logout.
- res.oidc.logout(); + res.oidc.logout({ returnTo: req.originalUrl });
206-208:redirectUrlis computed but ignored — honor it or stop setting itEither redirect to the provided URL or remove it from the decision payload to avoid dead code paths.
- case 'redirect': - if (decision.redirectUrl) { - // Use OIDC login method which handles the redirect properly - res.oidc.login({ returnTo: req.originalUrl }); - } else { - res.redirect('/'); - } + case 'redirect': + if (decision.redirectUrl) { + res.redirect(decision.redirectUrl); + } else { + // Use OIDC login method which handles the redirect properly + res.oidc.login({ returnTo: req.originalUrl }); + }Also applies to: 287-294
86-144: Minor: reduce token detail in logsYou already redact headers; still, consider dropping
tokenValueand cookie keys from logs to minimize signal for token state.apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-modal/meeting-modal.component.ts (1)
19-21: Add explicit types for meeting and occurrenceImproves template type safety and IDE help; aligns with shared interfaces.
+import { Meeting, MeetingOccurrence } from '@lfx-pcc/shared/interfaces'; @@ - public readonly meeting = this.config.data?.meeting; - public readonly occurrence = this.config.data?.occurrence; + public readonly meeting: Meeting | null = this.config.data?.meeting ?? null; + public readonly occurrence: MeetingOccurrence | null = this.config.data?.occurrence ?? null;apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-modal/meeting-modal.component.html (1)
7-7: Passing occurrence into card: LGTM; add test id for new binding pointThe new
[occurrenceInput]wiring looks correct. Add adata-testidon this instance to keep selectors stable in modal tests.- <lfx-meeting-card [meetingInput]="meeting" [occurrenceInput]="occurrence" [pastMeeting]="false" (meetingDeleted)="onDelete()"></lfx-meeting-card> + <lfx-meeting-card + data-testid="meeting-modal-card" + [meetingInput]="meeting" + [occurrenceInput]="occurrence" + [pastMeeting]="false" + (meetingDeleted)="onDelete()"></lfx-meeting-card>apps/lfx-pcc/src/server/routes/public-meetings.route.ts (2)
14-16: Comment says GET, route is POSTAlign the comment with the actual HTTP method to avoid confusion.
-// GET /public/api/meetings/:id/join-url - get the join URL for a meeting (public access, no authentication required) +// POST /public/api/meetings/:id/join-url - get the join URL for a meeting (public access, no authentication required) router.post('/:id/join-url', (req, res, next) => publicMeetingController.postMeetingJoinUrl(req, res, next));
14-16: Consider lightweight rate limiting on join-urlThis endpoint is a password-validation surface; add a small rate limit (e.g., per IP) to reduce brute-force attempts.
If desired, I can open a follow-up PR wiring
express-rate-limitat the router level.apps/lfx-pcc/src/server/errors/authentication.error.ts (1)
23-35: Includemetadatain AuthorizationError options for parity
AuthenticationErrorsupportsmetadata;AuthorizationErrorshould too for consistent structured logging.export class AuthorizationError extends BaseApiError { public constructor( message = 'Authorization required', options: { operation?: string; service?: string; path?: string; + metadata?: Record<string, any>; } = {} ) { super(message, 403, 'AUTHORIZATION_REQUIRED', options); } }apps/lfx-pcc/src/app/shared/services/meeting.service.ts (1)
110-116: Public meeting and join-url client: LGTM; tighten typing for request bodyThe API surface looks good. Consider introducing a typed request for the join body to reduce duplication and improve call-site clarity.
+export interface PublicJoinRequest { + email?: string; + name?: string; + organization?: string; +} … - public getPublicMeetingJoinUrl( - id: string, - password: string | null, - body?: { email?: string; name?: string; organization?: string } - ): Observable<MeetingJoinURL> { + public getPublicMeetingJoinUrl( + id: string, + password: string | null, + body?: PublicJoinRequest + ): Observable<MeetingJoinURL> {Also applies to: 123-141, 15-16
apps/lfx-pcc/src/app/modules/meeting/meeting.component.html (2)
9-13: Bind image src property and add test idPrefer property binding for URLs; add a stable selector per guidelines.
- @if (project()?.logo_url) { - <img src="{{ project()?.logo_url }}" alt="{{ project()?.name }}" class="w-full h-20 mb-6" /> - } + @if (project()?.logo_url) { + <img + data-testid="project-logo" + [src]="project()?.logo_url" + [alt]="project()?.name || 'Project Logo'" + class="w-full h-20 mb-6" /> + }
186-190: Open join in a new tab safely and add test idsMake the join behavior consistent and avoid anchor semantics when disabled. Route both variants through
onJoinMeeting()(which canwindow.open(url, '_blank', 'noopener,noreferrer')) and adddata-testidfor stability.- @if (meeting().join_url) { - <lfx-button size="small" [href]="meeting().join_url" severity="primary" label="Join Meeting" icon="fa-light fa-sign-in"></lfx-button> - } @else { - <lfx-button size="small" severity="primary" label="Join Meeting" icon="fa-light fa-sign-in" (click)="onJoinMeeting()"></lfx-button> - } + <lfx-button + data-testid="join-button-authenticated" + size="small" + severity="primary" + label="Join Meeting" + icon="fa-light fa-sign-in" + (click)="onJoinMeeting()"></lfx-button>- @if (meeting().join_url) { - <lfx-button - size="small" - [href]="joinForm.invalid ? undefined : meeting().join_url" - severity="primary" - label="Join Meeting" - icon="fa-light fa-sign-in" - [disabled]="joinForm.invalid"></lfx-button> - } @else { - <lfx-button - size="small" - severity="primary" - label="Join Meeting" - [disabled]="joinForm.invalid" - icon="fa-light fa-sign-in" - (click)="onJoinMeeting()"></lfx-button> - } + <lfx-button + data-testid="join-button-guest" + size="small" + severity="primary" + label="Join Meeting" + [disabled]="joinForm.invalid" + icon="fa-light fa-sign-in" + (click)="onJoinMeeting()"></lfx-button>Confirm
onJoinMeeting()opens the URL withwindow.open(url, '_blank', 'noopener,noreferrer')to preventwindow.openerattacks.Also applies to: 269-286
apps/lfx-pcc/src/server/controllers/public-meeting.controller.ts (2)
213-239: Avoid email enumeration in error responsesThrowing an
AuthorizationErrorwith “email address is not registered…” leaks registrant status. Return a generic authorization failure message.- if (registrants.resources.length === 0) { - throw new AuthorizationError('The email address is not registered for this restricted meeting', { + if (registrants.resources.length === 0) { + throw new AuthorizationError('Not authorized to access this meeting', { operation: 'post_meeting_join_url', service: 'public_meeting_controller', path: `/meetings/${id}`, }); }
139-146: M2M token: add caching and basic join-url rate limiting (follow-up)
- Cache
generateM2MTokenby issuer/audience until expiry to cut auth-provider load.- Add per-IP/per-meeting rate limiting around join-url to reduce password brute force.
I can provide a small in-memory cache (with TTL) and a router-scoped rate limiter if you want a follow-up patch.
Also applies to: 181-184, 189-211
apps/lfx-pcc/src/app/modules/meeting/meeting.component.ts (2)
22-22: Importfinalizefor safer subscription cleanup in onJoinMeeting().You’re manually toggling
isJoiningin bothnextanderror. Preferfinalizeto guarantee reset on all paths.-import { combineLatest, map, of, switchMap, tap } from 'rxjs'; +import { combineLatest, map, of, switchMap, tap, finalize } from 'rxjs';
115-117: Minor: normalize inputs.Consider trimming and lowercasing email at submit-time (already trimmed in the suggested diff) and trimming
namebefore use.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (19)
apps/lfx-pcc/src/app/modules/meeting/meeting.component.html(5 hunks)apps/lfx-pcc/src/app/modules/meeting/meeting.component.ts(4 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html(2 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts(3 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.html(2 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.ts(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-modal/meeting-modal.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-modal/meeting-modal.component.ts(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts(9 hunks)apps/lfx-pcc/src/app/shared/services/meeting.service.ts(2 hunks)apps/lfx-pcc/src/server/controllers/public-meeting.controller.ts(3 hunks)apps/lfx-pcc/src/server/errors/authentication.error.ts(1 hunks)apps/lfx-pcc/src/server/middleware/auth.middleware.ts(7 hunks)apps/lfx-pcc/src/server/routes/public-meetings.route.ts(1 hunks)apps/lfx-pcc/src/server/server.ts(1 hunks)apps/lfx-pcc/src/server/services/meeting.service.ts(1 hunks)apps/lfx-pcc/src/server/utils/m2m-token.util.ts(1 hunks)packages/shared/src/interfaces/auth.interface.ts(3 hunks)packages/shared/src/interfaces/meeting.interface.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer TypeScript interfaces over union types when modeling shapes and contracts
Files:
apps/lfx-pcc/src/server/utils/m2m-token.util.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-modal/meeting-modal.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.tsapps/lfx-pcc/src/server/routes/public-meetings.route.tsapps/lfx-pcc/src/server/services/meeting.service.tspackages/shared/src/interfaces/meeting.interface.tsapps/lfx-pcc/src/server/errors/authentication.error.tsapps/lfx-pcc/src/app/shared/services/meeting.service.tsapps/lfx-pcc/src/server/server.tsapps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.tsapps/lfx-pcc/src/server/controllers/public-meeting.controller.tspackages/shared/src/interfaces/auth.interface.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-pcc/src/app/modules/meeting/meeting.component.tsapps/lfx-pcc/src/server/middleware/auth.middleware.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-pcc/src/server/utils/m2m-token.util.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-modal/meeting-modal.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.tsapps/lfx-pcc/src/server/routes/public-meetings.route.tsapps/lfx-pcc/src/server/services/meeting.service.tspackages/shared/src/interfaces/meeting.interface.tsapps/lfx-pcc/src/server/errors/authentication.error.tsapps/lfx-pcc/src/app/shared/services/meeting.service.tsapps/lfx-pcc/src/server/server.tsapps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.tsapps/lfx-pcc/src/server/controllers/public-meeting.controller.tspackages/shared/src/interfaces/auth.interface.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-pcc/src/app/modules/meeting/meeting.component.tsapps/lfx-pcc/src/server/middleware/auth.middleware.ts
apps/lfx-pcc/src/**/*.{ts,html}
📄 CodeRabbit inference engine (CLAUDE.md)
Use LFX wrapper components instead of importing PrimeNG components directly
Files:
apps/lfx-pcc/src/server/utils/m2m-token.util.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-modal/meeting-modal.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.tsapps/lfx-pcc/src/server/routes/public-meetings.route.tsapps/lfx-pcc/src/server/services/meeting.service.tsapps/lfx-pcc/src/server/errors/authentication.error.tsapps/lfx-pcc/src/app/shared/services/meeting.service.tsapps/lfx-pcc/src/app/modules/meeting/meeting.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.htmlapps/lfx-pcc/src/server/server.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-modal/meeting-modal.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.tsapps/lfx-pcc/src/server/controllers/public-meeting.controller.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-pcc/src/app/modules/meeting/meeting.component.tsapps/lfx-pcc/src/server/middleware/auth.middleware.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.html
**/*.{ts,tsx,js,jsx,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
apps/lfx-pcc/src/server/utils/m2m-token.util.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-modal/meeting-modal.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.tsapps/lfx-pcc/src/server/routes/public-meetings.route.tsapps/lfx-pcc/src/server/services/meeting.service.tspackages/shared/src/interfaces/meeting.interface.tsapps/lfx-pcc/src/server/errors/authentication.error.tsapps/lfx-pcc/src/app/shared/services/meeting.service.tsapps/lfx-pcc/src/app/modules/meeting/meeting.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.htmlapps/lfx-pcc/src/server/server.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-modal/meeting-modal.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.tsapps/lfx-pcc/src/server/controllers/public-meeting.controller.tspackages/shared/src/interfaces/auth.interface.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-pcc/src/app/modules/meeting/meeting.component.tsapps/lfx-pcc/src/server/middleware/auth.middleware.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.html
apps/lfx-pcc/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-pcc/src/**/*.ts: In the Angular app, use direct imports for standalone components (no barrel imports)
When defining types for PrimeNG usage, reference PrimeNG’s component interfaces
Files:
apps/lfx-pcc/src/server/utils/m2m-token.util.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-modal/meeting-modal.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.tsapps/lfx-pcc/src/server/routes/public-meetings.route.tsapps/lfx-pcc/src/server/services/meeting.service.tsapps/lfx-pcc/src/server/errors/authentication.error.tsapps/lfx-pcc/src/app/shared/services/meeting.service.tsapps/lfx-pcc/src/server/server.tsapps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.tsapps/lfx-pcc/src/server/controllers/public-meeting.controller.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-pcc/src/app/modules/meeting/meeting.component.tsapps/lfx-pcc/src/server/middleware/auth.middleware.ts
packages/shared/src/interfaces/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all shared interfaces in the shared package
Files:
packages/shared/src/interfaces/meeting.interface.tspackages/shared/src/interfaces/auth.interface.ts
apps/lfx-pcc/src/**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-pcc/src/**/*.html: Add data-testid attributes to new components for reliable test targeting
Follow data-testid naming convention: [section]-[component]-[element]
Files:
apps/lfx-pcc/src/app/modules/meeting/meeting.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-modal/meeting-modal.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.html
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: linuxfoundation/lfx-v2-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T18:09:48.791Z
Learning: Public routes include /meeting and /public/api; use M2M tokens for server-side API calls from public endpoints
📚 Learning: 2025-09-05T18:09:48.792Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T18:09:48.792Z
Learning: Applies to apps/lfx-pcc/src/**/*.{ts,html} : Use LFX wrapper components instead of importing PrimeNG components directly
Applied to files:
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.tsapps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.tsapps/lfx-pcc/src/app/modules/meeting/meeting.component.ts
📚 Learning: 2025-09-05T18:09:48.791Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T18:09:48.791Z
Learning: Public routes include /meeting and /public/api; use M2M tokens for server-side API calls from public endpoints
Applied to files:
apps/lfx-pcc/src/server/routes/public-meetings.route.tsapps/lfx-pcc/src/server/controllers/public-meeting.controller.ts
📚 Learning: 2025-09-05T18:09:48.792Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T18:09:48.792Z
Learning: Applies to apps/lfx-pcc/src/**/*.ts : When defining types for PrimeNG usage, reference PrimeNG’s component interfaces
Applied to files:
apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts
📚 Learning: 2025-09-05T18:09:48.792Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T18:09:48.792Z
Learning: Applies to apps/lfx-pcc/src/**/*.ts : In the Angular app, use direct imports for standalone components (no barrel imports)
Applied to files:
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts
🧬 Code graph analysis (5)
apps/lfx-pcc/src/server/services/meeting.service.ts (2)
packages/shared/src/interfaces/api.interface.ts (1)
QueryServiceResponse(58-61)packages/shared/src/interfaces/meeting.interface.ts (1)
MeetingRegistrant(256-297)
apps/lfx-pcc/src/app/shared/services/meeting.service.ts (1)
packages/shared/src/interfaces/meeting.interface.ts (2)
Meeting(75-144)MeetingJoinURL(446-449)
apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts (2)
packages/shared/src/interfaces/meeting.interface.ts (2)
MeetingOccurrence(150-161)Meeting(75-144)packages/shared/src/interfaces/calendar.interface.ts (1)
CalendarEvent(10-36)
apps/lfx-pcc/src/server/controllers/public-meeting.controller.ts (5)
apps/lfx-pcc/src/server/helpers/logger.ts (2)
error(52-65)Logger(10-129)apps/lfx-pcc/src/server/utils/m2m-token.util.ts (1)
generateM2MToken(16-110)apps/lfx-pcc/src/server/helpers/validation.helper.ts (1)
validateUidParameter(30-49)apps/lfx-pcc/src/server/utils/security.util.ts (1)
validatePassword(63-65)apps/lfx-pcc/src/server/errors/authentication.error.ts (1)
AuthorizationError(24-35)
apps/lfx-pcc/src/server/middleware/auth.middleware.ts (1)
packages/shared/src/interfaces/auth.interface.ts (3)
TokenExtractionResult(130-135)AuthMiddlewareResult(141-150)AuthDecision(115-124)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests / Playwright E2E Tests
🔇 Additional comments (8)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-delete-confirmation/meeting-delete-confirmation.component.ts (2)
23-23: Removed RadioButton import matches disabled recurrence options — LGTM.
Keeps the component lean and aligned with the HTML change.
33-37: Ignore missingmeetingguard – this dialog is only ever opened in meeting-card.component.ts with a non‐nullmeeting(the caller doesif (!meeting) returnbefore calling.open(…, { data: { meeting } })), sothis.dialogConfig.data?.meetingwill always be defined.Likely an incorrect or invalid review comment.
apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts (1)
18-22: Ignore suggestion to replace AnimateOnScrollModule, ConfirmDialogModule, and DialogService imports with wrappers. No LFX wrapper components exist for these PrimeNG modules/services.Likely an incorrect or invalid review comment.
apps/lfx-pcc/src/server/server.ts (1)
199-201: Permissions router mount is correct Mounting at/api/projectsproduces the intended endpoints (/api/projects/:projectId/permissions, etc.) as defined inpermissions.route.ts.apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-modal/meeting-modal.component.ts (1)
20-20: Verify template bindingEnsure the template passes it through as
[occurrenceInput]="occurrence"to the meeting card.apps/lfx-pcc/src/app/modules/meeting/meeting.component.ts (3)
54-54: Writableprojectsignal change looks good.This matches the new flow where
projectis set from API response in the pipeline.
51-63: Ignore the type nit:authenticatedis correctly declared. userService.authenticated is a WritableSignal (see apps/lfx-pcc/src/app/shared/services/user.service.ts:15), so matching it with a WritableSignal field is valid.Likely an incorrect or invalid review comment.
19-21: Ignore this suggestion: no LFX wrapper exists for PrimeNG Toast, Tooltip or MessageService—keep the direct imports.Likely an incorrect or invalid review comment.
...lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.html
Show resolved
Hide resolved
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts
Show resolved
Hide resolved
When token refresh fails, the middleware was issuing IdP redirects for all routes, breaking XHR/Fetch clients on public API routes that expect JSON responses. This change: - Returns 401 error for API routes or non-GET requests on token refresh failure - Preserves logout redirect behavior for SSR GET requests - Prevents breaking public API clients while maintaining good UX for browser navigation Fixes token refresh handling for /public/api routes called via XHR/Fetch. 🤖 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
Signed-off-by: Asitha de Silva <asithade@gmail.com>
🧹 Deployment RemovedThe deployment for PR #80 has been removed. |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/lfx-pcc/src/server/middleware/auth.middleware.ts (1)
337-348: Propagate decision.statusCode into the AuthenticationError so the global handler returns 401/403File: apps/lfx-pcc/src/server/middleware/auth.middleware.ts (lines ~337-348)
apiErrorHandler uses BaseApiError.statusCode — set statusCode on the AuthenticationError options.- const error = new AuthenticationError( + const error = new AuthenticationError( decision.errorType === 'authorization' ? 'Insufficient permissions to access this resource' : 'Authentication required to access this resource', { operation: 'auth_middleware', service: 'authentication', path: req.path, + statusCode: decision.statusCode, } );
♻️ Duplicate comments (1)
apps/lfx-pcc/src/server/middleware/auth.middleware.ts (1)
381-385: Skip token extraction on optional API routes (public APIs use M2M)Avoids unnecessary refresh attempts and accidental 401s/logouts on public API calls. For SSR optional, still extract to enhance UX.
- if (routeConfig.tokenRequired || routeConfig.auth === 'optional') { - const tokenResult = await extractBearerToken(req); - hasToken = tokenResult.success; - needsLogout = tokenResult.needsLogout; - } + // Extract token for API routes only if required; for SSR 'optional', token enhances UX. + const shouldExtract = + routeConfig.type === 'api' ? !!routeConfig.tokenRequired : routeConfig.auth === 'optional'; + if (shouldExtract) { + const tokenResult = await extractBearerToken(req); + hasToken = tokenResult.success; + needsLogout = tokenResult.needsLogout; + }#!/bin/bash # Confirm '/public/api' calls aren’t triggering token refresh: rg -n -C2 "/public/api" apps/lfx-pcc/src # Confirm Request typing supports req.bearerToken: rg -nP -C2 "\binterface\s+Request\b|declare\s+global\b" packages | rg -n "bearerToken" -n
🧹 Nitpick comments (3)
apps/lfx-pcc/src/server/middleware/auth.middleware.ts (3)
112-121: Log message implies forced logout; clarify wordingWe no longer always logout on refresh failure (API/non-GET returns 401). Make the log neutral.
- 'Token refresh failed - logging user out' + 'Token refresh failed - marking needsLogout=true'
150-191: Smart handling of refresh failures (logout only for SSR GET) — niceThe split between SSR GET (logout) and API/non-GET (401) looks correct. Consider treating HEAD like GET to avoid odd behavior on HEAD requests.
- if (route.type === 'api' || req.method !== 'GET') { + const isSsrGetLike = route.type === 'ssr' && (req.method === 'GET' || req.method === 'HEAD'); + if (!isSsrGetLike) { ... - return { + return { action: 'error', errorType: 'authentication', statusCode: 401, }; } - // For SSR GET requests, proceed with logout redirect + // For SSR GET/HEAD requests, proceed with logout redirect
316-324: ‘redirectUrl’ isn’t used; standardize redirect signalingEither drop redirectUrl from the decision or set it to returnTo and use it here. Keeps intent consistent and avoids confusion.
- res.oidc.login({ returnTo: req.originalUrl }); + res.oidc.login({ returnTo: decision.redirectUrl || req.originalUrl });And when building the decision:
- redirectUrl: `/login?returnTo=${encodeURIComponent(req.originalUrl)}`, + redirectUrl: req.originalUrl,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/lfx-pcc/src/server/middleware/auth.middleware.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer TypeScript interfaces over union types when modeling shapes and contracts
Files:
apps/lfx-pcc/src/server/middleware/auth.middleware.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-pcc/src/server/middleware/auth.middleware.ts
apps/lfx-pcc/src/**/*.{ts,html}
📄 CodeRabbit inference engine (CLAUDE.md)
Use LFX wrapper components instead of importing PrimeNG components directly
Files:
apps/lfx-pcc/src/server/middleware/auth.middleware.ts
**/*.{ts,tsx,js,jsx,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
apps/lfx-pcc/src/server/middleware/auth.middleware.ts
apps/lfx-pcc/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-pcc/src/**/*.ts: In the Angular app, use direct imports for standalone components (no barrel imports)
When defining types for PrimeNG usage, reference PrimeNG’s component interfaces
Files:
apps/lfx-pcc/src/server/middleware/auth.middleware.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: linuxfoundation/lfx-v2-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T18:09:48.791Z
Learning: Public routes include /meeting and /public/api; use M2M tokens for server-side API calls from public endpoints
🧬 Code graph analysis (1)
apps/lfx-pcc/src/server/middleware/auth.middleware.ts (1)
packages/shared/src/interfaces/auth.interface.ts (3)
TokenExtractionResult(130-135)AuthMiddlewareResult(141-150)AuthDecision(115-124)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests / Playwright E2E Tests
🔇 Additional comments (2)
apps/lfx-pcc/src/server/middleware/auth.middleware.ts (2)
19-21: Public API route marked optional — aligns with M2M usageThis matches the public API/M2M approach. Ensure token extraction is skipped for optional API routes (see suggestion on Lines 381-385).
23-24: Incorrect — do not add '/meeting'; only '/meetings' exists. Ripgrep shows a single "meetings" route at apps/lfx-pcc/src/app/modules/project/project.routes.ts:12 and no "meeting" path in apps/lfx-pcc/src.Likely an incorrect or invalid review comment.
Summary
Implement meeting join URL functionality and improve authentication flow for public meeting pages.
JIRA Ticket: LFXV2-458
Changes Made
Frontend Updates
Backend Updates
/public/api/meetings/:id/join-urlendpoint for retrieving meeting join URLsShared Package Updates
Technical Details
Testing Checklist
Additional Notes