Adding github actions for oidc-ui#2000
Open
zesu22 wants to merge 4 commits into
Open
Conversation
Signed-off-by: Zeeshan Mehboob <zeeshan.mehboob@infosys.com>
Signed-off-by: Zeeshan Mehboob <zeeshan.mehboob@infosys.com>
WalkthroughAdds CI jobs for building, analyzing, and containerizing the OIDC UI; changes nginx to proxy directly to esignet upstream for API and well-known endpoints, adds CORS and CSP/Referrer headers, updates logging, and mirrors those changes in the Helm ConfigMap template. ChangesOIDC UI Infrastructure Setup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/push-trigger.yml (2)
104-123:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate OIDC UI Docker build on successful npm build.
build_dockers_oidc_uicurrently has noneeds, so it can run even ifbuild-oidc-uifails, which can publish an image from an unvalidated state.Suggested fix
build_dockers_oidc_ui: + needs: build-oidc-ui strategy: matrix: include:As per coding guidelines, external-call/pipeline sequencing hazards that can propagate bad artifacts should be fixed before release.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/push-trigger.yml around lines 104 - 123, The build_dockers_oidc_ui job currently runs without depending on the npm build step; update the job definition for build_dockers_oidc_ui to add a needs dependency on the job that performs the OIDC UI npm build (referencing the build-oidc-ui job name) so the docker build only runs after build-oidc-ui succeeds; modify the build_dockers_oidc_ui job to include needs: [build-oidc-ui] (or the actual job id used for the OIDC UI npm build) so failures in the npm build will block the docker publish.
20-24:⚠️ Potential issue | 🟠 MajorBranch negation is overridden by later
release*include
push.branchesexcludesrelease-branchvia!release-branchbut then re-includes it becauserelease-branchalso matches the laterrelease*pattern.Suggested fix
push: branches: - - "!release-branch" - master - develop - develop-go - release* + - "!release-branch"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/push-trigger.yml around lines 20 - 24, The "!release-branch" negation is being overridden by the later "release*" include in push.branches; move the negation so it appears after the broad include (or narrow the include so it doesn't match the specific branch) to ensure the exclusion takes effect. Update the push.branches list so either "release*" appears before "!release-branch" (or replace "release*" with a pattern that doesn't match the exact branch name), keeping the exact strings "!release-branch" and "release*" (or the new narrower pattern) so the exclusion on the branch named release-branch is honored.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/push-trigger.yml:
- Around line 81-90: The build-oidc-ui job (job id build-oidc-ui that uses
mosip/kattu/.github/workflows/npm-build.yml@develop) is relying on default
GitHub Actions token permissions; add an explicit least-privilege permissions
block to the job to restrict scopes to only what the reusable workflow actually
needs (for example: contents: read, actions: write, checks: read — only include
the exact scopes required by the npm-build.yml reusable workflow and any
post-build steps like Slack notifications), and if the reusable workflow
requires extra scopes grant them at this job level only rather than leaving
defaults broad.
- Line 82: The workflow uses mutable branch refs for reusable workflows; replace
the three occurrences of branch refs with their exact commit SHAs — update the
strings "mosip/kattu/.github/workflows/npm-build.yml@develop",
"mosip/kattu/.github/workflows/npm-sonar-analysis.yml@develop", and
"mosip/kattu/.github/workflows/docker-build.yml@master-java21" to use the full
commit SHA for the referenced commit in the mosip/kattu repo so the reusable
workflow calls are pinned to immutable commits.
In `@oidc-ui/nginx/nginx.conf`:
- Around line 52-67: The location block for /.well-known/jwks.json is wrong and
incomplete: confirm and update proxy_pass to the correct upstream path used in
Helm (e.g., /v1/esignet/oauth/.well-known/jwks.json) or adjust the location to
match the upstream; add an explicit OPTIONS handler inside the same location (or
a separate location = /.well-known/jwks.json if needed) that returns 204 with
the same CORS headers instead of proxying preflight to upstream (update the
existing add_header lines: Access-Control-Allow-Origin, -Allow-Methods,
-Allow-Headers to be included in the OPTIONS response), and change the types
mapping so .json and the JWKS response use application/json (remove or replace
the current mapping that sets json to text/plain) to comply with RFC 7517/8414;
refer to the location block, proxy_pass directive, add_header lines, and the
types block when making these edits.
---
Outside diff comments:
In @.github/workflows/push-trigger.yml:
- Around line 104-123: The build_dockers_oidc_ui job currently runs without
depending on the npm build step; update the job definition for
build_dockers_oidc_ui to add a needs dependency on the job that performs the
OIDC UI npm build (referencing the build-oidc-ui job name) so the docker build
only runs after build-oidc-ui succeeds; modify the build_dockers_oidc_ui job to
include needs: [build-oidc-ui] (or the actual job id used for the OIDC UI npm
build) so failures in the npm build will block the docker publish.
- Around line 20-24: The "!release-branch" negation is being overridden by the
later "release*" include in push.branches; move the negation so it appears after
the broad include (or narrow the include so it doesn't match the specific
branch) to ensure the exclusion takes effect. Update the push.branches list so
either "release*" appears before "!release-branch" (or replace "release*" with a
pattern that doesn't match the exact branch name), keeping the exact strings
"!release-branch" and "release*" (or the new narrower pattern) so the exclusion
on the branch named release-branch is honored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 92931941-44d2-427b-bd46-e02c0c40d08b
📒 Files selected for processing (2)
.github/workflows/push-trigger.ymloidc-ui/nginx/nginx.conf
Comment on lines
+81
to
+90
| build-oidc-ui: | ||
| uses: mosip/kattu/.github/workflows/npm-build.yml@develop | ||
| with: | ||
| SERVICE_LOCATION: oidc-ui | ||
| BUILD_ARTIFACT: oidc | ||
| NPM_BUILD_TYPE: BOB | ||
| NODE_VERSION: "18" | ||
| ZIP_DIR: build | ||
| secrets: | ||
| SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK }} |
There was a problem hiding this comment.
Set explicit least-privilege permissions for new OIDC UI jobs.
The new jobs rely on default token permissions (static warning at Lines 81-90 and 92-102), which is broader than necessary.
Suggested fix
on:
release:
types: [published]
+permissions:
+ contents: readIf any called reusable workflow needs additional scopes, grant them only at the specific job level.
As per coding guidelines, security posture gaps that weaken guarantees should be addressed before release.
Also applies to: 92-102
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 81-90: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[error] 82-82: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/push-trigger.yml around lines 81 - 90, The build-oidc-ui
job (job id build-oidc-ui that uses
mosip/kattu/.github/workflows/npm-build.yml@develop) is relying on default
GitHub Actions token permissions; add an explicit least-privilege permissions
block to the job to restrict scopes to only what the reusable workflow actually
needs (for example: contents: read, actions: write, checks: read — only include
the exact scopes required by the npm-build.yml reusable workflow and any
post-build steps like Slack notifications), and if the reusable workflow
requires extra scopes grant them at this job level only rather than leaving
defaults broad.
Source: Linters/SAST tools
| SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK }} | ||
|
|
||
| build-oidc-ui: | ||
| uses: mosip/kattu/.github/workflows/npm-build.yml@develop |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la .github/workflows || true
sed -n '60,140p' .github/workflows/push-trigger.ymlRepository: mosip/esignet
Length of output: 2772
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la .github/workflows || true
sed -n '60,140p' .github/workflows/push-trigger.ymlRepository: mosip/esignet
Length of output: 2772
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba .github/workflows/push-trigger.yml | sed -n '70,130p'Repository: mosip/esignet
Length of output: 100
🏁 Script executed:
#!/bin/bash
set -euo pipefail
awk 'NR>=70 && NR<=130 {print NR ":" $0}' .github/workflows/push-trigger.ymlRepository: mosip/esignet
Length of output: 2092
Pin reusable workflow references to immutable commit SHAs
.github/workflows/push-trigger.yml uses branch refs for reusable workflows that run with privileged CI:
- Line 82:
mosip/kattu/.github/workflows/npm-build.yml@develop - Line 95:
mosip/kattu/.github/workflows/npm-sonar-analysis.yml@develop - Line 113:
mosip/kattu/.github/workflows/docker-build.yml@master-java21
Replace each @<branch> with the exact full commit SHA of the referenced reusable workflow to prevent supply-chain drift.
Suggested fix
- uses: mosip/kattu/.github/workflows/npm-build.yml@develop
+ uses: mosip/kattu/.github/workflows/npm-build.yml@<commit-sha>
- uses: mosip/kattu/.github/workflows/npm-sonar-analysis.yml@develop
+ uses: mosip/kattu/.github/workflows/npm-sonar-analysis.yml@<commit-sha>
- uses: mosip/kattu/.github/workflows/docker-build.yml@master-java21
+ uses: mosip/kattu/.github/workflows/docker-build.yml@<commit-sha>🧰 Tools
🪛 zizmor (1.25.2)
[error] 82-82: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/push-trigger.yml at line 82, The workflow uses mutable
branch refs for reusable workflows; replace the three occurrences of branch refs
with their exact commit SHAs — update the strings
"mosip/kattu/.github/workflows/npm-build.yml@develop",
"mosip/kattu/.github/workflows/npm-sonar-analysis.yml@develop", and
"mosip/kattu/.github/workflows/docker-build.yml@master-java21" to use the full
commit SHA for the referenced commit in the mosip/kattu repo so the reusable
workflow calls are pinned to immutable commits.
Source: Linters/SAST tools
Comment on lines
52
to
+67
| location /.well-known/jwks.json { | ||
| proxy_pass http://esignet.esignet/v1/esignet/oauth/.well-known/jwks.json; | ||
| proxy_pass http://esignet.esignet/.well-known/jwks.json; | ||
| proxy_redirect off; | ||
| proxy_set_header Host $host; | ||
| proxy_set_header X-Real-IP $remote_addr; | ||
| proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; | ||
| proxy_set_header X-Forwarded-Host $server_name; | ||
| add_header Content-Security-Policy "default-src 'none'" always; | ||
| add_header Referrer-Policy "no-referrer" always; | ||
| proxy_connect_timeout 10s; | ||
| proxy_send_timeout 30s; | ||
| proxy_read_timeout 30s; | ||
| add_header 'Access-Control-Allow-Origin' $cors_allowed_origin always; | ||
| add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always; | ||
| add_header 'Access-Control-Allow-Headers' 'DNT,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Range,Authorization' always; | ||
| types { | ||
| text/plain log cer json txt; | ||
| } | ||
| } |
There was a problem hiding this comment.
Multiple issues with well-known endpoint configuration.
-
Path mismatch: Proxying to
/.well-known/jwks.jsondiffers from Helm config which uses/v1/esignet/oauth/.well-known/jwks.json. Verify the upstream service exposes this endpoint at the root path. -
Missing OPTIONS preflight handling: CORS headers are added but preflight OPTIONS requests will be proxied to upstream instead of being handled by nginx. Add explicit OPTIONS handling:
-
Incorrect MIME type: The
typesblock sets.jsontotext/plain. JWKS and OIDC discovery documents should returnapplication/jsonper RFC 7517 and RFC 8414.
Proposed fix for OPTIONS handling and MIME type
location /.well-known/jwks.json {
+ if ($request_method = 'OPTIONS') {
+ add_header 'Access-Control-Allow-Origin' $cors_allowed_origin;
+ add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS';
+ add_header 'Access-Control-Allow-Headers' 'DNT,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Range,Authorization';
+ add_header 'Access-Control-Max-Age' 1728000;
+ add_header 'Content-Type' 'text/plain; charset=utf-8';
+ add_header 'Content-Length' 0;
+ return 204;
+ }
proxy_pass http://esignet.esignet/.well-known/jwks.json;
...
types {
- text/plain log cer json txt;
+ application/json json;
+ text/plain log cer txt;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| location /.well-known/jwks.json { | |
| proxy_pass http://esignet.esignet/v1/esignet/oauth/.well-known/jwks.json; | |
| proxy_pass http://esignet.esignet/.well-known/jwks.json; | |
| proxy_redirect off; | |
| proxy_set_header Host $host; | |
| proxy_set_header X-Real-IP $remote_addr; | |
| proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; | |
| proxy_set_header X-Forwarded-Host $server_name; | |
| add_header Content-Security-Policy "default-src 'none'" always; | |
| add_header Referrer-Policy "no-referrer" always; | |
| proxy_connect_timeout 10s; | |
| proxy_send_timeout 30s; | |
| proxy_read_timeout 30s; | |
| add_header 'Access-Control-Allow-Origin' $cors_allowed_origin always; | |
| add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always; | |
| add_header 'Access-Control-Allow-Headers' 'DNT,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Range,Authorization' always; | |
| types { | |
| text/plain log cer json txt; | |
| } | |
| } | |
| location /.well-known/jwks.json { | |
| if ($request_method = 'OPTIONS') { | |
| add_header 'Access-Control-Allow-Origin' $cors_allowed_origin; | |
| add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS'; | |
| add_header 'Access-Control-Allow-Headers' 'DNT,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Range,Authorization'; | |
| add_header 'Access-Control-Max-Age' 1728000; | |
| add_header 'Content-Type' 'text/plain; charset=utf-8'; | |
| add_header 'Content-Length' 0; | |
| return 204; | |
| } | |
| proxy_pass http://esignet.esignet/.well-known/jwks.json; | |
| proxy_redirect off; | |
| proxy_set_header Host $host; | |
| proxy_set_header X-Real-IP $remote_addr; | |
| proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; | |
| proxy_set_header X-Forwarded-Host $server_name; | |
| add_header Content-Security-Policy "default-src 'none'" always; | |
| add_header Referrer-Policy "no-referrer" always; | |
| add_header 'Access-Control-Allow-Origin' $cors_allowed_origin always; | |
| add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always; | |
| add_header 'Access-Control-Allow-Headers' 'DNT,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Range,Authorization' always; | |
| types { | |
| application/json json; | |
| text/plain log cer txt; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/nginx/nginx.conf` around lines 52 - 67, The location block for
/.well-known/jwks.json is wrong and incomplete: confirm and update proxy_pass to
the correct upstream path used in Helm (e.g.,
/v1/esignet/oauth/.well-known/jwks.json) or adjust the location to match the
upstream; add an explicit OPTIONS handler inside the same location (or a
separate location = /.well-known/jwks.json if needed) that returns 204 with the
same CORS headers instead of proxying preflight to upstream (update the existing
add_header lines: Access-Control-Allow-Origin, -Allow-Methods, -Allow-Headers to
be included in the OPTIONS response), and change the types mapping so .json and
the JWKS response use application/json (remove or replace the current mapping
that sets json to text/plain) to comply with RFC 7517/8414; refer to the
location block, proxy_pass directive, add_header lines, and the types block when
making these edits.
Signed-off-by: Zeeshan Mehboob <zeeshan.mehboob@infosys.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@helm/oidc-ui/templates/configmap.yaml`:
- Around line 74-76: The responses that set Access-Control-Allow-Origin from the
request via $cors_allowed_origin need to also emit Vary: Origin to avoid caching
responses across origins; update each location block that calls add_header
'Access-Control-Allow-Origin' $cors_allowed_origin always (e.g., the
.well-known/* locations referencing $cors_allowed_origin) to also call
add_header 'Vary' 'Origin' always so caches and CDNs vary by the request Origin
header.
- Around line 44-45: In the nginx location blocks for "location /v1/esignet" and
"location /v1/esignet/actuator/" replace the proxy_pass that ends with a
trailing "/;" so it does not strip the location prefix (change the proxy_pass
reference to the host value without the trailing slash) and keep proxy_redirect
off; also add an add_header Vary Origin directive alongside your dynamic
Access-Control-Allow-Origin header so caches vary by Origin to avoid CORS
mix-ups. Ensure you update the proxy_pass occurrences that reference {{
.Values.oidc_ui.oidc_service_host }} and add the Vary header in the same
location blocks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2d94a261-fe38-4eaf-805e-4945d72246e9
📒 Files selected for processing (1)
helm/oidc-ui/templates/configmap.yaml
Comment on lines
+44
to
+45
| proxy_pass http://{{ .Values.oidc_ui.oidc_service_host }}/; | ||
| proxy_redirect off; |
There was a problem hiding this comment.
Preserve the /v1/esignet path in proxy_pass (remove the trailing /;)
proxy_pass http://{{ .Values.oidc_ui.oidc_service_host }}/; inside location /v1/esignet (and location /v1/esignet/actuator/) rewrites the upstream URI by stripping the matched location prefix, which can break /v1/esignet/* forwarding—use proxy_pass http://{{ .Values.oidc_ui.oidc_service_host }}; instead. Also add Vary: Origin with your dynamic Access-Control-Allow-Origin to avoid cache-mismatched CORS responses.
🔧 Suggested fix
location /v1/esignet {
- proxy_pass http://{{ .Values.oidc_ui.oidc_service_host }}/;
+ proxy_pass http://{{ .Values.oidc_ui.oidc_service_host }};
proxy_redirect off;
proxy_set_header Host $host;
proxy_set_header X-Real-IP $remote_addr;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Host $server_name;
add_header Content-Security-Policy "default-src 'none'" always;
add_header Referrer-Policy "no-referrer" always;
}
location /v1/esignet/actuator/ {
- proxy_pass http://{{ .Values.oidc_ui.oidc_service_host }}/;
+ proxy_pass http://{{ .Values.oidc_ui.oidc_service_host }};
proxy_redirect off;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@helm/oidc-ui/templates/configmap.yaml` around lines 44 - 45, In the nginx
location blocks for "location /v1/esignet" and "location /v1/esignet/actuator/"
replace the proxy_pass that ends with a trailing "/;" so it does not strip the
location prefix (change the proxy_pass reference to the host value without the
trailing slash) and keep proxy_redirect off; also add an add_header Vary Origin
directive alongside your dynamic Access-Control-Allow-Origin header so caches
vary by Origin to avoid CORS mix-ups. Ensure you update the proxy_pass
occurrences that reference {{ .Values.oidc_ui.oidc_service_host }} and add the
Vary header in the same location blocks.
Signed-off-by: Zeeshan Mehboob <zeeshan.mehboob@infosys.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
helm/oidc-ui/templates/configmap.yaml (2)
86-91:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame OPTIONS/MIME issues in remaining
.well-knownlocations.The
openid-configuration,oauth-authorization-server, andopenid-credential-issuerlocations all have the same missing OPTIONS handler andjson → text/plainMIME mapping issues described above.Also applies to: 103-108, 120-125
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helm/oidc-ui/templates/configmap.yaml` around lines 86 - 91, For each of the remaining .well-known location blocks (the locations serving openid-configuration, oauth-authorization-server, and openid-credential-issuer) add the same OPTIONS preflight handling used elsewhere (respond to OPTIONS with appropriate CORS headers and a 204/empty response) and fix the types block so JSON responses are mapped to application/json (not text/plain); update the location blocks referenced by their path names (e.g. /.well-known/openid-configuration, /.well-known/oauth-authorization-server, /.well-known/openid-credential-issuer) to include the OPTIONS handler and correct MIME mapping for json.
69-74:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCORS headers added but OPTIONS preflight not handled; JSON served as
text/plain.Two issues with the CORS implementation:
No OPTIONS preflight response: Browsers send OPTIONS before cross-origin GET with custom headers. Without explicit handling, these requests proxy to upstream which may not respond correctly, causing CORS failures.
Wrong Content-Type for JSON: Line 73 maps
.jsontotext/plain. JWKS endpoints must returnapplication/jsonper RFC 7517 for proper client parsing.🔧 Proposed fix: handle OPTIONS and fix MIME type
location /.well-known/jwks.json { + if ($request_method = 'OPTIONS') { + add_header 'Access-Control-Allow-Origin' '*' always; + add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always; + add_header 'Access-Control-Allow-Headers' 'DNT,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Range,Authorization' always; + add_header 'Access-Control-Max-Age' 1728000; + add_header 'Content-Length' 0; + return 204; + } proxy_pass http://{{ .Values.oidc_ui.oidc_service_host }}/v1/esignet/oauth/.well-known/jwks.json; ... add_header 'Access-Control-Allow-Origin' '*' always; add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always; add_header 'Access-Control-Allow-Headers' 'DNT,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Range,Authorization' always; types { - text/plain log cer json txt; + application/json json; + text/plain log cer txt; } }Apply the same pattern to
/.well-known/openid-configuration,/.well-known/oauth-authorization-server, and/.well-known/openid-credential-issuerlocations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helm/oidc-ui/templates/configmap.yaml` around lines 69 - 74, Update the nginx config in the ConfigMap so CORS preflight and correct MIME for JSON are handled: add an explicit handling block for OPTIONS requests (return 204 with the same add_header directives) inside the same location(s) where add_header 'Access-Control-Allow-Origin' ... is set (e.g., the locations serving /.well-known/openid-configuration, /.well-known/oauth-authorization-server, /.well-known/openid-credential-issuer), and change the types mapping so `.json` is mapped to `application/json` instead of `text/plain`; ensure the OPTIONS handler includes the Access-Control-Allow-Methods and Access-Control-Allow-Headers headers and does not proxy to upstream.oidc-ui/nginx/nginx.conf (1)
25-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winProxy path stripping may break upstream routing.
location /v1/esignet/withproxy_pass http://esignet.esignet/(trailing slash) causes nginx to strip the/v1/esignet/prefix before forwarding. A request to/v1/esignet/oauth/tokenbecomes/oauth/tokenupstream. Verify this is intentional—if the upstream expects the full/v1/esignet/...path, remove the trailing slash fromproxy_pass.🔧 If upstream expects the full path
location /v1/esignet/ { - proxy_pass http://esignet.esignet/; + proxy_pass http://esignet.esignet; proxy_redirect off;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@oidc-ui/nginx/nginx.conf` around lines 25 - 34, The proxy currently defined in the nginx location block "location /v1/esignet/" uses "proxy_pass http://esignet.esignet/" which strips the /v1/esignet/ prefix before forwarding; if the upstream service (esignet) expects the full /v1/esignet/... path, update the proxy_pass in that location to remove the trailing slash so it preserves the original request URI (i.e., change proxy_pass to http://esignet.esignet), and then test requests like /v1/esignet/oauth/token to confirm the upstream receives the full path; keep the location /v1/esignet/ and existing headers (proxy_set_header Host, X-Real-IP, X-Forwarded-For, X-Forwarded-Host) as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@helm/oidc-ui/templates/configmap.yaml`:
- Around line 86-91: For each of the remaining .well-known location blocks (the
locations serving openid-configuration, oauth-authorization-server, and
openid-credential-issuer) add the same OPTIONS preflight handling used elsewhere
(respond to OPTIONS with appropriate CORS headers and a 204/empty response) and
fix the types block so JSON responses are mapped to application/json (not
text/plain); update the location blocks referenced by their path names (e.g.
/.well-known/openid-configuration, /.well-known/oauth-authorization-server,
/.well-known/openid-credential-issuer) to include the OPTIONS handler and
correct MIME mapping for json.
- Around line 69-74: Update the nginx config in the ConfigMap so CORS preflight
and correct MIME for JSON are handled: add an explicit handling block for
OPTIONS requests (return 204 with the same add_header directives) inside the
same location(s) where add_header 'Access-Control-Allow-Origin' ... is set
(e.g., the locations serving /.well-known/openid-configuration,
/.well-known/oauth-authorization-server, /.well-known/openid-credential-issuer),
and change the types mapping so `.json` is mapped to `application/json` instead
of `text/plain`; ensure the OPTIONS handler includes the
Access-Control-Allow-Methods and Access-Control-Allow-Headers headers and does
not proxy to upstream.
In `@oidc-ui/nginx/nginx.conf`:
- Around line 25-34: The proxy currently defined in the nginx location block
"location /v1/esignet/" uses "proxy_pass http://esignet.esignet/" which strips
the /v1/esignet/ prefix before forwarding; if the upstream service (esignet)
expects the full /v1/esignet/... path, update the proxy_pass in that location to
remove the trailing slash so it preserves the original request URI (i.e., change
proxy_pass to http://esignet.esignet), and then test requests like
/v1/esignet/oauth/token to confirm the upstream receives the full path; keep the
location /v1/esignet/ and existing headers (proxy_set_header Host, X-Real-IP,
X-Forwarded-For, X-Forwarded-Host) as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d9c1cd23-41e8-4014-96c9-5028ea489a0a
📒 Files selected for processing (2)
helm/oidc-ui/templates/configmap.yamloidc-ui/nginx/nginx.conf
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
1 participant
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 by CodeRabbit
Chores
Security