From 2f795e30d253e6e4d1dc4fd61c9735b50fd23280 Mon Sep 17 00:00:00 2001 From: kiootic Date: Thu, 30 Nov 2023 13:59:22 +0800 Subject: [PATCH] Normalize user/repository for comparison --- internal/models/credential_id.go | 11 ++-- internal/models/credential_id_test.go | 65 ++++++++++++++++++++++++ internal/models/credential_index_test.go | 55 +++++++++----------- internal/sshkey/github_keys.go | 3 +- 4 files changed, 99 insertions(+), 35 deletions(-) create mode 100644 internal/models/credential_id_test.go diff --git a/internal/models/credential_id.go b/internal/models/credential_id.go index 162bfd7..5daada4 100644 --- a/internal/models/credential_id.go +++ b/internal/models/credential_id.go @@ -44,14 +44,17 @@ func (c CredentialID) Matches(r *config.ACLSubjectRule) bool { case CredentialIDKindUserID: return r.PageshipUser != "" && r.PageshipUser == data case CredentialIDKindGitHubUser: - return r.GitHubUser != "" && r.GitHubUser == data + return r.GitHubUser != "" && strings.EqualFold(r.GitHubUser, data) case CredentialIDGitHubRepositoryActions: - if r.GitHubRepositoryActions == "*" || r.GitHubRepositoryActions == data { + if r.GitHubRepositoryActions == "*" { + return true + } + if strings.EqualFold(r.GitHubRepositoryActions, data) { return true } - repoOwner, _, ok := strings.Cut(data, "/") - return ok && r.GitHubRepositoryActions == repoOwner+"/*" + repoOwner, _, ok := strings.Cut(strings.ToLower(data), "/") + return ok && strings.ToLower(r.GitHubRepositoryActions) == repoOwner+"/*" case CredentialIDIP: if r.IpRange == "" { return false diff --git a/internal/models/credential_id_test.go b/internal/models/credential_id_test.go new file mode 100644 index 0000000..92d567d --- /dev/null +++ b/internal/models/credential_id_test.go @@ -0,0 +1,65 @@ +package models_test + +import ( + "testing" + + "github.com/oursky/pageship/internal/config" + "github.com/oursky/pageship/internal/models" + "github.com/stretchr/testify/assert" +) + +func matchRule(rule *config.ACLSubjectRule, id models.CredentialID) bool { + return id.Matches(rule) +} + +func TestGitHubUserCredentials(t *testing.T) { + assert.True(t, matchRule( + &config.ACLSubjectRule{GitHubUser: "example"}, + models.CredentialGitHubUser("example"), + )) + assert.False(t, matchRule( + &config.ACLSubjectRule{GitHubUser: "example"}, + models.CredentialGitHubUser("foobar"), + )) + assert.True(t, matchRule( + &config.ACLSubjectRule{GitHubUser: "example"}, + models.CredentialGitHubUser("Example"), + )) + assert.True(t, matchRule( + &config.ACLSubjectRule{GitHubUser: "Example"}, + models.CredentialGitHubUser("example"), + )) +} + +func TestGitHubActionsCredentials(t *testing.T) { + assert.True(t, matchRule( + &config.ACLSubjectRule{GitHubRepositoryActions: "*"}, + models.CredentialGitHubRepositoryActions("oursky/pageship"), + )) + + assert.True(t, matchRule( + &config.ACLSubjectRule{GitHubRepositoryActions: "oursky/*"}, + models.CredentialGitHubRepositoryActions("oursky/pageship"), + )) + assert.True(t, matchRule( + &config.ACLSubjectRule{GitHubRepositoryActions: "oursky/*"}, + models.CredentialGitHubRepositoryActions("Oursky/Pageship"), + )) + assert.False(t, matchRule( + &config.ACLSubjectRule{GitHubRepositoryActions: "oursky/*"}, + models.CredentialGitHubRepositoryActions("github/example"), + )) + + assert.True(t, matchRule( + &config.ACLSubjectRule{GitHubRepositoryActions: "oursky/pageship"}, + models.CredentialGitHubRepositoryActions("oursky/pageship"), + )) + assert.True(t, matchRule( + &config.ACLSubjectRule{GitHubRepositoryActions: "oursky/pageship"}, + models.CredentialGitHubRepositoryActions("Oursky/Pageship"), + )) + assert.False(t, matchRule( + &config.ACLSubjectRule{GitHubRepositoryActions: "oursky/pageship"}, + models.CredentialGitHubRepositoryActions("Oursky/Example"), + )) +} diff --git a/internal/models/credential_index_test.go b/internal/models/credential_index_test.go index a950c86..bf65553 100644 --- a/internal/models/credential_index_test.go +++ b/internal/models/credential_index_test.go @@ -7,10 +7,9 @@ import ( "github.com/oursky/pageship/internal/config" "github.com/oursky/pageship/internal/models" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/suite" ) -func matchRule(rule *config.ACLSubjectRule, id models.CredentialID) bool { +func matchIndex(rule *config.ACLSubjectRule, id models.CredentialID) bool { index := make(map[string]struct{}) for _, key := range models.MakeCredentialRuleIndexKeys(rule) { index[string(key)] = struct{}{} @@ -24,101 +23,97 @@ func matchRule(rule *config.ACLSubjectRule, id models.CredentialID) bool { return false } -type GitHubActionsCredentialsTestSuite struct { - suite.Suite -} - -func TestGitHubActionsCredentials(t *testing.T) { - assert.True(t, matchRule( +func TestGitHubActionsCredentialsIndex(t *testing.T) { + assert.True(t, matchIndex( &config.ACLSubjectRule{GitHubRepositoryActions: "oursky/pageship"}, models.CredentialGitHubRepositoryActions("oursky/pageship"), )) - assert.False(t, matchRule( + assert.False(t, matchIndex( &config.ACLSubjectRule{GitHubRepositoryActions: "oursky/pageship"}, models.CredentialGitHubRepositoryActions("oursky/other"), )) - assert.False(t, matchRule( + assert.False(t, matchIndex( &config.ACLSubjectRule{GitHubRepositoryActions: "oursky/pageship"}, models.CredentialGitHubRepositoryActions("other/pageship"), )) - assert.True(t, matchRule( + assert.True(t, matchIndex( &config.ACLSubjectRule{GitHubRepositoryActions: "oursky/*"}, models.CredentialGitHubRepositoryActions("oursky/pageship"), )) - assert.True(t, matchRule( + assert.True(t, matchIndex( &config.ACLSubjectRule{GitHubRepositoryActions: "oursky/*"}, models.CredentialGitHubRepositoryActions("oursky/other"), )) - assert.False(t, matchRule( + assert.False(t, matchIndex( &config.ACLSubjectRule{GitHubRepositoryActions: "oursky/*"}, models.CredentialGitHubRepositoryActions("other/oursky"), )) - assert.True(t, matchRule( + assert.True(t, matchIndex( &config.ACLSubjectRule{GitHubRepositoryActions: "*"}, models.CredentialGitHubRepositoryActions("oursky/pageship"), )) - assert.True(t, matchRule( + assert.True(t, matchIndex( &config.ACLSubjectRule{GitHubRepositoryActions: "*"}, models.CredentialGitHubRepositoryActions("oursky/other"), )) - assert.True(t, matchRule( + assert.True(t, matchIndex( &config.ACLSubjectRule{GitHubRepositoryActions: "*"}, models.CredentialGitHubRepositoryActions("other/oursky"), )) } -func TestGitHubUserCredentials(t *testing.T) { - assert.True(t, matchRule( +func TestGitHubUserCredentialsIndex(t *testing.T) { + assert.True(t, matchIndex( &config.ACLSubjectRule{GitHubUser: "oursky"}, models.CredentialGitHubUser("oursky"), )) - assert.False(t, matchRule( + assert.False(t, matchIndex( &config.ACLSubjectRule{GitHubUser: "oursky"}, models.CredentialGitHubUser("other"), )) } -func TestIPCredentials(t *testing.T) { - assert.True(t, matchRule( +func TestIPCredentialsIndex(t *testing.T) { + assert.True(t, matchIndex( &config.ACLSubjectRule{IpRange: "127.0.0.1/32"}, models.CredentialIP("127.0.0.1"), )) - assert.False(t, matchRule( + assert.False(t, matchIndex( &config.ACLSubjectRule{IpRange: "127.0.0.1/32"}, models.CredentialIP("127.0.0.2"), )) - assert.True(t, matchRule( + assert.True(t, matchIndex( &config.ACLSubjectRule{IpRange: "127.0.0.1/16"}, models.CredentialIP("127.0.0.1"), )) - assert.True(t, matchRule( + assert.True(t, matchIndex( &config.ACLSubjectRule{IpRange: "127.0.0.1/16"}, models.CredentialIP("127.0.100.2"), )) - assert.True(t, matchRule( + assert.True(t, matchIndex( &config.ACLSubjectRule{IpRange: "2001:db8::/48"}, models.CredentialIP("2001:db8::1"), )) - assert.False(t, matchRule( + assert.False(t, matchIndex( &config.ACLSubjectRule{IpRange: "2001:db8::/48"}, models.CredentialIP("::1"), )) - assert.True(t, matchRule( + assert.True(t, matchIndex( &config.ACLSubjectRule{IpRange: "::/0"}, models.CredentialIP("192.168.0.1"), )) - assert.True(t, matchRule( + assert.True(t, matchIndex( &config.ACLSubjectRule{IpRange: "::ffff:192.168.0.1/120"}, models.CredentialIP("192.168.0.255"), )) } -func FuzzIPCredentials(f *testing.F) { +func FuzzIPCredentialsIndex(f *testing.F) { add := func(cidr string, ip string) { prefix := netip.MustParsePrefix(cidr) f.Add(prefix.Addr().AsSlice(), prefix.Bits(), netip.MustParseAddr(ip).AsSlice()) @@ -151,7 +146,7 @@ func FuzzIPCredentials(f *testing.F) { rule := &config.ACLSubjectRule{IpRange: ruleCIDR.String()} cred := models.CredentialIP(credIP.String()) - assert.True(t, matchRule( + assert.True(t, matchIndex( &config.ACLSubjectRule{IpRange: ruleCIDR.String()}, models.CredentialIP(credIP.String()), ), "range=%s;ip=%s;range_keys=%+v;cred_keys=%+v", diff --git a/internal/sshkey/github_keys.go b/internal/sshkey/github_keys.go index 6079379..a33e173 100644 --- a/internal/sshkey/github_keys.go +++ b/internal/sshkey/github_keys.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "net/url" + "strings" "time" "github.com/oursky/pageship/internal/cache" @@ -37,7 +38,7 @@ func NewGitHubKeys(ctx context.Context) (*GitHubKeys, error) { } func (g *GitHubKeys) PublicKey(username string) (map[string]struct{}, error) { - pkeys, err := g.cache.Load(username) + pkeys, err := g.cache.Load(strings.ToLower(username)) if err != nil { return nil, err }