Adding github actions for oidc-ui - #2000
Merged
Merged
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
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
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
Signed-off-by: Zeeshan Mehboob <zeeshan.mehboob@infosys.com>
anushasunkada
approved these changes
Jun 16, 2026
anushasunkada
approved these changes
Jun 17, 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
2 participants
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