Skip to content

Commit

Permalink
Merge pull request #1895 from mnecas/fix_gh_idp_validation
Browse files Browse the repository at this point in the history
OCM-7110 | fix: GitHub IDP validation
  • Loading branch information
openshift-merge-bot[bot] authored and gdbranco committed Apr 3, 2024
1 parent a583c12 commit 1f0dde9
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 4 deletions.
6 changes: 3 additions & 3 deletions cmd/create/idp/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func buildGithubIdp(cmd *cobra.Command,
Help: cmd.Flags().Lookup("hostname").Usage,
Default: githubHostname,
Validators: []interactive.Validator{
interactive.IsURL,
interactive.IsValidHostname,
},
})
if err != nil {
Expand All @@ -201,9 +201,9 @@ func buildGithubIdp(cmd *cobra.Command,
return idpBuilder, fmt.Errorf("CA is not expected when not using a hosted instance of Github Enterprise")
}
if githubHostname != "" {
_, err = url.ParseRequestURI(githubHostname)
err := interactive.IsValidHostname(githubHostname)
if err != nil {
return idpBuilder, fmt.Errorf("Expected a valid Hostname: %s", err)
return idpBuilder, err
}
// Set the hostname, if any
githubIDP = githubIDP.Hostname(githubHostname)
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ require (
go.uber.org/mock v0.3.0
gopkg.in/yaml.v3 v3.0.1
k8s.io/apimachinery v0.29.2
k8s.io/utils v0.0.0-20230726121419-3b25d923346b
)

require (
Expand Down Expand Up @@ -112,7 +113,6 @@ require (
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/square/go-jose.v2 v2.6.0 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect
)

replace github.com/golang/glog => github.com/kubermatic/glog-logrus v0.0.0-20180829085450-3fa5b9870d1d
18 changes: 18 additions & 0 deletions pkg/interactive/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ import (
"regexp"
"strconv"

"k8s.io/apimachinery/pkg/util/validation"
netutils "k8s.io/utils/net"

"github.com/AlecAivazis/survey/v2"
"github.com/AlecAivazis/survey/v2/core"
clustervalidations "github.com/openshift-online/ocm-common/pkg/cluster/validations"
Expand Down Expand Up @@ -59,6 +62,21 @@ func IsURL(val interface{}) error {
return err
}

func IsValidHostname(val interface{}) error {
if !_isValidHostname(val.(string)) {
return fmt.Errorf(fmt.Sprintf("'%s' hostname must be a valid DNS subdomain or IP address", val.(string)))
}
return nil
}

// _isValidHostname is same validation as in the Open Shift GitHub IDP CRD
// https://github.com/openshift/kubernetes/blob/91607f5d750ba4002f87d34a12ae1cfd45b45b81/openshift-kube-apiserver/admission/customresourcevalidation/oauth/helpers.go#L13
//
//nolint:lll
func _isValidHostname(hostname string) bool {
return len(validation.IsDNS1123Subdomain(hostname)) == 0 || netutils.ParseIPSloppy(hostname) != nil
}

func IsURLHttps(val interface{}) error {
parsedUri, err := _isUrl(val)
if err != nil {
Expand Down
13 changes: 13 additions & 0 deletions pkg/interactive/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,18 @@ var _ = Describe("Validation", func() {
Expect(err).NotTo(HaveOccurred())
})
})
Context("GitHub Hostname", func() {
It("Fails validation if hostname is 'https://github.com'", func() {
err := IsValidHostname("https://github.com")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring(
"'https://github.com' hostname must be a valid DNS subdomain or IP address"),
)
})
It("Passes validation if hostname is 'domain.customer.com'", func() {
err := IsValidHostname("domain.customer.com")
Expect(err).NotTo(HaveOccurred())
})
})

})

0 comments on commit 1f0dde9

Please sign in to comment.