Skip to content

Commit 00638f4

Browse files
committed
OCM-18900 | fix: Improve proxy validation error messages for special characters in passwords
1 parent 11289f1 commit 00638f4

File tree

3 files changed

+70
-9
lines changed

3 files changed

+70
-9
lines changed

cmd/list/dnsdomains/cmd.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,19 @@ func run(_ *cobra.Command, _ []string) {
7070
r := rosa.NewRuntime().WithOCM()
7171
defer r.Cleanup()
7272

73-
r.Reporter.Debugf("Loading dns domains for current org id")
74-
search := "user_defined='true'"
73+
organizationID, _, err := r.OCMClient.GetCurrentOrganization()
74+
if err != nil {
75+
_ = r.Reporter.Errorf("Failed to get current organization: %v", err)
76+
os.Exit(1)
77+
}
78+
79+
search := fmt.Sprintf("user_defined='true' and organization.id='%s'", organizationID)
7580
if args.all {
76-
search = ""
81+
search = "user_defined='true'"
7782
}
7883
dnsDomains, err := r.OCMClient.ListDNSDomains(search)
7984
if err != nil {
80-
r.Reporter.Errorf("Failed to list DNS Domains: %v", err)
85+
_ = r.Reporter.Errorf("Failed to list DNS Domains: %v", err)
8186
os.Exit(1)
8287
}
8388

@@ -88,7 +93,7 @@ func run(_ *cobra.Command, _ []string) {
8893
if output.HasFlag() {
8994
err = output.Print(dnsDomains)
9095
if err != nil {
91-
r.Reporter.Errorf("%s", err)
96+
_ = r.Reporter.Errorf("%s", err)
9297
os.Exit(1)
9398
}
9499
os.Exit(0)

pkg/ocm/helpers.go

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,12 +139,24 @@ func ValidateHTTPProxy(val interface{}) error {
139139
if httpProxy == "" {
140140
return nil
141141
}
142-
url, err := url.ParseRequestURI(httpProxy)
142+
143+
// Encode password in place
144+
if i := strings.LastIndex(httpProxy, "@"); i > 0 {
145+
if j := strings.LastIndex(httpProxy[:i], ":"); j > 0 {
146+
httpProxy = httpProxy[:j+1] + url.QueryEscape(httpProxy[j+1:i]) + httpProxy[i:]
147+
}
148+
}
149+
150+
parsedURL, err := url.ParseRequestURI(httpProxy)
143151
if err != nil {
144-
return fmt.Errorf("Invalid http-proxy value '%s'", httpProxy)
152+
return fmt.Errorf("Invalid http-proxy value '%s': %s", httpProxy, err)
145153
}
146-
if url.Scheme != "http" {
147-
return errors.Errorf("%s", "Expected http-proxy to have an http:// scheme")
154+
155+
if parsedURL.Scheme != "http" {
156+
if parsedURL.Scheme == "https" {
157+
return errors.Errorf("Invalid http-proxy value '%s': use --https-proxy for HTTPS proxies", httpProxy)
158+
}
159+
return errors.Errorf("Expected http-proxy to have an http:// scheme")
148160
}
149161
return nil
150162
}

pkg/ocm/helpers_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,3 +695,47 @@ var _ = Describe("GetAutoNodeRoleArn", func() {
695695
Expect(exists).To(BeTrue())
696696
})
697697
})
698+
699+
var _ = Describe("ValidateHTTPProxy", func() {
700+
It("accepts valid proxy URL with authentication", func() {
701+
err := ValidateHTTPProxy("http://user:[email protected]:8080")
702+
Expect(err).NotTo(HaveOccurred())
703+
})
704+
705+
It("accepts valid proxy URL without authentication", func() {
706+
err := ValidateHTTPProxy("http://proxy.example.com:8080")
707+
Expect(err).NotTo(HaveOccurred())
708+
})
709+
710+
It("accepts empty string", func() {
711+
err := ValidateHTTPProxy("")
712+
Expect(err).NotTo(HaveOccurred())
713+
})
714+
715+
It("auto-encodes password with forward slash", func() {
716+
err := ValidateHTTPProxy("http://proxyuser:QvoZjyy/[email protected]:8080")
717+
Expect(err).NotTo(HaveOccurred())
718+
})
719+
720+
It("auto-encodes password with special characters", func() {
721+
err := ValidateHTTPProxy("http://user:pass@[email protected]:8080")
722+
Expect(err).NotTo(HaveOccurred())
723+
})
724+
725+
It("accepts password with URL-encoded forward slash", func() {
726+
err := ValidateHTTPProxy("http://proxyuser:QvoZjyy%[email protected]:8080")
727+
Expect(err).NotTo(HaveOccurred())
728+
})
729+
730+
It("rejects HTTPS scheme and suggests using --https-proxy", func() {
731+
err := ValidateHTTPProxy("https://proxy.example.com:8080")
732+
Expect(err).To(HaveOccurred())
733+
Expect(err.Error()).To(ContainSubstring("use --https-proxy"))
734+
})
735+
736+
It("rejects missing protocol scheme", func() {
737+
err := ValidateHTTPProxy("proxy.example.com:8080")
738+
Expect(err).To(HaveOccurred())
739+
Expect(err.Error()).To(ContainSubstring("http:// scheme"))
740+
})
741+
})

0 commit comments

Comments
 (0)