fix: disclaimer popup logout/accept consistency and audit log endpoint - #3224
Open
ohdsi-trex wants to merge 3 commits into
Open
fix: disclaimer popup logout/accept consistency and audit log endpoint#3224ohdsi-trex wants to merge 3 commits into
ohdsi-trex wants to merge 3 commits into
Conversation
ohdsi-trex
requested review from
SantanM,
hengxian-jiang and
suwarnoong
as code owners
August 27, 2026 08:20
ohdsi-trex
requested review from
brandantck,
csafreen and
p-hoffmann
as code owners
August 27, 2026 08:20
Contributor
There was a problem hiding this comment.
Pull request overview
Moves disclaimer auditing to a portal-owned endpoint while ensuring audit failures cannot block acceptance or OIDC logout.
Changes:
- Adds the authorized portal audit endpoint and client.
- Makes disclaimer audit submission best-effort.
- Adds service/client tests and implementation plans.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
trex/plans/2026-08-27-disclaimer-logout-and-audit-removal.md |
Documents an alternative audit-removal plan. |
trex/plans/2026-08-26-disclaimer-logout-and-log-authorization.md |
Documents the earlier Trex authorization approach. |
plugins/ui/apps/portal/src/containers/shared/Legal/DisclaimerDialog.tsx |
Makes auditing non-blocking and preserves /logout navigation. |
plugins/ui/apps/portal/src/axios/system-portal.ts |
Adds the audit API client method. |
plugins/ui/apps/portal/src/axios/system-portal.test.ts |
Tests the audit request configuration. |
plugins/functions/portal/src/audit/audit.service.ts |
Logs disclaimer responses with user context. |
plugins/functions/portal/src/audit/audit.service.test.ts |
Tests audit service logging. |
plugins/functions/portal/src/audit/audit.module.ts |
Registers audit components. |
plugins/functions/portal/src/audit/audit.controller.ts |
Exposes the audit endpoint. |
plugins/functions/portal/src/app.module.ts |
Loads the audit module. |
plugins/functions/package.json |
Adds route authorization and tenant-viewer scope. |
d2e/plans/audit-log-endpoint.md |
Documents the selected audit-endpoint design. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+11
to
+12
| // TODO: Persist disclaimer audit records when a portal audit data model is available. | ||
| console.log("Disclaimer response", { userId, response }); |
Comment on lines
+1
to
+5
| # Disclaimer Logout and Audit Removal — Implementation Plan | ||
|
|
||
| ## Goal | ||
|
|
||
| Ensure the disclaimer dialog never calls `POST /d2e/trex/log`, so both its **Accept** and **Logout** actions are independent of the optional audit endpoint. The Logout action must immediately enter the portal's existing OIDC logout path, which resolves the configured provider's end-session endpoint consistently across deployment types. |
Comment on lines
+1
to
+5
| # Disclaimer Logout and Trex Log Authorization — Implementation Plan | ||
|
|
||
| **Goal:** Ensure declining the legal disclaimer always proceeds to the portal's existing OIDC logout flow, and permit any authenticated portal user to record a disclaimer response at `POST /d2e/trex/log`. | ||
|
|
||
| **Scope:** Two focused changes: one portal UI handler and one Trex core compatibility-route authorization change. The Trex route is not implemented in this repository: `services/trex/Dockerfile.v2` explicitly states that the D2E compatibility layer is supplied by the pinned `ghcr.io/ohdsi/trexsql` base image. Updating its guard therefore requires the upstream Trex source/package version that owns the route, followed by an image-reference update in this repository. |
Comment on lines
+1
to
+14
| import { Body, Controller, Middleware, Post } from "@danet/core"; | ||
| import { RequestContextMiddleware } from "../common/request-context.middleware.ts"; | ||
| import { AuditService } from "./audit.service.ts"; | ||
|
|
||
| @Middleware(RequestContextMiddleware) | ||
| @Controller("system-portal/audit") | ||
| export class AuditController { | ||
| constructor(private readonly auditService: AuditService) {} | ||
|
|
||
| @Post("log") | ||
| logDisclaimerResponse(@Body() body: { response: string }) { | ||
| this.auditService.logDisclaimerResponse(body.response); | ||
| } | ||
| } |
Resolve the user from thirdPartyToken.oid, falling back to GATEWAY__IDP_SUBJECT_PROP, the oid claim, then the Logto subject.
5 tasks
hengxian-jiang
approved these changes
Aug 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
$## Summary\n\n- Bug 1: Logout button was calling
/trex/logbefore reaching the OIDC end-session flow on Azure IDP setups. Logout now always navigates immediately through the existing/logoutroute, which invokes the configured OIDC session-end flow.\n- Bug 2: BothhandleAcceptandhandleLogoutawaitedPOST /trex/logwithout error handling, so a 403 could silently block logout. The portal now uses the D2E-ownedPOST /system-portal/audit/logendpoint and treats audit submission as best-effort.\n- AddedPOST /system-portal/audit/logto the portal user function, protected byportal.audit.log, which is granted toTENANT_VIEWER.\n- Updated the UI to call the new endpoint. Both Accept and Logout catch audit errors so a failed audit request never blocks the user action.\n\n## Verification\n\n-deno test --config plugins/functions/portal/deno.json plugins/functions/portal/src/audit/audit.service.test.ts\n- Authorization manifest JSON validation andgit diff --check\n\nPortal UI test execution remains blocked in this workspace becausereact-scriptsis not installed.