Skip to content

Commit dca6669

Browse files
olucasfreitasolucasfreitas
committed
OCM-18900 | fix: Improve error message for proxy passwords with special characters
1 parent 351c9c5 commit dca6669

File tree

7 files changed

+543
-10
lines changed

7 files changed

+543
-10
lines changed

‎cmd/create/cluster/cmd.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3053,15 +3053,15 @@ func run(cmd *cobra.Command, _ []string) {
30533053
Help: cmd.Flags().Lookup("https-proxy").Usage,
30543054
Default: httpsProxy,
30553055
Validators: []interactive.Validator{
3056-
interactive.IsURL,
3056+
ocm.ValidateHTTPSProxy,
30573057
},
30583058
})
30593059
if err != nil {
30603060
r.Reporter.Errorf("Expected a valid https proxy: %s", err)
30613061
os.Exit(1)
30623062
}
30633063
}
3064-
err = interactive.IsURL(httpsProxy)
3064+
err = ocm.ValidateHTTPSProxy(httpsProxy)
30653065
if err != nil {
30663066
r.Reporter.Errorf("%s", err)
30673067
os.Exit(1)

‎cmd/edit/cluster/cmd.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ func run(cmd *cobra.Command, _ []string) {
513513
}
514514
}
515515
if httpsProxy != nil && *httpsProxy != input.DoubleQuotesToRemove {
516-
err = interactive.IsURL(*httpsProxy)
516+
err = ocm.ValidateHTTPSProxy(*httpsProxy)
517517
if err != nil {
518518
r.Reporter.Errorf("%s", err)
519519
os.Exit(1)

‎pkg/helper/helpers.go‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,3 +268,67 @@ func FilterEmptyStrings(strings []string) []string {
268268
}
269269
return filteredResult
270270
}
271+
272+
type URLCredentialValidation struct {
273+
Field string
274+
InvalidChar rune
275+
HasInvalid bool
276+
}
277+
278+
func ValidateURLCredentials(urlString string) *URLCredentialValidation {
279+
if !strings.Contains(urlString, "@") || !strings.Contains(urlString, "://") {
280+
return &URLCredentialValidation{HasInvalid: false}
281+
}
282+
283+
schemeIdx := strings.Index(urlString, "://")
284+
if schemeIdx == -1 {
285+
return &URLCredentialValidation{HasInvalid: false}
286+
}
287+
288+
rest := urlString[schemeIdx+3:]
289+
atIdx := strings.LastIndex(rest, "@")
290+
if atIdx == -1 {
291+
return &URLCredentialValidation{HasInvalid: false}
292+
}
293+
294+
userinfo := rest[:atIdx]
295+
296+
// Check for multiple @ symbols first (indicates @ in credentials)
297+
if strings.Count(urlString, "@") > 1 {
298+
return &URLCredentialValidation{
299+
Field: "password",
300+
InvalidChar: '@',
301+
HasInvalid: true,
302+
}
303+
}
304+
305+
// Check username and password separately
306+
colonIdx := strings.Index(userinfo, ":")
307+
if colonIdx == -1 {
308+
return checkForInvalidChars(userinfo, "username")
309+
}
310+
311+
username := userinfo[:colonIdx]
312+
if result := checkForInvalidChars(username, "username"); result.HasInvalid {
313+
return result
314+
}
315+
316+
password := userinfo[colonIdx+1:]
317+
return checkForInvalidChars(password, "password")
318+
}
319+
320+
func checkForInvalidChars(value, field string) *URLCredentialValidation {
321+
invalidChars := "/:#?[]@!$&'()*+,;="
322+
323+
for _, char := range value {
324+
if strings.ContainsRune(invalidChars, char) {
325+
return &URLCredentialValidation{
326+
Field: field,
327+
InvalidChar: char,
328+
HasInvalid: true,
329+
}
330+
}
331+
}
332+
333+
return &URLCredentialValidation{HasInvalid: false}
334+
}

‎pkg/interactive/validation.go‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,11 @@ func IsURLHttps(val interface{}) error {
8989
if err != nil {
9090
return err
9191
}
92+
if parsedUri == nil {
93+
return nil
94+
}
9295
if parsedUri.Scheme != helper.ProtocolHttps {
93-
return fmt.Errorf("Expect URL '%s' to use an 'https://' scheme", val.(string))
96+
return fmt.Errorf("expect URL '%s' to use an 'https://' scheme", val.(string))
9497
}
9598
return nil
9699
}
@@ -106,8 +109,20 @@ func _isUrl(val interface{}) (*url.URL, error) {
106109
if s == "" {
107110
return nil, nil
108111
}
109-
parsedUri, err := url.ParseRequestURI(fmt.Sprintf("%v", val))
110-
return parsedUri, err
112+
113+
if credValidation := helper.ValidateURLCredentials(s); credValidation.HasInvalid {
114+
return nil, fmt.Errorf("invalid URL: %s contains invalid character '%c'",
115+
credValidation.Field, credValidation.InvalidChar)
116+
}
117+
118+
parsedUri, err := url.Parse(fmt.Sprintf("%v", val))
119+
if err != nil {
120+
return nil, err
121+
}
122+
if parsedUri.Scheme == "" || parsedUri.Host == "" {
123+
return nil, fmt.Errorf("url must have a scheme and host")
124+
}
125+
return parsedUri, nil
111126
}
112127

113128
// IsCert validates whether the given filepath is a valid cert file

‎pkg/interactive/validation_test.go‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,134 @@ var _ = Describe("Validation", func() {
108108
})
109109
})
110110

