fix: prevent rootpath duplication in OIDC redirect URLs, fixes #21857 #20790 #12195 #22254

Diasker · 2025-03-08T01:51:50Z

Fix rootPath duplication in OIDC callback URLs

Fixes #21857
Fixes #20790
Fixes #12195

Checklist

Problem Description

When ArgoCD is configured with a rootPath, there is an issue where the rootPath appears duplicated in OIDC authentication callback URLs. For example, when rootPath is set to /argocd, the callback URL becomes https://example.com/argocd/argocd/..., causing authentication to fail and preventing users from successfully logging in.

Note that the /argocd segment appears twice in the URL. This causes the authentication callback to fail, preventing users from successfully logging in.

This issue has been reported in multiple tickets:

Root Cause Analysis

After reviewing the code, I identified two functions in server.go that contribute to this issue:

withRootPath function: When rootPath is empty, it still creates an http.ServeMux and processes the path, instead of directly returning the original handler.
newRedirectServer function: When constructing the server address and handling redirect URLs, it doesn't properly handle the empty rootPath case, which can lead to rootPath being added multiple times in certain scenarios.

Fix Implementation

I've made the following changes to server.go:

Modified the withRootPath function to add handling for empty rootPath.
Modified the newRedirectServer function to improve address construction logic.
These changes ensure that rootPath is handled correctly in URL processing, preventing the path duplication issue.

Testing Validation

I've validated the fix through the following methods:

Unit Testing:
- I've added unit tests in server/rootpath_test.go to verify the fix.
- Tests cover scenarios with and without rootPath, as well as cases with rootPath duplication.
- All tests pass, confirming the effectiveness of the fix.
Here are the test results:
```
=== RUN   TestWithRootPathEmptyRootPath
--- PASS: TestWithRootPathEmptyRootPath (0.00s)
=== RUN   TestWithRootPathNonEmptyRootPath
--- PASS: TestWithRootPathNonEmptyRootPath (0.00s)
=== RUN   TestNewRedirectServerEmptyRootPath
--- PASS: TestNewRedirectServerEmptyRootPath (0.00s)
=== RUN   TestNewRedirectServerNonEmptyRootPath
--- PASS: TestNewRedirectServerNonEmptyRootPath (0.00s)
=== RUN   TestNewRedirectServerRootPathDuplication
--- PASS: TestNewRedirectServerRootPathDuplication (0.00s)
PASS
```
The key test TestNewRedirectServerRootPathDuplication verifies that when a request path already contains the rootPath (e.g., /argocd/applications), the redirect URL correctly becomes https://example.com:8080/argocd/applications instead of the duplicated https://example.com:8080/argocd/argocd/applications.
Building ArgoCD Image:
- Successfully built an ArgoCD image with the fix (available at docker.io/fuckery/argocd:rootpath-fix)
- The build process completed without any issues
Deployment Testing:
- Deployed an ArgoCD instance with the fix, configured with rootPath: /argocd
- Verified that the UI is accessible
- Verified that the OIDC login flow works correctly
- Verified that the redirect URL after login does not contain duplicated rootPath
Comparison Testing:
- Compared behavior before and after the fix
- Before: Redirect URL contained duplicated rootPath (/argocd/argocd/)
- After: Redirect URL is correct, without duplicated rootPath (/argocd/)

Impact Scope

This fix only affects ArgoCD instances configured with a rootPath parameter, particularly when using OIDC authentication. The fix does not impact other functionality or configurations.

Summary

This fix resolves the rootPath duplication issue in ArgoCD, allowing ArgoCD instances configured with rootPath to properly use OIDC authentication. The fix is minimal and focused, and has been validated through actual deployment testing.

Signed-off-by: Diasker <[email protected]>

bunnyshell · 2025-03-08T01:51:56Z

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

🚀 /bns:deploy to deploy the environment

codecov · 2025-03-08T02:29:54Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (master@75cb7fc). Learn more about missing BASE report.
⚠️ Report is 702 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##             master   #22254   +/-   ##
=========================================
  Coverage          ?   55.82%           
=========================================
  Files             ?      342           
  Lines             ?    57231           
  Branches          ?        0           
=========================================
  Hits              ?    31952           
  Misses            ?    22636           
  Partials          ?     2643

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

andrii-korotkov-verkada · 2025-03-09T20:27:10Z

