diff --git a/pkg/cli/gettoken/gettoken.go b/pkg/cli/gettoken/gettoken.go index 55022a5bb4..c60555bc41 100644 --- a/pkg/cli/gettoken/gettoken.go +++ b/pkg/cli/gettoken/gettoken.go @@ -144,8 +144,9 @@ func (o *GetTokenOptions) Validate() error { // external OIDC issuer. If not, it forces user to log in. func (o *GetTokenOptions) Run() error { tokenCacheKey := tokencache.Key{ - IssuerURL: o.IssuerURL, - ClientID: o.ClientID, + IssuerURL: o.IssuerURL, + ClientID: o.ClientID, + ExtraScopes: o.ExtraScopes, } // Ignoring the error because if there is any error occurred diff --git a/pkg/cli/gettoken/tokencache/tokencache.go b/pkg/cli/gettoken/tokencache/tokencache.go index 39045e8bfb..6f73262a6d 100644 --- a/pkg/cli/gettoken/tokencache/tokencache.go +++ b/pkg/cli/gettoken/tokencache/tokencache.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "path/filepath" + "sort" ) // Set stores the id token and the refresh token @@ -23,6 +24,8 @@ type Set struct { type Key struct { IssuerURL string ClientID string + // ExtraScopes are sorted internally by computeFilename to ensure deterministic cache keys. + ExtraScopes []string } // tokenCacheEntity is a internal structure for the in-memory token @@ -87,6 +90,13 @@ func (r *Repository) Save(dir string, key Key, tokenSet Set) error { } func computeFilename(key Key) (string, error) { + if key.ExtraScopes != nil { + sorted := make([]string, len(key.ExtraScopes)) + copy(sorted, key.ExtraScopes) + sort.Strings(sorted) + key.ExtraScopes = sorted + } + s := sha256.New() e := gob.NewEncoder(s) if err := e.Encode(&key); err != nil { diff --git a/pkg/cli/gettoken/tokencache/tokencache_test.go b/pkg/cli/gettoken/tokencache/tokencache_test.go new file mode 100644 index 0000000000..b2d3a1d69f --- /dev/null +++ b/pkg/cli/gettoken/tokencache/tokencache_test.go @@ -0,0 +1,140 @@ +package tokencache + +import ( + "os" + "path/filepath" + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestComputeFilename(t *testing.T) { + tests := []struct { + name string + k1 Key + k2 Key + wantEqual bool + }{ + { + name: "same keys without scopes produce same filename", + k1: Key{IssuerURL: "https://issuer.example.com", ClientID: "my-client"}, + k2: Key{IssuerURL: "https://issuer.example.com", ClientID: "my-client"}, + wantEqual: true, + }, + { + name: "identical sorted scopes produce same filename", + k1: Key{IssuerURL: "https://issuer.example.com", ClientID: "my-client", ExtraScopes: []string{"email", "profile"}}, + k2: Key{IssuerURL: "https://issuer.example.com", ClientID: "my-client", ExtraScopes: []string{"email", "profile"}}, + wantEqual: true, + }, + { + name: "presence of scopes changes filename", + k1: Key{IssuerURL: "https://issuer.example.com", ClientID: "my-client"}, + k2: Key{IssuerURL: "https://issuer.example.com", ClientID: "my-client", ExtraScopes: []string{"email", "profile"}}, + wantEqual: false, + }, + { + name: "differently ordered scopes produce same filename", + k1: Key{IssuerURL: "https://issuer.example.com", ClientID: "my-client", ExtraScopes: []string{"email", "profile"}}, + k2: Key{IssuerURL: "https://issuer.example.com", ClientID: "my-client", ExtraScopes: []string{"profile", "email"}}, + wantEqual: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f1, err := computeFilename(tt.k1) + if err != nil { + t.Fatalf("computeFilename(k1) error: %v", err) + } + f2, err := computeFilename(tt.k2) + if err != nil { + t.Fatalf("computeFilename(k2) error: %v", err) + } + + if tt.wantEqual && f1 != f2 { + t.Errorf("expected same filename, got %s and %s", f1, f2) + } + if !tt.wantEqual && f1 == f2 { + t.Errorf("expected different filenames, got %s for both", f1) + } + }) + } +} + +func TestSaveAndFindByKey(t *testing.T) { + dir := t.TempDir() + repo := &Repository{} + + key := Key{ + IssuerURL: "https://issuer.example.com", + ClientID: "my-client", + ExtraScopes: []string{"email", "profile"}, + } + want := Set{ + IDToken: "test-id-token", + RefreshToken: "test-refresh-token", + } + + if err := repo.Save(dir, key, want); err != nil { + t.Fatalf("Save() error: %v", err) + } + + got, err := repo.FindByKey(dir, key) + if err != nil { + t.Fatalf("FindByKey() error: %v", err) + } + + if diff := cmp.Diff(&want, got); diff != "" { + t.Errorf("FindByKey() mismatch (-want +got):\n%s", diff) + } +} + +func TestFindByKey_DifferentScopesMiss(t *testing.T) { + dir := t.TempDir() + repo := &Repository{} + + key := Key{ + IssuerURL: "https://issuer.example.com", + ClientID: "my-client", + ExtraScopes: []string{"email"}, + } + if err := repo.Save(dir, key, Set{IDToken: "token1", RefreshToken: "refresh1"}); err != nil { + t.Fatalf("Save() error: %v", err) + } + + differentKey := Key{ + IssuerURL: "https://issuer.example.com", + ClientID: "my-client", + ExtraScopes: []string{"email", "profile"}, + } + _, err := repo.FindByKey(dir, differentKey) + if err == nil { + t.Fatal("expected FindByKey to fail for a key with different scopes, but it succeeded") + } + if !os.IsNotExist(err) { + t.Errorf("expected file-not-found error, got: %v", err) + } +} + +func TestSave_FilePermissions(t *testing.T) { + dir := t.TempDir() + repo := &Repository{} + + key := Key{IssuerURL: "https://issuer.example.com", ClientID: "my-client"} + if err := repo.Save(dir, key, Set{IDToken: "tok"}); err != nil { + t.Fatalf("Save() error: %v", err) + } + + filename, err := computeFilename(key) + if err != nil { + t.Fatalf("computeFilename() error: %v", err) + } + info, err := os.Stat(filepath.Join(dir, filename)) + if err != nil { + t.Fatalf("Stat() error: %v", err) + } + if perm := info.Mode().Perm(); perm != 0600 { + t.Errorf("expected file permission 0600, got %o", perm) + } +}