feat: Add proxy-authorization flag to login command by onasser1 · Pull Request #5229 · akuity/kargo

onasser1 · 2025-10-16T21:51:48Z

Signed-off-by: Omar Nasser <omarnasserjr@gmail.com>

netlify · 2025-10-16T21:51:55Z

✅ Deploy Preview for docs-kargo-io ready!

Name	Link
🔨 Latest commit	`43413c0`
🔍 Latest deploy log	https://app.netlify.com/projects/docs-kargo-io/deploys/690268e185a3a600088af0ec
😎 Deploy Preview	https://deploy-preview-5229.docs.kargo.io
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

codecov · 2025-10-24T04:17:18Z

Codecov Report

❌ Patch coverage is 9.09091% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.96%. Comparing base (a877e35) to head (d290960).
⚠️ Report is 29 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/cli/cmd/login/login.go	0.00%	10 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5229      +/-   ##
==========================================
+ Coverage   55.04%   55.96%   +0.91%     
==========================================
  Files         401      404       +3     
  Lines       36017    29710    -6307     
==========================================
- Hits        19827    16628    -3199     
+ Misses      15227    12119    -3108     
  Partials      963      963

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

krancour · 2025-10-24T04:18:18Z

@onasser1 at a glance, this looks pretty good. I will review more thoroughly as soon as I am able.

From #3537:

I wonder, How would GCP IAP for example use that proxy-authorization from kargo?

I really don't know much about GCP IAP, and it's not a rabbit hole I am willing to fall into right now, but as I mentioned in #3537, the Proxy-Authorization header is formally defined in rfc7235. It's generally useful and not only an IAP thing.

If the header's verifiably being set, I think the objective has been achieved.

krancour · 2025-10-24T04:23:54Z

If the header's verifiably being set, I think the objective has been achieved.

Although, now that I've said this, I notice you're allowing this to be specified at login time and you are persisting it in Kargo's config file, but I see you're not yet incorporating this value into into the Proxy-Authorization header of the outbound requests, so it seems you may still have some work to do here.

Signed-off-by: Omar Nasser <omarnasserjr@gmail.com>

krancour · 2025-10-27T20:28:36Z

 	credential string,
+	proxyCredential string,
 	insecureTLS bool,


As the number of optional details here grows, it's probably better that we think about introducing a ClientOptions struct. The signature would change to:

func GetClient( serverAddress string, opts *ClientOptions, ) svcv1alpha1connect.KargoServiceClient

It should be tolerant of nil options.

I was going to suggest functional options here before I saw your comment.

e.g. WithCredentials, WithProxyCredentials

func GetClient( serverAddress string, opts ...ClientOption, ) svcv1alpha1connect.KargoServiceClient

Thoughts?

It's doable, of course, but I'm not sure I like it here. If you look at what the GetClient() function is configuring, it's an http.Client as well as options for the svcv1alpha1connect.KargoServiceClient. Options functions would need to pass around both of these. It's not pretty and if you wanted to tidy it up, you'd need to create some kind of options struct that gets passed around instead, and then we're more or less back to where we started. And because we're not really developing an API here, but simply trying to stop an argument list for something used internally from sprawling, I think we can skip this. It's tidy enough to bind flags directly to option struct fields and call it a day.

krancour · 2025-10-27T20:32:51Z

@onasser1 this seems to be on the right track. One small bit of feedback: https://github.com/akuity/kargo/pull/5229/files#r2466991433

@fuskovic do you mind giving this a review after the above is accounted for?

@steve-marmalade, this feature was your request. If you were willing to test it against your use case, it would be a big help.

krancour · 2025-10-28T15:50:05Z

