From 8e8fc696f13ce3396eabb92471bc4ae71a86274e Mon Sep 17 00:00:00 2001 From: Brian DeHamer Date: Thu, 12 Sep 2024 09:20:02 -0700 Subject: [PATCH 1/7] disable auth check for att trusted-root cmd Signed-off-by: Brian DeHamer --- pkg/cmd/attestation/trustedroot/trustedroot.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/cmd/attestation/trustedroot/trustedroot.go b/pkg/cmd/attestation/trustedroot/trustedroot.go index c9c3fdb04..a79c32ddb 100644 --- a/pkg/cmd/attestation/trustedroot/trustedroot.go +++ b/pkg/cmd/attestation/trustedroot/trustedroot.go @@ -94,6 +94,7 @@ func NewTrustedRootCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Com }, } + cmdutil.DisableAuthCheck(&trustedRootCmd) trustedRootCmd.Flags().StringVarP(&opts.TufUrl, "tuf-url", "", "", "URL to the TUF repository mirror") trustedRootCmd.Flags().StringVarP(&opts.TufRootPath, "tuf-root", "", "", "Path to the TUF root.json file on disk") trustedRootCmd.MarkFlagsRequiredTogether("tuf-url", "tuf-root") From cbe85253214d1ff3ce3c4fe707d55e99fe2adfaf Mon Sep 17 00:00:00 2001 From: Brian DeHamer Date: Fri, 13 Sep 2024 15:26:14 -0700 Subject: [PATCH 2/7] enforce auth for tenancy Signed-off-by: Brian DeHamer --- .../attestation/trustedroot/trustedroot.go | 9 ++ .../trustedroot/trustedroot_test.go | 97 +++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/pkg/cmd/attestation/trustedroot/trustedroot.go b/pkg/cmd/attestation/trustedroot/trustedroot.go index a79c32ddb..7dba916eb 100644 --- a/pkg/cmd/attestation/trustedroot/trustedroot.go +++ b/pkg/cmd/attestation/trustedroot/trustedroot.go @@ -69,6 +69,15 @@ func NewTrustedRootCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Com } if ghinstance.IsTenancy(opts.Hostname) { + c, err := f.Config() + if err != nil { + return err + } + + if token, _ := c.Authentication().ActiveToken(opts.Hostname); token == "" { + return fmt.Errorf("not authenticated with %s", opts.Hostname) + } + hc, err := f.HttpClient() if err != nil { return err diff --git a/pkg/cmd/attestation/trustedroot/trustedroot_test.go b/pkg/cmd/attestation/trustedroot/trustedroot_test.go index 70b5ae2a1..b7d5f6c2f 100644 --- a/pkg/cmd/attestation/trustedroot/trustedroot_test.go +++ b/pkg/cmd/attestation/trustedroot/trustedroot_test.go @@ -3,6 +3,7 @@ package trustedroot import ( "bytes" "fmt" + "net/http" "strings" "testing" @@ -10,8 +11,13 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/gh" + ghmock "github.com/cli/cli/v2/internal/gh/mock" + "github.com/cli/cli/v2/pkg/cmd/attestation/api" "github.com/cli/cli/v2/pkg/cmd/attestation/test" "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" ) @@ -19,6 +25,9 @@ func TestNewTrustedRootCmd(t *testing.T) { testIO, _, _, _ := iostreams.Test() f := &cmdutil.Factory{ IOStreams: testIO, + Config: func() (gh.Config, error) { + return &ghmock.ConfigMock{}, nil + }, } testcases := []struct { @@ -72,6 +81,83 @@ func TestNewTrustedRootCmd(t *testing.T) { } } +func TestNewTrustedRootWithTenancy(t *testing.T) { + testIO, _, _, _ := iostreams.Test() + var testReg httpmock.Registry + var metaResp = api.MetaResponse{ + Domains: api.Domain{ + ArtifactAttestations: api.ArtifactAttestations{ + TrustDomain: "foo", + }, + }, + } + testReg.Register(httpmock.REST(http.MethodGet, "meta"), + httpmock.StatusJSONResponse(200, &metaResp)) + + httpClientFunc := func() (*http.Client, error) { + reg := &testReg + client := &http.Client{} + httpmock.ReplaceTripper(client, reg) + return client, nil + } + + cli := "--hostname foo-bar.ghe.com" + + t.Run("Host with NO auth configured", func(t *testing.T) { + f := &cmdutil.Factory{ + IOStreams: testIO, + Config: func() (gh.Config, error) { + return &ghmock.ConfigMock{ + AuthenticationFunc: func() gh.AuthConfig { + return &MockAuthConfig{Token: ""} + }, + }, nil + }, + } + + cmd := NewTrustedRootCmd(f, func(_ *Options) error { + return nil + }) + + argv := strings.Split(cli, " ") + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + _, err := cmd.ExecuteC() + + assert.Error(t, err) + assert.ErrorContains(t, err, "not authenticated") + }) + + t.Run("Host wth auth configured", func(t *testing.T) { + f := &cmdutil.Factory{ + IOStreams: testIO, + Config: func() (gh.Config, error) { + return &ghmock.ConfigMock{ + AuthenticationFunc: func() gh.AuthConfig { + return &MockAuthConfig{Token: "TOKEN"} + }, + }, nil + }, + HttpClient: httpClientFunc, + } + + cmd := NewTrustedRootCmd(f, func(_ *Options) error { + return nil + }) + + argv := strings.Split(cli, " ") + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err := cmd.ExecuteC() + assert.NoError(t, err) + }) +} + var newTUFErrClient tufClientInstantiator = func(o *tuf.Options) (*tuf.Client, error) { return nil, fmt.Errorf("failed to create TUF client") } @@ -99,3 +185,14 @@ func TestGetTrustedRoot(t *testing.T) { }) } + +type MockAuthConfig struct { + config.AuthConfig + Token string +} + +var _ gh.AuthConfig = (*MockAuthConfig)(nil) + +func (c *MockAuthConfig) ActiveToken(host string) (string, string) { + return c.Token, "" +} From 3bcedfe7f071904af24b0d9096980e53844ca42a Mon Sep 17 00:00:00 2001 From: Brian DeHamer Date: Tue, 17 Sep 2024 14:18:00 -0700 Subject: [PATCH 3/7] Update pkg/cmd/attestation/trustedroot/trustedroot_test.go Co-authored-by: Fredrik Skogman --- pkg/cmd/attestation/trustedroot/trustedroot_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/attestation/trustedroot/trustedroot_test.go b/pkg/cmd/attestation/trustedroot/trustedroot_test.go index b7d5f6c2f..c3c1818dd 100644 --- a/pkg/cmd/attestation/trustedroot/trustedroot_test.go +++ b/pkg/cmd/attestation/trustedroot/trustedroot_test.go @@ -130,7 +130,7 @@ func TestNewTrustedRootWithTenancy(t *testing.T) { assert.ErrorContains(t, err, "not authenticated") }) - t.Run("Host wth auth configured", func(t *testing.T) { + t.Run("Host with auth configured", func(t *testing.T) { f := &cmdutil.Factory{ IOStreams: testIO, Config: func() (gh.Config, error) { From e7403b89d0c0be36dacae29f61cb040525ba7246 Mon Sep 17 00:00:00 2001 From: bagtoad <47394200+BagToad@users.noreply.github.com> Date: Wed, 18 Sep 2024 08:42:19 -0600 Subject: [PATCH 4/7] Add HasActiveToken to AuthConfig. Co-authored-by: William Martin --- internal/config/auth_config_test.go | 26 +++++++++++++++++++++++++- internal/config/config.go | 7 +++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/internal/config/auth_config_test.go b/internal/config/auth_config_test.go index ed000ff18..61245c650 100644 --- a/internal/config/auth_config_test.go +++ b/internal/config/auth_config_test.go @@ -52,8 +52,32 @@ func TestTokenFromKeyringForUserErrorsIfUsernameIsBlank(t *testing.T) { require.ErrorContains(t, err, "username cannot be blank") } +func TestHasActiveToken(t *testing.T) { + // Given the user has logged in for a host + authCfg := newTestAuthConfig(t) + _, err := authCfg.Login("github.com", "test-user", "test-token", "", false) + require.NoError(t, err) + + // When we check if that host has an active token + hasActiveToken := authCfg.HasActiveToken("github.com") + + // Then there is an active token + require.True(t, hasActiveToken, "expected there to be an active token") +} + +func TestHasNoActiveToken(t *testing.T) { + // Given there are no users logged in for a host + authCfg := newTestAuthConfig(t) + + // When we check if any host has an active token + hasActiveToken := authCfg.HasActiveToken("github.com") + + // Then there is no active token + require.False(t, hasActiveToken, "expected there to be no active token") +} + func TestTokenStoredInConfig(t *testing.T) { - // When the user has logged in insecurely + // Given the user has logged in insecurely authCfg := newTestAuthConfig(t) _, err := authCfg.Login("github.com", "test-user", "test-token", "", false) require.NoError(t, err) diff --git a/internal/config/config.go b/internal/config/config.go index 29b66b73b..f7c949a46 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -217,6 +217,13 @@ func (c *AuthConfig) ActiveToken(hostname string) (string, string) { return token, source } +// HasActiveToken returns true when a token for the hostname is +// present. +func (c *AuthConfig) HasActiveToken(hostname string) bool { + token, _ := c.ActiveToken(hostname) + return token != "" +} + // HasEnvToken returns true when a token has been specified in an // environment variable, else returns false. func (c *AuthConfig) HasEnvToken() bool { From 88d48f23654838ea7428ed47ed28f64f65405f6a Mon Sep 17 00:00:00 2001 From: bagtoad <47394200+BagToad@users.noreply.github.com> Date: Wed, 18 Sep 2024 10:32:58 -0600 Subject: [PATCH 5/7] Add HasActiveToken method to AuthConfig interface --- internal/gh/gh.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/gh/gh.go b/internal/gh/gh.go index c39734075..e4431fdab 100644 --- a/internal/gh/gh.go +++ b/internal/gh/gh.go @@ -93,6 +93,9 @@ type Migration interface { // with knowledge on how to access encrypted storage when neccesarry. // Behavior is scoped to authentication specific tasks. type AuthConfig interface { + // HasActiveToken returns true when a token for the hostname is present. + HasActiveToken(hostname string) bool + // ActiveToken will retrieve the active auth token for the given hostname, searching environment variables, // general configuration, and finally encrypted storage. ActiveToken(hostname string) (token string, source string) From d8e77d256fe83791cf675e0ff539e1390ffd98d5 Mon Sep 17 00:00:00 2001 From: bagtoad <47394200+BagToad@users.noreply.github.com> Date: Wed, 18 Sep 2024 10:35:11 -0600 Subject: [PATCH 6/7] Use new HasActiveToken method in trustedroot.go --- pkg/cmd/attestation/trustedroot/trustedroot.go | 2 +- .../attestation/trustedroot/trustedroot_test.go | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/attestation/trustedroot/trustedroot.go b/pkg/cmd/attestation/trustedroot/trustedroot.go index 7dba916eb..6f741dcd4 100644 --- a/pkg/cmd/attestation/trustedroot/trustedroot.go +++ b/pkg/cmd/attestation/trustedroot/trustedroot.go @@ -74,7 +74,7 @@ func NewTrustedRootCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Com return err } - if token, _ := c.Authentication().ActiveToken(opts.Hostname); token == "" { + if !c.Authentication().HasActiveToken(opts.Hostname) { return fmt.Errorf("not authenticated with %s", opts.Hostname) } diff --git a/pkg/cmd/attestation/trustedroot/trustedroot_test.go b/pkg/cmd/attestation/trustedroot/trustedroot_test.go index c3c1818dd..c4a259436 100644 --- a/pkg/cmd/attestation/trustedroot/trustedroot_test.go +++ b/pkg/cmd/attestation/trustedroot/trustedroot_test.go @@ -109,7 +109,7 @@ func TestNewTrustedRootWithTenancy(t *testing.T) { Config: func() (gh.Config, error) { return &ghmock.ConfigMock{ AuthenticationFunc: func() gh.AuthConfig { - return &MockAuthConfig{Token: ""} + return &stubAuthConfig{hasActiveToken: false} }, }, nil }, @@ -136,7 +136,7 @@ func TestNewTrustedRootWithTenancy(t *testing.T) { Config: func() (gh.Config, error) { return &ghmock.ConfigMock{ AuthenticationFunc: func() gh.AuthConfig { - return &MockAuthConfig{Token: "TOKEN"} + return &stubAuthConfig{hasActiveToken: true} }, }, nil }, @@ -186,13 +186,13 @@ func TestGetTrustedRoot(t *testing.T) { } -type MockAuthConfig struct { +type stubAuthConfig struct { config.AuthConfig - Token string + hasActiveToken bool } -var _ gh.AuthConfig = (*MockAuthConfig)(nil) +var _ gh.AuthConfig = (*stubAuthConfig)(nil) -func (c *MockAuthConfig) ActiveToken(host string) (string, string) { - return c.Token, "" +func (c *stubAuthConfig) HasActiveToken(host string) bool { + return c.hasActiveToken } From d24dfbeacfdc87ea108835ee79e9f766fd75f639 Mon Sep 17 00:00:00 2001 From: bagtoad <47394200+BagToad@users.noreply.github.com> Date: Wed, 18 Sep 2024 10:35:35 -0600 Subject: [PATCH 7/7] Update comment formatting --- internal/config/config.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index f7c949a46..1b56d30b2 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -217,8 +217,7 @@ func (c *AuthConfig) ActiveToken(hostname string) (string, string) { return token, source } -// HasActiveToken returns true when a token for the hostname is -// present. +// HasActiveToken returns true when a token for the hostname is present. func (c *AuthConfig) HasActiveToken(hostname string) bool { token, _ := c.ActiveToken(hostname) return token != ""