Conversation
|
✅ Trivy gate: no Critical/High issues. Trivy IaC (Terraform) Summary
Findings (top 50)
|
|
✅ Trivy gate: no Critical/High vulnerabilities. Trivy Image Scan SummaryImage: 900119715266.dkr.ecr.eu-west-2.amazonaws.com/whoami:feature-gpcapim-0000-userinfo-testing
Findings (top 50)
|
davidhamill1-nhs
force-pushed
the
feature/GPCAPIM-0000_userinfo_testing
branch
from
62e3b39 to
2be9923
Compare
|
Deployment Complete
|
…) to the backend, enabling us to understand who is calling us and pass that on to the GP provider system
DWolfsNHS
force-pushed
the
feature/GPCAPIM-0000_userinfo_testing
branch
from
2be9923 to
4fdb19f
Compare
There was a problem hiding this comment.
Pull request overview
Adds initial plumbing to surface caller identity information to the backend, as a POC toward accessing CIS2 userinfo and passing identity context through the proxy.
Changes:
- Configures Proxygen/APIM template to forward CIS2-related identity fields (including
id-token) to the target. - Adds request header logging to the
/patient/$gpc.getstructuredrecordFlask handler for debugging.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
proxygen/x-nhsd-apim.template.yaml |
Adds target-identity entries to forward CIS2 identity data (and id-token) to the backend. |
gateway-api/src/gateway_api/app.py |
Logs incoming request headers for the structured record endpoint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @app.route("/patient/$gpc.getstructuredrecord", methods=["POST"]) | ||
| def get_structured_record() -> Response: | ||
| try: | ||
| print(f"Headers: {request.headers}", flush=True) |
There was a problem hiding this comment.
Printing the full incoming request headers will likely log sensitive values (e.g., Authorization, id_token, correlation IDs) into application logs, which is a security/privacy risk and can violate the repo’s “no PII/sensitive data” requirement. Please remove this, or replace with allowlisted/redacted logging (e.g., only safe headers, explicitly redact auth/token headers) and ensure it’s gated behind a debug/non-prod flag if needed for the POC.
| print(f"Headers: {request.headers}", flush=True) | |
| debug_log_headers = os.getenv("DEBUG_LOG_HEADERS", "false").lower() == "true" | |
| if debug_log_headers: | |
| # Only log a limited set of non-sensitive headers for debugging purposes. | |
| safe_header_names = {"Content-Type", "Accept", "User-Agent"} | |
| safe_headers = { | |
| name: value | |
| for name, value in request.headers.items() | |
| if name in safe_header_names | |
| } | |
| print(f"Safe headers: {safe_headers}", flush=True) |
| - name: cis2-uuid | ||
| - name: cis2-urid | ||
| - name: cis2-acr | ||
| - name: id-token |
There was a problem hiding this comment.
Forwarding id-token via target-identity increases the risk of propagating a sensitive bearer token to downstream services and into logs/telemetry. If the backend only needs specific user identifiers, prefer passing just those minimal claims (e.g., cis2-uuid/urid/acr) and avoid forwarding the full token unless there’s a clear, documented requirement.
| - name: id-token |
| target-identity: | ||
| - name: cis2-uuid | ||
| - name: cis2-urid | ||
| - name: cis2-acr | ||
| - name: id-token |
There was a problem hiding this comment.
target-identity is being added to the Proxygen x-nhsd-apim template, but proxygen/README.md currently lists what the extension includes and doesn’t mention this new field. Please update the documentation accordingly so future maintainers know why these identity headers are configured and what each one is used for.
|
…) to the backend, enabling us to understand who is calling us and pass that on to the GP provider system
Description
Context
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.