From a4515052488001db21f550bca8b7ae0ac19df486 Mon Sep 17 00:00:00 2001 From: Christian Ihle Date: Tue, 3 Jan 2023 10:12:56 +0100 Subject: [PATCH 1/8] Support specifying bearerToken for git http token authentication. As an alternative to username and password with http basic authentication. Signed-off-by: Christian Ihle --- git/gogit/transport.go | 4 ++++ git/gogit/transport_test.go | 24 ++++++++++++++++++++++++ git/options.go | 16 +++++++++------- git/options_test.go | 2 ++ 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/git/gogit/transport.go b/git/gogit/transport.go index 62553c6c..50eddecb 100644 --- a/git/gogit/transport.go +++ b/git/gogit/transport.go @@ -43,6 +43,10 @@ func transportAuth(opts *git.AuthOptions, fallbackToDefaultKnownHosts bool) (tra Username: opts.Username, Password: opts.Password, }, nil + } else if opts.BearerToken != "" { + return &http.TokenAuth{ + Token: opts.BearerToken, + }, nil } return nil, nil case git.SSH: diff --git a/git/gogit/transport_test.go b/git/gogit/transport_test.go index b5708a72..d722bd6a 100644 --- a/git/gogit/transport_test.go +++ b/git/gogit/transport_test.go @@ -111,6 +111,18 @@ func Test_transportAuth(t *testing.T) { })) }, }, + { + name: "HTTP bearer token", + opts: &git.AuthOptions{ + Transport: git.HTTP, + BearerToken: "http-token", + }, + wantFunc: func(g *WithT, t transport.AuthMethod, opts *git.AuthOptions) { + g.Expect(t).To(Equal(&http.TokenAuth{ + Token: opts.BearerToken, + })) + }, + }, { name: "HTTPS basic auth", opts: &git.AuthOptions{ @@ -125,6 +137,18 @@ func Test_transportAuth(t *testing.T) { })) }, }, + { + name: "HTTPS bearer token", + opts: &git.AuthOptions{ + Transport: git.HTTPS, + BearerToken: "https-token", + }, + wantFunc: func(g *WithT, t transport.AuthMethod, opts *git.AuthOptions) { + g.Expect(t).To(Equal(&http.TokenAuth{ + Token: opts.BearerToken, + })) + }, + }, { name: "SSH private key", opts: &git.AuthOptions{ diff --git a/git/options.go b/git/options.go index 85b9ca39..dbd09004 100644 --- a/git/options.go +++ b/git/options.go @@ -38,13 +38,14 @@ const ( // AuthOptions are the authentication options for the Transport of // communication with a remote origin. type AuthOptions struct { - Transport TransportType - Host string - Username string - Password string - Identity []byte - KnownHosts []byte - CAFile []byte + Transport TransportType + Host string + Username string + Password string + BearerToken string + Identity []byte + KnownHosts []byte + CAFile []byte } // KexAlgos hosts the key exchange algorithms to be used for SSH connections. @@ -88,6 +89,7 @@ func NewAuthOptions(u url.URL, data map[string][]byte) (*AuthOptions, error) { if len(data) > 0 { opts.Username = string(data["username"]) opts.Password = string(data["password"]) + opts.BearerToken = string(data["bearerToken"]) opts.CAFile = data["caFile"] opts.Identity = data["identity"] opts.KnownHosts = data["known_hosts"] diff --git a/git/options_test.go b/git/options_test.go index e4adff13..8348ddba 100644 --- a/git/options_test.go +++ b/git/options_test.go @@ -186,6 +186,7 @@ func TestAuthOptionsFromData(t *testing.T) { data: map[string][]byte{ "username": []byte("example"), // This takes precedence over the one from the URL "password": []byte("secret"), + "bearerToken": []byte("token"), "identity": []byte(privateKeyFixture), "known_hosts": []byte(knownHostsFixture), "caFile": []byte("mock"), @@ -194,6 +195,7 @@ func TestAuthOptionsFromData(t *testing.T) { wantFunc: func(g *WithT, opts *AuthOptions) { g.Expect(opts.Username).To(Equal("example")) g.Expect(opts.Password).To(Equal("secret")) + g.Expect(opts.BearerToken).To(Equal("token")) g.Expect(opts.Identity).To(BeEquivalentTo(privateKeyFixture)) g.Expect(opts.KnownHosts).To(BeEquivalentTo(knownHostsFixture)) g.Expect(opts.CAFile).To(BeEquivalentTo("mock")) From 04d0d4878dd9755d7874031f1236ef0861d517ac Mon Sep 17 00:00:00 2001 From: Christian Ihle Date: Tue, 17 Jan 2023 14:35:10 +0100 Subject: [PATCH 2/8] Add some quick tests of basic auth in client.validateUrl() Signed-off-by: Christian Ihle --- git/gogit/client_test.go | 63 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/git/gogit/client_test.go b/git/gogit/client_test.go index 50b80433..f0d691e0 100644 --- a/git/gogit/client_test.go +++ b/git/gogit/client_test.go @@ -653,3 +653,66 @@ func TestHead(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) g.Expect(hash.String()).To(Equal(cc)) } + +func TestValidateUrl(t *testing.T) { + tests := []struct { + name string + transport git.TransportType + username string + password string + url string + credentialsOverHttp bool + expectedError string + }{ + { + name: "blocked: basic auth over http", + transport: git.HTTP, + username: "user", + password: "pass", + url: "http://url", + expectedError: "basic auth cannot be sent over HTTP", + }, + { + name: "allowed: basic auth over http with insecure enabled", + transport: git.HTTP, + username: "user", + password: "pass", + url: "http://url", + credentialsOverHttp: true, + }, + { + name: "allowed: basic auth over https", + transport: git.HTTPS, + username: "user", + password: "pass", + url: "https://url", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + opts := []ClientOption{WithDiskStorage()} + if tt.credentialsOverHttp { + opts = append(opts, WithInsecureCredentialsOverHTTP()) + } + + ggc, err := NewClient(t.TempDir(), &git.AuthOptions{ + Transport: tt.transport, + Username: tt.username, + Password: tt.password, + }, opts...) + g.Expect(err).ToNot(HaveOccurred()) + + err = ggc.validateUrl(tt.url) + + if tt.expectedError == "" { + g.Expect(err).To(BeNil()) + } else { + g.Expect(err).ToNot(BeNil()) + g.Expect(err.Error()).To(ContainSubstring(tt.expectedError)) + } + }) + } +} From 9b9b723460dcd0433fb45c3303f3a66dac91ec0a Mon Sep 17 00:00:00 2001 From: Christian Ihle Date: Tue, 17 Jan 2023 14:37:42 +0100 Subject: [PATCH 3/8] Validate that bearer token is not used over http Signed-off-by: Christian Ihle --- git/gogit/client.go | 9 ++++++--- git/gogit/client_test.go | 28 +++++++++++++++++++++++++--- git/gogit/clone_test.go | 20 +++++++++++++++++--- 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/git/gogit/client.go b/git/gogit/client.go index 3054ed35..1577d6ef 100644 --- a/git/gogit/client.go +++ b/git/gogit/client.go @@ -254,9 +254,12 @@ func (g *Client) validateUrl(u string) error { return errors.New("URL cannot contain credentials when using HTTP") } - if httpOrEmpty && g.authOpts != nil && - (g.authOpts.Username != "" || g.authOpts.Password != "") { - return errors.New("basic auth cannot be sent over HTTP") + if httpOrEmpty && g.authOpts != nil { + if g.authOpts.Username != "" || g.authOpts.Password != "" { + return errors.New("basic auth cannot be sent over HTTP") + } else if g.authOpts.BearerToken != "" { + return errors.New("bearer token cannot be sent over HTTP") + } } return nil diff --git a/git/gogit/client_test.go b/git/gogit/client_test.go index f0d691e0..c4b7afe6 100644 --- a/git/gogit/client_test.go +++ b/git/gogit/client_test.go @@ -660,6 +660,7 @@ func TestValidateUrl(t *testing.T) { transport git.TransportType username string password string + bearerToken string url string credentialsOverHttp bool expectedError string @@ -687,6 +688,26 @@ func TestValidateUrl(t *testing.T) { password: "pass", url: "https://url", }, + { + name: "blocked: bearer token over http", + transport: git.HTTP, + bearerToken: "token", + url: "http://url", + expectedError: "bearer token cannot be sent over HTTP", + }, + { + name: "allowed: bearer token over http with insecure enabled", + transport: git.HTTP, + bearerToken: "token", + url: "http://url", + credentialsOverHttp: true, + }, + { + name: "allowed: bearer token over https", + transport: git.HTTPS, + bearerToken: "token", + url: "https://url", + }, } for _, tt := range tests { @@ -699,9 +720,10 @@ func TestValidateUrl(t *testing.T) { } ggc, err := NewClient(t.TempDir(), &git.AuthOptions{ - Transport: tt.transport, - Username: tt.username, - Password: tt.password, + Transport: tt.transport, + Username: tt.username, + Password: tt.password, + BearerToken: tt.bearerToken, }, opts...) g.Expect(err).ToNot(HaveOccurred()) diff --git a/git/gogit/clone_test.go b/git/gogit/clone_test.go index 90cdc398..de2b4e3e 100644 --- a/git/gogit/clone_test.go +++ b/git/gogit/clone_test.go @@ -988,6 +988,7 @@ func TestClone_CredentialsOverHttp(t *testing.T) { name string username string password string + bearerToken string allowCredentialsOverHttp bool transformURL func(string) string expectCloneErr string @@ -1009,6 +1010,11 @@ func TestClone_CredentialsOverHttp(t *testing.T) { password: "pass", expectCloneErr: "basic auth cannot be sent over HTTP", }, + { + name: "blocked: bearer token over HTTP", + bearerToken: "token", + expectCloneErr: "bearer token cannot be sent over HTTP", + }, { name: "blocked: URL based credential over HTTP (name)", transformURL: func(s string) string { @@ -1069,6 +1075,13 @@ func TestClone_CredentialsOverHttp(t *testing.T) { allowCredentialsOverHttp: true, expectRequest: true, }, + { + name: "allowed: bearer token over HTTP", + bearerToken: "token", + expectCloneErr: "unable to clone", + allowCredentialsOverHttp: true, + expectRequest: true, + }, { name: "allowed: URL based credential over HTTP (name)", transformURL: func(s string) string { @@ -1129,9 +1142,10 @@ func TestClone_CredentialsOverHttp(t *testing.T) { } ggc, err := NewClient(tmpDir, &git.AuthOptions{ - Transport: git.HTTP, - Username: tt.username, - Password: tt.password, + Transport: git.HTTP, + Username: tt.username, + Password: tt.password, + BearerToken: tt.bearerToken, }, opts...) g.Expect(err).ToNot(HaveOccurred()) From fef9d6a24e385fbfb7a1b845498254418a947842 Mon Sep 17 00:00:00 2001 From: Christian Ihle Date: Fri, 20 Jan 2023 10:15:44 +0100 Subject: [PATCH 4/8] Add more test scenarios for NewAuthOptions Signed-off-by: Christian Ihle --- git/options_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/git/options_test.go b/git/options_test.go index 8348ddba..c905cfff 100644 --- a/git/options_test.go +++ b/git/options_test.go @@ -212,6 +212,27 @@ func TestAuthOptionsFromData(t *testing.T) { g.Expect(opts.Username).To(Equal(DefaultPublicKeyAuthUser)) }, }, + { + name: "Sets user for SSH from URL", + URL: "ssh://user@example.com", + data: map[string][]byte{ + "identity": []byte(privateKeyFixture), + "known_hosts": []byte(knownHostsFixture), + }, + wantFunc: func(g *WithT, opts *AuthOptions) { + g.Expect(opts.Username).To(Equal("user")) + }, + }, + { + name: "Sets caFile for HTTPS", + URL: "https://example.com", + data: map[string][]byte{ + "caFile": []byte("mock"), + }, + wantFunc: func(g *WithT, opts *AuthOptions) { + g.Expect(opts.CAFile).To(BeEquivalentTo("mock")) + }, + }, { name: "Sets transport from URL", URL: "http://git@example.com", @@ -232,6 +253,15 @@ func TestAuthOptionsFromData(t *testing.T) { g.Expect(opts.Password).To(Equal("secret")) }, }, + { + name: "Sets username and password from URL only", + URL: "https://user:pass@example.com", + data: nil, + wantFunc: func(g *WithT, opts *AuthOptions) { + g.Expect(opts.Username).To(Equal("user")) + g.Expect(opts.Password).To(Equal("pass")) + }, + }, { name: "Validates options", URL: "ssh://example.com", From b6c68885e09ae22687339efce59586ef1bf6e12c Mon Sep 17 00:00:00 2001 From: Christian Ihle Date: Fri, 20 Jan 2023 10:40:31 +0100 Subject: [PATCH 5/8] Refactor of NewAuthOptions to only fill the auth options that are relevant Signed-off-by: Christian Ihle --- git/options.go | 34 +++++++++++++++++------------ git/options_test.go | 53 ++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 67 insertions(+), 20 deletions(-) diff --git a/git/options.go b/git/options.go index dbd09004..33429973 100644 --- a/git/options.go +++ b/git/options.go @@ -87,25 +87,31 @@ func (o AuthOptions) Validate() error { func NewAuthOptions(u url.URL, data map[string][]byte) (*AuthOptions, error) { opts := newAuthOptions(u) if len(data) > 0 { - opts.Username = string(data["username"]) - opts.Password = string(data["password"]) - opts.BearerToken = string(data["bearerToken"]) - opts.CAFile = data["caFile"] - opts.Identity = data["identity"] - opts.KnownHosts = data["known_hosts"] + if opts.Transport == SSH { + opts.Identity = data["identity"] + opts.KnownHosts = data["known_hosts"] + opts.Username = u.User.Username() + // We fallback to using "git" as the username when cloning Git + // repositories through SSH since that's the conventional username used + // by Git providers. + if opts.Username == "" { + opts.Username = DefaultPublicKeyAuthUser + } + } else if token, found := data["bearerToken"]; found { + opts.CAFile = data["caFile"] + opts.BearerToken = string(token) + } else { + opts.CAFile = data["caFile"] + opts.Username = string(data["username"]) + opts.Password = string(data["password"]) + } } - if opts.Username == "" { + if opts.Transport != SSH && opts.Username == "" { opts.Username = u.User.Username() } - // We fallback to using "git" as the username when cloning Git - // repositories through SSH since that's the conventional username used - // by Git providers. - if opts.Username == "" && opts.Transport == SSH { - opts.Username = DefaultPublicKeyAuthUser - } - if opts.Password == "" { + if opts.Transport != SSH && opts.Password == "" { opts.Password, _ = u.User.Password() } diff --git a/git/options_test.go b/git/options_test.go index c905cfff..8d2cc765 100644 --- a/git/options_test.go +++ b/git/options_test.go @@ -181,12 +181,11 @@ func TestAuthOptionsFromData(t *testing.T) { wantErr string }{ { - name: "Sets values from Secret", - URL: "https://git@example.com", + name: "Sets only relevant values from Secret for HTTPS with basic auth", + URL: "https://example.com", data: map[string][]byte{ - "username": []byte("example"), // This takes precedence over the one from the URL + "username": []byte("example"), "password": []byte("secret"), - "bearerToken": []byte("token"), "identity": []byte(privateKeyFixture), "known_hosts": []byte(knownHostsFixture), "caFile": []byte("mock"), @@ -195,10 +194,52 @@ func TestAuthOptionsFromData(t *testing.T) { wantFunc: func(g *WithT, opts *AuthOptions) { g.Expect(opts.Username).To(Equal("example")) g.Expect(opts.Password).To(Equal("secret")) - g.Expect(opts.BearerToken).To(Equal("token")) + g.Expect(opts.BearerToken).To(Equal("")) + g.Expect(opts.Identity).To(BeNil()) + g.Expect(opts.KnownHosts).To(BeNil()) + g.Expect(opts.CAFile).To(BeEquivalentTo("mock")) + }, + }, + { + name: "Sets only relevant values from Secret for HTTPS with bearer token", + URL: "https://example.com", + data: map[string][]byte{ + "username": []byte("example"), + "password": []byte("secret"), + "bearerToken": []byte("token"), + "identity": []byte(privateKeyFixture), + "known_hosts": []byte(knownHostsFixture), + "caFile": []byte("mock"), + }, + + wantFunc: func(g *WithT, opts *AuthOptions) { + g.Expect(opts.Username).To(Equal("")) + g.Expect(opts.Password).To(Equal("")) + g.Expect(opts.BearerToken).To(Equal("token")) // Preferred over basic auth when provided + g.Expect(opts.Identity).To(BeNil()) + g.Expect(opts.KnownHosts).To(BeNil()) + g.Expect(opts.CAFile).To(BeEquivalentTo("mock")) + }, + }, + { + name: "Sets only relevant values from Secret for SSH", + URL: "ssh://example.com", + data: map[string][]byte{ + "username": []byte("example"), + "password": []byte("secret"), + "bearerToken": []byte("token"), + "identity": []byte(privateKeyFixture), + "known_hosts": []byte(knownHostsFixture), + "caFile": []byte("mock"), + }, + + wantFunc: func(g *WithT, opts *AuthOptions) { + g.Expect(opts.Username).To(Equal(DefaultPublicKeyAuthUser)) // Not the specified username + g.Expect(opts.Password).To(Equal("")) + g.Expect(opts.BearerToken).To(Equal("")) g.Expect(opts.Identity).To(BeEquivalentTo(privateKeyFixture)) g.Expect(opts.KnownHosts).To(BeEquivalentTo(knownHostsFixture)) - g.Expect(opts.CAFile).To(BeEquivalentTo("mock")) + g.Expect(opts.CAFile).To(BeNil()) }, }, { From cbf091cd4e48d5a24f38e0fc88182fd882d9b7d3 Mon Sep 17 00:00:00 2001 From: Christian Ihle Date: Fri, 20 Jan 2023 12:16:09 +0100 Subject: [PATCH 6/8] Add test to verify that username from Secret is preferred Signed-off-by: Christian Ihle --- git/options_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/git/options_test.go b/git/options_test.go index 8d2cc765..c5c26850 100644 --- a/git/options_test.go +++ b/git/options_test.go @@ -294,6 +294,19 @@ func TestAuthOptionsFromData(t *testing.T) { g.Expect(opts.Password).To(Equal("secret")) }, }, + { + name: "Sets username from Secret over username from URL", + URL: "http://example@example.com", + data: map[string][]byte{ + "username": []byte("user"), + "password": []byte("secret"), + }, + + wantFunc: func(g *WithT, opts *AuthOptions) { + g.Expect(opts.Username).To(Equal("user")) + g.Expect(opts.Password).To(Equal("secret")) + }, + }, { name: "Sets username and password from URL only", URL: "https://user:pass@example.com", From 767e77166909059ce767bb212a254ec22671da1c Mon Sep 17 00:00:00 2001 From: Christian Ihle Date: Fri, 20 Jan 2023 12:48:07 +0100 Subject: [PATCH 7/8] Validate that basic auth and bearer token cannot be set at the same time Signed-off-by: Christian Ihle --- git/gogit/client.go | 10 ++++++++++ git/gogit/client_test.go | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/git/gogit/client.go b/git/gogit/client.go index 1577d6ef..8cd8830f 100644 --- a/git/gogit/client.go +++ b/git/gogit/client.go @@ -245,6 +245,16 @@ func (g *Client) validateUrl(u string) error { return fmt.Errorf("cannot parse url: %w", err) } + if g.authOpts != nil { + httpOrHttps := g.authOpts.Transport == git.HTTP || g.authOpts.Transport == git.HTTPS + hasUsernameOrPassword := g.authOpts.Username != "" || g.authOpts.Password != "" + hasBearerToken := g.authOpts.BearerToken != "" + + if httpOrHttps && hasBearerToken && hasUsernameOrPassword { + return errors.New("basic auth and bearer token cannot be set at the same time") + } + } + if g.credentialsOverHTTP { return nil } diff --git a/git/gogit/client_test.go b/git/gogit/client_test.go index c4b7afe6..3323a483 100644 --- a/git/gogit/client_test.go +++ b/git/gogit/client_test.go @@ -708,6 +708,24 @@ func TestValidateUrl(t *testing.T) { bearerToken: "token", url: "https://url", }, + { + name: "blocked: basic auth and bearer token at the same time over http", + transport: git.HTTP, + username: "user", + password: "pass", + bearerToken: "token", + url: "http://url", + expectedError: "basic auth and bearer token cannot be set at the same time", + }, + { + name: "blocked: basic auth and bearer token at the same time over https", + transport: git.HTTPS, + username: "user", + password: "pass", + bearerToken: "token", + url: "https://url", + expectedError: "basic auth and bearer token cannot be set at the same time", + }, } for _, tt := range tests { From 659695fabb64da9acbd2738ec83c672185a8e550 Mon Sep 17 00:00:00 2001 From: Christian Ihle Date: Fri, 20 Jan 2023 13:56:00 +0100 Subject: [PATCH 8/8] Add back support for passphrase protected ssh keys Signed-off-by: Christian Ihle --- git/options.go | 1 + git/options_test.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/git/options.go b/git/options.go index 33429973..ea3653ae 100644 --- a/git/options.go +++ b/git/options.go @@ -91,6 +91,7 @@ func NewAuthOptions(u url.URL, data map[string][]byte) (*AuthOptions, error) { opts.Identity = data["identity"] opts.KnownHosts = data["known_hosts"] opts.Username = u.User.Username() + opts.Password = string(data["password"]) // We fallback to using "git" as the username when cloning Git // repositories through SSH since that's the conventional username used // by Git providers. diff --git a/git/options_test.go b/git/options_test.go index c5c26850..aa9e05d9 100644 --- a/git/options_test.go +++ b/git/options_test.go @@ -235,7 +235,7 @@ func TestAuthOptionsFromData(t *testing.T) { wantFunc: func(g *WithT, opts *AuthOptions) { g.Expect(opts.Username).To(Equal(DefaultPublicKeyAuthUser)) // Not the specified username - g.Expect(opts.Password).To(Equal("")) + g.Expect(opts.Password).To(Equal("secret")) // For passphrase protected ssh key g.Expect(opts.BearerToken).To(Equal("")) g.Expect(opts.Identity).To(BeEquivalentTo(privateKeyFixture)) g.Expect(opts.KnownHosts).To(BeEquivalentTo(knownHostsFixture))