server/server.go

+	if rootPath == "" {
+		addr = fmt.Sprintf("localhost:%d", port)
+	} else {
+		addr = fmt.Sprintf("localhost:%d/%s", port, strings.TrimRight(strings.TrimLeft(rootPath, "/"), "/"))


Just use strings.Trim(...)

andrii-korotkov-verkada · 2025-03-09T20:27:25Z

server/server.go

 			target := "https://" + req.Host
 			if rootPath != "" {
-				target += "/" + strings.TrimRight(strings.TrimLeft(rootPath, "/"), "/")
+				root := strings.TrimRight(strings.TrimLeft(rootPath, "/"), "/")


strings.Trim(...)

andrii-korotkov-verkada · 2025-03-09T20:28:37Z

server/server.go

+				// Check if the request path already contains rootPath
+				// If so, remove rootPath from the request path
+				prefix := "/" + root
+				req.URL.Path = strings.TrimPrefix(req.URL.Path, prefix)


Maybe modify how target is updated instead? req.URL.Path modification may bring req in the inconsistent state.

Thank you for your review and suggestions! I have made changes based on your feedback!

…truction Signed-off-by: Diasker <[email protected]>

Diasker · 2025-04-10T14:17:23Z

Hi, I have same problem. I am using helmchart deploy

Deployed Chart version: 7.8.22 Deployed App version: v2.14.9

in 2.14.9 version, Since the login phase is ok, after that the root path is duplicated.

server.basehref: /argocd server.rootpath: '/argocd'

but it's redirecting after login https://localhost/argocd/argocd/applications

Hi there. As you see now, the reviewers might be busy and haven’t had time to merge the fix for this yet.

In the meantime, you can try applying the changes from my PR (if applicable) directly to your setup, or you can test the pre-built image I’ve shared: fuckery/argocd:rootpath-fix. This image includes the fix for the root path duplication issue.

agaudreault

LGTM

MoShaaban13 · 2025-05-08T09:59:43Z

Hello is this applied on the latest docker image or just on this fuckery/argocd ?

Diasker · 2025-05-09T11:11:04Z

Hello is this applied on the latest docker image or just on this fuckery/argocd ?

Sorry, I'm not sure about that. But I think they haven't built the latest image just now after merging my PR.

…oj#21857 argoproj#20790 argoproj#12195 (argoproj#22254) Signed-off-by: Diasker <[email protected]>

…oj#21857 argoproj#20790 argoproj#12195 (argoproj#22254) Signed-off-by: Diasker <[email protected]> Signed-off-by: Kanika Rana <[email protected]>

…oj#21857 argoproj#20790 argoproj#12195 (argoproj#22254) Signed-off-by: Diasker <[email protected]> Signed-off-by: Oliver Gondža <[email protected]>

…oj#21857 argoproj#20790 argoproj#12195 (argoproj#22254) Signed-off-by: Diasker <[email protected]>

PandelisZ · 2025-08-18T18:52:43Z

For those also browsing:

Key highlights
nginx.ingress.kubernetes.io/backend-protocol: "HTTPS"

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: argocd-server-ingress
  namespace: argocd
  annotations:
    cert-manager.io/cluster-issuer: letsencrypt-prod
    nginx.ingress.kubernetes.io/ssl-passthrough: "true"
    # If you encounter a redirect loop or are getting a 307 response code
    # then you need to force the nginx ingress to connect to the backend using HTTPS.
    #
    nginx.ingress.kubernetes.io/backend-protocol: "HTTPS"
spec:
  ingressClassName: nginx
  rules:
  - host: argocd.example.com
    http:
      paths:
      - path: /
        pathType: Prefix
        backend:
          service:
            name: argocd-server
            port:
              name: https
  tls:
  - hosts:
    - argocd.example.com
    secretName: argocd-server-tls # as expected by argocd-server

fix: prevent rootPath duplication in redirect URLs

4c593df

Signed-off-by: Diasker <[email protected]>

Diasker requested a review from a team as a code owner March 8, 2025 01:51

Diasker changed the title ~~fix: prevent rootpath duplication in OIDC redirect URLs, fixes #21857 #20790 #12195 #22238~~ fix: prevent rootpath duplication in OIDC redirect URLs, fixes #21857 #20790 #12195 Mar 8, 2025

andrii-korotkov-verkada reviewed Mar 9, 2025

View reviewed changes

Address review feedback: use strings.Trim and improve target URL cons…

51e7eea

…truction Signed-off-by: Diasker <[email protected]>

andrii-korotkov-verkada approved these changes Mar 10, 2025

View reviewed changes

andrii-korotkov-verkada added the ready-for-review An approver should give a final review and merge the PR label Mar 10, 2025

github-project-automation bot added this to Argo CD Review Mar 10, 2025

github-project-automation bot moved this to Ready for final review in Argo CD Review Mar 10, 2025

Merge branch 'master' into fix-rootpath-duplication

70743f3

Diasker requested a review from andrii-korotkov-verkada April 4, 2025 14:34

agaudreault approved these changes May 7, 2025

View reviewed changes

agaudreault merged commit d4a20e4 into argoproj:master May 7, 2025
27 checks passed

LyhengTep pushed a commit to LyhengTep/argo-cd that referenced this pull request May 10, 2025

fix: prevent rootpath duplication in OIDC redirect URLs, fixes argopr…

8528ac9

…oj#21857 argoproj#20790 argoproj#12195 (argoproj#22254) Signed-off-by: Diasker <[email protected]>

tylerrosnett pushed a commit to StateFarmIns/argo-cd that referenced this pull request May 27, 2025

fix: prevent rootpath duplication in OIDC redirect URLs, fixes argopr…

e219fa4

…oj#21857 argoproj#20790 argoproj#12195 (argoproj#22254) Signed-off-by: Diasker <[email protected]>

chansuke pushed a commit to chansuke/argo-cd that referenced this pull request Jun 4, 2025

fix: prevent rootpath duplication in OIDC redirect URLs, fixes argopr…

47c5762

…oj#21857 argoproj#20790 argoproj#12195 (argoproj#22254) Signed-off-by: Diasker <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: prevent rootpath duplication in OIDC redirect URLs, fixes #21857 #20790 #12195 #22254

fix: prevent rootpath duplication in OIDC redirect URLs, fixes #21857 #20790 #12195 #22254

Uh oh!

Diasker commented Mar 8, 2025

Uh oh!

bunnyshell bot commented Mar 8, 2025 •

edited

Loading

Uh oh!

codecov bot commented Mar 8, 2025 •

edited

Loading

Uh oh!

andrii-korotkov-verkada Mar 9, 2025

Uh oh!

andrii-korotkov-verkada Mar 9, 2025

Uh oh!

andrii-korotkov-verkada Mar 9, 2025

Uh oh!

Diasker Mar 10, 2025

Uh oh!

Diasker commented Apr 10, 2025

Uh oh!

agaudreault left a comment

Uh oh!

Uh oh!

MoShaaban13 commented May 8, 2025

Uh oh!

Diasker commented May 9, 2025

Uh oh!

PandelisZ commented Aug 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fix: prevent rootpath duplication in OIDC redirect URLs, fixes #21857 #20790 #12195 #22254

fix: prevent rootpath duplication in OIDC redirect URLs, fixes #21857 #20790 #12195 #22254

Uh oh!

Conversation

Diasker commented Mar 8, 2025

Fix rootPath duplication in OIDC callback URLs

Checklist

Problem Description

Root Cause Analysis

Fix Implementation

Testing Validation

Impact Scope

Summary

Uh oh!

bunnyshell bot commented Mar 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ Preview Environment deleted from Bunnyshell

Uh oh!

codecov bot commented Mar 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

andrii-korotkov-verkada Mar 9, 2025

Choose a reason for hiding this comment

Uh oh!

andrii-korotkov-verkada Mar 9, 2025

Choose a reason for hiding this comment

Uh oh!

andrii-korotkov-verkada Mar 9, 2025

Choose a reason for hiding this comment

Uh oh!

Diasker Mar 10, 2025

Choose a reason for hiding this comment

Uh oh!

Diasker commented Apr 10, 2025

Uh oh!

agaudreault left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MoShaaban13 commented May 8, 2025

Uh oh!

Diasker commented May 9, 2025

Uh oh!

PandelisZ commented Aug 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bunnyshell bot commented Mar 8, 2025 •

edited

Loading

codecov bot commented Mar 8, 2025 •

edited

Loading