111+
Context("IsURL", func() {
112+
It("accepts empty string", func() {
113+
err := IsURL("")
114+
Expect(err).NotTo(HaveOccurred())
115+
})
116+
117+
It("accepts nil", func() {
118+
err := IsURL(nil)
119+
Expect(err).NotTo(HaveOccurred())
120+
})
121+
122+
It("accepts valid http URL", func() {
123+
err := IsURL("http://example.com")
124+
Expect(err).NotTo(HaveOccurred())
125+
})
126+
127+
It("accepts valid https URL", func() {
128+
err := IsURL("https://example.com")
129+
Expect(err).NotTo(HaveOccurred())
130+
})
131+
132+
It("accepts URL with authentication", func() {
133+
err := IsURL("http://user:[email protected]:8080")
134+
Expect(err).NotTo(HaveOccurred())
135+
})
136+
137+
It("rejects URL with unencoded special characters in password", func() {
138+
err := IsURL("http://proxyuser:QvoZjyy/[email protected]:8080")
139+
Expect(err).To(HaveOccurred())
140+
})
141+
142+
It("accepts URL with percent-encoded special characters in password", func() {
143+
err := IsURL("http://proxyuser:QvoZjyy%[email protected]:8080")
144+
Expect(err).NotTo(HaveOccurred())
145+
})
146+
147+
It("rejects URL with unencoded @ in password", func() {
148+
err := IsURL("http://user:pass@[email protected]:8080")
149+
Expect(err).To(HaveOccurred())
150+
Expect(err.Error()).To(Equal("invalid URL: password contains invalid character '@'"))
151+
})
152+
153+
It("accepts URL with percent-encoded @ in password", func() {
154+
err := IsURL("http://user:pass%[email protected]:8080")
155+
Expect(err).NotTo(HaveOccurred())
156+
})
157+
158+
It("rejects URL with extra colon in password", func() {
159+
err := IsURL("http://user:pass:[email protected]:8080")
160+
Expect(err).To(HaveOccurred())
161+
Expect(err.Error()).To(Equal("invalid URL: password contains invalid character ':'"))
162+
})
163+
164+
It("accepts URL with percent-encoded colon in password", func() {
165+
err := IsURL("http://user:pass%[email protected]:8080")
166+
Expect(err).NotTo(HaveOccurred())
167+
})
168+
169+
It("rejects URL without scheme", func() {
170+
err := IsURL("example.com")
171+
Expect(err).To(HaveOccurred())
172+
Expect(err.Error()).To(ContainSubstring("url must have a scheme and host"))
173+
})
174+
175+
It("rejects URL without host", func() {
176+
err := IsURL("http://")
177+
Expect(err).To(HaveOccurred())
178+
Expect(err.Error()).To(ContainSubstring("url must have a scheme and host"))
179+
})
180+
181+
It("rejects non-string input", func() {
182+
err := IsURL(123)
183+
Expect(err).To(HaveOccurred())
184+
Expect(err.Error()).To(ContainSubstring("can only validate strings"))
185+
})
186+
})
187+
188+
Context("IsURLHttps", func() {
189+
It("accepts empty string", func() {
190+
err := IsURLHttps("")
191+
Expect(err).NotTo(HaveOccurred())
192+
})
193+
194+
It("accepts nil", func() {
195+
err := IsURLHttps(nil)
196+
Expect(err).NotTo(HaveOccurred())
197+
})
198+
199+
It("accepts valid https URL", func() {
200+
err := IsURLHttps("https://example.com")
201+
Expect(err).NotTo(HaveOccurred())
202+
})
203+
204+
It("accepts https URL with authentication", func() {
205+
err := IsURLHttps("https://user:[email protected]:8080")
206+
Expect(err).NotTo(HaveOccurred())
207+
})
208+
209+
It("rejects https URL with unencoded special characters in password", func() {
210+
err := IsURLHttps("https://proxyuser:QvoZjyy/[email protected]:8080")
211+
Expect(err).To(HaveOccurred())
212+
Expect(err.Error()).To(Equal("invalid URL: password contains invalid character '/'"))
213+
})
214+
215+
It("accepts https URL with percent-encoded special characters in password", func() {
216+
err := IsURLHttps("https://proxyuser:QvoZjyy%[email protected]:8080")
217+
Expect(err).NotTo(HaveOccurred())
218+
})
219+
220+
It("rejects http URL", func() {
221+
err := IsURLHttps("http://example.com")
222+
Expect(err).To(HaveOccurred())
223+
Expect(err.Error()).To(ContainSubstring("expect URL 'http://example.com' to use an 'https://' scheme"))
224+
})
225+
226+
It("rejects URL without scheme", func() {
227+
err := IsURLHttps("example.com")
228+
Expect(err).To(HaveOccurred())
229+
Expect(err.Error()).To(ContainSubstring("url must have a scheme and host"))
230+
})
231+
232+
It("rejects non-string input", func() {
233+
err := IsURLHttps(123)
234+
Expect(err).To(HaveOccurred())
235+
Expect(err.Error()).To(ContainSubstring("can only validate strings"))
236+
})
237+
})
238+
111239
})
112240

113241
var _ = Describe("SubnetsValidator", func() {

0 commit comments

Comments
 (0)