Adds regex validation for git url ssh and base auth case with additional tests

Signed-off-by: Caroline Scherf <fcaroline@vmware.com>

Author: fcaroline2020

pkg/secret/factory.go

@@ -6,6 +6,7 @@ package secret
 import (
 	"encoding/json"
 	"github.com/google/go-containerregistry/pkg/name"
+	"os"
 	"regexp"
 	"sort"
 	"strings"
@@ -106,22 +107,29 @@ func (f *Factory) validate() error {
 	}
 
 	if f.GitUser != "" {
-		if !(strings.HasPrefix(f.GitUrl, "http://") || strings.HasPrefix(f.GitUrl, "https://")) {
-			return errors.Errorf("must provide a valid git url for basic auth (ex. https://github.com)")
-		}
+		//if !(strings.HasPrefix(f.GitUrl, "http://") || strings.HasPrefix(f.GitUrl, "https://")) {
+
+		//}
 		// regex checks if slash appears after a period, ex: www.github.com/repo
-		if match, _ := regexp.Match(`\.\w+/`, []byte(f.GitUrl)); match {
-			return errors.Errorf("git url should be a valid url without the repository path (ex. https://github.com)")
+		if match, _ := regexp.Match(`^(?:https?://)(?:[a-zA-Z0-9\-\.]+(?::\d+)?)(?:\/(?:$|[^/]))?$`, []byte(f.GitUrl)); !match {
+			return errors.Errorf("must provide a valid git url without the repository path for basic auth (ex. https://github.com)")
 		}
+		//	return errors.Errorf("git url should be a valid url without the repository path (ex. https://github.com)")
+		//}
 	}
 
-	if f.GitSshKeyFile != "" && !strings.HasPrefix(f.GitUrl, "git@") {
+	if f.GitSshKeyFile != "" && !validateGitSshAddress(f.GitUrl) {
 		return errors.Errorf("must provide a valid git url for SSH (ex. git@github.com)")
 	}
 
 	return nil
 }
 
+func validateGitSshAddress(address string) bool {
+	re := regexp.MustCompile(`^git@github\.com$`)
+	return re.MatchString(address)
+}
+
 func (f *Factory) getSecretKind() (secretKind, error) {
 	if f.DockerhubId != "" {
 		return dockerHubKind, nil

pkg/secret/factory_test.go

@@ -4,6 +4,7 @@
 package secret_test
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/sclevine/spec"
@@ -99,23 +100,19 @@ func testSecretFactory(t *testing.T, when spec.G, it spec.S) {
 
 	when("sub params are mixed with git", func() {
 		it("returns an error message", func() {
-			it("returns an error message", func() {
-				factory.GitUrl = "some-git"
-				factory.RegistryUser = "some-reg-user"
-				factory.GitUser = "some-git-user"
-				_, _, err := factory.MakeSecret("test-name", "test-namespace")
-				require.EqualError(t, err, "extraneous parameters: registry-user")
-			})
+			factory.GitUrl = "some-git"
+			factory.RegistryUser = "some-reg-user"
+			factory.GitUser = "some-git-user"
+			_, _, err := factory.MakeSecret("test-name", "test-namespace")
+			require.EqualError(t, err, "extraneous parameters: registry-user")
 		})
 	})
 
 	when("neither git basic auth nor git ssh are provided", func() {
 		it("returns an error message", func() {
-			it("returns an error message", func() {
-				factory.GitUrl = "some-git"
-				_, _, err := factory.MakeSecret("test-name", "test-namespace")
-				require.EqualError(t, err, "missing parameter git-user or git-ssh-key")
-			})
+			factory.GitUrl = "some-git"
+			_, _, err := factory.MakeSecret("test-name", "test-namespace")
+			require.EqualError(t, err, "missing parameter git-user or git-ssh-key")
 		})
 	})
 
@@ -130,29 +127,69 @@ func testSecretFactory(t *testing.T, when spec.G, it spec.S) {
 	})
 
 	when("using git basic auth", func() {
-		it("validates that the git url begins with http:// or https://", func() {
-			factory.GitUrl = "some-git"
-			factory.GitUser = "some-git-user"
-			_, _, err := factory.MakeSecret("test-name", "test-namespace")
-			require.EqualError(t, err, "must provide a valid git url for basic auth (ex. https://github.com)")
+		it("validates that the git url is correct", func() {
+			validGitUrls := []string{
+				"https://github.com",
+				"http://github.com",
+				"https://github.enterprise:1234",
+				"http://github.enterprise:1234",
+			}
+			for _, testUrl := range validGitUrls {
+				factory.GitUrl = testUrl
+				factory.GitUser = "some-git-user"
+				s, _, err := factory.MakeSecret("test-name", "test-namespace")
+				require.NotNilf(t, s, "factory.GitUrl = \"%s\" secret should not be nil", factory.GitUrl)
+				require.NoError(t, err, fmt.Sprintf("factory.GitUrl = \"%s\" should not have errors", factory.GitUrl))
+			}
 		})
-	})
 
-	when("using git basic auth", func() {
-		it("validates that the git url does not contain additional repo info", func() {
-			factory.GitUrl = "https://some-git.com/test"
-			factory.GitUser = "some-git-user"
-			_, _, err := factory.MakeSecret("test-name", "test-namespace")
-			require.EqualError(t, err, "git url should be a valid url without the repository path (ex. https://github.com)")
+		it("validates that the git url is correct", func() {
+			invalidGitUrls := []string{
+				"some-git",
+				"https://some-git.com/test",
+			}
+			for _, testUrl := range invalidGitUrls {
+				factory.GitUrl = testUrl
+				factory.GitUser = "some-git-user"
+				s, _, err := factory.MakeSecret("test-name", "test-namespace")
+				require.Nilf(t, s, "factory.GitUrl = \"%s\" secret should be nil", factory.GitUrl)
+				require.EqualError(t, err, "must provide a valid git url without the repository path for basic auth (ex. https://github.com)")
+			}
 		})
 	})
 
 	when("using git ssh keys", func() {
-		it("validates that the git url begins with git@", func() {
-			factory.GitUrl = "some-git"
-			factory.GitSshKeyFile = "some-ssh-key"
-			_, _, err := factory.MakeSecret("test-name", "test-namespace")
-			require.EqualError(t, err, "must provide a valid git url for SSH (ex. git@github.com)")
+		it("creates a secret when a valid ssh git url is passed", func() {
+			validGitSshUrls := []string{
+				"git@github.com",
+			}
+			for _, testUrl := range validGitSshUrls {
+				factory.GitUrl = testUrl
+				factory.GitSshKeyFile = "./testdata/some-ssh-key.pem"
+				s, _, err := factory.MakeSecret("test-name", "test-namespace")
+				require.NotNilf(t, s, "factory.GitUrl = \"%s\" secret should not be nil", factory.GitUrl)
+				require.NoError(t, err, fmt.Sprintf("factory.GitUrl = \"%s\" should not have errors", factory.GitUrl))
+			}
+		})
+
+		it("prints an error when the git url is not valid", func() {
+			invalidGitSshUrls := []string{
+				"some-git",
+				"git@github.com:owner",
+				"git@github.com:owner/repo",
+				"git@github.com:443/user/repo.git",
+				"git@github.com:user/repo.git",
+				"git@github.com:buildpacks-community/kpack-cli.git",
+				"ssh://git@ssh.github.com:443/YOUR-USERNAME/YOUR-REPOSITORY.git",
+			}
+			for _, testUrl := range invalidGitSshUrls {
+				factory.GitUrl = testUrl
+				factory.GitUrl = "some-git"
+				factory.GitSshKeyFile = "./testdata/some-ssh-key.pem"
+				s, _, err := factory.MakeSecret("test-name", "test-namespace")
+				require.Nilf(t, s, "factory.GitUrl = \"%s\" secret should be nil", factory.GitUrl)
+				require.EqualError(t, err, "must provide a valid git url for SSH (ex. git@github.com)")
+			}
 		})
 	})
 }