+		return svcv1alpha1connect.NewKargoServiceClient(httpClient, serverAddress)
+	}
+
+	if credential != "" && proxyCredential == "" {


All the logic that follows this can be greatly simplified to something like this:

interceptor := &authInterceptor{} if credential == "" { interceptor.credential = credential } if proxyCredential == "" { interceptor.proxyCredential = proxyCredential } return svcv1alpha1connect.NewKargoServiceClient( httpClient, serverAddress, connect.WithClientOptions(connect.WithInterceptors(interceptor)), )

Signed-off-by: Omar Nasser <omarnasserjr@gmail.com>

onasser1 · 2025-10-29T19:25:08Z

+	Credentials      string
+	ProxyCredentials string
+	InsecureTLS      bool


With your naming ClientOptions, did you mean to create a totally new struct? I thought the same and tried to create a new ClientOptions struct but go linting has complained about the naming.

pkg/cli/client/client.go:22:6: exported: type name will be used as client.ClientOptions by other packages, and that stutters; consider calling this Options (revive) type ClientOptions struct {}

So I tried to use the existent Options struct and add the credential fields. I was afraid if this would cause a break to the dependencies, but it didn't.

Shall we go with the Options struct or I have to create a new struct with a different name than ClientOptions?
@fuskovic

I forgot an Options struct already existed in this package was all.

The way you're currently doing things (this particular bit) is good.

krancour · 2025-10-29T21:42:16Z

+	if o.UseProxyAuth {
+		for o.ProxyAuthCredentials == "" {
+			prompt := &survey.Password{
+				Message: "Proxy Authorization token",
+			}
+			if err = survey.AskOne(prompt, &o.ProxyAuthCredentials); err != nil {
+				return err
+			}
+		}
+	}


You really ought to be collecting this information before any login attempt, because if credentials for a proxy are required, they're require during login as well.

Any login function that interacts with the Kargo API server will need to use this.

adminLogin() would need to use it for its interactions with the API server's "public config" endpoint as well as the login endpoint (where password is exchanged for a bearer token).

ssoLogin() would need to use it for its interactions with the API server's "public config" endpoint and if the OIDC identity provider's issuer URL has the same hostname as the Kargo API server, it would need to be used for all interactions with the identity provider as well. This is a rather complex thing to do.

I think kubeconfigLogin() probably doesn't require any modifications since it has not interactions with the Kargo API server.

krancour · 2025-10-29T22:47:29Z

After looking at this in greater depth, it's much more complicated than I initially anticipated. Combining that with its security implications, if this gets merged, I'd like to personally be the one signing off on it. Please note, however, that this remains a low priority.

krancour · 2025-10-29T22:49:24Z

+
+func setProxyAuthHeader(header http.Header, proxyCred string) {
+	if proxyCred != "" {
+		header.Set(proxyAuthHeaderKey, fmt.Sprintf("Bearer %s", proxyCred))


I'm not sure it's a given that the credential is a bearer token.

So what value for the <auth-scheme> should we use here?

krancour · 2026-02-12T20:30:33Z

See #3537 (comment)

feat: Add proxy-authorization flag to login command

d290960

Signed-off-by: Omar Nasser <omarnasserjr@gmail.com>

onasser1 requested a review from a team as a code owner October 16, 2025 21:51

onasser1 mentioned this pull request Oct 16, 2025

Support Proxy-Authorization header in kargo CLI #3537

Closed

3 tasks

krancour self-requested a review October 17, 2025 14:34

krancour assigned onasser1 and krancour Oct 24, 2025

krancour added kind/enhancement An entirely new feature area/security Has security implications and needs to be handled with great caution area/cli Affects the kargo CLI priority/low Low commitment from maintainers; progress is likely to be community-driven labels Oct 24, 2025

krancour added this to the v1.9.0 milestone Oct 24, 2025

Provide usage to proxy authorization in outbound requests

07d7c61

Signed-off-by: Omar Nasser <omarnasserjr@gmail.com>

krancour reviewed Oct 27, 2025

View reviewed changes

krancour requested a review from fuskovic October 27, 2025 20:32

krancour assigned fuskovic and unassigned krancour Oct 27, 2025

krancour reviewed Oct 28, 2025

View reviewed changes

Apply review suggestions

43413c0

Signed-off-by: Omar Nasser <omarnasserjr@gmail.com>

onasser1 commented Oct 29, 2025

View reviewed changes

krancour reviewed Oct 29, 2025

View reviewed changes

krancour added the do not merge yet Denotes a PR that a maintainer is explicitly requesting NOT yet be merged by their peers label Oct 29, 2025

krancour unassigned fuskovic Oct 29, 2025

krancour reviewed Oct 29, 2025

View reviewed changes

krancour removed this from the v1.9.0 milestone Jan 6, 2026

krancour added this to the v1.10.0 milestone Jan 6, 2026

krancour closed this Feb 12, 2026

Conversation

onasser1 commented Oct 16, 2025

Uh oh!

netlify Bot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for docs-kargo-io ready!

Uh oh!

codecov Bot commented Oct 24, 2025

Codecov Report

Uh oh!

krancour commented Oct 24, 2025

Uh oh!

krancour commented Oct 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krancour Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krancour commented Oct 27, 2025

Uh oh!

krancour Oct 28, 2025 • edited by fuskovic Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krancour Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krancour commented Oct 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krancour commented Feb 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

netlify Bot commented Oct 16, 2025 •

edited

Loading

krancour Oct 28, 2025 •

edited

Loading

krancour Oct 28, 2025 •

edited by fuskovic

Loading

krancour Oct 29, 2025 •

edited

Loading