From fe068185f5b1304ee1a2af937f12aa40b48a3c7a Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Tue, 11 Apr 2023 20:23:22 +0800 Subject: [PATCH 01/34] draft dynamic store Signed-off-by: Sylvia Lei --- dynamic_store.go | 120 ++++++++++++++++++++++++++++++++++++++ dynamic_store_test.go | 115 ++++++++++++++++++++++++++++++++++++ testdata/auth_config.json | 29 +++++++++ 3 files changed, 264 insertions(+) create mode 100644 dynamic_store.go create mode 100644 dynamic_store_test.go create mode 100644 testdata/auth_config.json diff --git a/dynamic_store.go b/dynamic_store.go new file mode 100644 index 0000000..41092e8 --- /dev/null +++ b/dynamic_store.go @@ -0,0 +1,120 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package credentials + +import ( + "context" + "encoding/json" + "fmt" + "os" + + "oras.land/oras-go/v2/registry/remote/auth" +) + +// StoreOptions provides options for NewStore. +type StoreOptions struct { + // AllowPlainText allows saving credentials in plain text in configuration file. + AllowPlainText bool +} + +const ( + configFieldCredStore = "credsStore" + configFieldCredHelpers = "credHelpers" +) + +type dynamicStore struct { + configPath string + credStore string + credHelpers map[string]string + fileStore *FileStore +} + +// TODO: when to use default store? +func NewStore(configPath string, opts StoreOptions) (Store, error) { + ds := &dynamicStore{ + configPath: configPath, + } + + configFile, err := os.Open(configPath) + if err != nil { + if os.IsNotExist(err) { + // TODO: no error? + return ds, nil + } + return nil, fmt.Errorf("failed to open config file at %s: %w", configPath, err) + } + defer configFile.Close() + + // init file store + ds.fileStore, err = NewFileStore(ds.configPath) + if err != nil { + return nil, err + } + if !opts.AllowPlainText { + ds.fileStore.DisableSave = true + } + + var content map[string]json.RawMessage + if err := json.NewDecoder(configFile).Decode(&content); err != nil { + return nil, fmt.Errorf("failed to decode config file at %s: %w: %v", configPath, ErrInvalidConfigFormat, err) + } + ds.credStore = string(content[configFieldCredStore]) + credHelperBytes, ok := content[configFieldCredHelpers] + if !ok { + return ds, nil + } + if err := json.Unmarshal(credHelperBytes, &ds.credHelpers); err != nil { + return nil, fmt.Errorf("failed to decode config file at %s: %w: %v", configPath, ErrInvalidConfigFormat, err) + } + return ds, nil +} + +// Get retrieves credentials from the store for the given server address. +func (ds *dynamicStore) Get(ctx context.Context, serverAddress string) (auth.Credential, error) { + store, err := ds.getStore(serverAddress) + if err != nil { + return auth.EmptyCredential, nil + } + return store.Get(ctx, serverAddress) +} + +// Put saves credentials into the store for the given server address. +func (ds *dynamicStore) Put(ctx context.Context, serverAddress string, cred auth.Credential) error { + store, err := ds.getStore(serverAddress) + if err != nil { + return err + } + return store.Put(ctx, serverAddress, cred) +} + +// Delete removes credentials from the store for the given server address. +func (ds *dynamicStore) Delete(ctx context.Context, serverAddress string) error { + store, err := ds.getStore(serverAddress) + if err != nil { + return err + } + return store.Delete(ctx, serverAddress) +} + +func (ds *dynamicStore) getStore(serverAddress string) (Store, error) { + if helper, ok := ds.credHelpers[serverAddress]; ok { + return NewNativeStore(helper), nil + } + if ds.credStore != "" { + return NewNativeStore(ds.credStore), nil + } + return ds.fileStore, nil +} diff --git a/dynamic_store_test.go b/dynamic_store_test.go new file mode 100644 index 0000000..0bbf0d0 --- /dev/null +++ b/dynamic_store_test.go @@ -0,0 +1,115 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package credentials + +import ( + "context" + "reflect" + "testing" + + "oras.land/oras-go/v2/registry/remote/auth" +) + +func TestDynamicStore_Get(t *testing.T) { + ctx := context.Background() + ds, err := NewStore("testdata/auth_config.json", StoreOptions{}) + if err != nil { + t.Fatalf("NewDynamicStore() error = %v", err) + } + + tests := []struct { + name string + serverAddress string + want auth.Credential + wantErr bool + }{ + { + name: "Username and password", + serverAddress: "registry1.example.com", + want: auth.Credential{ + Username: "username", + Password: "password", + }, + }, + { + name: "Identity token", + serverAddress: "registry2.example.com", + want: auth.Credential{ + RefreshToken: "identity_token", + }, + }, + { + name: "Registry token", + serverAddress: "registry3.example.com", + want: auth.Credential{ + AccessToken: "registry_token", + }, + }, + { + name: "Username and password, identity token and registry token", + serverAddress: "registry4.example.com", + want: auth.Credential{ + Username: "username", + Password: "password", + RefreshToken: "identity_token", + AccessToken: "registry_token", + }, + }, + { + name: "Empty credential", + serverAddress: "registry5.example.com", + want: auth.EmptyCredential, + }, + { + name: "Username and password, no auth", + serverAddress: "registry6.example.com", + want: auth.Credential{ + Username: "username", + Password: "password", + }, + }, + { + name: "Auth overriding Username and password", + serverAddress: "registry7.example.com", + want: auth.Credential{ + Username: "username", + Password: "password", + }, + }, + { + name: "Not in auths", + serverAddress: "foo.example.com", + want: auth.EmptyCredential, + }, + { + name: "No record", + serverAddress: "registry999.example.com", + want: auth.EmptyCredential, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ds.Get(ctx, tt.serverAddress) + if (err != nil) != tt.wantErr { + t.Errorf("DynamicStore.Get() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("DynamicStore.Get() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/testdata/auth_config.json b/testdata/auth_config.json new file mode 100644 index 0000000..faa294b --- /dev/null +++ b/testdata/auth_config.json @@ -0,0 +1,29 @@ +{ + "auths": { + "registry1.example.com": { + "auth": "dXNlcm5hbWU6cGFzc3dvcmQ=" + }, + "registry2.example.com": { + "identitytoken": "identity_token" + }, + "registry3.example.com": { + "registrytoken": "registry_token" + }, + "registry4.example.com": { + "auth": "dXNlcm5hbWU6cGFzc3dvcmQ=", + "identitytoken": "identity_token", + "registrytoken": "registry_token" + }, + "registry5.example.com": {}, + "registry6.example.com": { + "username": "username", + "password": "password" + }, + "registry7.example.com": { + "auth": "dXNlcm5hbWU6cGFzc3dvcmQ=", + "username": "foo", + "password": "bar" + } + }, + "key": "val" +} From 8b441cc66d6800ee9a8e56c771dc9c8ea49258ac Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Wed, 12 Apr 2023 16:51:05 +0800 Subject: [PATCH 02/34] add docs Signed-off-by: Sylvia Lei --- dynamic_store.go | 77 +++++++++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 37 deletions(-) diff --git a/dynamic_store.go b/dynamic_store.go index 41092e8..c21d03b 100644 --- a/dynamic_store.go +++ b/dynamic_store.go @@ -24,59 +24,47 @@ import ( "oras.land/oras-go/v2/registry/remote/auth" ) -// StoreOptions provides options for NewStore. -type StoreOptions struct { - // AllowPlainText allows saving credentials in plain text in configuration file. - AllowPlainText bool +// credentialConfig contains the config fields related to credentials. +// Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L28-L29 +type credentialConfig struct { + CredentialsStore string `json:"credsStore,omitempty"` + CredentialHelpers map[string]string `json:"credHelpers,omitempty"` } -const ( - configFieldCredStore = "credsStore" - configFieldCredHelpers = "credHelpers" -) - +// dynamicStore dynamically determines which store to use based on the settings +// in the config file. type dynamicStore struct { - configPath string - credStore string - credHelpers map[string]string - fileStore *FileStore + credentialConfig + configPath string + fileStore *FileStore + options StoreOptions } -// TODO: when to use default store? +// StoreOptions provides options for NewStore. +type StoreOptions struct { + // AllowPlaintext allows saving credentials in plaintext in the config file. + AllowPlaintext bool +} + +// NewStore returns a store based on given config file. func NewStore(configPath string, opts StoreOptions) (Store, error) { ds := &dynamicStore{ configPath: configPath, + options: opts, } configFile, err := os.Open(configPath) if err != nil { if os.IsNotExist(err) { - // TODO: no error? + // the config file can be created when needed return ds, nil } return nil, fmt.Errorf("failed to open config file at %s: %w", configPath, err) } defer configFile.Close() - // init file store - ds.fileStore, err = NewFileStore(ds.configPath) - if err != nil { - return nil, err - } - if !opts.AllowPlainText { - ds.fileStore.DisableSave = true - } - - var content map[string]json.RawMessage - if err := json.NewDecoder(configFile).Decode(&content); err != nil { - return nil, fmt.Errorf("failed to decode config file at %s: %w: %v", configPath, ErrInvalidConfigFormat, err) - } - ds.credStore = string(content[configFieldCredStore]) - credHelperBytes, ok := content[configFieldCredHelpers] - if !ok { - return ds, nil - } - if err := json.Unmarshal(credHelperBytes, &ds.credHelpers); err != nil { + // decode credential config if the config file exists + if err := json.NewDecoder(configFile).Decode(&ds.credentialConfig); err != nil { return nil, fmt.Errorf("failed to decode config file at %s: %w: %v", configPath, ErrInvalidConfigFormat, err) } return ds, nil @@ -109,12 +97,27 @@ func (ds *dynamicStore) Delete(ctx context.Context, serverAddress string) error return store.Delete(ctx, serverAddress) } +// getStore returns a store for the given server address. func (ds *dynamicStore) getStore(serverAddress string) (Store, error) { - if helper, ok := ds.credHelpers[serverAddress]; ok { + // 1. Look for a server-specific credential helper first + if helper := ds.CredentialHelpers[serverAddress]; helper != "" { return NewNativeStore(helper), nil } - if ds.credStore != "" { - return NewNativeStore(ds.credStore), nil + // 2. Then look for the configured native store + if ds.CredentialsStore != "" { + return NewNativeStore(ds.CredentialsStore), nil + } + // 3. Finally use a file store + if ds.fileStore == nil { + // lazy loading + var err error + ds.fileStore, err = NewFileStore(ds.configPath) + if err != nil { + return nil, fmt.Errorf("failed to initialize file store: %w", err) + } + if !ds.options.AllowPlaintext { + ds.fileStore.DisableSave = true + } } return ds.fileStore, nil } From db2481b5e8fda2cf188f3ab30631b166554f309c Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Wed, 12 Apr 2023 19:06:40 +0800 Subject: [PATCH 03/34] add tests Signed-off-by: Sylvia Lei --- dynamic_store.go | 73 ++++++----- dynamic_store_test.go | 201 +++++++++++++++++++++++-------- testdata/auth_config.json | 29 ----- testdata/credHelpers_config.json | 15 +++ testdata/credsStore_config.json | 6 + 5 files changed, 215 insertions(+), 109 deletions(-) delete mode 100644 testdata/auth_config.json create mode 100644 testdata/credHelpers_config.json create mode 100644 testdata/credsStore_config.json diff --git a/dynamic_store.go b/dynamic_store.go index c21d03b..5f204f2 100644 --- a/dynamic_store.go +++ b/dynamic_store.go @@ -20,6 +20,7 @@ import ( "encoding/json" "fmt" "os" + "sync" "oras.land/oras-go/v2/registry/remote/auth" ) @@ -35,9 +36,29 @@ type credentialConfig struct { // in the config file. type dynamicStore struct { credentialConfig - configPath string - fileStore *FileStore - options StoreOptions + configPath string + options StoreOptions + fileStore *FileStore + fileStoreOnce sync.Once +} + +// LoadConfig loads the config file for ds. +func (ds *dynamicStore) LoadConfig() error { + configFile, err := os.Open(ds.configPath) + if err != nil { + if os.IsNotExist(err) { + // the config file can be created when needed + return nil + } + return fmt.Errorf("failed to open config file at %s: %w", ds.configPath, err) + } + defer configFile.Close() + + // decode credential config if the config file exists + if err := json.NewDecoder(configFile).Decode(&ds.credentialConfig); err != nil { + return fmt.Errorf("failed to decode config file at %s: %w: %v", ds.configPath, ErrInvalidConfigFormat, err) + } + return nil } // StoreOptions provides options for NewStore. @@ -53,19 +74,8 @@ func NewStore(configPath string, opts StoreOptions) (Store, error) { options: opts, } - configFile, err := os.Open(configPath) - if err != nil { - if os.IsNotExist(err) { - // the config file can be created when needed - return ds, nil - } - return nil, fmt.Errorf("failed to open config file at %s: %w", configPath, err) - } - defer configFile.Close() - - // decode credential config if the config file exists - if err := json.NewDecoder(configFile).Decode(&ds.credentialConfig); err != nil { - return nil, fmt.Errorf("failed to decode config file at %s: %w: %v", configPath, ErrInvalidConfigFormat, err) + if err := ds.LoadConfig(); err != nil { + return nil, err } return ds, nil } @@ -74,7 +84,7 @@ func NewStore(configPath string, opts StoreOptions) (Store, error) { func (ds *dynamicStore) Get(ctx context.Context, serverAddress string) (auth.Credential, error) { store, err := ds.getStore(serverAddress) if err != nil { - return auth.EmptyCredential, nil + return auth.EmptyCredential, err } return store.Get(ctx, serverAddress) } @@ -97,27 +107,36 @@ func (ds *dynamicStore) Delete(ctx context.Context, serverAddress string) error return store.Delete(ctx, serverAddress) } -// getStore returns a store for the given server address. -func (ds *dynamicStore) getStore(serverAddress string) (Store, error) { +// getHelperSuffix returns the credential helper suffix for the given server +// address. +func (ds *dynamicStore) getHelperSuffix(serverAddress string) string { // 1. Look for a server-specific credential helper first if helper := ds.CredentialHelpers[serverAddress]; helper != "" { - return NewNativeStore(helper), nil + return helper } // 2. Then look for the configured native store - if ds.CredentialsStore != "" { - return NewNativeStore(ds.CredentialsStore), nil + return ds.CredentialsStore +} + +// getStore returns a store for the given server address. +func (ds *dynamicStore) getStore(serverAddress string) (Store, error) { + if helper := ds.getHelperSuffix(serverAddress); helper != "" { + return NewNativeStore(helper), nil } - // 3. Finally use a file store - if ds.fileStore == nil { - // lazy loading - var err error + + // lazy loading file store when no native store is available + var err error + ds.fileStoreOnce.Do(func() { ds.fileStore, err = NewFileStore(ds.configPath) if err != nil { - return nil, fmt.Errorf("failed to initialize file store: %w", err) + return } if !ds.options.AllowPlaintext { ds.fileStore.DisableSave = true } + }) + if err != nil { + return nil, fmt.Errorf("failed to initialize file store: %w", err) } return ds.fileStore, nil } diff --git a/dynamic_store_test.go b/dynamic_store_test.go index 0bbf0d0..d7b6496 100644 --- a/dynamic_store_test.go +++ b/dynamic_store_test.go @@ -23,92 +23,187 @@ import ( "oras.land/oras-go/v2/registry/remote/auth" ) -func TestDynamicStore_Get(t *testing.T) { +func Test_dynamicStore_Get_fileStore(t *testing.T) { ctx := context.Background() - ds, err := NewStore("testdata/auth_config.json", StoreOptions{}) - if err != nil { - t.Fatalf("NewDynamicStore() error = %v", err) - } - tests := []struct { name string + configPath string serverAddress string want auth.Credential wantErr bool }{ { - name: "Username and password", - serverAddress: "registry1.example.com", + name: "registry3.example.com", + configPath: "testdata/credHelpers_config.json", + serverAddress: "registry3.example.com", want: auth.Credential{ - Username: "username", - Password: "password", + Username: "foo", + Password: "bar", }, }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ds, err := NewStore(tt.configPath, StoreOptions{}) + if err != nil { + t.Fatal("NewStore() error =", err) + } + got, err := ds.Get(ctx, tt.serverAddress) + if (err != nil) != tt.wantErr { + t.Errorf("dynamicStore.Get() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("dynamicStore.Get() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_dynamicStore_getHelperSuffix(t *testing.T) { + tests := []struct { + name string + configPath string + serverAddress string + want string + }{ { - name: "Identity token", + name: "Get cred helper: registry_helper1", + configPath: "testdata/credHelpers_config.json", + serverAddress: "registry1.example.com", + want: "registry1-helper", + }, + { + name: "Get cred helper: registry_helper2", + configPath: "testdata/credHelpers_config.json", serverAddress: "registry2.example.com", - want: auth.Credential{ - RefreshToken: "identity_token", - }, + want: "registry2-helper", }, { - name: "Registry token", + name: "Empty cred helper configured", + configPath: "testdata/credHelpers_config.json", serverAddress: "registry3.example.com", - want: auth.Credential{ - AccessToken: "registry_token", - }, + want: "", }, { - name: "Username and password, identity token and registry token", - serverAddress: "registry4.example.com", - want: auth.Credential{ - Username: "username", - Password: "password", - RefreshToken: "identity_token", - AccessToken: "registry_token", - }, + name: "No cred helper and creds store configured", + configPath: "testdata/credHelpers_config.json", + serverAddress: "whatever.example.com", + want: "", }, { - name: "Empty credential", - serverAddress: "registry5.example.com", - want: auth.EmptyCredential, + name: "Choose cred helper over creds store", + configPath: "testdata/credsStore_config.json", + serverAddress: "test.example.com", + want: "test-helper", }, { - name: "Username and password, no auth", - serverAddress: "registry6.example.com", - want: auth.Credential{ - Username: "username", - Password: "password", - }, + name: "No cred helper configured, choose cred store", + configPath: "testdata/credsStore_config.json", + serverAddress: "whatever.example.com", + want: "teststore", }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ds := &dynamicStore{ + configPath: tt.configPath, + } + if err := ds.LoadConfig(); err != nil { + t.Fatal("dynamicStore.LoadConfig() error =", err) + } + if got := ds.getHelperSuffix(tt.serverAddress); got != tt.want { + t.Errorf("dynamicStore.getHelperSuffix() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_dynamicStore_getStore_nativeStore(t *testing.T) { + tests := []struct { + name string + configPath string + serverAddress string + }{ { - name: "Auth overriding Username and password", - serverAddress: "registry7.example.com", - want: auth.Credential{ - Username: "username", - Password: "password", - }, + name: "Cred helper configured for registry1.example.com", + configPath: "testdata/credHelpers_config.json", + serverAddress: "registry1.example.com", + }, + { + name: "Cred helper configured for registry2.example.com", + configPath: "testdata/credHelpers_config.json", + serverAddress: "registry2.example.com", }, { - name: "Not in auths", - serverAddress: "foo.example.com", - want: auth.EmptyCredential, + name: "Cred helper configured for test.example.com", + configPath: "testdata/credsStore_config.json", + serverAddress: "test.example.com", }, { - name: "No record", - serverAddress: "registry999.example.com", - want: auth.EmptyCredential, + name: "No cred helper configured, use creds store", + configPath: "testdata/credsStore_config.json", + serverAddress: "whaterver.example.com", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := ds.Get(ctx, tt.serverAddress) - if (err != nil) != tt.wantErr { - t.Errorf("DynamicStore.Get() error = %v, wantErr %v", err, tt.wantErr) - return + ds := &dynamicStore{ + configPath: tt.configPath, } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("DynamicStore.Get() = %v, want %v", got, tt.want) + if err := ds.LoadConfig(); err != nil { + t.Fatal("dynamicStore.LoadConfig() error =", err) + } + gotStore, err := ds.getStore(tt.serverAddress) + if err != nil { + t.Fatal("dynamicStore.getStore() error =", err) + } + if _, ok := gotStore.(*NativeStore); !ok { + t.Errorf("gotStore is not a native store") + } + }) + } +} + +func Test_dynamicStore_getStore_fileStore(t *testing.T) { + tests := []struct { + name string + configPath string + serverAddress string + }{ + { + name: "Empty cred helper configured for registry3.example.com", + configPath: "testdata/credHelpers_config.json", + serverAddress: "registry3.example.com", + }, + { + name: "No cred helper configured", + configPath: "testdata/credHelpers_config.json", + serverAddress: "whatever.example.com", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ds := &dynamicStore{ + configPath: tt.configPath, + } + if err := ds.LoadConfig(); err != nil { + t.Fatal("dynamicStore.LoadConfig() error =", err) + } + gotStore, err := ds.getStore(tt.serverAddress) + if err != nil { + t.Fatal("dynamicStore.getStore() error =", err) + } + if gotStore != ds.fileStore { + t.Errorf("gotStore is not a file store") + } + // get again, the same file store should be returned + gotStore, err = ds.getStore(tt.serverAddress) + if err != nil { + t.Fatal("dynamicStore.getStore() error =", err) + } + if gotStore != ds.fileStore { + t.Errorf("gotStore is not a file store") } }) } diff --git a/testdata/auth_config.json b/testdata/auth_config.json deleted file mode 100644 index faa294b..0000000 --- a/testdata/auth_config.json +++ /dev/null @@ -1,29 +0,0 @@ -{ - "auths": { - "registry1.example.com": { - "auth": "dXNlcm5hbWU6cGFzc3dvcmQ=" - }, - "registry2.example.com": { - "identitytoken": "identity_token" - }, - "registry3.example.com": { - "registrytoken": "registry_token" - }, - "registry4.example.com": { - "auth": "dXNlcm5hbWU6cGFzc3dvcmQ=", - "identitytoken": "identity_token", - "registrytoken": "registry_token" - }, - "registry5.example.com": {}, - "registry6.example.com": { - "username": "username", - "password": "password" - }, - "registry7.example.com": { - "auth": "dXNlcm5hbWU6cGFzc3dvcmQ=", - "username": "foo", - "password": "bar" - } - }, - "key": "val" -} diff --git a/testdata/credHelpers_config.json b/testdata/credHelpers_config.json new file mode 100644 index 0000000..f33a98e --- /dev/null +++ b/testdata/credHelpers_config.json @@ -0,0 +1,15 @@ +{ + "auths": { + "registry1.example.com": { + "auth": "dXNlcm5hbWU6cGFzc3dvcmQ=" + }, + "registry3.example.com": { + "auth": "Zm9vOmJhcg==" + } + }, + "credHelpers": { + "registry1.example.com": "registry1-helper", + "registry2.example.com": "registry2-helper", + "registry3.example.com": "" + } +} diff --git a/testdata/credsStore_config.json b/testdata/credsStore_config.json new file mode 100644 index 0000000..9c9c6f6 --- /dev/null +++ b/testdata/credsStore_config.json @@ -0,0 +1,6 @@ +{ + "credHelpers": { + "test.example.com": "test-helper" + }, + "credsStore": "teststore" +} \ No newline at end of file From a9f71db9856d85daba3029538ef541f4b3851acb Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Thu, 13 Apr 2023 15:44:40 +0800 Subject: [PATCH 04/34] improve docs Signed-off-by: Sylvia Lei --- dynamic_store.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/dynamic_store.go b/dynamic_store.go index 5f204f2..7b20822 100644 --- a/dynamic_store.go +++ b/dynamic_store.go @@ -47,7 +47,7 @@ func (ds *dynamicStore) LoadConfig() error { configFile, err := os.Open(ds.configPath) if err != nil { if os.IsNotExist(err) { - // the config file can be created when needed + // the config file can be created later when needed return nil } return fmt.Errorf("failed to open config file at %s: %w", ds.configPath, err) @@ -63,8 +63,13 @@ func (ds *dynamicStore) LoadConfig() error { // StoreOptions provides options for NewStore. type StoreOptions struct { - // AllowPlaintext allows saving credentials in plaintext in the config file. - AllowPlaintext bool + // AllowPlaintextSave allows saving credentials in plaintext in the config + // file. + // - If AllowPlaintextSave is set to false (default value), Put() will + // return an error when native store is not available. + // - If AllowPlaintextSave is set to true, Put() will save credentials in + // plaintext in the config file when native store is not available. + AllowPlaintextSave bool } // NewStore returns a store based on given config file. @@ -131,7 +136,7 @@ func (ds *dynamicStore) getStore(serverAddress string) (Store, error) { if err != nil { return } - if !ds.options.AllowPlaintext { + if !ds.options.AllowPlaintextSave { ds.fileStore.DisableSave = true } }) From 1e51c1a66980c892d85932a8488c8ccc30153274 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Thu, 13 Apr 2023 16:21:09 +0800 Subject: [PATCH 05/34] rebase Signed-off-by: Sylvia Lei --- dynamic_store.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/dynamic_store.go b/dynamic_store.go index 7b20822..238bf45 100644 --- a/dynamic_store.go +++ b/dynamic_store.go @@ -63,13 +63,13 @@ func (ds *dynamicStore) LoadConfig() error { // StoreOptions provides options for NewStore. type StoreOptions struct { - // AllowPlaintextSave allows saving credentials in plaintext in the config + // AllowPlaintextPut allows saving credentials in plaintext in the config // file. - // - If AllowPlaintextSave is set to false (default value), Put() will - // return an error when native store is not available. - // - If AllowPlaintextSave is set to true, Put() will save credentials in + // - If AllowPlaintextPut is set to false (default value), Put() will + // return an error when native store is not available. + // - If AllowPlaintextPut is set to true, Put() will save credentials in // plaintext in the config file when native store is not available. - AllowPlaintextSave bool + AllowPlaintextPut bool } // NewStore returns a store based on given config file. @@ -95,6 +95,8 @@ func (ds *dynamicStore) Get(ctx context.Context, serverAddress string) (auth.Cre } // Put saves credentials into the store for the given server address. +// Returns ErrPlaintextPutDisabled if native store is not available and +// StoreOptions.AllowPlaintextPut is set to false. func (ds *dynamicStore) Put(ctx context.Context, serverAddress string, cred auth.Credential) error { store, err := ds.getStore(serverAddress) if err != nil { @@ -136,8 +138,8 @@ func (ds *dynamicStore) getStore(serverAddress string) (Store, error) { if err != nil { return } - if !ds.options.AllowPlaintextSave { - ds.fileStore.DisableSave = true + if !ds.options.AllowPlaintextPut { + ds.fileStore.DisablePut = true } }) if err != nil { From fdd8bf4e382e331a7706d3d816dac4fb39594b5a Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Fri, 14 Apr 2023 19:20:08 +0800 Subject: [PATCH 06/34] refactor config Signed-off-by: Sylvia Lei --- config.go | 169 ++++++++++++++++++++++++++++++++++++++++++++++++++ file_store.go | 131 +++----------------------------------- 2 files changed, 178 insertions(+), 122 deletions(-) create mode 100644 config.go diff --git a/config.go b/config.go new file mode 100644 index 0000000..4c4e0ac --- /dev/null +++ b/config.go @@ -0,0 +1,169 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package credentials + +import ( + "bytes" + "encoding/json" + "fmt" + "os" + "path/filepath" + "sync" + + "github.com/oras-project/oras-credentials-go/internal/ioutil" +) + +type config struct { + // Path is the path to the config file. + Path string + // Content is the Content of the config file. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L17-L45 + Content map[string]json.RawMessage + // AuthsCache is a cache of the auths field of the config field. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L19 + AuthsCache map[string]json.RawMessage + CredentialsStore string `json:"credsStore,omitempty"` + CredentialHelpers map[string]string `json:"credHelpers,omitempty"` + // rwLock is a read-write-lock for the file store. + rwLock sync.RWMutex +} + +// authConfig contains authorization information for connecting to a Registry. +// References: +// - https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L17-L45 +// - https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/types/authconfig.go#L3-L22 +type authConfig struct { + // Auth is a base64-encoded string of "{username}:{password}". + Auth string `json:"auth,omitempty"` + // IdentityToken is used to authenticate the user and get. + // an access token for the registry. + IdentityToken string `json:"identitytoken,omitempty"` + // RegistryToken is a bearer token to be sent to a registry. + RegistryToken string `json:"registrytoken,omitempty"` + + Username string `json:"username,omitempty"` // legacy field for compatibility + Password string `json:"password,omitempty"` // legacy field for compatibility +} + +func loadConfigFile(configPath string) (*config, error) { + cfg := &config{Path: configPath} + configFile, err := os.Open(configPath) + if err != nil { + if os.IsNotExist(err) { + // init content map and auths cache if the content file does not exist + cfg.Content = make(map[string]json.RawMessage) + cfg.AuthsCache = make(map[string]json.RawMessage) + return cfg, nil + } + return nil, fmt.Errorf("failed to open config file at %s: %w", configPath, err) + } + defer configFile.Close() + + // decode config content if the config file exists + if err := json.NewDecoder(configFile).Decode(&cfg.Content); err != nil { + return nil, fmt.Errorf("failed to decode config file at %s: %w: %v", configPath, ErrInvalidConfigFormat, err) + } + authsBytes, ok := cfg.Content[configFieldAuths] + if !ok { + // init auths cache + cfg.AuthsCache = make(map[string]json.RawMessage) + return cfg, nil + } + if err := json.Unmarshal(authsBytes, &cfg.AuthsCache); err != nil { + return nil, fmt.Errorf("failed to unmarshal auths field: %w: %v", ErrInvalidConfigFormat, err) + } + return cfg, nil +} + +func (cfg config) getAuthConfig(serverAddress string) (authConfig, error) { + cfg.rwLock.RLock() + defer cfg.rwLock.RUnlock() + + authCfgBytes, ok := cfg.AuthsCache[serverAddress] + if !ok { + return authConfig{}, nil + } + var authCfg authConfig + if err := json.Unmarshal(authCfgBytes, &authCfg); err != nil { + return authConfig{}, fmt.Errorf("failed to unmarshal auth field: %w: %v", ErrInvalidConfigFormat, err) + } + return authCfg, nil +} + +func (cfg config) putAuthConfig(serverAddress string, authCfg authConfig) error { + cfg.rwLock.Lock() + defer cfg.rwLock.Unlock() + + authCfgBytes, err := json.Marshal(authCfg) + if err != nil { + return fmt.Errorf("failed to marshal auth field: %w", err) + } + cfg.AuthsCache[serverAddress] = authCfgBytes + return cfg.saveFile() +} + +func (cfg *config) deleteAuthConfig(serverAddress string) error { + cfg.rwLock.Lock() + defer cfg.rwLock.Unlock() + + if _, ok := cfg.AuthsCache[serverAddress]; !ok { + // no ops + return nil + } + delete(cfg.AuthsCache, serverAddress) + return cfg.saveFile() +} + +func (cfg *config) isAuthConfigured() bool { + return cfg.CredentialsStore != "" || + len(cfg.CredentialHelpers) > 0 || + len(cfg.AuthsCache) > 0 +} + +func (cfg *config) saveFile() (returnErr error) { + // marshal content + authsBytes, err := json.Marshal(cfg.AuthsCache) + if err != nil { + return fmt.Errorf("failed to marshal credentials: %w", err) + } + cfg.Content[configFieldAuths] = authsBytes + jsonBytes, err := json.MarshalIndent(cfg.Content, "", "\t") + if err != nil { + return fmt.Errorf("failed to marshal config: %w", err) + } + + // write the content to a ingest file for atomicity + configDir := filepath.Dir(cfg.Path) + if err := os.MkdirAll(configDir, 0700); err != nil { + return fmt.Errorf("failed to make directory %s: %w", configDir, err) + } + ingest, err := ioutil.Ingest(configDir, bytes.NewReader(jsonBytes)) + if err != nil { + return fmt.Errorf("failed to save config file: %w", err) + } + defer func() { + if returnErr != nil { + // clean up the ingest file in case of error + os.Remove(ingest) + } + }() + + // overwrite the config file + if err := os.Rename(ingest, cfg.Path); err != nil { + return fmt.Errorf("failed to save config file: %w", err) + } + return nil +} diff --git a/file_store.go b/file_store.go index 0e5cdab..df52818 100644 --- a/file_store.go +++ b/file_store.go @@ -16,18 +16,12 @@ limitations under the License. package credentials import ( - "bytes" "context" "encoding/base64" - "encoding/json" "errors" "fmt" - "os" - "path/filepath" "strings" - "sync" - "github.com/oras-project/oras-credentials-go/internal/ioutil" "oras.land/oras-go/v2/registry/remote/auth" ) @@ -38,16 +32,7 @@ type FileStore struct { // If DisablePut is set to true, Put() will return ErrPlaintextPutDisabled. DisablePut bool - // configPath is the path to the config file. - configPath string - // content is the content of the config file. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L17-L45 - content map[string]json.RawMessage - // authsCache is a cache of the auths field of the config field. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L19 - authsCache map[string]json.RawMessage - // rwLock is a read-write-lock for the file store. - rwLock sync.RWMutex + config *config } // configFieldAuths is the "auths" field in the config file. @@ -62,23 +47,6 @@ var ( ErrPlaintextPutDisabled = errors.New("putting plaintext credentials is disabled") ) -// authConfig contains authorization information for connecting to a Registry. -// References: -// - https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L17-L45 -// - https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/types/authconfig.go#L3-L22 -type authConfig struct { - // Auth is a base64-encoded string of "{username}:{password}". - Auth string `json:"auth,omitempty"` - // IdentityToken is used to authenticate the user and get. - // an access token for the registry. - IdentityToken string `json:"identitytoken,omitempty"` - // RegistryToken is a bearer token to be sent to a registry. - RegistryToken string `json:"registrytoken,omitempty"` - - Username string `json:"username,omitempty"` // legacy field for compatibility - Password string `json:"password,omitempty"` // legacy field for compatibility -} - // newAuthConfig creates an authConfig based on cred. func newAuthConfig(cred auth.Credential) authConfig { return authConfig{ @@ -109,47 +77,18 @@ func (ac authConfig) Credential() (auth.Credential, error) { // NewFileStore creates a new file credentials store. func NewFileStore(configPath string) (*FileStore, error) { - fs := &FileStore{configPath: configPath} - configFile, err := os.Open(configPath) + cfg, err := loadConfigFile(configPath) if err != nil { - if os.IsNotExist(err) { - // init content map and auths cache if the content file does not exist - fs.content = make(map[string]json.RawMessage) - fs.authsCache = make(map[string]json.RawMessage) - return fs, nil - } - return nil, fmt.Errorf("failed to open config file at %s: %w", configPath, err) - } - defer configFile.Close() - - // decode config content if the config file exists - if err := json.NewDecoder(configFile).Decode(&fs.content); err != nil { - return nil, fmt.Errorf("failed to decode config file at %s: %w: %v", configPath, ErrInvalidConfigFormat, err) - } - authsBytes, ok := fs.content[configFieldAuths] - if !ok { - // init auths cache - fs.authsCache = make(map[string]json.RawMessage) - return fs, nil - } - if err := json.Unmarshal(authsBytes, &fs.authsCache); err != nil { - return nil, fmt.Errorf("failed to unmarshal auths field: %w: %v", ErrInvalidConfigFormat, err) + return nil, err } - return fs, nil + return &FileStore{config: cfg}, nil } // Get retrieves credentials from the store for the given server address. func (fs *FileStore) Get(_ context.Context, serverAddress string) (auth.Credential, error) { - fs.rwLock.RLock() - defer fs.rwLock.RUnlock() - - authCfgBytes, ok := fs.authsCache[serverAddress] - if !ok { - return auth.EmptyCredential, nil - } - var authCfg authConfig - if err := json.Unmarshal(authCfgBytes, &authCfg); err != nil { - return auth.EmptyCredential, fmt.Errorf("failed to unmarshal auth field: %w: %v", ErrInvalidConfigFormat, err) + authCfg, err := fs.config.getAuthConfig(serverAddress) + if err != nil { + return auth.EmptyCredential, err } return authCfg.Credential() } @@ -161,65 +100,13 @@ func (fs *FileStore) Put(_ context.Context, serverAddress string, cred auth.Cred return ErrPlaintextPutDisabled } - fs.rwLock.Lock() - defer fs.rwLock.Unlock() - authCfg := newAuthConfig(cred) - authCfgBytes, err := json.Marshal(authCfg) - if err != nil { - return fmt.Errorf("failed to marshal auth field: %w", err) - } - fs.authsCache[serverAddress] = authCfgBytes - return fs.saveFile() + return fs.config.putAuthConfig(serverAddress, authCfg) } // Delete removes credentials from the store for the given server address. func (fs *FileStore) Delete(_ context.Context, serverAddress string) error { - fs.rwLock.Lock() - defer fs.rwLock.Unlock() - - if _, ok := fs.authsCache[serverAddress]; !ok { - // no ops - return nil - } - delete(fs.authsCache, serverAddress) - return fs.saveFile() -} - -// saveFile saves fs.content into fs.configPath. -func (fs *FileStore) saveFile() (returnErr error) { - // marshal content - authsBytes, err := json.Marshal(fs.authsCache) - if err != nil { - return fmt.Errorf("failed to marshal credentials: %w", err) - } - fs.content[configFieldAuths] = authsBytes - jsonBytes, err := json.MarshalIndent(fs.content, "", "\t") - if err != nil { - return fmt.Errorf("failed to marshal config: %w", err) - } - - // write the content to a ingest file for atomicity - configDir := filepath.Dir(fs.configPath) - if err := os.MkdirAll(configDir, 0700); err != nil { - return fmt.Errorf("failed to make directory %s: %w", configDir, err) - } - ingest, err := ioutil.Ingest(configDir, bytes.NewReader(jsonBytes)) - if err != nil { - return fmt.Errorf("failed to save config file: %w", err) - } - defer func() { - if returnErr != nil { - // clean up the ingest file in case of error - os.Remove(ingest) - } - }() - - // overwrite the config file - if err := os.Rename(ingest, fs.configPath); err != nil { - return fmt.Errorf("failed to save config file: %w", err) - } - return nil + return fs.config.deleteAuthConfig(serverAddress) } // encodeAuth base64-encodes username and password into base64(username:password). From edbb72b09dab4b75b79ef6806b2cbd3dc8ee1f4b Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Fri, 14 Apr 2023 19:27:04 +0800 Subject: [PATCH 07/34] refactor more Signed-off-by: Sylvia Lei --- config.go | 65 ++++++++++++++++++++++++++++++++++++++++++++++ file_store.go | 71 +++------------------------------------------------ 2 files changed, 68 insertions(+), 68 deletions(-) diff --git a/config.go b/config.go index 4c4e0ac..ca343d0 100644 --- a/config.go +++ b/config.go @@ -17,13 +17,17 @@ package credentials import ( "bytes" + "encoding/base64" "encoding/json" + "errors" "fmt" "os" "path/filepath" + "strings" "sync" "github.com/oras-project/oras-credentials-go/internal/ioutil" + "oras.land/oras-go/v2/registry/remote/auth" ) type config struct { @@ -58,6 +62,41 @@ type authConfig struct { Password string `json:"password,omitempty"` // legacy field for compatibility } +// configFieldAuths is the "auths" field in the config file. +// Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L19 +const configFieldAuths = "auths" + +// ErrInvalidConfigFormat is returned when the config format is invalid. +var ErrInvalidConfigFormat = errors.New("invalid config format") + +// newAuthConfig creates an authConfig based on cred. +func newAuthConfig(cred auth.Credential) authConfig { + return authConfig{ + Auth: encodeAuth(cred.Username, cred.Password), + IdentityToken: cred.RefreshToken, + RegistryToken: cred.AccessToken, + } +} + +// Credential returns an auth.Credential based on ac. +func (ac authConfig) Credential() (auth.Credential, error) { + cred := auth.Credential{ + Username: ac.Username, + Password: ac.Password, + RefreshToken: ac.IdentityToken, + AccessToken: ac.RegistryToken, + } + if ac.Auth != "" { + var err error + // override username and password + cred.Username, cred.Password, err = decodeAuth(ac.Auth) + if err != nil { + return auth.EmptyCredential, fmt.Errorf("failed to decode auth field: %w: %v", ErrInvalidConfigFormat, err) + } + } + return cred, nil +} + func loadConfigFile(configPath string) (*config, error) { cfg := &config{Path: configPath} configFile, err := os.Open(configPath) @@ -167,3 +206,29 @@ func (cfg *config) saveFile() (returnErr error) { } return nil } + +// encodeAuth base64-encodes username and password into base64(username:password). +func encodeAuth(username, password string) string { + if username == "" && password == "" { + return "" + } + return base64.StdEncoding.EncodeToString([]byte(username + ":" + password)) +} + +// decodeAuth decodes a base64 encoded string and returns username and password. +func decodeAuth(authStr string) (username string, password string, err error) { + if authStr == "" { + return "", "", nil + } + + decoded, err := base64.StdEncoding.DecodeString(authStr) + if err != nil { + return "", "", err + } + decodedStr := string(decoded) + username, password, ok := strings.Cut(decodedStr, ":") + if !ok { + return "", "", fmt.Errorf("auth '%s' does not conform the base64(username:password) format", decodedStr) + } + return username, password, nil +} diff --git a/file_store.go b/file_store.go index df52818..2b971ea 100644 --- a/file_store.go +++ b/file_store.go @@ -17,10 +17,7 @@ package credentials import ( "context" - "encoding/base64" "errors" - "fmt" - "strings" "oras.land/oras-go/v2/registry/remote/auth" ) @@ -35,45 +32,9 @@ type FileStore struct { config *config } -// configFieldAuths is the "auths" field in the config file. -// Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L19 -const configFieldAuths = "auths" - -var ( - // ErrInvalidConfigFormat is returned when the config format is invalid. - ErrInvalidConfigFormat = errors.New("invalid config format") - // ErrPlaintextPutDisabled is returned by Put() when DisablePut is set - // to true. - ErrPlaintextPutDisabled = errors.New("putting plaintext credentials is disabled") -) - -// newAuthConfig creates an authConfig based on cred. -func newAuthConfig(cred auth.Credential) authConfig { - return authConfig{ - Auth: encodeAuth(cred.Username, cred.Password), - IdentityToken: cred.RefreshToken, - RegistryToken: cred.AccessToken, - } -} - -// Credential returns an auth.Credential based on ac. -func (ac authConfig) Credential() (auth.Credential, error) { - cred := auth.Credential{ - Username: ac.Username, - Password: ac.Password, - RefreshToken: ac.IdentityToken, - AccessToken: ac.RegistryToken, - } - if ac.Auth != "" { - var err error - // override username and password - cred.Username, cred.Password, err = decodeAuth(ac.Auth) - if err != nil { - return auth.EmptyCredential, fmt.Errorf("failed to decode auth field: %w: %v", ErrInvalidConfigFormat, err) - } - } - return cred, nil -} +// ErrPlaintextPutDisabled is returned by Put() when DisablePut is set +// to true. +var ErrPlaintextPutDisabled = errors.New("putting plaintext credentials is disabled") // NewFileStore creates a new file credentials store. func NewFileStore(configPath string) (*FileStore, error) { @@ -108,29 +69,3 @@ func (fs *FileStore) Put(_ context.Context, serverAddress string, cred auth.Cred func (fs *FileStore) Delete(_ context.Context, serverAddress string) error { return fs.config.deleteAuthConfig(serverAddress) } - -// encodeAuth base64-encodes username and password into base64(username:password). -func encodeAuth(username, password string) string { - if username == "" && password == "" { - return "" - } - return base64.StdEncoding.EncodeToString([]byte(username + ":" + password)) -} - -// decodeAuth decodes a base64 encoded string and returns username and password. -func decodeAuth(authStr string) (username string, password string, err error) { - if authStr == "" { - return "", "", nil - } - - decoded, err := base64.StdEncoding.DecodeString(authStr) - if err != nil { - return "", "", err - } - decodedStr := string(decoded) - username, password, ok := strings.Cut(decodedStr, ":") - if !ok { - return "", "", fmt.Errorf("auth '%s' does not conform the base64(username:password) format", decodedStr) - } - return username, password, nil -} From 16b2c25af184aa16eff98bac0bffa9b416545cc5 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Fri, 14 Apr 2023 19:51:07 +0800 Subject: [PATCH 08/34] refactor dynamic store Signed-off-by: Sylvia Lei --- config.go | 37 ++++++++++++++++++------- dynamic_store.go | 64 ++++++++----------------------------------- dynamic_store_test.go | 37 +++++++++---------------- file_store.go | 4 +++ 4 files changed, 56 insertions(+), 86 deletions(-) diff --git a/config.go b/config.go index ca343d0..1f1c101 100644 --- a/config.go +++ b/config.go @@ -30,6 +30,9 @@ import ( "oras.land/oras-go/v2/registry/remote/auth" ) +// TODO: detect default store +// TODO: save creds store + type config struct { // Path is the path to the config file. Path string @@ -62,9 +65,13 @@ type authConfig struct { Password string `json:"password,omitempty"` // legacy field for compatibility } -// configFieldAuths is the "auths" field in the config file. -// Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L19 -const configFieldAuths = "auths" +const ( + // configFieldAuths is the "auths" field in the config file. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L19 + configFieldAuths = "auths" + configFieldCredentialsStore = "credsStore" + configFieldCredentialHelpers = "credHelpers" +) // ErrInvalidConfigFormat is returned when the config format is invalid. var ErrInvalidConfigFormat = errors.New("invalid config format") @@ -115,15 +122,25 @@ func loadConfigFile(configPath string) (*config, error) { if err := json.NewDecoder(configFile).Decode(&cfg.Content); err != nil { return nil, fmt.Errorf("failed to decode config file at %s: %w: %v", configPath, ErrInvalidConfigFormat, err) } - authsBytes, ok := cfg.Content[configFieldAuths] - if !ok { - // init auths cache - cfg.AuthsCache = make(map[string]json.RawMessage) - return cfg, nil + + if credsStoreBytes, ok := cfg.Content[configFieldCredentialsStore]; ok { + if err := json.Unmarshal(credsStoreBytes, &cfg.CredentialsStore); err != nil { + return nil, fmt.Errorf("failed to unmarshal creds store field: %w: %v", ErrInvalidConfigFormat, err) + } + } + if credHelpersBytes, ok := cfg.Content[configFieldCredentialHelpers]; ok { + if err := json.Unmarshal(credHelpersBytes, &cfg.CredentialHelpers); err != nil { + return nil, fmt.Errorf("failed to unmarshal cred helpers field: %w: %v", ErrInvalidConfigFormat, err) + } } - if err := json.Unmarshal(authsBytes, &cfg.AuthsCache); err != nil { - return nil, fmt.Errorf("failed to unmarshal auths field: %w: %v", ErrInvalidConfigFormat, err) + if authsBytes, ok := cfg.Content[configFieldAuths]; ok { + if err := json.Unmarshal(authsBytes, &cfg.AuthsCache); err != nil { + return nil, fmt.Errorf("failed to unmarshal auths field: %w: %v", ErrInvalidConfigFormat, err) + } + } else { + cfg.AuthsCache = make(map[string]json.RawMessage) } + return cfg, nil } diff --git a/dynamic_store.go b/dynamic_store.go index 238bf45..bbb623a 100644 --- a/dynamic_store.go +++ b/dynamic_store.go @@ -17,10 +17,6 @@ package credentials import ( "context" - "encoding/json" - "fmt" - "os" - "sync" "oras.land/oras-go/v2/registry/remote/auth" ) @@ -35,30 +31,8 @@ type credentialConfig struct { // dynamicStore dynamically determines which store to use based on the settings // in the config file. type dynamicStore struct { - credentialConfig - configPath string - options StoreOptions - fileStore *FileStore - fileStoreOnce sync.Once -} - -// LoadConfig loads the config file for ds. -func (ds *dynamicStore) LoadConfig() error { - configFile, err := os.Open(ds.configPath) - if err != nil { - if os.IsNotExist(err) { - // the config file can be created later when needed - return nil - } - return fmt.Errorf("failed to open config file at %s: %w", ds.configPath, err) - } - defer configFile.Close() - - // decode credential config if the config file exists - if err := json.NewDecoder(configFile).Decode(&ds.credentialConfig); err != nil { - return fmt.Errorf("failed to decode config file at %s: %w: %v", ds.configPath, ErrInvalidConfigFormat, err) - } - return nil + config *config + options StoreOptions } // StoreOptions provides options for NewStore. @@ -74,15 +48,15 @@ type StoreOptions struct { // NewStore returns a store based on given config file. func NewStore(configPath string, opts StoreOptions) (Store, error) { - ds := &dynamicStore{ - configPath: configPath, - options: opts, - } - - if err := ds.LoadConfig(); err != nil { + cfg, err := loadConfigFile(configPath) + if err != nil { return nil, err } - return ds, nil + + return &dynamicStore{ + config: cfg, + options: opts, + }, nil } // Get retrieves credentials from the store for the given server address. @@ -118,11 +92,11 @@ func (ds *dynamicStore) Delete(ctx context.Context, serverAddress string) error // address. func (ds *dynamicStore) getHelperSuffix(serverAddress string) string { // 1. Look for a server-specific credential helper first - if helper := ds.CredentialHelpers[serverAddress]; helper != "" { + if helper := ds.config.CredentialHelpers[serverAddress]; helper != "" { return helper } // 2. Then look for the configured native store - return ds.CredentialsStore + return ds.config.CredentialsStore } // getStore returns a store for the given server address. @@ -131,19 +105,5 @@ func (ds *dynamicStore) getStore(serverAddress string) (Store, error) { return NewNativeStore(helper), nil } - // lazy loading file store when no native store is available - var err error - ds.fileStoreOnce.Do(func() { - ds.fileStore, err = NewFileStore(ds.configPath) - if err != nil { - return - } - if !ds.options.AllowPlaintextPut { - ds.fileStore.DisablePut = true - } - }) - if err != nil { - return nil, fmt.Errorf("failed to initialize file store: %w", err) - } - return ds.fileStore, nil + return newFileStore(ds.config) } diff --git a/dynamic_store_test.go b/dynamic_store_test.go index d7b6496..0b1c830 100644 --- a/dynamic_store_test.go +++ b/dynamic_store_test.go @@ -106,12 +106,11 @@ func Test_dynamicStore_getHelperSuffix(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ds := &dynamicStore{ - configPath: tt.configPath, - } - if err := ds.LoadConfig(); err != nil { - t.Fatal("dynamicStore.LoadConfig() error =", err) + cfg, err := loadConfigFile(tt.configPath) + if err != nil { + t.Fatal("loadConfigFile() error =", err) } + ds := &dynamicStore{config: cfg} if got := ds.getHelperSuffix(tt.serverAddress); got != tt.want { t.Errorf("dynamicStore.getHelperSuffix() = %v, want %v", got, tt.want) } @@ -148,12 +147,11 @@ func Test_dynamicStore_getStore_nativeStore(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ds := &dynamicStore{ - configPath: tt.configPath, - } - if err := ds.LoadConfig(); err != nil { - t.Fatal("dynamicStore.LoadConfig() error =", err) + cfg, err := loadConfigFile(tt.configPath) + if err != nil { + t.Fatal("loadConfigFile() error =", err) } + ds := &dynamicStore{config: cfg} gotStore, err := ds.getStore(tt.serverAddress) if err != nil { t.Fatal("dynamicStore.getStore() error =", err) @@ -184,25 +182,16 @@ func Test_dynamicStore_getStore_fileStore(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ds := &dynamicStore{ - configPath: tt.configPath, - } - if err := ds.LoadConfig(); err != nil { - t.Fatal("dynamicStore.LoadConfig() error =", err) - } - gotStore, err := ds.getStore(tt.serverAddress) + cfg, err := loadConfigFile(tt.configPath) if err != nil { - t.Fatal("dynamicStore.getStore() error =", err) + t.Fatal("loadConfigFile() error =", err) } - if gotStore != ds.fileStore { - t.Errorf("gotStore is not a file store") - } - // get again, the same file store should be returned - gotStore, err = ds.getStore(tt.serverAddress) + ds := &dynamicStore{config: cfg} + gotStore, err := ds.getStore(tt.serverAddress) if err != nil { t.Fatal("dynamicStore.getStore() error =", err) } - if gotStore != ds.fileStore { + if _, ok := gotStore.(*FileStore); !ok { t.Errorf("gotStore is not a file store") } }) diff --git a/file_store.go b/file_store.go index 2b971ea..5e9e3de 100644 --- a/file_store.go +++ b/file_store.go @@ -45,6 +45,10 @@ func NewFileStore(configPath string) (*FileStore, error) { return &FileStore{config: cfg}, nil } +func newFileStore(cfg *config) (*FileStore, error) { + return &FileStore{config: cfg}, nil +} + // Get retrieves credentials from the store for the given server address. func (fs *FileStore) Get(_ context.Context, serverAddress string) (auth.Credential, error) { authCfg, err := fs.config.getAuthConfig(serverAddress) From 3f4a9f28d92bf3ab6d1f46be8950f00f5de11b71 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Mon, 17 Apr 2023 14:29:40 +0800 Subject: [PATCH 09/34] rename fileds Signed-off-by: Sylvia Lei --- config.go | 55 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/config.go b/config.go index 1f1c101..89a5291 100644 --- a/config.go +++ b/config.go @@ -34,16 +34,17 @@ import ( // TODO: save creds store type config struct { - // Path is the path to the config file. - Path string - // Content is the Content of the config file. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L17-L45 - Content map[string]json.RawMessage - // AuthsCache is a cache of the auths field of the config field. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L19 - AuthsCache map[string]json.RawMessage CredentialsStore string `json:"credsStore,omitempty"` CredentialHelpers map[string]string `json:"credHelpers,omitempty"` + + // path is the path to the config file. + path string + // content is the content of the config file. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L17-L45 + content map[string]json.RawMessage + // authsCache is a cache of the auths field of the config field. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L19 + authsCache map[string]json.RawMessage // rwLock is a read-write-lock for the file store. rwLock sync.RWMutex } @@ -105,13 +106,13 @@ func (ac authConfig) Credential() (auth.Credential, error) { } func loadConfigFile(configPath string) (*config, error) { - cfg := &config{Path: configPath} + cfg := &config{path: configPath} configFile, err := os.Open(configPath) if err != nil { if os.IsNotExist(err) { // init content map and auths cache if the content file does not exist - cfg.Content = make(map[string]json.RawMessage) - cfg.AuthsCache = make(map[string]json.RawMessage) + cfg.content = make(map[string]json.RawMessage) + cfg.authsCache = make(map[string]json.RawMessage) return cfg, nil } return nil, fmt.Errorf("failed to open config file at %s: %w", configPath, err) @@ -119,26 +120,26 @@ func loadConfigFile(configPath string) (*config, error) { defer configFile.Close() // decode config content if the config file exists - if err := json.NewDecoder(configFile).Decode(&cfg.Content); err != nil { + if err := json.NewDecoder(configFile).Decode(&cfg.content); err != nil { return nil, fmt.Errorf("failed to decode config file at %s: %w: %v", configPath, ErrInvalidConfigFormat, err) } - if credsStoreBytes, ok := cfg.Content[configFieldCredentialsStore]; ok { + if credsStoreBytes, ok := cfg.content[configFieldCredentialsStore]; ok { if err := json.Unmarshal(credsStoreBytes, &cfg.CredentialsStore); err != nil { return nil, fmt.Errorf("failed to unmarshal creds store field: %w: %v", ErrInvalidConfigFormat, err) } } - if credHelpersBytes, ok := cfg.Content[configFieldCredentialHelpers]; ok { + if credHelpersBytes, ok := cfg.content[configFieldCredentialHelpers]; ok { if err := json.Unmarshal(credHelpersBytes, &cfg.CredentialHelpers); err != nil { return nil, fmt.Errorf("failed to unmarshal cred helpers field: %w: %v", ErrInvalidConfigFormat, err) } } - if authsBytes, ok := cfg.Content[configFieldAuths]; ok { - if err := json.Unmarshal(authsBytes, &cfg.AuthsCache); err != nil { + if authsBytes, ok := cfg.content[configFieldAuths]; ok { + if err := json.Unmarshal(authsBytes, &cfg.authsCache); err != nil { return nil, fmt.Errorf("failed to unmarshal auths field: %w: %v", ErrInvalidConfigFormat, err) } } else { - cfg.AuthsCache = make(map[string]json.RawMessage) + cfg.authsCache = make(map[string]json.RawMessage) } return cfg, nil @@ -148,7 +149,7 @@ func (cfg config) getAuthConfig(serverAddress string) (authConfig, error) { cfg.rwLock.RLock() defer cfg.rwLock.RUnlock() - authCfgBytes, ok := cfg.AuthsCache[serverAddress] + authCfgBytes, ok := cfg.authsCache[serverAddress] if !ok { return authConfig{}, nil } @@ -167,7 +168,7 @@ func (cfg config) putAuthConfig(serverAddress string, authCfg authConfig) error if err != nil { return fmt.Errorf("failed to marshal auth field: %w", err) } - cfg.AuthsCache[serverAddress] = authCfgBytes + cfg.authsCache[serverAddress] = authCfgBytes return cfg.saveFile() } @@ -175,34 +176,34 @@ func (cfg *config) deleteAuthConfig(serverAddress string) error { cfg.rwLock.Lock() defer cfg.rwLock.Unlock() - if _, ok := cfg.AuthsCache[serverAddress]; !ok { + if _, ok := cfg.authsCache[serverAddress]; !ok { // no ops return nil } - delete(cfg.AuthsCache, serverAddress) + delete(cfg.authsCache, serverAddress) return cfg.saveFile() } func (cfg *config) isAuthConfigured() bool { return cfg.CredentialsStore != "" || len(cfg.CredentialHelpers) > 0 || - len(cfg.AuthsCache) > 0 + len(cfg.authsCache) > 0 } func (cfg *config) saveFile() (returnErr error) { // marshal content - authsBytes, err := json.Marshal(cfg.AuthsCache) + authsBytes, err := json.Marshal(cfg.authsCache) if err != nil { return fmt.Errorf("failed to marshal credentials: %w", err) } - cfg.Content[configFieldAuths] = authsBytes - jsonBytes, err := json.MarshalIndent(cfg.Content, "", "\t") + cfg.content[configFieldAuths] = authsBytes + jsonBytes, err := json.MarshalIndent(cfg.content, "", "\t") if err != nil { return fmt.Errorf("failed to marshal config: %w", err) } // write the content to a ingest file for atomicity - configDir := filepath.Dir(cfg.Path) + configDir := filepath.Dir(cfg.path) if err := os.MkdirAll(configDir, 0700); err != nil { return fmt.Errorf("failed to make directory %s: %w", configDir, err) } @@ -218,7 +219,7 @@ func (cfg *config) saveFile() (returnErr error) { }() // overwrite the config file - if err := os.Rename(ingest, cfg.Path); err != nil { + if err := os.Rename(ingest, cfg.path); err != nil { return fmt.Errorf("failed to save config file: %w", err) } return nil From 0df1d76bf138c718a6dbb10edc32ac8342cf758f Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Mon, 17 Apr 2023 15:08:37 +0800 Subject: [PATCH 10/34] marshal cred store Signed-off-by: Sylvia Lei --- config.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/config.go b/config.go index 89a5291..e139c6b 100644 --- a/config.go +++ b/config.go @@ -31,7 +31,7 @@ import ( ) // TODO: detect default store -// TODO: save creds store +// TODO: do we need to set cred helpers? type config struct { CredentialsStore string `json:"credsStore,omitempty"` @@ -192,6 +192,18 @@ func (cfg *config) isAuthConfigured() bool { func (cfg *config) saveFile() (returnErr error) { // marshal content + credHelpersBytes, err := json.Marshal(cfg.CredentialHelpers) + if err != nil { + return fmt.Errorf("failed to marshal cred helpers: %w", err) + } + cfg.content[configFieldCredentialHelpers] = credHelpersBytes + + credsStoreBytes, err := json.Marshal(cfg.CredentialsStore) + if err != nil { + return fmt.Errorf("failed to marshal creds store: %w", err) + } + cfg.content[configFieldCredentialsStore] = credsStoreBytes + authsBytes, err := json.Marshal(cfg.authsCache) if err != nil { return fmt.Errorf("failed to marshal credentials: %w", err) From f119c09545c812378a0ceb54239b77b7b21004ee Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Mon, 17 Apr 2023 16:19:51 +0800 Subject: [PATCH 11/34] refactor internal Signed-off-by: Sylvia Lei --- dynamic_store.go | 5 +- dynamic_store_test.go | 13 +- file_store.go | 15 +- file_store_test.go | 124 +----- config.go => internal/config/config.go | 528 ++++++++++++------------- internal/config/config_test.go | 107 +++++ 6 files changed, 399 insertions(+), 393 deletions(-) rename config.go => internal/config/config.go (87%) create mode 100644 internal/config/config_test.go diff --git a/dynamic_store.go b/dynamic_store.go index bbb623a..88f5857 100644 --- a/dynamic_store.go +++ b/dynamic_store.go @@ -18,6 +18,7 @@ package credentials import ( "context" + "github.com/oras-project/oras-credentials-go/internal/config" "oras.land/oras-go/v2/registry/remote/auth" ) @@ -31,7 +32,7 @@ type credentialConfig struct { // dynamicStore dynamically determines which store to use based on the settings // in the config file. type dynamicStore struct { - config *config + config *config.Config options StoreOptions } @@ -48,7 +49,7 @@ type StoreOptions struct { // NewStore returns a store based on given config file. func NewStore(configPath string, opts StoreOptions) (Store, error) { - cfg, err := loadConfigFile(configPath) + cfg, err := config.LoadConfigFile(configPath) if err != nil { return nil, err } diff --git a/dynamic_store_test.go b/dynamic_store_test.go index 0b1c830..b71fb8c 100644 --- a/dynamic_store_test.go +++ b/dynamic_store_test.go @@ -20,6 +20,7 @@ import ( "reflect" "testing" + "github.com/oras-project/oras-credentials-go/internal/config" "oras.land/oras-go/v2/registry/remote/auth" ) @@ -106,9 +107,9 @@ func Test_dynamicStore_getHelperSuffix(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cfg, err := loadConfigFile(tt.configPath) + cfg, err := config.LoadConfigFile(tt.configPath) if err != nil { - t.Fatal("loadConfigFile() error =", err) + t.Fatal("config.LoadConfigFile() error =", err) } ds := &dynamicStore{config: cfg} if got := ds.getHelperSuffix(tt.serverAddress); got != tt.want { @@ -147,9 +148,9 @@ func Test_dynamicStore_getStore_nativeStore(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cfg, err := loadConfigFile(tt.configPath) + cfg, err := config.LoadConfigFile(tt.configPath) if err != nil { - t.Fatal("loadConfigFile() error =", err) + t.Fatal("config.LoadConfigFile() error =", err) } ds := &dynamicStore{config: cfg} gotStore, err := ds.getStore(tt.serverAddress) @@ -182,9 +183,9 @@ func Test_dynamicStore_getStore_fileStore(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cfg, err := loadConfigFile(tt.configPath) + cfg, err := config.LoadConfigFile(tt.configPath) if err != nil { - t.Fatal("loadConfigFile() error =", err) + t.Fatal("config.LoadConfigFile() error =", err) } ds := &dynamicStore{config: cfg} gotStore, err := ds.getStore(tt.serverAddress) diff --git a/file_store.go b/file_store.go index 5e9e3de..5d8dc4a 100644 --- a/file_store.go +++ b/file_store.go @@ -19,6 +19,7 @@ import ( "context" "errors" + "github.com/oras-project/oras-credentials-go/internal/config" "oras.land/oras-go/v2/registry/remote/auth" ) @@ -29,7 +30,7 @@ type FileStore struct { // If DisablePut is set to true, Put() will return ErrPlaintextPutDisabled. DisablePut bool - config *config + config *config.Config } // ErrPlaintextPutDisabled is returned by Put() when DisablePut is set @@ -38,20 +39,20 @@ var ErrPlaintextPutDisabled = errors.New("putting plaintext credentials is disab // NewFileStore creates a new file credentials store. func NewFileStore(configPath string) (*FileStore, error) { - cfg, err := loadConfigFile(configPath) + cfg, err := config.LoadConfigFile(configPath) if err != nil { return nil, err } return &FileStore{config: cfg}, nil } -func newFileStore(cfg *config) (*FileStore, error) { +func newFileStore(cfg *config.Config) (*FileStore, error) { return &FileStore{config: cfg}, nil } // Get retrieves credentials from the store for the given server address. func (fs *FileStore) Get(_ context.Context, serverAddress string) (auth.Credential, error) { - authCfg, err := fs.config.getAuthConfig(serverAddress) + authCfg, err := fs.config.GetAuthConfig(serverAddress) if err != nil { return auth.EmptyCredential, err } @@ -65,11 +66,11 @@ func (fs *FileStore) Put(_ context.Context, serverAddress string, cred auth.Cred return ErrPlaintextPutDisabled } - authCfg := newAuthConfig(cred) - return fs.config.putAuthConfig(serverAddress, authCfg) + authCfg := config.NewAuthConfig(cred) + return fs.config.PutAuthConfig(serverAddress, authCfg) } // Delete removes credentials from the store for the given server address. func (fs *FileStore) Delete(_ context.Context, serverAddress string) error { - return fs.config.deleteAuthConfig(serverAddress) + return fs.config.DeleteAuthConfig(serverAddress) } diff --git a/file_store_test.go b/file_store_test.go index 86e50d3..3bc12a2 100644 --- a/file_store_test.go +++ b/file_store_test.go @@ -79,28 +79,28 @@ func TestNewFileStore_badFormat(t *testing.T) { tests := []struct { name string configPath string - wantErr error + wantErr bool }{ { name: "Bad JSON format", configPath: "testdata/bad_config", - wantErr: ErrInvalidConfigFormat, + wantErr: true, }, { name: "Invalid auths format", configPath: "testdata/invalid_auths_config.json", - wantErr: ErrInvalidConfigFormat, + wantErr: true, }, { name: "No auths field", configPath: "testdata/no_auths_config.json", - wantErr: nil, + wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { _, err := NewFileStore(tt.configPath) - if !errors.Is(err, tt.wantErr) { + if (err != nil) != tt.wantErr { t.Errorf("NewFileStore() error = %v, wantErr %v", err, tt.wantErr) return } @@ -210,31 +210,31 @@ func TestFileStore_Get_invalidConfig(t *testing.T) { name string serverAddress string want auth.Credential - wantErr error + wantErr bool }{ { name: "Invalid auth encode", serverAddress: "registry1.example.com", want: auth.EmptyCredential, - wantErr: ErrInvalidConfigFormat, + wantErr: true, }, { name: "Invalid auths format", serverAddress: "registry2.example.com", want: auth.EmptyCredential, - wantErr: ErrInvalidConfigFormat, + wantErr: true, }, { name: "Invalid type", serverAddress: "registry3.example.com", want: auth.EmptyCredential, - wantErr: ErrInvalidConfigFormat, + wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := fs.Get(ctx, tt.serverAddress) - if !errors.Is(err, tt.wantErr) { + if (err != nil) != tt.wantErr { t.Errorf("FileStore.Get() error = %v, wantErr %v", err, tt.wantErr) return } @@ -849,107 +849,3 @@ func TestFileStore_Delete_notExistConfig(t *testing.T) { t.Errorf("Stat(%s) error = %v, wantErr %v", configPath, err, wantErr) } } - -func Test_encodeAuth(t *testing.T) { - tests := []struct { - name string - username string - password string - want string - }{ - { - name: "Username and password", - username: "username", - password: "password", - want: "dXNlcm5hbWU6cGFzc3dvcmQ=", - }, - { - name: "Username only", - username: "username", - password: "", - want: "dXNlcm5hbWU6", - }, - { - name: "Password only", - username: "", - password: "password", - want: "OnBhc3N3b3Jk", - }, - { - name: "Empty username and empty password", - username: "", - password: "", - want: "", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := encodeAuth(tt.username, tt.password); got != tt.want { - t.Errorf("encodeAuth() = %v, want %v", got, tt.want) - } - }) - } -} - -func Test_decodeAuth(t *testing.T) { - tests := []struct { - name string - authStr string - username string - password string - wantErr bool - }{ - { - name: "Valid base64", - authStr: "dXNlcm5hbWU6cGFzc3dvcmQ=", // username:password - username: "username", - password: "password", - }, - { - name: "Valid base64, username only", - authStr: "dXNlcm5hbWU6", // username: - username: "username", - }, - { - name: "Valid base64, password only", - authStr: "OnBhc3N3b3Jk", // :password - password: "password", - }, - { - name: "Valid base64, bad format", - authStr: "d2hhdGV2ZXI=", // whatever - username: "", - password: "", - wantErr: true, - }, - { - name: "Invalid base64", - authStr: "whatever", - username: "", - password: "", - wantErr: true, - }, - { - name: "Empty string", - authStr: "", - username: "", - password: "", - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - gotUsername, gotPassword, err := decodeAuth(tt.authStr) - if (err != nil) != tt.wantErr { - t.Errorf("decodeAuth() error = %v, wantErr %v", err, tt.wantErr) - return - } - if gotUsername != tt.username { - t.Errorf("decodeAuth() got = %v, want %v", gotUsername, tt.username) - } - if gotPassword != tt.password { - t.Errorf("decodeAuth() got1 = %v, want %v", gotPassword, tt.password) - } - }) - } -} diff --git a/config.go b/internal/config/config.go similarity index 87% rename from config.go rename to internal/config/config.go index e139c6b..f57d7b0 100644 --- a/config.go +++ b/internal/config/config.go @@ -1,264 +1,264 @@ -/* -Copyright The ORAS Authors. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - -http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package credentials - -import ( - "bytes" - "encoding/base64" - "encoding/json" - "errors" - "fmt" - "os" - "path/filepath" - "strings" - "sync" - - "github.com/oras-project/oras-credentials-go/internal/ioutil" - "oras.land/oras-go/v2/registry/remote/auth" -) - -// TODO: detect default store -// TODO: do we need to set cred helpers? - -type config struct { - CredentialsStore string `json:"credsStore,omitempty"` - CredentialHelpers map[string]string `json:"credHelpers,omitempty"` - - // path is the path to the config file. - path string - // content is the content of the config file. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L17-L45 - content map[string]json.RawMessage - // authsCache is a cache of the auths field of the config field. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L19 - authsCache map[string]json.RawMessage - // rwLock is a read-write-lock for the file store. - rwLock sync.RWMutex -} - -// authConfig contains authorization information for connecting to a Registry. -// References: -// - https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L17-L45 -// - https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/types/authconfig.go#L3-L22 -type authConfig struct { - // Auth is a base64-encoded string of "{username}:{password}". - Auth string `json:"auth,omitempty"` - // IdentityToken is used to authenticate the user and get. - // an access token for the registry. - IdentityToken string `json:"identitytoken,omitempty"` - // RegistryToken is a bearer token to be sent to a registry. - RegistryToken string `json:"registrytoken,omitempty"` - - Username string `json:"username,omitempty"` // legacy field for compatibility - Password string `json:"password,omitempty"` // legacy field for compatibility -} - -const ( - // configFieldAuths is the "auths" field in the config file. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L19 - configFieldAuths = "auths" - configFieldCredentialsStore = "credsStore" - configFieldCredentialHelpers = "credHelpers" -) - -// ErrInvalidConfigFormat is returned when the config format is invalid. -var ErrInvalidConfigFormat = errors.New("invalid config format") - -// newAuthConfig creates an authConfig based on cred. -func newAuthConfig(cred auth.Credential) authConfig { - return authConfig{ - Auth: encodeAuth(cred.Username, cred.Password), - IdentityToken: cred.RefreshToken, - RegistryToken: cred.AccessToken, - } -} - -// Credential returns an auth.Credential based on ac. -func (ac authConfig) Credential() (auth.Credential, error) { - cred := auth.Credential{ - Username: ac.Username, - Password: ac.Password, - RefreshToken: ac.IdentityToken, - AccessToken: ac.RegistryToken, - } - if ac.Auth != "" { - var err error - // override username and password - cred.Username, cred.Password, err = decodeAuth(ac.Auth) - if err != nil { - return auth.EmptyCredential, fmt.Errorf("failed to decode auth field: %w: %v", ErrInvalidConfigFormat, err) - } - } - return cred, nil -} - -func loadConfigFile(configPath string) (*config, error) { - cfg := &config{path: configPath} - configFile, err := os.Open(configPath) - if err != nil { - if os.IsNotExist(err) { - // init content map and auths cache if the content file does not exist - cfg.content = make(map[string]json.RawMessage) - cfg.authsCache = make(map[string]json.RawMessage) - return cfg, nil - } - return nil, fmt.Errorf("failed to open config file at %s: %w", configPath, err) - } - defer configFile.Close() - - // decode config content if the config file exists - if err := json.NewDecoder(configFile).Decode(&cfg.content); err != nil { - return nil, fmt.Errorf("failed to decode config file at %s: %w: %v", configPath, ErrInvalidConfigFormat, err) - } - - if credsStoreBytes, ok := cfg.content[configFieldCredentialsStore]; ok { - if err := json.Unmarshal(credsStoreBytes, &cfg.CredentialsStore); err != nil { - return nil, fmt.Errorf("failed to unmarshal creds store field: %w: %v", ErrInvalidConfigFormat, err) - } - } - if credHelpersBytes, ok := cfg.content[configFieldCredentialHelpers]; ok { - if err := json.Unmarshal(credHelpersBytes, &cfg.CredentialHelpers); err != nil { - return nil, fmt.Errorf("failed to unmarshal cred helpers field: %w: %v", ErrInvalidConfigFormat, err) - } - } - if authsBytes, ok := cfg.content[configFieldAuths]; ok { - if err := json.Unmarshal(authsBytes, &cfg.authsCache); err != nil { - return nil, fmt.Errorf("failed to unmarshal auths field: %w: %v", ErrInvalidConfigFormat, err) - } - } else { - cfg.authsCache = make(map[string]json.RawMessage) - } - - return cfg, nil -} - -func (cfg config) getAuthConfig(serverAddress string) (authConfig, error) { - cfg.rwLock.RLock() - defer cfg.rwLock.RUnlock() - - authCfgBytes, ok := cfg.authsCache[serverAddress] - if !ok { - return authConfig{}, nil - } - var authCfg authConfig - if err := json.Unmarshal(authCfgBytes, &authCfg); err != nil { - return authConfig{}, fmt.Errorf("failed to unmarshal auth field: %w: %v", ErrInvalidConfigFormat, err) - } - return authCfg, nil -} - -func (cfg config) putAuthConfig(serverAddress string, authCfg authConfig) error { - cfg.rwLock.Lock() - defer cfg.rwLock.Unlock() - - authCfgBytes, err := json.Marshal(authCfg) - if err != nil { - return fmt.Errorf("failed to marshal auth field: %w", err) - } - cfg.authsCache[serverAddress] = authCfgBytes - return cfg.saveFile() -} - -func (cfg *config) deleteAuthConfig(serverAddress string) error { - cfg.rwLock.Lock() - defer cfg.rwLock.Unlock() - - if _, ok := cfg.authsCache[serverAddress]; !ok { - // no ops - return nil - } - delete(cfg.authsCache, serverAddress) - return cfg.saveFile() -} - -func (cfg *config) isAuthConfigured() bool { - return cfg.CredentialsStore != "" || - len(cfg.CredentialHelpers) > 0 || - len(cfg.authsCache) > 0 -} - -func (cfg *config) saveFile() (returnErr error) { - // marshal content - credHelpersBytes, err := json.Marshal(cfg.CredentialHelpers) - if err != nil { - return fmt.Errorf("failed to marshal cred helpers: %w", err) - } - cfg.content[configFieldCredentialHelpers] = credHelpersBytes - - credsStoreBytes, err := json.Marshal(cfg.CredentialsStore) - if err != nil { - return fmt.Errorf("failed to marshal creds store: %w", err) - } - cfg.content[configFieldCredentialsStore] = credsStoreBytes - - authsBytes, err := json.Marshal(cfg.authsCache) - if err != nil { - return fmt.Errorf("failed to marshal credentials: %w", err) - } - cfg.content[configFieldAuths] = authsBytes - jsonBytes, err := json.MarshalIndent(cfg.content, "", "\t") - if err != nil { - return fmt.Errorf("failed to marshal config: %w", err) - } - - // write the content to a ingest file for atomicity - configDir := filepath.Dir(cfg.path) - if err := os.MkdirAll(configDir, 0700); err != nil { - return fmt.Errorf("failed to make directory %s: %w", configDir, err) - } - ingest, err := ioutil.Ingest(configDir, bytes.NewReader(jsonBytes)) - if err != nil { - return fmt.Errorf("failed to save config file: %w", err) - } - defer func() { - if returnErr != nil { - // clean up the ingest file in case of error - os.Remove(ingest) - } - }() - - // overwrite the config file - if err := os.Rename(ingest, cfg.path); err != nil { - return fmt.Errorf("failed to save config file: %w", err) - } - return nil -} - -// encodeAuth base64-encodes username and password into base64(username:password). -func encodeAuth(username, password string) string { - if username == "" && password == "" { - return "" - } - return base64.StdEncoding.EncodeToString([]byte(username + ":" + password)) -} - -// decodeAuth decodes a base64 encoded string and returns username and password. -func decodeAuth(authStr string) (username string, password string, err error) { - if authStr == "" { - return "", "", nil - } - - decoded, err := base64.StdEncoding.DecodeString(authStr) - if err != nil { - return "", "", err - } - decodedStr := string(decoded) - username, password, ok := strings.Cut(decodedStr, ":") - if !ok { - return "", "", fmt.Errorf("auth '%s' does not conform the base64(username:password) format", decodedStr) - } - return username, password, nil -} +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package config + +import ( + "bytes" + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "sync" + + "github.com/oras-project/oras-credentials-go/internal/ioutil" + "oras.land/oras-go/v2/registry/remote/auth" +) + +// TODO: detect default store +// TODO: do we need to set cred helpers? +// TODO: when to store cred store when use default cred store? +type Config struct { + CredentialsStore string `json:"credsStore,omitempty"` + CredentialHelpers map[string]string `json:"credHelpers,omitempty"` + + // path is the path to the config file. + path string + // content is the content of the config file. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L17-L45 + content map[string]json.RawMessage + // authsCache is a cache of the auths field of the config field. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L19 + authsCache map[string]json.RawMessage + // rwLock is a read-write-lock for the file store. + rwLock sync.RWMutex +} + +// AuthConfig contains authorization information for connecting to a Registry. +// References: +// - https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L17-L45 +// - https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/types/authconfig.go#L3-L22 +type AuthConfig struct { + // Auth is a base64-encoded string of "{username}:{password}". + Auth string `json:"auth,omitempty"` + // IdentityToken is used to authenticate the user and get. + // an access token for the registry. + IdentityToken string `json:"identitytoken,omitempty"` + // RegistryToken is a bearer token to be sent to a registry. + RegistryToken string `json:"registrytoken,omitempty"` + + Username string `json:"username,omitempty"` // legacy field for compatibility + Password string `json:"password,omitempty"` // legacy field for compatibility +} + +const ( + // configFieldAuths is the "auths" field in the config file. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L19 + configFieldAuths = "auths" + configFieldCredentialsStore = "credsStore" + configFieldCredentialHelpers = "credHelpers" +) + +// ErrInvalidConfigFormat is returned when the config format is invalid. +var ErrInvalidConfigFormat = errors.New("invalid config format") + +// NewAuthConfig creates an authConfig based on cred. +func NewAuthConfig(cred auth.Credential) AuthConfig { + return AuthConfig{ + Auth: encodeAuth(cred.Username, cred.Password), + IdentityToken: cred.RefreshToken, + RegistryToken: cred.AccessToken, + } +} + +// Credential returns an auth.Credential based on ac. +func (ac AuthConfig) Credential() (auth.Credential, error) { + cred := auth.Credential{ + Username: ac.Username, + Password: ac.Password, + RefreshToken: ac.IdentityToken, + AccessToken: ac.RegistryToken, + } + if ac.Auth != "" { + var err error + // override username and password + cred.Username, cred.Password, err = decodeAuth(ac.Auth) + if err != nil { + return auth.EmptyCredential, fmt.Errorf("failed to decode auth field: %w: %v", ErrInvalidConfigFormat, err) + } + } + return cred, nil +} + +func LoadConfigFile(configPath string) (*Config, error) { + cfg := &Config{path: configPath} + configFile, err := os.Open(configPath) + if err != nil { + if os.IsNotExist(err) { + // init content map and auths cache if the content file does not exist + cfg.content = make(map[string]json.RawMessage) + cfg.authsCache = make(map[string]json.RawMessage) + return cfg, nil + } + return nil, fmt.Errorf("failed to open config file at %s: %w", configPath, err) + } + defer configFile.Close() + + // decode config content if the config file exists + if err := json.NewDecoder(configFile).Decode(&cfg.content); err != nil { + return nil, fmt.Errorf("failed to decode config file at %s: %w: %v", configPath, ErrInvalidConfigFormat, err) + } + + if credsStoreBytes, ok := cfg.content[configFieldCredentialsStore]; ok { + if err := json.Unmarshal(credsStoreBytes, &cfg.CredentialsStore); err != nil { + return nil, fmt.Errorf("failed to unmarshal creds store field: %w: %v", ErrInvalidConfigFormat, err) + } + } + if credHelpersBytes, ok := cfg.content[configFieldCredentialHelpers]; ok { + if err := json.Unmarshal(credHelpersBytes, &cfg.CredentialHelpers); err != nil { + return nil, fmt.Errorf("failed to unmarshal cred helpers field: %w: %v", ErrInvalidConfigFormat, err) + } + } + if authsBytes, ok := cfg.content[configFieldAuths]; ok { + if err := json.Unmarshal(authsBytes, &cfg.authsCache); err != nil { + return nil, fmt.Errorf("failed to unmarshal auths field: %w: %v", ErrInvalidConfigFormat, err) + } + } else { + cfg.authsCache = make(map[string]json.RawMessage) + } + + return cfg, nil +} + +func (cfg Config) GetAuthConfig(serverAddress string) (AuthConfig, error) { + cfg.rwLock.RLock() + defer cfg.rwLock.RUnlock() + + authCfgBytes, ok := cfg.authsCache[serverAddress] + if !ok { + return AuthConfig{}, nil + } + var authCfg AuthConfig + if err := json.Unmarshal(authCfgBytes, &authCfg); err != nil { + return AuthConfig{}, fmt.Errorf("failed to unmarshal auth field: %w: %v", ErrInvalidConfigFormat, err) + } + return authCfg, nil +} + +func (cfg Config) PutAuthConfig(serverAddress string, authCfg AuthConfig) error { + cfg.rwLock.Lock() + defer cfg.rwLock.Unlock() + + authCfgBytes, err := json.Marshal(authCfg) + if err != nil { + return fmt.Errorf("failed to marshal auth field: %w", err) + } + cfg.authsCache[serverAddress] = authCfgBytes + return cfg.saveFile() +} + +func (cfg *Config) DeleteAuthConfig(serverAddress string) error { + cfg.rwLock.Lock() + defer cfg.rwLock.Unlock() + + if _, ok := cfg.authsCache[serverAddress]; !ok { + // no ops + return nil + } + delete(cfg.authsCache, serverAddress) + return cfg.saveFile() +} + +func (cfg *Config) IsAuthConfigured() bool { + return cfg.CredentialsStore != "" || + len(cfg.CredentialHelpers) > 0 || + len(cfg.authsCache) > 0 +} + +func (cfg *Config) saveFile() (returnErr error) { + // marshal content + credHelpersBytes, err := json.Marshal(cfg.CredentialHelpers) + if err != nil { + return fmt.Errorf("failed to marshal cred helpers: %w", err) + } + cfg.content[configFieldCredentialHelpers] = credHelpersBytes + + credsStoreBytes, err := json.Marshal(cfg.CredentialsStore) + if err != nil { + return fmt.Errorf("failed to marshal creds store: %w", err) + } + cfg.content[configFieldCredentialsStore] = credsStoreBytes + + authsBytes, err := json.Marshal(cfg.authsCache) + if err != nil { + return fmt.Errorf("failed to marshal credentials: %w", err) + } + cfg.content[configFieldAuths] = authsBytes + jsonBytes, err := json.MarshalIndent(cfg.content, "", "\t") + if err != nil { + return fmt.Errorf("failed to marshal config: %w", err) + } + + // write the content to a ingest file for atomicity + configDir := filepath.Dir(cfg.path) + if err := os.MkdirAll(configDir, 0700); err != nil { + return fmt.Errorf("failed to make directory %s: %w", configDir, err) + } + ingest, err := ioutil.Ingest(configDir, bytes.NewReader(jsonBytes)) + if err != nil { + return fmt.Errorf("failed to save config file: %w", err) + } + defer func() { + if returnErr != nil { + // clean up the ingest file in case of error + os.Remove(ingest) + } + }() + + // overwrite the config file + if err := os.Rename(ingest, cfg.path); err != nil { + return fmt.Errorf("failed to save config file: %w", err) + } + return nil +} + +// encodeAuth base64-encodes username and password into base64(username:password). +func encodeAuth(username, password string) string { + if username == "" && password == "" { + return "" + } + return base64.StdEncoding.EncodeToString([]byte(username + ":" + password)) +} + +// decodeAuth decodes a base64 encoded string and returns username and password. +func decodeAuth(authStr string) (username string, password string, err error) { + if authStr == "" { + return "", "", nil + } + + decoded, err := base64.StdEncoding.DecodeString(authStr) + if err != nil { + return "", "", err + } + decodedStr := string(decoded) + username, password, ok := strings.Cut(decodedStr, ":") + if !ok { + return "", "", fmt.Errorf("auth '%s' does not conform the base64(username:password) format", decodedStr) + } + return username, password, nil +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 0000000..db732cb --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,107 @@ +package config + +import "testing" + +func Test_encodeAuth(t *testing.T) { + tests := []struct { + name string + username string + password string + want string + }{ + { + name: "Username and password", + username: "username", + password: "password", + want: "dXNlcm5hbWU6cGFzc3dvcmQ=", + }, + { + name: "Username only", + username: "username", + password: "", + want: "dXNlcm5hbWU6", + }, + { + name: "Password only", + username: "", + password: "password", + want: "OnBhc3N3b3Jk", + }, + { + name: "Empty username and empty password", + username: "", + password: "", + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := encodeAuth(tt.username, tt.password); got != tt.want { + t.Errorf("encodeAuth() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_decodeAuth(t *testing.T) { + tests := []struct { + name string + authStr string + username string + password string + wantErr bool + }{ + { + name: "Valid base64", + authStr: "dXNlcm5hbWU6cGFzc3dvcmQ=", // username:password + username: "username", + password: "password", + }, + { + name: "Valid base64, username only", + authStr: "dXNlcm5hbWU6", // username: + username: "username", + }, + { + name: "Valid base64, password only", + authStr: "OnBhc3N3b3Jk", // :password + password: "password", + }, + { + name: "Valid base64, bad format", + authStr: "d2hhdGV2ZXI=", // whatever + username: "", + password: "", + wantErr: true, + }, + { + name: "Invalid base64", + authStr: "whatever", + username: "", + password: "", + wantErr: true, + }, + { + name: "Empty string", + authStr: "", + username: "", + password: "", + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotUsername, gotPassword, err := decodeAuth(tt.authStr) + if (err != nil) != tt.wantErr { + t.Errorf("decodeAuth() error = %v, wantErr %v", err, tt.wantErr) + return + } + if gotUsername != tt.username { + t.Errorf("decodeAuth() got = %v, want %v", gotUsername, tt.username) + } + if gotPassword != tt.password { + t.Errorf("decodeAuth() got1 = %v, want %v", gotPassword, tt.password) + } + }) + } +} From be7367a37b2cb936f98e18841c4e2e01c4c162d3 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Mon, 17 Apr 2023 19:57:51 +0800 Subject: [PATCH 12/34] add tests Signed-off-by: Sylvia Lei --- dynamic_store.go | 14 +++-- dynamic_store_test.go | 123 ++++++++++++++++++++++++++++++++++++++---- file_store.go | 3 +- 3 files changed, 120 insertions(+), 20 deletions(-) diff --git a/dynamic_store.go b/dynamic_store.go index 88f5857..4b84e9a 100644 --- a/dynamic_store.go +++ b/dynamic_store.go @@ -22,13 +22,6 @@ import ( "oras.land/oras-go/v2/registry/remote/auth" ) -// credentialConfig contains the config fields related to credentials. -// Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L28-L29 -type credentialConfig struct { - CredentialsStore string `json:"credsStore,omitempty"` - CredentialHelpers map[string]string `json:"credHelpers,omitempty"` -} - // dynamicStore dynamically determines which store to use based on the settings // in the config file. type dynamicStore struct { @@ -106,5 +99,10 @@ func (ds *dynamicStore) getStore(serverAddress string) (Store, error) { return NewNativeStore(helper), nil } - return newFileStore(ds.config) + fs, err := newFileStore(ds.config) + if err != nil { + return nil, err + } + fs.DisablePut = !ds.options.AllowPlaintextPut + return fs, nil } diff --git a/dynamic_store_test.go b/dynamic_store_test.go index b71fb8c..9145aac 100644 --- a/dynamic_store_test.go +++ b/dynamic_store_test.go @@ -17,13 +17,23 @@ package credentials import ( "context" + "encoding/json" + "errors" + "os" + "path/filepath" "reflect" "testing" - "github.com/oras-project/oras-credentials-go/internal/config" "oras.land/oras-go/v2/registry/remote/auth" ) +func Test_dynamicStore_interface(t *testing.T) { + var ds interface{} = &dynamicStore{} + if _, ok := ds.(Store); !ok { + t.Error("&dynamicStore{} does not conform Store") + } +} + func Test_dynamicStore_Get_fileStore(t *testing.T) { ctx := context.Background() tests := []struct { @@ -61,6 +71,74 @@ func Test_dynamicStore_Get_fileStore(t *testing.T) { } } +func Test_dynamicStore_Put_AllowPlainTextPut(t *testing.T) { + // prepare test content + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "config.json") + serverAddr := "newtest.example.com" + cred := auth.Credential{ + Username: "username", + Password: "password", + } + ctx := context.Background() + + cfg := testConfig{ + AuthConfigs: map[string]testAuthConfig{ + "test.example.com": {}, + }, + SomeConfigField: 123, + } + jsonStr, err := json.Marshal(cfg) + if err != nil { + t.Fatalf("failed to marshal config: %v", err) + } + if err := os.WriteFile(configPath, jsonStr, 0666); err != nil { + t.Fatalf("failed to write config file: %v", err) + } + + // test default option + ds, err := NewStore(configPath, StoreOptions{}) + if err != nil { + t.Fatal("NewStore() error =", err) + } + err = ds.Put(ctx, serverAddr, cred) + if wantErr := ErrPlaintextPutDisabled; !errors.Is(err, wantErr) { + t.Errorf("dynamicStore.Put() error = %v, wantErr %v", err, wantErr) + } + + // test AllowPlainTextPut = true + ds, err = NewStore(configPath, StoreOptions{AllowPlaintextPut: true}) + if err != nil { + t.Fatal("NewStore() error =", err) + } + if err := ds.Put(ctx, serverAddr, cred); err != nil { + t.Error("dynamicStore.Put() error =", err) + } + + // verify config file + configFile, err := os.Open(configPath) + if err != nil { + t.Fatalf("failed to open config file: %v", err) + } + defer configFile.Close() + var gotCfg testConfig + if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { + t.Fatalf("failed to decode config file: %v", err) + } + wantCfg := testConfig{ + AuthConfigs: map[string]testAuthConfig{ + "test.example.com": {}, + serverAddr: { + Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", + }, + }, + SomeConfigField: cfg.SomeConfigField, + } + if !reflect.DeepEqual(gotCfg, wantCfg) { + t.Errorf("Decoded config = %v, want %v", gotCfg, wantCfg) + } +} + func Test_dynamicStore_getHelperSuffix(t *testing.T) { tests := []struct { name string @@ -107,11 +185,14 @@ func Test_dynamicStore_getHelperSuffix(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cfg, err := config.LoadConfigFile(tt.configPath) + store, err := NewStore(tt.configPath, StoreOptions{}) if err != nil { - t.Fatal("config.LoadConfigFile() error =", err) + t.Fatal("NewStore() error =", err) + } + ds, ok := store.(*dynamicStore) + if !ok { + t.Fatal("Store is not a dynamicStore") } - ds := &dynamicStore{config: cfg} if got := ds.getHelperSuffix(tt.serverAddress); got != tt.want { t.Errorf("dynamicStore.getHelperSuffix() = %v, want %v", got, tt.want) } @@ -148,11 +229,14 @@ func Test_dynamicStore_getStore_nativeStore(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cfg, err := config.LoadConfigFile(tt.configPath) + store, err := NewStore(tt.configPath, StoreOptions{}) if err != nil { - t.Fatal("config.LoadConfigFile() error =", err) + t.Fatal("NewStore() error =", err) + } + ds, ok := store.(*dynamicStore) + if !ok { + t.Fatal("Store is not a dynamicStore") } - ds := &dynamicStore{config: cfg} gotStore, err := ds.getStore(tt.serverAddress) if err != nil { t.Fatal("dynamicStore.getStore() error =", err) @@ -183,18 +267,35 @@ func Test_dynamicStore_getStore_fileStore(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cfg, err := config.LoadConfigFile(tt.configPath) + store, err := NewStore(tt.configPath, StoreOptions{}) if err != nil { - t.Fatal("config.LoadConfigFile() error =", err) + t.Fatal("NewStore() error =", err) + } + ds, ok := store.(*dynamicStore) + if !ok { + t.Fatal("Store is not a dynamicStore") } - ds := &dynamicStore{config: cfg} gotStore, err := ds.getStore(tt.serverAddress) if err != nil { t.Fatal("dynamicStore.getStore() error =", err) } - if _, ok := gotStore.(*FileStore); !ok { + gotFS1, ok := gotStore.(*FileStore) + if !ok { t.Errorf("gotStore is not a file store") } + + // get again, the two file stores should be based on the same config instance + gotStore, err = ds.getStore(tt.serverAddress) + if err != nil { + t.Fatal("dynamicStore.getStore() error =", err) + } + gotFS2, ok := gotStore.(*FileStore) + if !ok { + t.Errorf("gotStore is not a file store") + } + if gotFS1.config != gotFS2.config { + t.Errorf("gotFS1 and gotFS2 are not based on the same config") + } }) } } diff --git a/file_store.go b/file_store.go index 5d8dc4a..b76c8f9 100644 --- a/file_store.go +++ b/file_store.go @@ -43,9 +43,10 @@ func NewFileStore(configPath string) (*FileStore, error) { if err != nil { return nil, err } - return &FileStore{config: cfg}, nil + return newFileStore(cfg) } +// newFileStore creates a file credentials store based on the given config instance. func newFileStore(cfg *config.Config) (*FileStore, error) { return &FileStore{config: cfg}, nil } From 928c0e04a91299e19c1e23ad9a0bcc980a9e3771 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Tue, 18 Apr 2023 20:06:23 +0800 Subject: [PATCH 13/34] support default helper Signed-off-by: Sylvia Lei --- default_native_helper_darwin.go | 20 +++++++++++++ default_native_helper_linux.go | 26 +++++++++++++++++ default_native_helper_windows.go | 20 +++++++++++++ dynamic_store.go | 16 +++++++++++ dynamic_store_test.go | 49 ++++++++++++++++++++++++-------- internal/config/config.go | 31 +++++++++++--------- 6 files changed, 136 insertions(+), 26 deletions(-) create mode 100644 default_native_helper_darwin.go create mode 100644 default_native_helper_linux.go create mode 100644 default_native_helper_windows.go diff --git a/default_native_helper_darwin.go b/default_native_helper_darwin.go new file mode 100644 index 0000000..34a019b --- /dev/null +++ b/default_native_helper_darwin.go @@ -0,0 +1,20 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package credentials + +func getPlatformDefaultHelperSuffix() string { + return "osxkeychain" +} diff --git a/default_native_helper_linux.go b/default_native_helper_linux.go new file mode 100644 index 0000000..393c013 --- /dev/null +++ b/default_native_helper_linux.go @@ -0,0 +1,26 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package credentials + +import "os/exec" + +func getPlatformDefaultHelperSuffix() string { + if _, err := exec.LookPath("pass"); err == nil { + return "pass" + } + + return "secretservice" +} diff --git a/default_native_helper_windows.go b/default_native_helper_windows.go new file mode 100644 index 0000000..9cd6c76 --- /dev/null +++ b/default_native_helper_windows.go @@ -0,0 +1,20 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package credentials + +func getPlatformDefaultHelperSuffix() string { + return "wincred" +} diff --git a/dynamic_store.go b/dynamic_store.go index 4b84e9a..59735d7 100644 --- a/dynamic_store.go +++ b/dynamic_store.go @@ -17,6 +17,7 @@ package credentials import ( "context" + "os/exec" "github.com/oras-project/oras-credentials-go/internal/config" "oras.land/oras-go/v2/registry/remote/auth" @@ -46,6 +47,13 @@ func NewStore(configPath string, opts StoreOptions) (Store, error) { if err != nil { return nil, err } + if !cfg.IsAuthConfigured() { + if defaultCredsStore := getDefaultHelperSuffix(); defaultCredsStore != "" { + cfg.CredentialsStore = defaultCredsStore + // ignore save error + cfg.SaveFile() + } + } return &dynamicStore{ config: cfg, @@ -106,3 +114,11 @@ func (ds *dynamicStore) getStore(serverAddress string) (Store, error) { fs.DisablePut = !ds.options.AllowPlaintextPut return fs, nil } + +func getDefaultHelperSuffix() string { + platformDefault := getPlatformDefaultHelperSuffix() + if _, err := exec.LookPath(remoteCredentialsPrefix + platformDefault); err == nil { + return platformDefault + } + return "" +} diff --git a/dynamic_store_test.go b/dynamic_store_test.go index 9145aac..25c3d62 100644 --- a/dynamic_store_test.go +++ b/dynamic_store_test.go @@ -34,6 +34,40 @@ func Test_dynamicStore_interface(t *testing.T) { } } +func Test_dynamicStore_defaultHelper(t *testing.T) { + // no auth configured, should use default helper + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "no_auths_config.json") + jsonBytes := []byte(`{"auths":{}}`) + if err := os.WriteFile(configPath, jsonBytes, 0666); err != nil { + t.Fatalf("failed to write config file: %v", err) + } + + store, err := NewStore(configPath, StoreOptions{}) + if err != nil { + t.Fatal("NewStore() error =", err) + } + ds := store.(*dynamicStore) + if got := ds.config.CredentialsStore; got == "" { + t.Error("dynamicStore.config.CredentialsStore is not set") + } + + // auth configured, should not use default helper + configPath = filepath.Join(tempDir, "auths_config.json") + jsonBytes = []byte(`{"auths":{"xxx":{}}}`) + if err := os.WriteFile(configPath, jsonBytes, 0666); err != nil { + t.Fatalf("failed to write config file: %v", err) + } + store, err = NewStore(configPath, StoreOptions{}) + if err != nil { + t.Fatal("NewStore() error =", err) + } + ds = store.(*dynamicStore) + if got := ds.config.CredentialsStore; got != "" { + t.Errorf("dynamicStore.config.CredentialsStore = %v, want empty", got) + } +} + func Test_dynamicStore_Get_fileStore(t *testing.T) { ctx := context.Background() tests := []struct { @@ -189,10 +223,7 @@ func Test_dynamicStore_getHelperSuffix(t *testing.T) { if err != nil { t.Fatal("NewStore() error =", err) } - ds, ok := store.(*dynamicStore) - if !ok { - t.Fatal("Store is not a dynamicStore") - } + ds := store.(*dynamicStore) if got := ds.getHelperSuffix(tt.serverAddress); got != tt.want { t.Errorf("dynamicStore.getHelperSuffix() = %v, want %v", got, tt.want) } @@ -233,10 +264,7 @@ func Test_dynamicStore_getStore_nativeStore(t *testing.T) { if err != nil { t.Fatal("NewStore() error =", err) } - ds, ok := store.(*dynamicStore) - if !ok { - t.Fatal("Store is not a dynamicStore") - } + ds := store.(*dynamicStore) gotStore, err := ds.getStore(tt.serverAddress) if err != nil { t.Fatal("dynamicStore.getStore() error =", err) @@ -271,10 +299,7 @@ func Test_dynamicStore_getStore_fileStore(t *testing.T) { if err != nil { t.Fatal("NewStore() error =", err) } - ds, ok := store.(*dynamicStore) - if !ok { - t.Fatal("Store is not a dynamicStore") - } + ds := store.(*dynamicStore) gotStore, err := ds.getStore(tt.serverAddress) if err != nil { t.Fatal("dynamicStore.getStore() error =", err) diff --git a/internal/config/config.go b/internal/config/config.go index f57d7b0..b868896 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -30,9 +30,6 @@ import ( "oras.land/oras-go/v2/registry/remote/auth" ) -// TODO: detect default store -// TODO: do we need to set cred helpers? -// TODO: when to store cred store when use default cred store? type Config struct { CredentialsStore string `json:"credsStore,omitempty"` CredentialHelpers map[string]string `json:"credHelpers,omitempty"` @@ -169,7 +166,7 @@ func (cfg Config) PutAuthConfig(serverAddress string, authCfg AuthConfig) error return fmt.Errorf("failed to marshal auth field: %w", err) } cfg.authsCache[serverAddress] = authCfgBytes - return cfg.saveFile() + return cfg.SaveFile() } func (cfg *Config) DeleteAuthConfig(serverAddress string) error { @@ -181,7 +178,7 @@ func (cfg *Config) DeleteAuthConfig(serverAddress string) error { return nil } delete(cfg.authsCache, serverAddress) - return cfg.saveFile() + return cfg.SaveFile() } func (cfg *Config) IsAuthConfigured() bool { @@ -190,19 +187,25 @@ func (cfg *Config) IsAuthConfigured() bool { len(cfg.authsCache) > 0 } -func (cfg *Config) saveFile() (returnErr error) { +func (cfg *Config) SaveFile() (returnErr error) { // marshal content - credHelpersBytes, err := json.Marshal(cfg.CredentialHelpers) - if err != nil { - return fmt.Errorf("failed to marshal cred helpers: %w", err) + if len(cfg.CredentialHelpers) > 0 { + // omit empty + credHelpersBytes, err := json.Marshal(cfg.CredentialHelpers) + if err != nil { + return fmt.Errorf("failed to marshal cred helpers: %w", err) + } + cfg.content[configFieldCredentialHelpers] = credHelpersBytes } - cfg.content[configFieldCredentialHelpers] = credHelpersBytes - credsStoreBytes, err := json.Marshal(cfg.CredentialsStore) - if err != nil { - return fmt.Errorf("failed to marshal creds store: %w", err) + if cfg.CredentialsStore != "" { + // omit empty + credsStoreBytes, err := json.Marshal(cfg.CredentialsStore) + if err != nil { + return fmt.Errorf("failed to marshal creds store: %w", err) + } + cfg.content[configFieldCredentialsStore] = credsStoreBytes } - cfg.content[configFieldCredentialsStore] = credsStoreBytes authsBytes, err := json.Marshal(cfg.authsCache) if err != nil { From 64223399d91e1c84726b711076242badc750c907 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Wed, 19 Apr 2023 15:44:10 +0800 Subject: [PATCH 14/34] config doc Signed-off-by: Sylvia Lei --- dynamic_store.go | 6 +-- dynamic_store_test.go | 4 +- internal/config/config.go | 86 +++++++++++++++++++++++++-------------- 3 files changed, 60 insertions(+), 36 deletions(-) diff --git a/dynamic_store.go b/dynamic_store.go index 59735d7..c788ac0 100644 --- a/dynamic_store.go +++ b/dynamic_store.go @@ -49,7 +49,7 @@ func NewStore(configPath string, opts StoreOptions) (Store, error) { } if !cfg.IsAuthConfigured() { if defaultCredsStore := getDefaultHelperSuffix(); defaultCredsStore != "" { - cfg.CredentialsStore = defaultCredsStore + cfg.CredentialsStoreCache = defaultCredsStore // ignore save error cfg.SaveFile() } @@ -94,11 +94,11 @@ func (ds *dynamicStore) Delete(ctx context.Context, serverAddress string) error // address. func (ds *dynamicStore) getHelperSuffix(serverAddress string) string { // 1. Look for a server-specific credential helper first - if helper := ds.config.CredentialHelpers[serverAddress]; helper != "" { + if helper := ds.config.CredentialHelpersCache[serverAddress]; helper != "" { return helper } // 2. Then look for the configured native store - return ds.config.CredentialsStore + return ds.config.CredentialsStoreCache } // getStore returns a store for the given server address. diff --git a/dynamic_store_test.go b/dynamic_store_test.go index 25c3d62..eace29d 100644 --- a/dynamic_store_test.go +++ b/dynamic_store_test.go @@ -48,7 +48,7 @@ func Test_dynamicStore_defaultHelper(t *testing.T) { t.Fatal("NewStore() error =", err) } ds := store.(*dynamicStore) - if got := ds.config.CredentialsStore; got == "" { + if got := ds.config.CredentialsStoreCache; got == "" { t.Error("dynamicStore.config.CredentialsStore is not set") } @@ -63,7 +63,7 @@ func Test_dynamicStore_defaultHelper(t *testing.T) { t.Fatal("NewStore() error =", err) } ds = store.(*dynamicStore) - if got := ds.config.CredentialsStore; got != "" { + if got := ds.config.CredentialsStoreCache; got != "" { t.Errorf("dynamicStore.config.CredentialsStore = %v, want empty", got) } } diff --git a/internal/config/config.go b/internal/config/config.go index b868896..27fb672 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -30,17 +30,23 @@ import ( "oras.land/oras-go/v2/registry/remote/auth" ) +// Config represents a docker configuration file. +// Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L17-L44 type Config struct { - CredentialsStore string `json:"credsStore,omitempty"` - CredentialHelpers map[string]string `json:"credHelpers,omitempty"` + // CredentialsStoreCache is a cache of the credsStore field of the config. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L28 + CredentialsStoreCache string `json:"credsStore,omitempty"` + // CredentialHelpersCache is a cache of the credHelpers field of the config. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L29 + CredentialHelpersCache map[string]string `json:"credHelpers,omitempty"` // path is the path to the config file. path string // content is the content of the config file. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L17-L45 + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L17-L44 content map[string]json.RawMessage - // authsCache is a cache of the auths field of the config field. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L19 + // authsCache is a cache of the auths field of the config. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L19 authsCache map[string]json.RawMessage // rwLock is a read-write-lock for the file store. rwLock sync.RWMutex @@ -48,8 +54,8 @@ type Config struct { // AuthConfig contains authorization information for connecting to a Registry. // References: -// - https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L17-L45 -// - https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/types/authconfig.go#L3-L22 +// - https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L17-L45 +// - https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/types/authconfig.go#L3-L22 type AuthConfig struct { // Auth is a base64-encoded string of "{username}:{password}". Auth string `json:"auth,omitempty"` @@ -65,9 +71,11 @@ type AuthConfig struct { const ( // configFieldAuths is the "auths" field in the config file. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.1/cli/config/configfile/file.go#L19 - configFieldAuths = "auths" - configFieldCredentialsStore = "credsStore" + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L19 + configFieldAuths = "auths" + // configFieldCredentialsStore is the "credsStore" field in the config file. + configFieldCredentialsStore = "credsStore" + // configFieldCredentialHelpers is the "credHelpers" field in the config file. configFieldCredentialHelpers = "credHelpers" ) @@ -102,14 +110,16 @@ func (ac AuthConfig) Credential() (auth.Credential, error) { return cred, nil } +// LoadConfigFile loads Config from the given config path. func LoadConfigFile(configPath string) (*Config, error) { cfg := &Config{path: configPath} configFile, err := os.Open(configPath) if err != nil { if os.IsNotExist(err) { - // init content map and auths cache if the content file does not exist + // init content and caches if the content file does not exist cfg.content = make(map[string]json.RawMessage) cfg.authsCache = make(map[string]json.RawMessage) + cfg.CredentialHelpersCache = make(map[string]string) return cfg, nil } return nil, fmt.Errorf("failed to open config file at %s: %w", configPath, err) @@ -122,15 +132,19 @@ func LoadConfigFile(configPath string) (*Config, error) { } if credsStoreBytes, ok := cfg.content[configFieldCredentialsStore]; ok { - if err := json.Unmarshal(credsStoreBytes, &cfg.CredentialsStore); err != nil { + if err := json.Unmarshal(credsStoreBytes, &cfg.CredentialsStoreCache); err != nil { return nil, fmt.Errorf("failed to unmarshal creds store field: %w: %v", ErrInvalidConfigFormat, err) } } + if credHelpersBytes, ok := cfg.content[configFieldCredentialHelpers]; ok { - if err := json.Unmarshal(credHelpersBytes, &cfg.CredentialHelpers); err != nil { + if err := json.Unmarshal(credHelpersBytes, &cfg.CredentialHelpersCache); err != nil { return nil, fmt.Errorf("failed to unmarshal cred helpers field: %w: %v", ErrInvalidConfigFormat, err) } + } else { + cfg.CredentialHelpersCache = make(map[string]string) } + if authsBytes, ok := cfg.content[configFieldAuths]; ok { if err := json.Unmarshal(authsBytes, &cfg.authsCache); err != nil { return nil, fmt.Errorf("failed to unmarshal auths field: %w: %v", ErrInvalidConfigFormat, err) @@ -142,7 +156,8 @@ func LoadConfigFile(configPath string) (*Config, error) { return cfg, nil } -func (cfg Config) GetAuthConfig(serverAddress string) (AuthConfig, error) { +// GetAuthConfig returns an AuthConfig for serverAddress. +func (cfg *Config) GetAuthConfig(serverAddress string) (AuthConfig, error) { cfg.rwLock.RLock() defer cfg.rwLock.RUnlock() @@ -157,7 +172,8 @@ func (cfg Config) GetAuthConfig(serverAddress string) (AuthConfig, error) { return authCfg, nil } -func (cfg Config) PutAuthConfig(serverAddress string, authCfg AuthConfig) error { +// PutAuthConfig puts authCfg for serverAddress. +func (cfg *Config) PutAuthConfig(serverAddress string, authCfg AuthConfig) error { cfg.rwLock.Lock() defer cfg.rwLock.Unlock() @@ -169,6 +185,7 @@ func (cfg Config) PutAuthConfig(serverAddress string, authCfg AuthConfig) error return cfg.SaveFile() } +// DeleteAuthConfig deletes the corresponding AuthConfig for serverAddress. func (cfg *Config) DeleteAuthConfig(serverAddress string) error { cfg.rwLock.Lock() defer cfg.rwLock.Unlock() @@ -181,30 +198,29 @@ func (cfg *Config) DeleteAuthConfig(serverAddress string) error { return cfg.SaveFile() } -func (cfg *Config) IsAuthConfigured() bool { - return cfg.CredentialsStore != "" || - len(cfg.CredentialHelpers) > 0 || - len(cfg.authsCache) > 0 -} - +// SaveFile saves Config into the file. func (cfg *Config) SaveFile() (returnErr error) { // marshal content - if len(cfg.CredentialHelpers) > 0 { - // omit empty - credHelpersBytes, err := json.Marshal(cfg.CredentialHelpers) + if cfg.CredentialsStoreCache != "" { + credsStoreBytes, err := json.Marshal(cfg.CredentialsStoreCache) if err != nil { - return fmt.Errorf("failed to marshal cred helpers: %w", err) + return fmt.Errorf("failed to marshal creds store: %w", err) } - cfg.content[configFieldCredentialHelpers] = credHelpersBytes + cfg.content[configFieldCredentialsStore] = credsStoreBytes + } else { + // omit empty + delete(cfg.content, configFieldCredentialsStore) } - if cfg.CredentialsStore != "" { - // omit empty - credsStoreBytes, err := json.Marshal(cfg.CredentialsStore) + if len(cfg.CredentialHelpersCache) > 0 { + credHelpersBytes, err := json.Marshal(cfg.CredentialHelpersCache) if err != nil { - return fmt.Errorf("failed to marshal creds store: %w", err) + return fmt.Errorf("failed to marshal cred helpers: %w", err) } - cfg.content[configFieldCredentialsStore] = credsStoreBytes + cfg.content[configFieldCredentialHelpers] = credHelpersBytes + } else { + // omit empty + delete(cfg.content, configFieldCredentialHelpers) } authsBytes, err := json.Marshal(cfg.authsCache) @@ -240,6 +256,14 @@ func (cfg *Config) SaveFile() (returnErr error) { return nil } +// IsAuthConfigured returns whether there is authentication configured in this +// config file or not. +func (cfg *Config) IsAuthConfigured() bool { + return cfg.CredentialsStoreCache != "" || + len(cfg.CredentialHelpersCache) > 0 || + len(cfg.authsCache) > 0 +} + // encodeAuth base64-encodes username and password into base64(username:password). func encodeAuth(username, password string) string { if username == "" && password == "" { From 895d28d480c3cab7c01b63b969a07acaa52ef341 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Wed, 19 Apr 2023 16:05:20 +0800 Subject: [PATCH 15/34] refactor dynamic store Signed-off-by: Sylvia Lei --- default_native_helper_darwin.go | 2 + default_native_helper_linux.go | 2 + default_native_helper_unsupported.go | 25 +++++++++++ default_native_helper_windows.go | 2 + dynamic_store.go | 8 ++-- dynamic_store_test.go | 34 -------------- internal/config/config.go | 67 ++++++++++++++++++---------- 7 files changed, 79 insertions(+), 61 deletions(-) create mode 100644 default_native_helper_unsupported.go diff --git a/default_native_helper_darwin.go b/default_native_helper_darwin.go index 34a019b..abb158d 100644 --- a/default_native_helper_darwin.go +++ b/default_native_helper_darwin.go @@ -15,6 +15,8 @@ limitations under the License. package credentials +// getPlatformDefaultHelperSuffix returns the platform default credential +// helper suffix. func getPlatformDefaultHelperSuffix() string { return "osxkeychain" } diff --git a/default_native_helper_linux.go b/default_native_helper_linux.go index 393c013..cd5de94 100644 --- a/default_native_helper_linux.go +++ b/default_native_helper_linux.go @@ -17,6 +17,8 @@ package credentials import "os/exec" +// getPlatformDefaultHelperSuffix returns the platform default credential +// helper suffix. func getPlatformDefaultHelperSuffix() string { if _, err := exec.LookPath("pass"); err == nil { return "pass" diff --git a/default_native_helper_unsupported.go b/default_native_helper_unsupported.go new file mode 100644 index 0000000..7403d87 --- /dev/null +++ b/default_native_helper_unsupported.go @@ -0,0 +1,25 @@ +//go:build !windows && !darwin && !linux +// +build !windows,!darwin,!linux + +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package credentials + +// getPlatformDefaultHelperSuffix returns the platform default credential +// helper suffix. +func getPlatformDefaultHelperSuffix() string { + return "" +} diff --git a/default_native_helper_windows.go b/default_native_helper_windows.go index 9cd6c76..b97135f 100644 --- a/default_native_helper_windows.go +++ b/default_native_helper_windows.go @@ -15,6 +15,8 @@ limitations under the License. package credentials +// getPlatformDefaultHelperSuffix returns the platform default credential +// helper suffix. func getPlatformDefaultHelperSuffix() string { return "wincred" } diff --git a/dynamic_store.go b/dynamic_store.go index c788ac0..98a0d84 100644 --- a/dynamic_store.go +++ b/dynamic_store.go @@ -49,9 +49,8 @@ func NewStore(configPath string, opts StoreOptions) (Store, error) { } if !cfg.IsAuthConfigured() { if defaultCredsStore := getDefaultHelperSuffix(); defaultCredsStore != "" { - cfg.CredentialsStoreCache = defaultCredsStore // ignore save error - cfg.SaveFile() + cfg.PutCredentialsStore(defaultCredsStore) } } @@ -94,11 +93,11 @@ func (ds *dynamicStore) Delete(ctx context.Context, serverAddress string) error // address. func (ds *dynamicStore) getHelperSuffix(serverAddress string) string { // 1. Look for a server-specific credential helper first - if helper := ds.config.CredentialHelpersCache[serverAddress]; helper != "" { + if helper := ds.config.GetCredentialHelpers(serverAddress); helper != "" { return helper } // 2. Then look for the configured native store - return ds.config.CredentialsStoreCache + return ds.config.GetCredentialsStore() } // getStore returns a store for the given server address. @@ -115,6 +114,7 @@ func (ds *dynamicStore) getStore(serverAddress string) (Store, error) { return fs, nil } +// getDefaultHelperSuffix returns the default credential helper suffix. func getDefaultHelperSuffix() string { platformDefault := getPlatformDefaultHelperSuffix() if _, err := exec.LookPath(remoteCredentialsPrefix + platformDefault); err == nil { diff --git a/dynamic_store_test.go b/dynamic_store_test.go index eace29d..5d25d42 100644 --- a/dynamic_store_test.go +++ b/dynamic_store_test.go @@ -34,40 +34,6 @@ func Test_dynamicStore_interface(t *testing.T) { } } -func Test_dynamicStore_defaultHelper(t *testing.T) { - // no auth configured, should use default helper - tempDir := t.TempDir() - configPath := filepath.Join(tempDir, "no_auths_config.json") - jsonBytes := []byte(`{"auths":{}}`) - if err := os.WriteFile(configPath, jsonBytes, 0666); err != nil { - t.Fatalf("failed to write config file: %v", err) - } - - store, err := NewStore(configPath, StoreOptions{}) - if err != nil { - t.Fatal("NewStore() error =", err) - } - ds := store.(*dynamicStore) - if got := ds.config.CredentialsStoreCache; got == "" { - t.Error("dynamicStore.config.CredentialsStore is not set") - } - - // auth configured, should not use default helper - configPath = filepath.Join(tempDir, "auths_config.json") - jsonBytes = []byte(`{"auths":{"xxx":{}}}`) - if err := os.WriteFile(configPath, jsonBytes, 0666); err != nil { - t.Fatalf("failed to write config file: %v", err) - } - store, err = NewStore(configPath, StoreOptions{}) - if err != nil { - t.Fatal("NewStore() error =", err) - } - ds = store.(*dynamicStore) - if got := ds.config.CredentialsStoreCache; got != "" { - t.Errorf("dynamicStore.config.CredentialsStore = %v, want empty", got) - } -} - func Test_dynamicStore_Get_fileStore(t *testing.T) { ctx := context.Background() tests := []struct { diff --git a/internal/config/config.go b/internal/config/config.go index 27fb672..2c83435 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -33,23 +33,22 @@ import ( // Config represents a docker configuration file. // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L17-L44 type Config struct { - // CredentialsStoreCache is a cache of the credsStore field of the config. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L28 - CredentialsStoreCache string `json:"credsStore,omitempty"` - // CredentialHelpersCache is a cache of the credHelpers field of the config. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L29 - CredentialHelpersCache map[string]string `json:"credHelpers,omitempty"` - // path is the path to the config file. path string + // rwLock is a read-write-lock for the file store. + rwLock sync.RWMutex // content is the content of the config file. // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L17-L44 content map[string]json.RawMessage // authsCache is a cache of the auths field of the config. // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L19 authsCache map[string]json.RawMessage - // rwLock is a read-write-lock for the file store. - rwLock sync.RWMutex + // credentialsStoreCache is a cache of the credsStore field of the config. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L28 + credentialsStoreCache string + // credentialHelpersCache is a cache of the credHelpers field of the config. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L29 + credentialHelpersCache map[string]string } // AuthConfig contains authorization information for connecting to a Registry. @@ -119,7 +118,7 @@ func LoadConfigFile(configPath string) (*Config, error) { // init content and caches if the content file does not exist cfg.content = make(map[string]json.RawMessage) cfg.authsCache = make(map[string]json.RawMessage) - cfg.CredentialHelpersCache = make(map[string]string) + cfg.credentialHelpersCache = make(map[string]string) return cfg, nil } return nil, fmt.Errorf("failed to open config file at %s: %w", configPath, err) @@ -132,17 +131,17 @@ func LoadConfigFile(configPath string) (*Config, error) { } if credsStoreBytes, ok := cfg.content[configFieldCredentialsStore]; ok { - if err := json.Unmarshal(credsStoreBytes, &cfg.CredentialsStoreCache); err != nil { + if err := json.Unmarshal(credsStoreBytes, &cfg.credentialsStoreCache); err != nil { return nil, fmt.Errorf("failed to unmarshal creds store field: %w: %v", ErrInvalidConfigFormat, err) } } if credHelpersBytes, ok := cfg.content[configFieldCredentialHelpers]; ok { - if err := json.Unmarshal(credHelpersBytes, &cfg.CredentialHelpersCache); err != nil { + if err := json.Unmarshal(credHelpersBytes, &cfg.credentialHelpersCache); err != nil { return nil, fmt.Errorf("failed to unmarshal cred helpers field: %w: %v", ErrInvalidConfigFormat, err) } } else { - cfg.CredentialHelpersCache = make(map[string]string) + cfg.credentialHelpersCache = make(map[string]string) } if authsBytes, ok := cfg.content[configFieldAuths]; ok { @@ -182,7 +181,7 @@ func (cfg *Config) PutAuthConfig(serverAddress string, authCfg AuthConfig) error return fmt.Errorf("failed to marshal auth field: %w", err) } cfg.authsCache[serverAddress] = authCfgBytes - return cfg.SaveFile() + return cfg.saveFile() } // DeleteAuthConfig deletes the corresponding AuthConfig for serverAddress. @@ -195,14 +194,36 @@ func (cfg *Config) DeleteAuthConfig(serverAddress string) error { return nil } delete(cfg.authsCache, serverAddress) - return cfg.SaveFile() + return cfg.saveFile() +} + +// GetCredentialHelpers returns the credential helpers for serverAddress. +func (cfg *Config) GetCredentialHelpers(serverAddress string) string { + return cfg.credentialHelpersCache[serverAddress] +} + +// GetCredentialHelpers returns the configured credentials store. +func (cfg *Config) GetCredentialsStore() string { + cfg.rwLock.RLock() + defer cfg.rwLock.RUnlock() + + return cfg.credentialsStoreCache +} + +// PutCredentialsStore puts the configured credentials store. +func (cfg *Config) PutCredentialsStore(credsStore string) error { + cfg.rwLock.Lock() + defer cfg.rwLock.Unlock() + + cfg.credentialsStoreCache = credsStore + return cfg.saveFile() } -// SaveFile saves Config into the file. -func (cfg *Config) SaveFile() (returnErr error) { +// saveFile saves Config into the file. +func (cfg *Config) saveFile() (returnErr error) { // marshal content - if cfg.CredentialsStoreCache != "" { - credsStoreBytes, err := json.Marshal(cfg.CredentialsStoreCache) + if cfg.credentialsStoreCache != "" { + credsStoreBytes, err := json.Marshal(cfg.credentialsStoreCache) if err != nil { return fmt.Errorf("failed to marshal creds store: %w", err) } @@ -212,8 +233,8 @@ func (cfg *Config) SaveFile() (returnErr error) { delete(cfg.content, configFieldCredentialsStore) } - if len(cfg.CredentialHelpersCache) > 0 { - credHelpersBytes, err := json.Marshal(cfg.CredentialHelpersCache) + if len(cfg.credentialHelpersCache) > 0 { + credHelpersBytes, err := json.Marshal(cfg.credentialHelpersCache) if err != nil { return fmt.Errorf("failed to marshal cred helpers: %w", err) } @@ -259,8 +280,8 @@ func (cfg *Config) SaveFile() (returnErr error) { // IsAuthConfigured returns whether there is authentication configured in this // config file or not. func (cfg *Config) IsAuthConfigured() bool { - return cfg.CredentialsStoreCache != "" || - len(cfg.CredentialHelpersCache) > 0 || + return cfg.credentialsStoreCache != "" || + len(cfg.credentialHelpersCache) > 0 || len(cfg.authsCache) > 0 } From 7a7e7b306125dde48b7977f7773c53f9beac4573 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Wed, 19 Apr 2023 16:15:34 +0800 Subject: [PATCH 16/34] fix Signed-off-by: Sylvia Lei --- dynamic_store.go | 6 ++++-- testdata/credsStore_config.json | 12 ++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/dynamic_store.go b/dynamic_store.go index 98a0d84..0d58cd7 100644 --- a/dynamic_store.go +++ b/dynamic_store.go @@ -17,6 +17,7 @@ package credentials import ( "context" + "fmt" "os/exec" "github.com/oras-project/oras-credentials-go/internal/config" @@ -49,8 +50,9 @@ func NewStore(configPath string, opts StoreOptions) (Store, error) { } if !cfg.IsAuthConfigured() { if defaultCredsStore := getDefaultHelperSuffix(); defaultCredsStore != "" { - // ignore save error - cfg.PutCredentialsStore(defaultCredsStore) + if err := cfg.PutCredentialsStore(defaultCredsStore); err != nil { + return nil, fmt.Errorf("failed to detect default creds store: %w", err) + } } } diff --git a/testdata/credsStore_config.json b/testdata/credsStore_config.json index 9c9c6f6..40eb384 100644 --- a/testdata/credsStore_config.json +++ b/testdata/credsStore_config.json @@ -1,6 +1,6 @@ -{ - "credHelpers": { - "test.example.com": "test-helper" - }, - "credsStore": "teststore" -} \ No newline at end of file +{ + "credHelpers": { + "test.example.com": "test-helper" + }, + "credsStore": "teststore" +} From 2e1c6b333e0d035970389c10004a0f2ae8254e75 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Wed, 19 Apr 2023 16:21:17 +0800 Subject: [PATCH 17/34] rename internal Signed-off-by: Sylvia Lei --- file_store.go | 11 +++-------- file_store_test.go | 2 +- internal/config/config.go | 19 ++++++++++--------- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/file_store.go b/file_store.go index b76c8f9..6efb965 100644 --- a/file_store.go +++ b/file_store.go @@ -53,11 +53,7 @@ func newFileStore(cfg *config.Config) (*FileStore, error) { // Get retrieves credentials from the store for the given server address. func (fs *FileStore) Get(_ context.Context, serverAddress string) (auth.Credential, error) { - authCfg, err := fs.config.GetAuthConfig(serverAddress) - if err != nil { - return auth.EmptyCredential, err - } - return authCfg.Credential() + return fs.config.GetCredential(serverAddress) } // Put saves credentials into the store for the given server address. @@ -67,11 +63,10 @@ func (fs *FileStore) Put(_ context.Context, serverAddress string, cred auth.Cred return ErrPlaintextPutDisabled } - authCfg := config.NewAuthConfig(cred) - return fs.config.PutAuthConfig(serverAddress, authCfg) + return fs.config.PutCredential(serverAddress, cred) } // Delete removes credentials from the store for the given server address. func (fs *FileStore) Delete(_ context.Context, serverAddress string) error { - return fs.config.DeleteAuthConfig(serverAddress) + return fs.config.DeleteCredential(serverAddress) } diff --git a/file_store_test.go b/file_store_test.go index 3bc12a2..213ac46 100644 --- a/file_store_test.go +++ b/file_store_test.go @@ -540,7 +540,7 @@ func TestFileStore_Put_updateOld(t *testing.T) { } } -func TestStore_Put_disableSave(t *testing.T) { +func TestStore_Put_disablePut(t *testing.T) { tempDir := t.TempDir() configPath := filepath.Join(tempDir, "config.json") ctx := context.Background() diff --git a/internal/config/config.go b/internal/config/config.go index 2c83435..3b5dbce 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -155,27 +155,28 @@ func LoadConfigFile(configPath string) (*Config, error) { return cfg, nil } -// GetAuthConfig returns an AuthConfig for serverAddress. -func (cfg *Config) GetAuthConfig(serverAddress string) (AuthConfig, error) { +// GetAuthConfig returns an auth.Credential for serverAddress. +func (cfg *Config) GetCredential(serverAddress string) (auth.Credential, error) { cfg.rwLock.RLock() defer cfg.rwLock.RUnlock() authCfgBytes, ok := cfg.authsCache[serverAddress] if !ok { - return AuthConfig{}, nil + return auth.EmptyCredential, nil } var authCfg AuthConfig if err := json.Unmarshal(authCfgBytes, &authCfg); err != nil { - return AuthConfig{}, fmt.Errorf("failed to unmarshal auth field: %w: %v", ErrInvalidConfigFormat, err) + return auth.EmptyCredential, fmt.Errorf("failed to unmarshal auth field: %w: %v", ErrInvalidConfigFormat, err) } - return authCfg, nil + return authCfg.Credential() } -// PutAuthConfig puts authCfg for serverAddress. -func (cfg *Config) PutAuthConfig(serverAddress string, authCfg AuthConfig) error { +// PutAuthConfig puts cred for serverAddress. +func (cfg *Config) PutCredential(serverAddress string, cred auth.Credential) error { cfg.rwLock.Lock() defer cfg.rwLock.Unlock() + authCfg := NewAuthConfig(cred) authCfgBytes, err := json.Marshal(authCfg) if err != nil { return fmt.Errorf("failed to marshal auth field: %w", err) @@ -184,8 +185,8 @@ func (cfg *Config) PutAuthConfig(serverAddress string, authCfg AuthConfig) error return cfg.saveFile() } -// DeleteAuthConfig deletes the corresponding AuthConfig for serverAddress. -func (cfg *Config) DeleteAuthConfig(serverAddress string) error { +// DeleteAuthConfig deletes the corresponding credential for serverAddress. +func (cfg *Config) DeleteCredential(serverAddress string) error { cfg.rwLock.Lock() defer cfg.rwLock.Unlock() From ee0f73c3258716877f32289e3ce17b512dc29f11 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Wed, 19 Apr 2023 16:58:40 +0800 Subject: [PATCH 18/34] fix Signed-off-by: Sylvia Lei --- dynamic_store_test.go | 95 ++++++++++++++++++++++++++++++--------- internal/config/config.go | 6 ++- 2 files changed, 77 insertions(+), 24 deletions(-) diff --git a/dynamic_store_test.go b/dynamic_store_test.go index 5d25d42..2cdade3 100644 --- a/dynamic_store_test.go +++ b/dynamic_store_test.go @@ -34,44 +34,95 @@ func Test_dynamicStore_interface(t *testing.T) { } } -func Test_dynamicStore_Get_fileStore(t *testing.T) { - ctx := context.Background() +func Test_dynamicStore(t *testing.T) { + // prepare test content + tempDir := t.TempDir() + authConfiguredPath := filepath.Join(tempDir, "auth_configured.json") + config := testConfig{ + AuthConfigs: map[string]testAuthConfig{ + "xxx": {}, + }, + SomeConfigField: 123, + } + jsonStr, err := json.Marshal(config) + if err != nil { + t.Fatalf("failed to marshal config: %v", err) + } + if err := os.WriteFile(authConfiguredPath, jsonStr, 0666); err != nil { + t.Fatalf("failed to write config file: %v", err) + } + + noAuthConfiguredPath := filepath.Join(tempDir, "no_auth_configured.json") + config = testConfig{ + SomeConfigField: 123, + } + jsonStr, err = json.Marshal(config) + if err != nil { + t.Fatalf("failed to marshal config: %v", err) + } + if err := os.WriteFile(noAuthConfiguredPath, jsonStr, 0666); err != nil { + t.Fatalf("failed to write config file: %v", err) + } + tests := []struct { - name string - configPath string - serverAddress string - want auth.Credential - wantErr bool + name string + configPath string }{ { - name: "registry3.example.com", - configPath: "testdata/credHelpers_config.json", - serverAddress: "registry3.example.com", - want: auth.Credential{ - Username: "foo", - Password: "bar", - }, + name: "Authentication configured", + configPath: authConfiguredPath, + }, + { + name: "No authentication configured", + configPath: noAuthConfiguredPath, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ds, err := NewStore(tt.configPath, StoreOptions{}) + ds, err := NewStore(tt.configPath, StoreOptions{AllowPlaintextPut: true}) if err != nil { t.Fatal("NewStore() error =", err) } - got, err := ds.Get(ctx, tt.serverAddress) - if (err != nil) != tt.wantErr { - t.Errorf("dynamicStore.Get() error = %v, wantErr %v", err, tt.wantErr) - return + serverAddr := "test.example.com" + cred := auth.Credential{ + Username: "username", + Password: "password", + } + ctx := context.Background() + + // test put + if err := ds.Put(ctx, serverAddr, cred); err != nil { + t.Fatal("dynamicStore.Get() error =", err) + } + + // test get + got, err := ds.Get(ctx, serverAddr) + if err != nil { + t.Fatal("dynamicStore.Get() error =", err) + } + if want := cred; got != want { + t.Errorf("dynamicStore.Get() = %v, want %v", got, want) + } + + // test delete + err = ds.Delete(ctx, serverAddr) + if err != nil { + t.Fatal("dynamicStore.Delete() error =", err) + } + + // verify delete + got, err = ds.Get(ctx, serverAddr) + if err != nil { + t.Fatal("dynamicStore.Get() error =", err) } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("dynamicStore.Get() = %v, want %v", got, tt.want) + if want := auth.EmptyCredential; got != want { + t.Errorf("dynamicStore.Get() = %v, want %v", got, want) } }) } } -func Test_dynamicStore_Put_AllowPlainTextPut(t *testing.T) { +func Test_dynamicStore_fileStore_AllowPlainTextPut(t *testing.T) { // prepare test content tempDir := t.TempDir() configPath := filepath.Join(tempDir, "config.json") diff --git a/internal/config/config.go b/internal/config/config.go index 3b5dbce..1686b4b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -140,7 +140,8 @@ func LoadConfigFile(configPath string) (*Config, error) { if err := json.Unmarshal(credHelpersBytes, &cfg.credentialHelpersCache); err != nil { return nil, fmt.Errorf("failed to unmarshal cred helpers field: %w: %v", ErrInvalidConfigFormat, err) } - } else { + } + if cfg.credentialHelpersCache == nil { cfg.credentialHelpersCache = make(map[string]string) } @@ -148,7 +149,8 @@ func LoadConfigFile(configPath string) (*Config, error) { if err := json.Unmarshal(authsBytes, &cfg.authsCache); err != nil { return nil, fmt.Errorf("failed to unmarshal auths field: %w: %v", ErrInvalidConfigFormat, err) } - } else { + } + if cfg.authsCache == nil { cfg.authsCache = make(map[string]json.RawMessage) } From 263529cb8b74acd170c4595222893e0b6633d2b5 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Thu, 20 Apr 2023 11:28:16 +0800 Subject: [PATCH 19/34] fix license + reference Signed-off-by: Sylvia Lei --- default_native_helper_darwin.go | 1 + default_native_helper_linux.go | 1 + default_native_helper_unsupported.go | 1 + default_native_helper_windows.go | 1 + internal/config/config_test.go | 15 +++++++++++++++ 5 files changed, 19 insertions(+) diff --git a/default_native_helper_darwin.go b/default_native_helper_darwin.go index abb158d..1a9aca6 100644 --- a/default_native_helper_darwin.go +++ b/default_native_helper_darwin.go @@ -17,6 +17,7 @@ package credentials // getPlatformDefaultHelperSuffix returns the platform default credential // helper suffix. +// Reference: https://docs.docker.com/engine/reference/commandline/login/#default-behavior func getPlatformDefaultHelperSuffix() string { return "osxkeychain" } diff --git a/default_native_helper_linux.go b/default_native_helper_linux.go index cd5de94..f182923 100644 --- a/default_native_helper_linux.go +++ b/default_native_helper_linux.go @@ -19,6 +19,7 @@ import "os/exec" // getPlatformDefaultHelperSuffix returns the platform default credential // helper suffix. +// Reference: https://docs.docker.com/engine/reference/commandline/login/#default-behavior func getPlatformDefaultHelperSuffix() string { if _, err := exec.LookPath("pass"); err == nil { return "pass" diff --git a/default_native_helper_unsupported.go b/default_native_helper_unsupported.go index 7403d87..c506b84 100644 --- a/default_native_helper_unsupported.go +++ b/default_native_helper_unsupported.go @@ -20,6 +20,7 @@ package credentials // getPlatformDefaultHelperSuffix returns the platform default credential // helper suffix. +// Reference: https://docs.docker.com/engine/reference/commandline/login/#default-behavior func getPlatformDefaultHelperSuffix() string { return "" } diff --git a/default_native_helper_windows.go b/default_native_helper_windows.go index b97135f..e334cc7 100644 --- a/default_native_helper_windows.go +++ b/default_native_helper_windows.go @@ -17,6 +17,7 @@ package credentials // getPlatformDefaultHelperSuffix returns the platform default credential // helper suffix. +// Reference: https://docs.docker.com/engine/reference/commandline/login/#default-behavior func getPlatformDefaultHelperSuffix() string { return "wincred" } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index db732cb..040287e 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1,3 +1,18 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package config import "testing" From 9e693d5ab37573aaf0930f9077f6392d4177d569 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Thu, 20 Apr 2023 15:38:41 +0800 Subject: [PATCH 20/34] rename + remove unnecessary code Signed-off-by: Sylvia Lei --- dynamic_store.go | 2 +- internal/config/config.go | 38 +++++++++++++++++--------------------- 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/dynamic_store.go b/dynamic_store.go index 0d58cd7..35f1437 100644 --- a/dynamic_store.go +++ b/dynamic_store.go @@ -95,7 +95,7 @@ func (ds *dynamicStore) Delete(ctx context.Context, serverAddress string) error // address. func (ds *dynamicStore) getHelperSuffix(serverAddress string) string { // 1. Look for a server-specific credential helper first - if helper := ds.config.GetCredentialHelpers(serverAddress); helper != "" { + if helper := ds.config.GetCredentialHelper(serverAddress); helper != "" { return helper } // 2. Then look for the configured native store diff --git a/internal/config/config.go b/internal/config/config.go index 1686b4b..0001b38 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -43,12 +43,12 @@ type Config struct { // authsCache is a cache of the auths field of the config. // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L19 authsCache map[string]json.RawMessage - // credentialsStoreCache is a cache of the credsStore field of the config. + // credentialsStore is the credsStore field of the config. // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L28 - credentialsStoreCache string - // credentialHelpersCache is a cache of the credHelpers field of the config. + credentialsStore string + // credentialHelpers is the credHelpers field of the config. // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L29 - credentialHelpersCache map[string]string + credentialHelpers map[string]string } // AuthConfig contains authorization information for connecting to a Registry. @@ -118,7 +118,6 @@ func LoadConfigFile(configPath string) (*Config, error) { // init content and caches if the content file does not exist cfg.content = make(map[string]json.RawMessage) cfg.authsCache = make(map[string]json.RawMessage) - cfg.credentialHelpersCache = make(map[string]string) return cfg, nil } return nil, fmt.Errorf("failed to open config file at %s: %w", configPath, err) @@ -131,19 +130,16 @@ func LoadConfigFile(configPath string) (*Config, error) { } if credsStoreBytes, ok := cfg.content[configFieldCredentialsStore]; ok { - if err := json.Unmarshal(credsStoreBytes, &cfg.credentialsStoreCache); err != nil { + if err := json.Unmarshal(credsStoreBytes, &cfg.credentialsStore); err != nil { return nil, fmt.Errorf("failed to unmarshal creds store field: %w: %v", ErrInvalidConfigFormat, err) } } if credHelpersBytes, ok := cfg.content[configFieldCredentialHelpers]; ok { - if err := json.Unmarshal(credHelpersBytes, &cfg.credentialHelpersCache); err != nil { + if err := json.Unmarshal(credHelpersBytes, &cfg.credentialHelpers); err != nil { return nil, fmt.Errorf("failed to unmarshal cred helpers field: %w: %v", ErrInvalidConfigFormat, err) } } - if cfg.credentialHelpersCache == nil { - cfg.credentialHelpersCache = make(map[string]string) - } if authsBytes, ok := cfg.content[configFieldAuths]; ok { if err := json.Unmarshal(authsBytes, &cfg.authsCache); err != nil { @@ -200,9 +196,9 @@ func (cfg *Config) DeleteCredential(serverAddress string) error { return cfg.saveFile() } -// GetCredentialHelpers returns the credential helpers for serverAddress. -func (cfg *Config) GetCredentialHelpers(serverAddress string) string { - return cfg.credentialHelpersCache[serverAddress] +// GetCredentialHelper returns the credential helpers for serverAddress. +func (cfg *Config) GetCredentialHelper(serverAddress string) string { + return cfg.credentialHelpers[serverAddress] } // GetCredentialHelpers returns the configured credentials store. @@ -210,7 +206,7 @@ func (cfg *Config) GetCredentialsStore() string { cfg.rwLock.RLock() defer cfg.rwLock.RUnlock() - return cfg.credentialsStoreCache + return cfg.credentialsStore } // PutCredentialsStore puts the configured credentials store. @@ -218,15 +214,15 @@ func (cfg *Config) PutCredentialsStore(credsStore string) error { cfg.rwLock.Lock() defer cfg.rwLock.Unlock() - cfg.credentialsStoreCache = credsStore + cfg.credentialsStore = credsStore return cfg.saveFile() } // saveFile saves Config into the file. func (cfg *Config) saveFile() (returnErr error) { // marshal content - if cfg.credentialsStoreCache != "" { - credsStoreBytes, err := json.Marshal(cfg.credentialsStoreCache) + if cfg.credentialsStore != "" { + credsStoreBytes, err := json.Marshal(cfg.credentialsStore) if err != nil { return fmt.Errorf("failed to marshal creds store: %w", err) } @@ -236,8 +232,8 @@ func (cfg *Config) saveFile() (returnErr error) { delete(cfg.content, configFieldCredentialsStore) } - if len(cfg.credentialHelpersCache) > 0 { - credHelpersBytes, err := json.Marshal(cfg.credentialHelpersCache) + if len(cfg.credentialHelpers) > 0 { + credHelpersBytes, err := json.Marshal(cfg.credentialHelpers) if err != nil { return fmt.Errorf("failed to marshal cred helpers: %w", err) } @@ -283,8 +279,8 @@ func (cfg *Config) saveFile() (returnErr error) { // IsAuthConfigured returns whether there is authentication configured in this // config file or not. func (cfg *Config) IsAuthConfigured() bool { - return cfg.credentialsStoreCache != "" || - len(cfg.credentialHelpersCache) > 0 || + return cfg.credentialsStore != "" || + len(cfg.credentialHelpers) > 0 || len(cfg.authsCache) > 0 } From d44d9069875246527dca328fdb85ed86025ccc08 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Thu, 20 Apr 2023 15:52:34 +0800 Subject: [PATCH 21/34] reformat Signed-off-by: Sylvia Lei --- internal/config/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 0001b38..4d7d243 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -58,8 +58,8 @@ type Config struct { type AuthConfig struct { // Auth is a base64-encoded string of "{username}:{password}". Auth string `json:"auth,omitempty"` - // IdentityToken is used to authenticate the user and get. - // an access token for the registry. + // IdentityToken is used to authenticate the user and get an access token + // for the registry. IdentityToken string `json:"identitytoken,omitempty"` // RegistryToken is a bearer token to be sent to a registry. RegistryToken string `json:"registrytoken,omitempty"` From 129aef2ce57d6a8dc171df58ed485d57e4157b30 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Thu, 20 Apr 2023 17:16:04 +0800 Subject: [PATCH 22/34] address comments Signed-off-by: Sylvia Lei --- dynamic_store.go | 22 +++---------------- file_store.go | 6 ++--- internal/config/config.go | 13 +++-------- native_store.go | 10 +++++++++ ...helper_darwin.go => native_store_darwin.go | 0 ..._unsupported.go => native_store_generic.go | 1 - ...e_helper_linux.go => native_store_linux.go | 0 ...lper_windows.go => native_store_windows.go | 0 8 files changed, 19 insertions(+), 33 deletions(-) rename default_native_helper_darwin.go => native_store_darwin.go (100%) rename default_native_helper_unsupported.go => native_store_generic.go (96%) rename default_native_helper_linux.go => native_store_linux.go (100%) rename default_native_helper_windows.go => native_store_windows.go (100%) diff --git a/dynamic_store.go b/dynamic_store.go index 35f1437..43fa4b4 100644 --- a/dynamic_store.go +++ b/dynamic_store.go @@ -17,8 +17,6 @@ package credentials import ( "context" - "fmt" - "os/exec" "github.com/oras-project/oras-credentials-go/internal/config" "oras.land/oras-go/v2/registry/remote/auth" @@ -50,9 +48,7 @@ func NewStore(configPath string, opts StoreOptions) (Store, error) { } if !cfg.IsAuthConfigured() { if defaultCredsStore := getDefaultHelperSuffix(); defaultCredsStore != "" { - if err := cfg.PutCredentialsStore(defaultCredsStore); err != nil { - return nil, fmt.Errorf("failed to detect default creds store: %w", err) - } + cfg.SetCredentialsStore(defaultCredsStore) } } @@ -99,7 +95,7 @@ func (ds *dynamicStore) getHelperSuffix(serverAddress string) string { return helper } // 2. Then look for the configured native store - return ds.config.GetCredentialsStore() + return ds.config.CredentialsStore() } // getStore returns a store for the given server address. @@ -108,19 +104,7 @@ func (ds *dynamicStore) getStore(serverAddress string) (Store, error) { return NewNativeStore(helper), nil } - fs, err := newFileStore(ds.config) - if err != nil { - return nil, err - } + fs := newFileStore(ds.config) fs.DisablePut = !ds.options.AllowPlaintextPut return fs, nil } - -// getDefaultHelperSuffix returns the default credential helper suffix. -func getDefaultHelperSuffix() string { - platformDefault := getPlatformDefaultHelperSuffix() - if _, err := exec.LookPath(remoteCredentialsPrefix + platformDefault); err == nil { - return platformDefault - } - return "" -} diff --git a/file_store.go b/file_store.go index 6efb965..47afece 100644 --- a/file_store.go +++ b/file_store.go @@ -43,12 +43,12 @@ func NewFileStore(configPath string) (*FileStore, error) { if err != nil { return nil, err } - return newFileStore(cfg) + return newFileStore(cfg), nil } // newFileStore creates a file credentials store based on the given config instance. -func newFileStore(cfg *config.Config) (*FileStore, error) { - return &FileStore{config: cfg}, nil +func newFileStore(cfg *config.Config) *FileStore { + return &FileStore{config: cfg} } // Get retrieves credentials from the store for the given server address. diff --git a/internal/config/config.go b/internal/config/config.go index 4d7d243..5619111 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -202,20 +202,13 @@ func (cfg *Config) GetCredentialHelper(serverAddress string) string { } // GetCredentialHelpers returns the configured credentials store. -func (cfg *Config) GetCredentialsStore() string { - cfg.rwLock.RLock() - defer cfg.rwLock.RUnlock() - +func (cfg *Config) CredentialsStore() string { return cfg.credentialsStore } -// PutCredentialsStore puts the configured credentials store. -func (cfg *Config) PutCredentialsStore(credsStore string) error { - cfg.rwLock.Lock() - defer cfg.rwLock.Unlock() - +// SetCredentialsStore puts the configured credentials store. +func (cfg *Config) SetCredentialsStore(credsStore string) { cfg.credentialsStore = credsStore - return cfg.saveFile() } // saveFile saves Config into the file. diff --git a/native_store.go b/native_store.go index 5e5c7a3..7e7f19f 100644 --- a/native_store.go +++ b/native_store.go @@ -17,6 +17,7 @@ package credentials import ( "context" + "os/exec" "github.com/docker/docker-credential-helpers/client" "github.com/docker/docker-credential-helpers/credentials" @@ -81,3 +82,12 @@ func (ns *nativeStore) Put(_ context.Context, serverAddress string, cred auth.Cr func (ns *nativeStore) Delete(_ context.Context, serverAddress string) error { return client.Erase(ns.programFunc, serverAddress) } + +// getDefaultHelperSuffix returns the default credential helper suffix. +func getDefaultHelperSuffix() string { + platformDefault := getPlatformDefaultHelperSuffix() + if _, err := exec.LookPath(remoteCredentialsPrefix + platformDefault); err == nil { + return platformDefault + } + return "" +} diff --git a/default_native_helper_darwin.go b/native_store_darwin.go similarity index 100% rename from default_native_helper_darwin.go rename to native_store_darwin.go diff --git a/default_native_helper_unsupported.go b/native_store_generic.go similarity index 96% rename from default_native_helper_unsupported.go rename to native_store_generic.go index c506b84..5c7d4a3 100644 --- a/default_native_helper_unsupported.go +++ b/native_store_generic.go @@ -1,5 +1,4 @@ //go:build !windows && !darwin && !linux -// +build !windows,!darwin,!linux /* Copyright The ORAS Authors. diff --git a/default_native_helper_linux.go b/native_store_linux.go similarity index 100% rename from default_native_helper_linux.go rename to native_store_linux.go diff --git a/default_native_helper_windows.go b/native_store_windows.go similarity index 100% rename from default_native_helper_windows.go rename to native_store_windows.go From 78512871ae94e5df1e02ab21c40fa9a6c49fdf9d Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Thu, 20 Apr 2023 18:31:56 +0800 Subject: [PATCH 23/34] address comments Signed-off-by: Sylvia Lei --- dynamic_store.go | 11 +++++++++-- internal/config/config.go | 28 ++++++++-------------------- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/dynamic_store.go b/dynamic_store.go index 43fa4b4..6bf74db 100644 --- a/dynamic_store.go +++ b/dynamic_store.go @@ -41,6 +41,13 @@ type StoreOptions struct { } // NewStore returns a store based on given config file. +// If no any authentication is configured in the config file, a platform-default +// native store will be used. +// - Windows: "wincred" +// - Linux: "pass" or "secretservice" +// - MacOS: "osxkeychain" +// +// Reference: https://docs.docker.com/engine/reference/commandline/login/#credentials-store func NewStore(configPath string, opts StoreOptions) (Store, error) { cfg, err := config.LoadConfigFile(configPath) if err != nil { @@ -48,7 +55,7 @@ func NewStore(configPath string, opts StoreOptions) (Store, error) { } if !cfg.IsAuthConfigured() { if defaultCredsStore := getDefaultHelperSuffix(); defaultCredsStore != "" { - cfg.SetCredentialsStore(defaultCredsStore) + cfg.CredentialsStore = defaultCredsStore } } @@ -95,7 +102,7 @@ func (ds *dynamicStore) getHelperSuffix(serverAddress string) string { return helper } // 2. Then look for the configured native store - return ds.config.CredentialsStore() + return ds.config.CredentialsStore } // getStore returns a store for the given server address. diff --git a/internal/config/config.go b/internal/config/config.go index 5619111..c4ff5aa 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -33,6 +33,10 @@ import ( // Config represents a docker configuration file. // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L17-L44 type Config struct { + // CredentialsStore is the credsStore field of the config. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L28 + CredentialsStore string + // path is the path to the config file. path string // rwLock is a read-write-lock for the file store. @@ -43,9 +47,6 @@ type Config struct { // authsCache is a cache of the auths field of the config. // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L19 authsCache map[string]json.RawMessage - // credentialsStore is the credsStore field of the config. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L28 - credentialsStore string // credentialHelpers is the credHelpers field of the config. // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L29 credentialHelpers map[string]string @@ -130,7 +131,7 @@ func LoadConfigFile(configPath string) (*Config, error) { } if credsStoreBytes, ok := cfg.content[configFieldCredentialsStore]; ok { - if err := json.Unmarshal(credsStoreBytes, &cfg.credentialsStore); err != nil { + if err := json.Unmarshal(credsStoreBytes, &cfg.CredentialsStore); err != nil { return nil, fmt.Errorf("failed to unmarshal creds store field: %w: %v", ErrInvalidConfigFormat, err) } } @@ -201,21 +202,11 @@ func (cfg *Config) GetCredentialHelper(serverAddress string) string { return cfg.credentialHelpers[serverAddress] } -// GetCredentialHelpers returns the configured credentials store. -func (cfg *Config) CredentialsStore() string { - return cfg.credentialsStore -} - -// SetCredentialsStore puts the configured credentials store. -func (cfg *Config) SetCredentialsStore(credsStore string) { - cfg.credentialsStore = credsStore -} - // saveFile saves Config into the file. func (cfg *Config) saveFile() (returnErr error) { // marshal content - if cfg.credentialsStore != "" { - credsStoreBytes, err := json.Marshal(cfg.credentialsStore) + if cfg.CredentialsStore != "" { + credsStoreBytes, err := json.Marshal(cfg.CredentialsStore) if err != nil { return fmt.Errorf("failed to marshal creds store: %w", err) } @@ -231,9 +222,6 @@ func (cfg *Config) saveFile() (returnErr error) { return fmt.Errorf("failed to marshal cred helpers: %w", err) } cfg.content[configFieldCredentialHelpers] = credHelpersBytes - } else { - // omit empty - delete(cfg.content, configFieldCredentialHelpers) } authsBytes, err := json.Marshal(cfg.authsCache) @@ -272,7 +260,7 @@ func (cfg *Config) saveFile() (returnErr error) { // IsAuthConfigured returns whether there is authentication configured in this // config file or not. func (cfg *Config) IsAuthConfigured() bool { - return cfg.credentialsStore != "" || + return cfg.CredentialsStore != "" || len(cfg.credentialHelpers) > 0 || len(cfg.authsCache) > 0 } From dae7c9a4dfbe26c961a0e9af0280833935e1da44 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Thu, 20 Apr 2023 19:07:58 +0800 Subject: [PATCH 24/34] rebase Signed-off-by: Sylvia Lei --- dynamic_store_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dynamic_store_test.go b/dynamic_store_test.go index 2cdade3..05e8f5c 100644 --- a/dynamic_store_test.go +++ b/dynamic_store_test.go @@ -286,7 +286,7 @@ func Test_dynamicStore_getStore_nativeStore(t *testing.T) { if err != nil { t.Fatal("dynamicStore.getStore() error =", err) } - if _, ok := gotStore.(*NativeStore); !ok { + if _, ok := gotStore.(*nativeStore); !ok { t.Errorf("gotStore is not a native store") } }) From ec7bfc92f85fb2452e908f1dbe42e92eb721f131 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Thu, 20 Apr 2023 19:10:11 +0800 Subject: [PATCH 25/34] moved to store.go Signed-off-by: Sylvia Lei --- dynamic_store.go | 117 -------------- dynamic_store_test.go | 343 ------------------------------------------ store.go | 95 ++++++++++++ store_test.go | 312 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 407 insertions(+), 460 deletions(-) delete mode 100644 dynamic_store.go delete mode 100644 dynamic_store_test.go diff --git a/dynamic_store.go b/dynamic_store.go deleted file mode 100644 index 6bf74db..0000000 --- a/dynamic_store.go +++ /dev/null @@ -1,117 +0,0 @@ -/* -Copyright The ORAS Authors. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - -http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package credentials - -import ( - "context" - - "github.com/oras-project/oras-credentials-go/internal/config" - "oras.land/oras-go/v2/registry/remote/auth" -) - -// dynamicStore dynamically determines which store to use based on the settings -// in the config file. -type dynamicStore struct { - config *config.Config - options StoreOptions -} - -// StoreOptions provides options for NewStore. -type StoreOptions struct { - // AllowPlaintextPut allows saving credentials in plaintext in the config - // file. - // - If AllowPlaintextPut is set to false (default value), Put() will - // return an error when native store is not available. - // - If AllowPlaintextPut is set to true, Put() will save credentials in - // plaintext in the config file when native store is not available. - AllowPlaintextPut bool -} - -// NewStore returns a store based on given config file. -// If no any authentication is configured in the config file, a platform-default -// native store will be used. -// - Windows: "wincred" -// - Linux: "pass" or "secretservice" -// - MacOS: "osxkeychain" -// -// Reference: https://docs.docker.com/engine/reference/commandline/login/#credentials-store -func NewStore(configPath string, opts StoreOptions) (Store, error) { - cfg, err := config.LoadConfigFile(configPath) - if err != nil { - return nil, err - } - if !cfg.IsAuthConfigured() { - if defaultCredsStore := getDefaultHelperSuffix(); defaultCredsStore != "" { - cfg.CredentialsStore = defaultCredsStore - } - } - - return &dynamicStore{ - config: cfg, - options: opts, - }, nil -} - -// Get retrieves credentials from the store for the given server address. -func (ds *dynamicStore) Get(ctx context.Context, serverAddress string) (auth.Credential, error) { - store, err := ds.getStore(serverAddress) - if err != nil { - return auth.EmptyCredential, err - } - return store.Get(ctx, serverAddress) -} - -// Put saves credentials into the store for the given server address. -// Returns ErrPlaintextPutDisabled if native store is not available and -// StoreOptions.AllowPlaintextPut is set to false. -func (ds *dynamicStore) Put(ctx context.Context, serverAddress string, cred auth.Credential) error { - store, err := ds.getStore(serverAddress) - if err != nil { - return err - } - return store.Put(ctx, serverAddress, cred) -} - -// Delete removes credentials from the store for the given server address. -func (ds *dynamicStore) Delete(ctx context.Context, serverAddress string) error { - store, err := ds.getStore(serverAddress) - if err != nil { - return err - } - return store.Delete(ctx, serverAddress) -} - -// getHelperSuffix returns the credential helper suffix for the given server -// address. -func (ds *dynamicStore) getHelperSuffix(serverAddress string) string { - // 1. Look for a server-specific credential helper first - if helper := ds.config.GetCredentialHelper(serverAddress); helper != "" { - return helper - } - // 2. Then look for the configured native store - return ds.config.CredentialsStore -} - -// getStore returns a store for the given server address. -func (ds *dynamicStore) getStore(serverAddress string) (Store, error) { - if helper := ds.getHelperSuffix(serverAddress); helper != "" { - return NewNativeStore(helper), nil - } - - fs := newFileStore(ds.config) - fs.DisablePut = !ds.options.AllowPlaintextPut - return fs, nil -} diff --git a/dynamic_store_test.go b/dynamic_store_test.go deleted file mode 100644 index 05e8f5c..0000000 --- a/dynamic_store_test.go +++ /dev/null @@ -1,343 +0,0 @@ -/* -Copyright The ORAS Authors. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - -http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package credentials - -import ( - "context" - "encoding/json" - "errors" - "os" - "path/filepath" - "reflect" - "testing" - - "oras.land/oras-go/v2/registry/remote/auth" -) - -func Test_dynamicStore_interface(t *testing.T) { - var ds interface{} = &dynamicStore{} - if _, ok := ds.(Store); !ok { - t.Error("&dynamicStore{} does not conform Store") - } -} - -func Test_dynamicStore(t *testing.T) { - // prepare test content - tempDir := t.TempDir() - authConfiguredPath := filepath.Join(tempDir, "auth_configured.json") - config := testConfig{ - AuthConfigs: map[string]testAuthConfig{ - "xxx": {}, - }, - SomeConfigField: 123, - } - jsonStr, err := json.Marshal(config) - if err != nil { - t.Fatalf("failed to marshal config: %v", err) - } - if err := os.WriteFile(authConfiguredPath, jsonStr, 0666); err != nil { - t.Fatalf("failed to write config file: %v", err) - } - - noAuthConfiguredPath := filepath.Join(tempDir, "no_auth_configured.json") - config = testConfig{ - SomeConfigField: 123, - } - jsonStr, err = json.Marshal(config) - if err != nil { - t.Fatalf("failed to marshal config: %v", err) - } - if err := os.WriteFile(noAuthConfiguredPath, jsonStr, 0666); err != nil { - t.Fatalf("failed to write config file: %v", err) - } - - tests := []struct { - name string - configPath string - }{ - { - name: "Authentication configured", - configPath: authConfiguredPath, - }, - { - name: "No authentication configured", - configPath: noAuthConfiguredPath, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ds, err := NewStore(tt.configPath, StoreOptions{AllowPlaintextPut: true}) - if err != nil { - t.Fatal("NewStore() error =", err) - } - serverAddr := "test.example.com" - cred := auth.Credential{ - Username: "username", - Password: "password", - } - ctx := context.Background() - - // test put - if err := ds.Put(ctx, serverAddr, cred); err != nil { - t.Fatal("dynamicStore.Get() error =", err) - } - - // test get - got, err := ds.Get(ctx, serverAddr) - if err != nil { - t.Fatal("dynamicStore.Get() error =", err) - } - if want := cred; got != want { - t.Errorf("dynamicStore.Get() = %v, want %v", got, want) - } - - // test delete - err = ds.Delete(ctx, serverAddr) - if err != nil { - t.Fatal("dynamicStore.Delete() error =", err) - } - - // verify delete - got, err = ds.Get(ctx, serverAddr) - if err != nil { - t.Fatal("dynamicStore.Get() error =", err) - } - if want := auth.EmptyCredential; got != want { - t.Errorf("dynamicStore.Get() = %v, want %v", got, want) - } - }) - } -} - -func Test_dynamicStore_fileStore_AllowPlainTextPut(t *testing.T) { - // prepare test content - tempDir := t.TempDir() - configPath := filepath.Join(tempDir, "config.json") - serverAddr := "newtest.example.com" - cred := auth.Credential{ - Username: "username", - Password: "password", - } - ctx := context.Background() - - cfg := testConfig{ - AuthConfigs: map[string]testAuthConfig{ - "test.example.com": {}, - }, - SomeConfigField: 123, - } - jsonStr, err := json.Marshal(cfg) - if err != nil { - t.Fatalf("failed to marshal config: %v", err) - } - if err := os.WriteFile(configPath, jsonStr, 0666); err != nil { - t.Fatalf("failed to write config file: %v", err) - } - - // test default option - ds, err := NewStore(configPath, StoreOptions{}) - if err != nil { - t.Fatal("NewStore() error =", err) - } - err = ds.Put(ctx, serverAddr, cred) - if wantErr := ErrPlaintextPutDisabled; !errors.Is(err, wantErr) { - t.Errorf("dynamicStore.Put() error = %v, wantErr %v", err, wantErr) - } - - // test AllowPlainTextPut = true - ds, err = NewStore(configPath, StoreOptions{AllowPlaintextPut: true}) - if err != nil { - t.Fatal("NewStore() error =", err) - } - if err := ds.Put(ctx, serverAddr, cred); err != nil { - t.Error("dynamicStore.Put() error =", err) - } - - // verify config file - configFile, err := os.Open(configPath) - if err != nil { - t.Fatalf("failed to open config file: %v", err) - } - defer configFile.Close() - var gotCfg testConfig - if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { - t.Fatalf("failed to decode config file: %v", err) - } - wantCfg := testConfig{ - AuthConfigs: map[string]testAuthConfig{ - "test.example.com": {}, - serverAddr: { - Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", - }, - }, - SomeConfigField: cfg.SomeConfigField, - } - if !reflect.DeepEqual(gotCfg, wantCfg) { - t.Errorf("Decoded config = %v, want %v", gotCfg, wantCfg) - } -} - -func Test_dynamicStore_getHelperSuffix(t *testing.T) { - tests := []struct { - name string - configPath string - serverAddress string - want string - }{ - { - name: "Get cred helper: registry_helper1", - configPath: "testdata/credHelpers_config.json", - serverAddress: "registry1.example.com", - want: "registry1-helper", - }, - { - name: "Get cred helper: registry_helper2", - configPath: "testdata/credHelpers_config.json", - serverAddress: "registry2.example.com", - want: "registry2-helper", - }, - { - name: "Empty cred helper configured", - configPath: "testdata/credHelpers_config.json", - serverAddress: "registry3.example.com", - want: "", - }, - { - name: "No cred helper and creds store configured", - configPath: "testdata/credHelpers_config.json", - serverAddress: "whatever.example.com", - want: "", - }, - { - name: "Choose cred helper over creds store", - configPath: "testdata/credsStore_config.json", - serverAddress: "test.example.com", - want: "test-helper", - }, - { - name: "No cred helper configured, choose cred store", - configPath: "testdata/credsStore_config.json", - serverAddress: "whatever.example.com", - want: "teststore", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - store, err := NewStore(tt.configPath, StoreOptions{}) - if err != nil { - t.Fatal("NewStore() error =", err) - } - ds := store.(*dynamicStore) - if got := ds.getHelperSuffix(tt.serverAddress); got != tt.want { - t.Errorf("dynamicStore.getHelperSuffix() = %v, want %v", got, tt.want) - } - }) - } -} - -func Test_dynamicStore_getStore_nativeStore(t *testing.T) { - tests := []struct { - name string - configPath string - serverAddress string - }{ - { - name: "Cred helper configured for registry1.example.com", - configPath: "testdata/credHelpers_config.json", - serverAddress: "registry1.example.com", - }, - { - name: "Cred helper configured for registry2.example.com", - configPath: "testdata/credHelpers_config.json", - serverAddress: "registry2.example.com", - }, - { - name: "Cred helper configured for test.example.com", - configPath: "testdata/credsStore_config.json", - serverAddress: "test.example.com", - }, - { - name: "No cred helper configured, use creds store", - configPath: "testdata/credsStore_config.json", - serverAddress: "whaterver.example.com", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - store, err := NewStore(tt.configPath, StoreOptions{}) - if err != nil { - t.Fatal("NewStore() error =", err) - } - ds := store.(*dynamicStore) - gotStore, err := ds.getStore(tt.serverAddress) - if err != nil { - t.Fatal("dynamicStore.getStore() error =", err) - } - if _, ok := gotStore.(*nativeStore); !ok { - t.Errorf("gotStore is not a native store") - } - }) - } -} - -func Test_dynamicStore_getStore_fileStore(t *testing.T) { - tests := []struct { - name string - configPath string - serverAddress string - }{ - { - name: "Empty cred helper configured for registry3.example.com", - configPath: "testdata/credHelpers_config.json", - serverAddress: "registry3.example.com", - }, - { - name: "No cred helper configured", - configPath: "testdata/credHelpers_config.json", - serverAddress: "whatever.example.com", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - store, err := NewStore(tt.configPath, StoreOptions{}) - if err != nil { - t.Fatal("NewStore() error =", err) - } - ds := store.(*dynamicStore) - gotStore, err := ds.getStore(tt.serverAddress) - if err != nil { - t.Fatal("dynamicStore.getStore() error =", err) - } - gotFS1, ok := gotStore.(*FileStore) - if !ok { - t.Errorf("gotStore is not a file store") - } - - // get again, the two file stores should be based on the same config instance - gotStore, err = ds.getStore(tt.serverAddress) - if err != nil { - t.Fatal("dynamicStore.getStore() error =", err) - } - gotFS2, ok := gotStore.(*FileStore) - if !ok { - t.Errorf("gotStore is not a file store") - } - if gotFS1.config != gotFS2.config { - t.Errorf("gotFS1 and gotFS2 are not based on the same config") - } - }) - } -} diff --git a/store.go b/store.go index b3de935..52262a0 100644 --- a/store.go +++ b/store.go @@ -18,6 +18,7 @@ package credentials import ( "context" + "github.com/oras-project/oras-credentials-go/internal/config" "oras.land/oras-go/v2/registry/remote/auth" ) @@ -31,6 +32,100 @@ type Store interface { Delete(ctx context.Context, serverAddress string) error } +// dynamicStore dynamically determines which store to use based on the settings +// in the config file. +type dynamicStore struct { + config *config.Config + options StoreOptions +} + +// StoreOptions provides options for NewStore. +type StoreOptions struct { + // AllowPlaintextPut allows saving credentials in plaintext in the config + // file. + // - If AllowPlaintextPut is set to false (default value), Put() will + // return an error when native store is not available. + // - If AllowPlaintextPut is set to true, Put() will save credentials in + // plaintext in the config file when native store is not available. + AllowPlaintextPut bool +} + +// NewStore returns a store based on given config file. +// If no any authentication is configured in the config file, a platform-default +// native store will be used. +// - Windows: "wincred" +// - Linux: "pass" or "secretservice" +// - MacOS: "osxkeychain" +// +// Reference: https://docs.docker.com/engine/reference/commandline/login/#credentials-store +func NewStore(configPath string, opts StoreOptions) (Store, error) { + cfg, err := config.LoadConfigFile(configPath) + if err != nil { + return nil, err + } + if !cfg.IsAuthConfigured() { + if defaultCredsStore := getDefaultHelperSuffix(); defaultCredsStore != "" { + cfg.CredentialsStore = defaultCredsStore + } + } + + return &dynamicStore{ + config: cfg, + options: opts, + }, nil +} + +// Get retrieves credentials from the store for the given server address. +func (ds *dynamicStore) Get(ctx context.Context, serverAddress string) (auth.Credential, error) { + store, err := ds.getStore(serverAddress) + if err != nil { + return auth.EmptyCredential, err + } + return store.Get(ctx, serverAddress) +} + +// Put saves credentials into the store for the given server address. +// Returns ErrPlaintextPutDisabled if native store is not available and +// StoreOptions.AllowPlaintextPut is set to false. +func (ds *dynamicStore) Put(ctx context.Context, serverAddress string, cred auth.Credential) error { + store, err := ds.getStore(serverAddress) + if err != nil { + return err + } + return store.Put(ctx, serverAddress, cred) +} + +// Delete removes credentials from the store for the given server address. +func (ds *dynamicStore) Delete(ctx context.Context, serverAddress string) error { + store, err := ds.getStore(serverAddress) + if err != nil { + return err + } + return store.Delete(ctx, serverAddress) +} + +// getHelperSuffix returns the credential helper suffix for the given server +// address. +func (ds *dynamicStore) getHelperSuffix(serverAddress string) string { + // 1. Look for a server-specific credential helper first + if helper := ds.config.GetCredentialHelper(serverAddress); helper != "" { + return helper + } + // 2. Then look for the configured native store + return ds.config.CredentialsStore +} + +// getStore returns a store for the given server address. +func (ds *dynamicStore) getStore(serverAddress string) (Store, error) { + if helper := ds.getHelperSuffix(serverAddress); helper != "" { + return NewNativeStore(helper), nil + } + + fs := newFileStore(ds.config) + fs.DisablePut = !ds.options.AllowPlaintextPut + return fs, nil +} + // storeWithFallbacks is a store that has multiple fallback stores. type storeWithFallbacks struct { stores []Store diff --git a/store_test.go b/store_test.go index 4960356..cda812f 100644 --- a/store_test.go +++ b/store_test.go @@ -17,12 +17,324 @@ package credentials import ( "context" + "encoding/json" + "errors" + "os" + "path/filepath" "reflect" "testing" "oras.land/oras-go/v2/registry/remote/auth" ) +func Test_dynamicStore(t *testing.T) { + // prepare test content + tempDir := t.TempDir() + authConfiguredPath := filepath.Join(tempDir, "auth_configured.json") + config := testConfig{ + AuthConfigs: map[string]testAuthConfig{ + "xxx": {}, + }, + SomeConfigField: 123, + } + jsonStr, err := json.Marshal(config) + if err != nil { + t.Fatalf("failed to marshal config: %v", err) + } + if err := os.WriteFile(authConfiguredPath, jsonStr, 0666); err != nil { + t.Fatalf("failed to write config file: %v", err) + } + + noAuthConfiguredPath := filepath.Join(tempDir, "no_auth_configured.json") + config = testConfig{ + SomeConfigField: 123, + } + jsonStr, err = json.Marshal(config) + if err != nil { + t.Fatalf("failed to marshal config: %v", err) + } + if err := os.WriteFile(noAuthConfiguredPath, jsonStr, 0666); err != nil { + t.Fatalf("failed to write config file: %v", err) + } + + tests := []struct { + name string + configPath string + }{ + { + name: "Authentication configured", + configPath: authConfiguredPath, + }, + { + name: "No authentication configured", + configPath: noAuthConfiguredPath, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ds, err := NewStore(tt.configPath, StoreOptions{AllowPlaintextPut: true}) + if err != nil { + t.Fatal("NewStore() error =", err) + } + serverAddr := "test.example.com" + cred := auth.Credential{ + Username: "username", + Password: "password", + } + ctx := context.Background() + + // test put + if err := ds.Put(ctx, serverAddr, cred); err != nil { + t.Fatal("dynamicStore.Get() error =", err) + } + + // test get + got, err := ds.Get(ctx, serverAddr) + if err != nil { + t.Fatal("dynamicStore.Get() error =", err) + } + if want := cred; got != want { + t.Errorf("dynamicStore.Get() = %v, want %v", got, want) + } + + // test delete + err = ds.Delete(ctx, serverAddr) + if err != nil { + t.Fatal("dynamicStore.Delete() error =", err) + } + + // verify delete + got, err = ds.Get(ctx, serverAddr) + if err != nil { + t.Fatal("dynamicStore.Get() error =", err) + } + if want := auth.EmptyCredential; got != want { + t.Errorf("dynamicStore.Get() = %v, want %v", got, want) + } + }) + } +} + +func Test_dynamicStore_fileStore_AllowPlainTextPut(t *testing.T) { + // prepare test content + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "config.json") + serverAddr := "newtest.example.com" + cred := auth.Credential{ + Username: "username", + Password: "password", + } + ctx := context.Background() + + cfg := testConfig{ + AuthConfigs: map[string]testAuthConfig{ + "test.example.com": {}, + }, + SomeConfigField: 123, + } + jsonStr, err := json.Marshal(cfg) + if err != nil { + t.Fatalf("failed to marshal config: %v", err) + } + if err := os.WriteFile(configPath, jsonStr, 0666); err != nil { + t.Fatalf("failed to write config file: %v", err) + } + + // test default option + ds, err := NewStore(configPath, StoreOptions{}) + if err != nil { + t.Fatal("NewStore() error =", err) + } + err = ds.Put(ctx, serverAddr, cred) + if wantErr := ErrPlaintextPutDisabled; !errors.Is(err, wantErr) { + t.Errorf("dynamicStore.Put() error = %v, wantErr %v", err, wantErr) + } + + // test AllowPlainTextPut = true + ds, err = NewStore(configPath, StoreOptions{AllowPlaintextPut: true}) + if err != nil { + t.Fatal("NewStore() error =", err) + } + if err := ds.Put(ctx, serverAddr, cred); err != nil { + t.Error("dynamicStore.Put() error =", err) + } + + // verify config file + configFile, err := os.Open(configPath) + if err != nil { + t.Fatalf("failed to open config file: %v", err) + } + defer configFile.Close() + var gotCfg testConfig + if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { + t.Fatalf("failed to decode config file: %v", err) + } + wantCfg := testConfig{ + AuthConfigs: map[string]testAuthConfig{ + "test.example.com": {}, + serverAddr: { + Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", + }, + }, + SomeConfigField: cfg.SomeConfigField, + } + if !reflect.DeepEqual(gotCfg, wantCfg) { + t.Errorf("Decoded config = %v, want %v", gotCfg, wantCfg) + } +} + +func Test_dynamicStore_getHelperSuffix(t *testing.T) { + tests := []struct { + name string + configPath string + serverAddress string + want string + }{ + { + name: "Get cred helper: registry_helper1", + configPath: "testdata/credHelpers_config.json", + serverAddress: "registry1.example.com", + want: "registry1-helper", + }, + { + name: "Get cred helper: registry_helper2", + configPath: "testdata/credHelpers_config.json", + serverAddress: "registry2.example.com", + want: "registry2-helper", + }, + { + name: "Empty cred helper configured", + configPath: "testdata/credHelpers_config.json", + serverAddress: "registry3.example.com", + want: "", + }, + { + name: "No cred helper and creds store configured", + configPath: "testdata/credHelpers_config.json", + serverAddress: "whatever.example.com", + want: "", + }, + { + name: "Choose cred helper over creds store", + configPath: "testdata/credsStore_config.json", + serverAddress: "test.example.com", + want: "test-helper", + }, + { + name: "No cred helper configured, choose cred store", + configPath: "testdata/credsStore_config.json", + serverAddress: "whatever.example.com", + want: "teststore", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + store, err := NewStore(tt.configPath, StoreOptions{}) + if err != nil { + t.Fatal("NewStore() error =", err) + } + ds := store.(*dynamicStore) + if got := ds.getHelperSuffix(tt.serverAddress); got != tt.want { + t.Errorf("dynamicStore.getHelperSuffix() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_dynamicStore_getStore_nativeStore(t *testing.T) { + tests := []struct { + name string + configPath string + serverAddress string + }{ + { + name: "Cred helper configured for registry1.example.com", + configPath: "testdata/credHelpers_config.json", + serverAddress: "registry1.example.com", + }, + { + name: "Cred helper configured for registry2.example.com", + configPath: "testdata/credHelpers_config.json", + serverAddress: "registry2.example.com", + }, + { + name: "Cred helper configured for test.example.com", + configPath: "testdata/credsStore_config.json", + serverAddress: "test.example.com", + }, + { + name: "No cred helper configured, use creds store", + configPath: "testdata/credsStore_config.json", + serverAddress: "whaterver.example.com", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + store, err := NewStore(tt.configPath, StoreOptions{}) + if err != nil { + t.Fatal("NewStore() error =", err) + } + ds := store.(*dynamicStore) + gotStore, err := ds.getStore(tt.serverAddress) + if err != nil { + t.Fatal("dynamicStore.getStore() error =", err) + } + if _, ok := gotStore.(*nativeStore); !ok { + t.Errorf("gotStore is not a native store") + } + }) + } +} + +func Test_dynamicStore_getStore_fileStore(t *testing.T) { + tests := []struct { + name string + configPath string + serverAddress string + }{ + { + name: "Empty cred helper configured for registry3.example.com", + configPath: "testdata/credHelpers_config.json", + serverAddress: "registry3.example.com", + }, + { + name: "No cred helper configured", + configPath: "testdata/credHelpers_config.json", + serverAddress: "whatever.example.com", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + store, err := NewStore(tt.configPath, StoreOptions{}) + if err != nil { + t.Fatal("NewStore() error =", err) + } + ds := store.(*dynamicStore) + gotStore, err := ds.getStore(tt.serverAddress) + if err != nil { + t.Fatal("dynamicStore.getStore() error =", err) + } + gotFS1, ok := gotStore.(*FileStore) + if !ok { + t.Errorf("gotStore is not a file store") + } + + // get again, the two file stores should be based on the same config instance + gotStore, err = ds.getStore(tt.serverAddress) + if err != nil { + t.Fatal("dynamicStore.getStore() error =", err) + } + gotFS2, ok := gotStore.(*FileStore) + if !ok { + t.Errorf("gotStore is not a file store") + } + if gotFS1.config != gotFS2.config { + t.Errorf("gotFS1 and gotFS2 are not based on the same config") + } + }) + } +} + func TestStoreWithFallbacks(t *testing.T) { // Initialize a StoreWithFallbacks primaryStore := &testStore{} From f47742d97a686eccdec8a02a97640e605c20a9c2 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Thu, 20 Apr 2023 19:33:30 +0800 Subject: [PATCH 26/34] minor improvement Signed-off-by: Sylvia Lei --- internal/config/config.go | 8 +------- store.go | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index c4ff5aa..92b876b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -216,13 +216,7 @@ func (cfg *Config) saveFile() (returnErr error) { delete(cfg.content, configFieldCredentialsStore) } - if len(cfg.credentialHelpers) > 0 { - credHelpersBytes, err := json.Marshal(cfg.credentialHelpers) - if err != nil { - return fmt.Errorf("failed to marshal cred helpers: %w", err) - } - cfg.content[configFieldCredentialHelpers] = credHelpersBytes - } + // skip saving credentialHelpers as it's never set authsBytes, err := json.Marshal(cfg.authsCache) if err != nil { diff --git a/store.go b/store.go index 52262a0..f63aec6 100644 --- a/store.go +++ b/store.go @@ -55,7 +55,7 @@ type StoreOptions struct { // native store will be used. // - Windows: "wincred" // - Linux: "pass" or "secretservice" -// - MacOS: "osxkeychain" +// - macOS: "osxkeychain" // // Reference: https://docs.docker.com/engine/reference/commandline/login/#credentials-store func NewStore(configPath string, opts StoreOptions) (Store, error) { From c7b4d5102bd5607d2c0823bf392709d79d9c5b81 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Fri, 21 Apr 2023 15:35:23 +0800 Subject: [PATCH 27/34] refactor dynamic store Signed-off-by: Sylvia Lei --- internal/config/config.go | 32 +++++-- store.go | 46 +++++++--- store_test.go | 173 ++++++++++++++++++++++++-------------- 3 files changed, 168 insertions(+), 83 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 92b876b..3ecfc81 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -33,10 +33,6 @@ import ( // Config represents a docker configuration file. // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L17-L44 type Config struct { - // CredentialsStore is the credsStore field of the config. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L28 - CredentialsStore string - // path is the path to the config file. path string // rwLock is a read-write-lock for the file store. @@ -47,6 +43,9 @@ type Config struct { // authsCache is a cache of the auths field of the config. // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L19 authsCache map[string]json.RawMessage + // credentialsStore is the credsStore field of the config. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L28 + credentialsStore string // credentialHelpers is the credHelpers field of the config. // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L29 credentialHelpers map[string]string @@ -131,7 +130,7 @@ func LoadConfigFile(configPath string) (*Config, error) { } if credsStoreBytes, ok := cfg.content[configFieldCredentialsStore]; ok { - if err := json.Unmarshal(credsStoreBytes, &cfg.CredentialsStore); err != nil { + if err := json.Unmarshal(credsStoreBytes, &cfg.credentialsStore); err != nil { return nil, fmt.Errorf("failed to unmarshal creds store field: %w: %v", ErrInvalidConfigFormat, err) } } @@ -202,11 +201,28 @@ func (cfg *Config) GetCredentialHelper(serverAddress string) string { return cfg.credentialHelpers[serverAddress] } +// CredentialsStore returns the configured credentials store. +func (cfg *Config) CredentialsStore() string { + cfg.rwLock.RLock() + defer cfg.rwLock.RUnlock() + + return cfg.credentialsStore +} + +// SetCredentialsStore puts the configured credentials store. +func (cfg *Config) SetCredentialsStore(credsStore string) error { + cfg.rwLock.Lock() + defer cfg.rwLock.Unlock() + + cfg.credentialsStore = credsStore + return cfg.saveFile() +} + // saveFile saves Config into the file. func (cfg *Config) saveFile() (returnErr error) { // marshal content - if cfg.CredentialsStore != "" { - credsStoreBytes, err := json.Marshal(cfg.CredentialsStore) + if cfg.credentialsStore != "" { + credsStoreBytes, err := json.Marshal(cfg.credentialsStore) if err != nil { return fmt.Errorf("failed to marshal creds store: %w", err) } @@ -254,7 +270,7 @@ func (cfg *Config) saveFile() (returnErr error) { // IsAuthConfigured returns whether there is authentication configured in this // config file or not. func (cfg *Config) IsAuthConfigured() bool { - return cfg.CredentialsStore != "" || + return cfg.credentialsStore != "" || len(cfg.credentialHelpers) > 0 || len(cfg.authsCache) > 0 } diff --git a/store.go b/store.go index f63aec6..c948c18 100644 --- a/store.go +++ b/store.go @@ -17,6 +17,8 @@ package credentials import ( "context" + "fmt" + "sync" "github.com/oras-project/oras-credentials-go/internal/config" "oras.land/oras-go/v2/registry/remote/auth" @@ -35,8 +37,10 @@ type Store interface { // dynamicStore dynamically determines which store to use based on the settings // in the config file. type dynamicStore struct { - config *config.Config - options StoreOptions + config *config.Config + options StoreOptions + detectedCredsStore string + setCredsStoreOnce sync.Once } // StoreOptions provides options for NewStore. @@ -51,7 +55,7 @@ type StoreOptions struct { } // NewStore returns a store based on given config file. -// If no any authentication is configured in the config file, a platform-default +// If no authentication is configured in the config file, a platform-default // native store will be used. // - Windows: "wincred" // - Linux: "pass" or "secretservice" @@ -63,16 +67,15 @@ func NewStore(configPath string, opts StoreOptions) (Store, error) { if err != nil { return nil, err } - if !cfg.IsAuthConfigured() { - if defaultCredsStore := getDefaultHelperSuffix(); defaultCredsStore != "" { - cfg.CredentialsStore = defaultCredsStore - } - } - - return &dynamicStore{ + ds := &dynamicStore{ config: cfg, options: opts, - }, nil + } + if !cfg.IsAuthConfigured() { + // no authentication configured, detect the default credentials store + ds.detectedCredsStore = getDefaultHelperSuffix() + } + return ds, nil } // Get retrieves credentials from the store for the given server address. @@ -87,12 +90,23 @@ func (ds *dynamicStore) Get(ctx context.Context, serverAddress string) (auth.Cre // Put saves credentials into the store for the given server address. // Returns ErrPlaintextPutDisabled if native store is not available and // StoreOptions.AllowPlaintextPut is set to false. -func (ds *dynamicStore) Put(ctx context.Context, serverAddress string, cred auth.Credential) error { +func (ds *dynamicStore) Put(ctx context.Context, serverAddress string, cred auth.Credential) (returnErr error) { store, err := ds.getStore(serverAddress) if err != nil { return err } - return store.Put(ctx, serverAddress, cred) + if err := store.Put(ctx, serverAddress, cred); err != nil { + return err + } + // save the detected creds store back to the config file on first put + ds.setCredsStoreOnce.Do(func() { + if ds.detectedCredsStore != "" { + if err := ds.config.SetCredentialsStore(ds.detectedCredsStore); err != nil { + returnErr = fmt.Errorf("failed to set credsStore: %w", err) + } + } + }) + return returnErr } // Delete removes credentials from the store for the given server address. @@ -112,7 +126,11 @@ func (ds *dynamicStore) getHelperSuffix(serverAddress string) string { return helper } // 2. Then look for the configured native store - return ds.config.CredentialsStore + if credsStore := ds.config.CredentialsStore(); credsStore != "" { + return credsStore + } + // 3. Use the detected default store + return ds.detectedCredsStore } // getStore returns a store for the given server address. diff --git a/store_test.go b/store_test.go index cda812f..1137420 100644 --- a/store_test.go +++ b/store_test.go @@ -27,11 +27,18 @@ import ( "oras.land/oras-go/v2/registry/remote/auth" ) -func Test_dynamicStore(t *testing.T) { +type testStoreConfig struct { + SomeConfigField int `json:"some_config_field"` + AuthConfigs map[string]testAuthConfig `json:"auths"` + CredentialsStore string `json:"credsStore,omitempty"` + CredentialHelpers map[string]string `json:"credHelpers,omitempty"` +} + +func Test_dynamicStore_authConfigured(t *testing.T) { // prepare test content tempDir := t.TempDir() - authConfiguredPath := filepath.Join(tempDir, "auth_configured.json") - config := testConfig{ + configPath := filepath.Join(tempDir, "auth_configured.json") + config := testStoreConfig{ AuthConfigs: map[string]testAuthConfig{ "xxx": {}, }, @@ -41,77 +48,121 @@ func Test_dynamicStore(t *testing.T) { if err != nil { t.Fatalf("failed to marshal config: %v", err) } - if err := os.WriteFile(authConfiguredPath, jsonStr, 0666); err != nil { + if err := os.WriteFile(configPath, jsonStr, 0666); err != nil { t.Fatalf("failed to write config file: %v", err) } - noAuthConfiguredPath := filepath.Join(tempDir, "no_auth_configured.json") - config = testConfig{ + store, err := NewStore(configPath, StoreOptions{AllowPlaintextPut: true}) + if err != nil { + t.Fatal("NewStore() error =", err) + } + ds := store.(*dynamicStore) + serverAddr := "test.example.com" + cred := auth.Credential{ + Username: "username", + Password: "password", + } + ctx := context.Background() + + // test put + if err := ds.Put(ctx, serverAddr, cred); err != nil { + t.Fatal("dynamicStore.Get() error =", err) + } + + // test get + got, err := ds.Get(ctx, serverAddr) + if err != nil { + t.Fatal("dynamicStore.Get() error =", err) + } + if want := cred; got != want { + t.Errorf("dynamicStore.Get() = %v, want %v", got, want) + } + + // test delete + err = ds.Delete(ctx, serverAddr) + if err != nil { + t.Fatal("dynamicStore.Delete() error =", err) + } + + // verify delete + got, err = ds.Get(ctx, serverAddr) + if err != nil { + t.Fatal("dynamicStore.Get() error =", err) + } + if want := auth.EmptyCredential; got != want { + t.Errorf("dynamicStore.Get() = %v, want %v", got, want) + } +} + +func Test_dynamicStore_noAuthConfigured(t *testing.T) { + // prepare test content + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "no_auth_configured.json") + cfg := testStoreConfig{ SomeConfigField: 123, } - jsonStr, err = json.Marshal(config) + jsonStr, err := json.Marshal(cfg) if err != nil { t.Fatalf("failed to marshal config: %v", err) } - if err := os.WriteFile(noAuthConfiguredPath, jsonStr, 0666); err != nil { + if err := os.WriteFile(configPath, jsonStr, 0666); err != nil { t.Fatalf("failed to write config file: %v", err) } - tests := []struct { - name string - configPath string - }{ - { - name: "Authentication configured", - configPath: authConfiguredPath, - }, - { - name: "No authentication configured", - configPath: noAuthConfiguredPath, - }, + store, err := NewStore(configPath, StoreOptions{AllowPlaintextPut: true}) + if err != nil { + t.Fatal("NewStore() error =", err) } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ds, err := NewStore(tt.configPath, StoreOptions{AllowPlaintextPut: true}) - if err != nil { - t.Fatal("NewStore() error =", err) - } - serverAddr := "test.example.com" - cred := auth.Credential{ - Username: "username", - Password: "password", - } - ctx := context.Background() + ds := store.(*dynamicStore) + serverAddr := "test.example.com" + cred := auth.Credential{ + Username: "username", + Password: "password", + } + ctx := context.Background() - // test put - if err := ds.Put(ctx, serverAddr, cred); err != nil { - t.Fatal("dynamicStore.Get() error =", err) - } + // Get() should not set detected store back to config + if _, err := ds.Get(ctx, serverAddr); err != nil { + t.Fatal("dynamicStore.Get() error =", err) + } + if got := ds.config.CredentialsStore(); got != "" { + t.Errorf("ds.config.CredentialsStore() = %v, want empty", got) + } - // test get - got, err := ds.Get(ctx, serverAddr) - if err != nil { - t.Fatal("dynamicStore.Get() error =", err) - } - if want := cred; got != want { - t.Errorf("dynamicStore.Get() = %v, want %v", got, want) - } + // test put + if err := ds.Put(ctx, serverAddr, cred); err != nil { + t.Fatal("dynamicStore.Put() error =", err) + } - // test delete - err = ds.Delete(ctx, serverAddr) - if err != nil { - t.Fatal("dynamicStore.Delete() error =", err) - } + // Put() should set detected store back to config + if defaultStore := getDefaultHelperSuffix(); defaultStore != "" { + if got := ds.config.CredentialsStore(); got != defaultStore { + t.Errorf("ds.config.CredentialsStore() = %v, want %v", got, defaultStore) + } + } - // verify delete - got, err = ds.Get(ctx, serverAddr) - if err != nil { - t.Fatal("dynamicStore.Get() error =", err) - } - if want := auth.EmptyCredential; got != want { - t.Errorf("dynamicStore.Get() = %v, want %v", got, want) - } - }) + // test get + got, err := ds.Get(ctx, serverAddr) + if err != nil { + t.Fatal("dynamicStore.Get() error =", err) + } + if want := cred; got != want { + t.Errorf("dynamicStore.Get() = %v, want %v", got, want) + } + + // test delete + err = ds.Delete(ctx, serverAddr) + if err != nil { + t.Fatal("dynamicStore.Delete() error =", err) + } + + // verify delete + got, err = ds.Get(ctx, serverAddr) + if err != nil { + t.Fatal("dynamicStore.Get() error =", err) + } + if want := auth.EmptyCredential; got != want { + t.Errorf("dynamicStore.Get() = %v, want %v", got, want) } } @@ -126,7 +177,7 @@ func Test_dynamicStore_fileStore_AllowPlainTextPut(t *testing.T) { } ctx := context.Background() - cfg := testConfig{ + cfg := testStoreConfig{ AuthConfigs: map[string]testAuthConfig{ "test.example.com": {}, }, @@ -165,11 +216,11 @@ func Test_dynamicStore_fileStore_AllowPlainTextPut(t *testing.T) { t.Fatalf("failed to open config file: %v", err) } defer configFile.Close() - var gotCfg testConfig + var gotCfg testStoreConfig if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { t.Fatalf("failed to decode config file: %v", err) } - wantCfg := testConfig{ + wantCfg := testStoreConfig{ AuthConfigs: map[string]testAuthConfig{ "test.example.com": {}, serverAddr: { From 28b49a295995159ebb8d2025558576e5de004abc Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Fri, 21 Apr 2023 16:10:45 +0800 Subject: [PATCH 28/34] update doc Signed-off-by: Sylvia Lei --- store.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/store.go b/store.go index c948c18..a903e38 100644 --- a/store.go +++ b/store.go @@ -54,8 +54,16 @@ type StoreOptions struct { AllowPlaintextPut bool } -// NewStore returns a store based on given config file. -// If no authentication is configured in the config file, a platform-default +// NewStore returns a Store based on the given configuration file. +// +// For Get(), Put() and Delete(), the returned Store will dynamically determine which underlying credentials +// store to used for the given server address. +// The underlying credentials store is determined in the following order: +// 1. Native server-specific credential helper +// 2. Native credentials store +// 3. The plain-text config file itself +// +// If the config file has no authentication information, a platform-default // native store will be used. // - Windows: "wincred" // - Linux: "pass" or "secretservice" From 38586e0dfb0ab99974e3b1f20cc6d1256cb0b5b0 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Fri, 21 Apr 2023 16:16:20 +0800 Subject: [PATCH 29/34] minor change Signed-off-by: Sylvia Lei --- internal/config/config.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 3ecfc81..82009d7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -218,6 +218,14 @@ func (cfg *Config) SetCredentialsStore(credsStore string) error { return cfg.saveFile() } +// IsAuthConfigured returns whether there is authentication configured in this +// config file or not. +func (cfg *Config) IsAuthConfigured() bool { + return cfg.credentialsStore != "" || + len(cfg.credentialHelpers) > 0 || + len(cfg.authsCache) > 0 +} + // saveFile saves Config into the file. func (cfg *Config) saveFile() (returnErr error) { // marshal content @@ -267,14 +275,6 @@ func (cfg *Config) saveFile() (returnErr error) { return nil } -// IsAuthConfigured returns whether there is authentication configured in this -// config file or not. -func (cfg *Config) IsAuthConfigured() bool { - return cfg.credentialsStore != "" || - len(cfg.credentialHelpers) > 0 || - len(cfg.authsCache) > 0 -} - // encodeAuth base64-encodes username and password into base64(username:password). func encodeAuth(username, password string) string { if username == "" && password == "" { From 14da58bd873d3f593e5f43c00265365c6ccce4a2 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Fri, 21 Apr 2023 17:15:16 +0800 Subject: [PATCH 30/34] improve tests Signed-off-by: Sylvia Lei --- file_store_test.go | 75 ++++----- internal/config/config_test.go | 275 ++++++++++++++++++++++++++++++++- internal/config/testconfig.go | 39 +++++ store_test.go | 24 ++- 4 files changed, 349 insertions(+), 64 deletions(-) create mode 100644 internal/config/testconfig.go diff --git a/file_store_test.go b/file_store_test.go index 213ac46..2ac0c45 100644 --- a/file_store_test.go +++ b/file_store_test.go @@ -24,27 +24,10 @@ import ( "reflect" "testing" + "github.com/oras-project/oras-credentials-go/internal/config" "oras.land/oras-go/v2/registry/remote/auth" ) -type testAuthConfig struct { - SomeAuthField string `json:"some_auth_field,omitempty"` - Username string `json:"username,omitempty"` - Password string `json:"password,omitempty"` - Auth string `json:"auth,omitempty"` - - // IdentityToken is used to authenticate the user and get - // an access token for the registry. - IdentityToken string `json:"identitytoken,omitempty"` - // RegistryToken is a bearer token to be sent to a registry - RegistryToken string `json:"registrytoken,omitempty"` -} - -type testConfig struct { - SomeConfigField int `json:"some_config_field"` - AuthConfigs map[string]testAuthConfig `json:"auths"` -} - func TestNewFileStore_badPath(t *testing.T) { tempDir := t.TempDir() @@ -343,12 +326,12 @@ func TestFileStore_Put_notExistConfig(t *testing.T) { } defer configFile.Close() - var cfg testConfig + var cfg config.TestConfig if err := json.NewDecoder(configFile).Decode(&cfg); err != nil { t.Fatalf("failed to decode config file: %v", err) } - want := testConfig{ - AuthConfigs: map[string]testAuthConfig{ + want := config.TestConfig{ + AuthConfigs: map[string]config.TestAuthConfig{ server: { Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", IdentityToken: "refresh_token", @@ -384,8 +367,8 @@ func TestFileStore_Put_addNew(t *testing.T) { AccessToken: "access_token", } - cfg := testConfig{ - AuthConfigs: map[string]testAuthConfig{ + cfg := config.TestConfig{ + AuthConfigs: map[string]config.TestAuthConfig{ server1: { SomeAuthField: "whatever", Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", @@ -425,12 +408,12 @@ func TestFileStore_Put_addNew(t *testing.T) { t.Fatalf("failed to open config file: %v", err) } defer configFile.Close() - var gotCfg testConfig + var gotCfg config.TestConfig if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { t.Fatalf("failed to decode config file: %v", err) } - wantCfg := testConfig{ - AuthConfigs: map[string]testAuthConfig{ + wantCfg := config.TestConfig{ + AuthConfigs: map[string]config.TestAuthConfig{ server1: { SomeAuthField: "whatever", Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", @@ -474,8 +457,8 @@ func TestFileStore_Put_updateOld(t *testing.T) { // prepare test content server := "registry.example.com" - cfg := testConfig{ - AuthConfigs: map[string]testAuthConfig{ + cfg := config.TestConfig{ + AuthConfigs: map[string]config.TestAuthConfig{ server: { SomeAuthField: "whatever", Username: "foo", @@ -513,12 +496,12 @@ func TestFileStore_Put_updateOld(t *testing.T) { t.Fatalf("failed to open config file: %v", err) } defer configFile.Close() - var gotCfg testConfig + var gotCfg config.TestConfig if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { t.Fatalf("failed to decode config file: %v", err) } - wantCfg := testConfig{ - AuthConfigs: map[string]testAuthConfig{ + wantCfg := config.TestConfig{ + AuthConfigs: map[string]config.TestAuthConfig{ server: { Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", RegistryToken: "access_token", @@ -585,8 +568,8 @@ func TestFileStore_Delete(t *testing.T) { AccessToken: "access_token_2", } - cfg := testConfig{ - AuthConfigs: map[string]testAuthConfig{ + cfg := config.TestConfig{ + AuthConfigs: map[string]config.TestAuthConfig{ server1: { Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", IdentityToken: cred1.RefreshToken, @@ -639,12 +622,12 @@ func TestFileStore_Delete(t *testing.T) { t.Fatalf("failed to open config file: %v", err) } defer configFile.Close() - var gotCfg testConfig + var gotCfg config.TestConfig if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { t.Fatalf("failed to decode config file: %v", err) } - wantCfg := testConfig{ - AuthConfigs: map[string]testAuthConfig{ + wantCfg := config.TestConfig{ + AuthConfigs: map[string]config.TestAuthConfig{ server2: cfg.AuthConfigs[server2], }, SomeConfigField: cfg.SomeConfigField, @@ -684,8 +667,8 @@ func TestFileStore_Delete_lastConfig(t *testing.T) { AccessToken: "access_token", } - cfg := testConfig{ - AuthConfigs: map[string]testAuthConfig{ + cfg := config.TestConfig{ + AuthConfigs: map[string]config.TestAuthConfig{ server: { Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", IdentityToken: cred.RefreshToken, @@ -726,12 +709,12 @@ func TestFileStore_Delete_lastConfig(t *testing.T) { t.Fatalf("failed to open config file: %v", err) } defer configFile.Close() - var gotCfg testConfig + var gotCfg config.TestConfig if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { t.Fatalf("failed to decode config file: %v", err) } - wantCfg := testConfig{ - AuthConfigs: map[string]testAuthConfig{}, + wantCfg := config.TestConfig{ + AuthConfigs: map[string]config.TestAuthConfig{}, SomeConfigField: cfg.SomeConfigField, } if !reflect.DeepEqual(gotCfg, wantCfg) { @@ -761,8 +744,8 @@ func TestFileStore_Delete_notExistRecord(t *testing.T) { RefreshToken: "refresh_token", AccessToken: "access_token", } - cfg := testConfig{ - AuthConfigs: map[string]testAuthConfig{ + cfg := config.TestConfig{ + AuthConfigs: map[string]config.TestAuthConfig{ server: { Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", IdentityToken: cred.RefreshToken, @@ -803,12 +786,12 @@ func TestFileStore_Delete_notExistRecord(t *testing.T) { t.Fatalf("failed to open config file: %v", err) } defer configFile.Close() - var gotCfg testConfig + var gotCfg config.TestConfig if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { t.Fatalf("failed to decode config file: %v", err) } - wantCfg := testConfig{ - AuthConfigs: map[string]testAuthConfig{ + wantCfg := config.TestConfig{ + AuthConfigs: map[string]config.TestAuthConfig{ server: cfg.AuthConfigs[server], }, SomeConfigField: cfg.SomeConfigField, diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 040287e..7102fb2 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -15,9 +15,278 @@ limitations under the License. package config -import "testing" +import ( + "encoding/json" + "os" + "path/filepath" + "reflect" + "testing" +) -func Test_encodeAuth(t *testing.T) { +func TestConfig_IsAuthConfigured(t *testing.T) { + tempDir := t.TempDir() + + tests := []struct { + name string + fileName string + shouldCreateFile bool + cfg TestConfig + want bool + }{ + { + name: "not existing file", + fileName: "config.json", + shouldCreateFile: false, + cfg: TestConfig{}, + want: false, + }, + { + name: "no auth", + fileName: "config.json", + shouldCreateFile: true, + cfg: TestConfig{ + SomeConfigField: 123, + }, + want: false, + }, + { + name: "empty auths exist", + fileName: "empty_auths.json", + shouldCreateFile: true, + cfg: TestConfig{ + AuthConfigs: map[string]TestAuthConfig{}, + }, + want: false, + }, + { + name: "auths exist, but no credential", + fileName: "no_cred_auths.json", + shouldCreateFile: true, + cfg: TestConfig{ + AuthConfigs: map[string]TestAuthConfig{ + "test.example.com": {}, + }, + }, + want: true, + }, + { + name: "auths exist", + fileName: "auths.json", + shouldCreateFile: true, + cfg: TestConfig{ + AuthConfigs: map[string]TestAuthConfig{ + "test.example.com": { + Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", + }, + }, + }, + want: true, + }, + { + name: "credsStore exists", + fileName: "credsStore.json", + shouldCreateFile: true, + cfg: TestConfig{ + CredentialsStore: "teststore", + }, + want: true, + }, + { + name: "empty credHelpers exist", + fileName: "empty_credsStore.json", + shouldCreateFile: true, + cfg: TestConfig{ + CredentialHelpers: map[string]string{}, + }, + want: false, + }, + { + name: "credHelpers exist", + fileName: "credsStore.json", + shouldCreateFile: true, + cfg: TestConfig{ + CredentialHelpers: map[string]string{ + "test.example.com": "testhelper", + }, + }, + want: true, + }, + { + name: "all exist", + fileName: "credsStore.json", + shouldCreateFile: true, + cfg: TestConfig{ + SomeConfigField: 123, + AuthConfigs: map[string]TestAuthConfig{ + "test.example.com": {}, + }, + CredentialsStore: "teststore", + CredentialHelpers: map[string]string{ + "test.example.com": "testhelper", + }, + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // prepare test content + configPath := filepath.Join(tempDir, tt.fileName) + if tt.shouldCreateFile { + jsonStr, err := json.Marshal(tt.cfg) + if err != nil { + t.Fatalf("failed to marshal config: %v", err) + } + if err := os.WriteFile(configPath, jsonStr, 0666); err != nil { + t.Fatalf("failed to write config file: %v", err) + } + } + + cfg, err := LoadConfigFile(configPath) + if err != nil { + t.Fatal("LoadConfigFile() error =", err) + } + if got := cfg.IsAuthConfigured(); got != tt.want { + t.Errorf("IsAuthConfigured() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestConfig_saveFile(t *testing.T) { + tempDir := t.TempDir() + tests := []struct { + name string + fileName string + shouldCreateFile bool + oldCfg TestConfig + newCfg TestConfig + wantCfg TestConfig + }{ + { + name: "set credsStore in a non-existing file", + fileName: "config.json", + oldCfg: TestConfig{}, + newCfg: TestConfig{ + CredentialsStore: "teststore", + }, + wantCfg: TestConfig{ + AuthConfigs: make(map[string]TestAuthConfig), + CredentialsStore: "teststore", + }, + shouldCreateFile: false, + }, + { + name: "set credsStore in empty file", + fileName: "empty.json", + oldCfg: TestConfig{}, + newCfg: TestConfig{ + CredentialsStore: "teststore", + }, + wantCfg: TestConfig{ + AuthConfigs: make(map[string]TestAuthConfig), + CredentialsStore: "teststore", + }, + shouldCreateFile: true, + }, + { + name: "set credsStore in a no-auth-configured file", + fileName: "empty.json", + oldCfg: TestConfig{ + SomeConfigField: 123, + }, + newCfg: TestConfig{ + CredentialsStore: "teststore", + }, + wantCfg: TestConfig{ + SomeConfigField: 123, + AuthConfigs: make(map[string]TestAuthConfig), + CredentialsStore: "teststore", + }, + shouldCreateFile: true, + }, + { + name: "Set credsStore and credHelpers in an auth-configured file", + fileName: "auth_configured.json", + oldCfg: TestConfig{ + SomeConfigField: 123, + AuthConfigs: map[string]TestAuthConfig{ + "registry1.example.com": { + SomeAuthField: "something", + Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", + }, + }, + CredentialsStore: "oldstore", + CredentialHelpers: map[string]string{ + "registry2.example.com": "testhelper", + }, + }, + newCfg: TestConfig{ + AuthConfigs: make(map[string]TestAuthConfig), + SomeConfigField: 123, + CredentialsStore: "newstore", + CredentialHelpers: map[string]string{ + "xxx": "yyy", + }, + }, + wantCfg: TestConfig{ + SomeConfigField: 123, + AuthConfigs: map[string]TestAuthConfig{ + "registry1.example.com": { + SomeAuthField: "something", + Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", + }, + }, + CredentialsStore: "newstore", + CredentialHelpers: map[string]string{ + "registry2.example.com": "testhelper", // cred helpers will not be updated + }, + }, + shouldCreateFile: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // prepare test content + configPath := filepath.Join(tempDir, tt.fileName) + if tt.shouldCreateFile { + jsonStr, err := json.Marshal(tt.oldCfg) + if err != nil { + t.Fatalf("failed to marshal config: %v", err) + } + if err := os.WriteFile(configPath, jsonStr, 0666); err != nil { + t.Fatalf("failed to write config file: %v", err) + } + } + + cfg, err := LoadConfigFile(configPath) + if err != nil { + t.Fatal("LoadConfigFile() error =", err) + } + cfg.credentialsStore = tt.newCfg.CredentialsStore + cfg.credentialHelpers = tt.newCfg.CredentialHelpers + if err := cfg.saveFile(); err != nil { + t.Fatal("saveFile() error =", err) + } + + // verify config file + configFile, err := os.Open(configPath) + if err != nil { + t.Fatalf("failed to open config file: %v", err) + } + defer configFile.Close() + var gotCfg TestConfig + if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { + t.Fatalf("failed to decode config file: %v", err) + } + if !reflect.DeepEqual(gotCfg, tt.wantCfg) { + t.Errorf("Decoded config = %v, want %v", gotCfg, tt.wantCfg) + } + }) + } +} + +func TestConfig_encodeAuth(t *testing.T) { tests := []struct { name string username string @@ -58,7 +327,7 @@ func Test_encodeAuth(t *testing.T) { } } -func Test_decodeAuth(t *testing.T) { +func TestConfig_decodeAuth(t *testing.T) { tests := []struct { name string authStr string diff --git a/internal/config/testconfig.go b/internal/config/testconfig.go new file mode 100644 index 0000000..8a8dc05 --- /dev/null +++ b/internal/config/testconfig.go @@ -0,0 +1,39 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package config + +// TestConfig represents the structure of a config file for testing purpose. +type TestConfig struct { + AuthConfigs map[string]TestAuthConfig `json:"auths"` + CredentialsStore string `json:"credsStore,omitempty"` + CredentialHelpers map[string]string `json:"credHelpers,omitempty"` + SomeConfigField int `json:"some_config_field"` +} + +// TestAuthConfig represents the structure of the "auths" field of a config file +// for testing purpose. +type TestAuthConfig struct { + SomeAuthField string `json:"some_auth_field,omitempty"` + Username string `json:"username,omitempty"` + Password string `json:"password,omitempty"` + Auth string `json:"auth,omitempty"` + + // IdentityToken is used to authenticate the user and get + // an access token for the registry. + IdentityToken string `json:"identitytoken,omitempty"` + // RegistryToken is a bearer token to be sent to a registry + RegistryToken string `json:"registrytoken,omitempty"` +} diff --git a/store_test.go b/store_test.go index 1137420..3871eed 100644 --- a/store_test.go +++ b/store_test.go @@ -24,22 +24,16 @@ import ( "reflect" "testing" + "github.com/oras-project/oras-credentials-go/internal/config" "oras.land/oras-go/v2/registry/remote/auth" ) -type testStoreConfig struct { - SomeConfigField int `json:"some_config_field"` - AuthConfigs map[string]testAuthConfig `json:"auths"` - CredentialsStore string `json:"credsStore,omitempty"` - CredentialHelpers map[string]string `json:"credHelpers,omitempty"` -} - func Test_dynamicStore_authConfigured(t *testing.T) { // prepare test content tempDir := t.TempDir() configPath := filepath.Join(tempDir, "auth_configured.json") - config := testStoreConfig{ - AuthConfigs: map[string]testAuthConfig{ + config := config.TestConfig{ + AuthConfigs: map[string]config.TestAuthConfig{ "xxx": {}, }, SomeConfigField: 123, @@ -98,7 +92,7 @@ func Test_dynamicStore_noAuthConfigured(t *testing.T) { // prepare test content tempDir := t.TempDir() configPath := filepath.Join(tempDir, "no_auth_configured.json") - cfg := testStoreConfig{ + cfg := config.TestConfig{ SomeConfigField: 123, } jsonStr, err := json.Marshal(cfg) @@ -177,8 +171,8 @@ func Test_dynamicStore_fileStore_AllowPlainTextPut(t *testing.T) { } ctx := context.Background() - cfg := testStoreConfig{ - AuthConfigs: map[string]testAuthConfig{ + cfg := config.TestConfig{ + AuthConfigs: map[string]config.TestAuthConfig{ "test.example.com": {}, }, SomeConfigField: 123, @@ -216,12 +210,12 @@ func Test_dynamicStore_fileStore_AllowPlainTextPut(t *testing.T) { t.Fatalf("failed to open config file: %v", err) } defer configFile.Close() - var gotCfg testStoreConfig + var gotCfg config.TestConfig if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { t.Fatalf("failed to decode config file: %v", err) } - wantCfg := testStoreConfig{ - AuthConfigs: map[string]testAuthConfig{ + wantCfg := config.TestConfig{ + AuthConfigs: map[string]config.TestAuthConfig{ "test.example.com": {}, serverAddr: { Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", From aaf75484403f66377058591e7be9cb09ffb9cd47 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Fri, 21 Apr 2023 18:54:57 +0800 Subject: [PATCH 31/34] minor fix name Signed-off-by: Sylvia Lei --- internal/config/config_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 7102fb2..20b3180 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -286,7 +286,7 @@ func TestConfig_saveFile(t *testing.T) { } } -func TestConfig_encodeAuth(t *testing.T) { +func Test_encodeAuth(t *testing.T) { tests := []struct { name string username string @@ -327,7 +327,7 @@ func TestConfig_encodeAuth(t *testing.T) { } } -func TestConfig_decodeAuth(t *testing.T) { +func Test_decodeAuth(t *testing.T) { tests := []struct { name string authStr string From 9a9b85b6b9ee6c0e009dbfd03cba81da8dd57d40 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Fri, 21 Apr 2023 19:10:40 +0800 Subject: [PATCH 32/34] check colons in username Signed-off-by: Sylvia Lei --- file_store.go | 27 ++++++++++++++-- file_store_test.go | 78 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 101 insertions(+), 4 deletions(-) diff --git a/file_store.go b/file_store.go index 47afece..bbe61ba 100644 --- a/file_store.go +++ b/file_store.go @@ -18,6 +18,8 @@ package credentials import ( "context" "errors" + "fmt" + "strings" "github.com/oras-project/oras-credentials-go/internal/config" "oras.land/oras-go/v2/registry/remote/auth" @@ -33,9 +35,14 @@ type FileStore struct { config *config.Config } -// ErrPlaintextPutDisabled is returned by Put() when DisablePut is set -// to true. -var ErrPlaintextPutDisabled = errors.New("putting plaintext credentials is disabled") +var ( + // ErrPlaintextPutDisabled is returned by Put() when DisablePut is set + // to true. + ErrPlaintextPutDisabled = errors.New("putting plaintext credentials is disabled") + // ErrBadCredentialFormat is returned by Put() when the credential format + // is bad. + ErrBadCredentialFormat = errors.New("bad credential format") +) // NewFileStore creates a new file credentials store. func NewFileStore(configPath string) (*FileStore, error) { @@ -62,6 +69,9 @@ func (fs *FileStore) Put(_ context.Context, serverAddress string, cred auth.Cred if fs.DisablePut { return ErrPlaintextPutDisabled } + if err := validateCredentialFormat(cred); err != nil { + return err + } return fs.config.PutCredential(serverAddress, cred) } @@ -70,3 +80,14 @@ func (fs *FileStore) Put(_ context.Context, serverAddress string, cred auth.Cred func (fs *FileStore) Delete(_ context.Context, serverAddress string) error { return fs.config.DeleteCredential(serverAddress) } + +// validateCredentialFormat validates the format of cred. +func validateCredentialFormat(cred auth.Credential) error { + if strings.ContainsRune(cred.Username, ':') { + // Username and password will be encoded in the base64(username:password) + // format in the file. The decoded result will be wrong if username + // contains colon(s). + return fmt.Errorf("%w: colons(:) are not allowed in username", ErrBadCredentialFormat) + } + return nil +} diff --git a/file_store_test.go b/file_store_test.go index 2ac0c45..bab8ffc 100644 --- a/file_store_test.go +++ b/file_store_test.go @@ -523,7 +523,7 @@ func TestFileStore_Put_updateOld(t *testing.T) { } } -func TestStore_Put_disablePut(t *testing.T) { +func TestFileStore_Put_disablePut(t *testing.T) { tempDir := t.TempDir() configPath := filepath.Join(tempDir, "config.json") ctx := context.Background() @@ -547,6 +547,51 @@ func TestStore_Put_disablePut(t *testing.T) { } } +func TestFileStore_Put_usernameContainsColon(t *testing.T) { + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "config.json") + ctx := context.Background() + + fs, err := NewFileStore(configPath) + if err != nil { + t.Fatal("NewFileStore() error =", err) + } + serverAddr := "test.example.com" + cred := auth.Credential{ + Username: "x:y", + Password: "z", + } + if err := fs.Put(ctx, serverAddr, cred); err == nil { + t.Fatal("FileStore.Put() error is nil, want", ErrBadCredentialFormat) + } +} + +func TestFileStore_Put_passwordContainsColon(t *testing.T) { + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "config.json") + ctx := context.Background() + + fs, err := NewFileStore(configPath) + if err != nil { + t.Fatal("NewFileStore() error =", err) + } + serverAddr := "test.example.com" + cred := auth.Credential{ + Username: "y", + Password: "y:z", + } + if err := fs.Put(ctx, serverAddr, cred); err != nil { + t.Fatal("FileStore.Put() error =", err) + } + got, err := fs.Get(ctx, serverAddr) + if err != nil { + t.Fatal("FileStore.Get() error =", err) + } + if !reflect.DeepEqual(got, cred) { + t.Errorf("FileStore.Get() = %v, want %v", got, cred) + } +} + func TestFileStore_Delete(t *testing.T) { tempDir := t.TempDir() configPath := filepath.Join(tempDir, "config.json") @@ -832,3 +877,34 @@ func TestFileStore_Delete_notExistConfig(t *testing.T) { t.Errorf("Stat(%s) error = %v, wantErr %v", configPath, err, wantErr) } } + +func Test_validateCredentialFormat(t *testing.T) { + tests := []struct { + name string + cred auth.Credential + wantErr error + }{ + { + name: "Username contains colon", + cred: auth.Credential{ + Username: "x:y", + Password: "z", + }, + wantErr: ErrBadCredentialFormat, + }, + { + name: "Password contains colon", + cred: auth.Credential{ + Username: "x", + Password: "y:z", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := validateCredentialFormat(tt.cred); !errors.Is(err, tt.wantErr) { + t.Errorf("validateCredentialFormat() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} From daf65219522c55dfd507cf3e24d7909e0f43f8d7 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Fri, 21 Apr 2023 19:39:17 +0800 Subject: [PATCH 33/34] improve test Signed-off-by: Sylvia Lei --- file_store_test.go | 58 +++++++-------- internal/config/config_test.go | 74 ++++++++++--------- .../{testconfig.go => configtest/config.go} | 18 ++--- store_test.go | 18 ++--- 4 files changed, 85 insertions(+), 83 deletions(-) rename internal/config/{testconfig.go => configtest/config.go} (67%) diff --git a/file_store_test.go b/file_store_test.go index bab8ffc..a4b78b5 100644 --- a/file_store_test.go +++ b/file_store_test.go @@ -24,7 +24,7 @@ import ( "reflect" "testing" - "github.com/oras-project/oras-credentials-go/internal/config" + "github.com/oras-project/oras-credentials-go/internal/config/configtest" "oras.land/oras-go/v2/registry/remote/auth" ) @@ -326,12 +326,12 @@ func TestFileStore_Put_notExistConfig(t *testing.T) { } defer configFile.Close() - var cfg config.TestConfig + var cfg configtest.Config if err := json.NewDecoder(configFile).Decode(&cfg); err != nil { t.Fatalf("failed to decode config file: %v", err) } - want := config.TestConfig{ - AuthConfigs: map[string]config.TestAuthConfig{ + want := configtest.Config{ + AuthConfigs: map[string]configtest.AuthConfig{ server: { Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", IdentityToken: "refresh_token", @@ -367,8 +367,8 @@ func TestFileStore_Put_addNew(t *testing.T) { AccessToken: "access_token", } - cfg := config.TestConfig{ - AuthConfigs: map[string]config.TestAuthConfig{ + cfg := configtest.Config{ + AuthConfigs: map[string]configtest.AuthConfig{ server1: { SomeAuthField: "whatever", Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", @@ -408,12 +408,12 @@ func TestFileStore_Put_addNew(t *testing.T) { t.Fatalf("failed to open config file: %v", err) } defer configFile.Close() - var gotCfg config.TestConfig + var gotCfg configtest.Config if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { t.Fatalf("failed to decode config file: %v", err) } - wantCfg := config.TestConfig{ - AuthConfigs: map[string]config.TestAuthConfig{ + wantCfg := configtest.Config{ + AuthConfigs: map[string]configtest.AuthConfig{ server1: { SomeAuthField: "whatever", Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", @@ -457,8 +457,8 @@ func TestFileStore_Put_updateOld(t *testing.T) { // prepare test content server := "registry.example.com" - cfg := config.TestConfig{ - AuthConfigs: map[string]config.TestAuthConfig{ + cfg := configtest.Config{ + AuthConfigs: map[string]configtest.AuthConfig{ server: { SomeAuthField: "whatever", Username: "foo", @@ -496,12 +496,12 @@ func TestFileStore_Put_updateOld(t *testing.T) { t.Fatalf("failed to open config file: %v", err) } defer configFile.Close() - var gotCfg config.TestConfig + var gotCfg configtest.Config if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { t.Fatalf("failed to decode config file: %v", err) } - wantCfg := config.TestConfig{ - AuthConfigs: map[string]config.TestAuthConfig{ + wantCfg := configtest.Config{ + AuthConfigs: map[string]configtest.AuthConfig{ server: { Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", RegistryToken: "access_token", @@ -613,8 +613,8 @@ func TestFileStore_Delete(t *testing.T) { AccessToken: "access_token_2", } - cfg := config.TestConfig{ - AuthConfigs: map[string]config.TestAuthConfig{ + cfg := configtest.Config{ + AuthConfigs: map[string]configtest.AuthConfig{ server1: { Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", IdentityToken: cred1.RefreshToken, @@ -667,12 +667,12 @@ func TestFileStore_Delete(t *testing.T) { t.Fatalf("failed to open config file: %v", err) } defer configFile.Close() - var gotCfg config.TestConfig + var gotCfg configtest.Config if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { t.Fatalf("failed to decode config file: %v", err) } - wantCfg := config.TestConfig{ - AuthConfigs: map[string]config.TestAuthConfig{ + wantCfg := configtest.Config{ + AuthConfigs: map[string]configtest.AuthConfig{ server2: cfg.AuthConfigs[server2], }, SomeConfigField: cfg.SomeConfigField, @@ -712,8 +712,8 @@ func TestFileStore_Delete_lastConfig(t *testing.T) { AccessToken: "access_token", } - cfg := config.TestConfig{ - AuthConfigs: map[string]config.TestAuthConfig{ + cfg := configtest.Config{ + AuthConfigs: map[string]configtest.AuthConfig{ server: { Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", IdentityToken: cred.RefreshToken, @@ -754,12 +754,12 @@ func TestFileStore_Delete_lastConfig(t *testing.T) { t.Fatalf("failed to open config file: %v", err) } defer configFile.Close() - var gotCfg config.TestConfig + var gotCfg configtest.Config if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { t.Fatalf("failed to decode config file: %v", err) } - wantCfg := config.TestConfig{ - AuthConfigs: map[string]config.TestAuthConfig{}, + wantCfg := configtest.Config{ + AuthConfigs: map[string]configtest.AuthConfig{}, SomeConfigField: cfg.SomeConfigField, } if !reflect.DeepEqual(gotCfg, wantCfg) { @@ -789,8 +789,8 @@ func TestFileStore_Delete_notExistRecord(t *testing.T) { RefreshToken: "refresh_token", AccessToken: "access_token", } - cfg := config.TestConfig{ - AuthConfigs: map[string]config.TestAuthConfig{ + cfg := configtest.Config{ + AuthConfigs: map[string]configtest.AuthConfig{ server: { Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", IdentityToken: cred.RefreshToken, @@ -831,12 +831,12 @@ func TestFileStore_Delete_notExistRecord(t *testing.T) { t.Fatalf("failed to open config file: %v", err) } defer configFile.Close() - var gotCfg config.TestConfig + var gotCfg configtest.Config if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { t.Fatalf("failed to decode config file: %v", err) } - wantCfg := config.TestConfig{ - AuthConfigs: map[string]config.TestAuthConfig{ + wantCfg := configtest.Config{ + AuthConfigs: map[string]configtest.AuthConfig{ server: cfg.AuthConfigs[server], }, SomeConfigField: cfg.SomeConfigField, diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 20b3180..ee25a0a 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -21,6 +21,8 @@ import ( "path/filepath" "reflect" "testing" + + "github.com/oras-project/oras-credentials-go/internal/config/configtest" ) func TestConfig_IsAuthConfigured(t *testing.T) { @@ -30,21 +32,21 @@ func TestConfig_IsAuthConfigured(t *testing.T) { name string fileName string shouldCreateFile bool - cfg TestConfig + cfg configtest.Config want bool }{ { name: "not existing file", fileName: "config.json", shouldCreateFile: false, - cfg: TestConfig{}, + cfg: configtest.Config{}, want: false, }, { name: "no auth", fileName: "config.json", shouldCreateFile: true, - cfg: TestConfig{ + cfg: configtest.Config{ SomeConfigField: 123, }, want: false, @@ -53,8 +55,8 @@ func TestConfig_IsAuthConfigured(t *testing.T) { name: "empty auths exist", fileName: "empty_auths.json", shouldCreateFile: true, - cfg: TestConfig{ - AuthConfigs: map[string]TestAuthConfig{}, + cfg: configtest.Config{ + AuthConfigs: map[string]configtest.AuthConfig{}, }, want: false, }, @@ -62,8 +64,8 @@ func TestConfig_IsAuthConfigured(t *testing.T) { name: "auths exist, but no credential", fileName: "no_cred_auths.json", shouldCreateFile: true, - cfg: TestConfig{ - AuthConfigs: map[string]TestAuthConfig{ + cfg: configtest.Config{ + AuthConfigs: map[string]configtest.AuthConfig{ "test.example.com": {}, }, }, @@ -73,8 +75,8 @@ func TestConfig_IsAuthConfigured(t *testing.T) { name: "auths exist", fileName: "auths.json", shouldCreateFile: true, - cfg: TestConfig{ - AuthConfigs: map[string]TestAuthConfig{ + cfg: configtest.Config{ + AuthConfigs: map[string]configtest.AuthConfig{ "test.example.com": { Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", }, @@ -86,7 +88,7 @@ func TestConfig_IsAuthConfigured(t *testing.T) { name: "credsStore exists", fileName: "credsStore.json", shouldCreateFile: true, - cfg: TestConfig{ + cfg: configtest.Config{ CredentialsStore: "teststore", }, want: true, @@ -95,7 +97,7 @@ func TestConfig_IsAuthConfigured(t *testing.T) { name: "empty credHelpers exist", fileName: "empty_credsStore.json", shouldCreateFile: true, - cfg: TestConfig{ + cfg: configtest.Config{ CredentialHelpers: map[string]string{}, }, want: false, @@ -104,7 +106,7 @@ func TestConfig_IsAuthConfigured(t *testing.T) { name: "credHelpers exist", fileName: "credsStore.json", shouldCreateFile: true, - cfg: TestConfig{ + cfg: configtest.Config{ CredentialHelpers: map[string]string{ "test.example.com": "testhelper", }, @@ -115,9 +117,9 @@ func TestConfig_IsAuthConfigured(t *testing.T) { name: "all exist", fileName: "credsStore.json", shouldCreateFile: true, - cfg: TestConfig{ + cfg: configtest.Config{ SomeConfigField: 123, - AuthConfigs: map[string]TestAuthConfig{ + AuthConfigs: map[string]configtest.AuthConfig{ "test.example.com": {}, }, CredentialsStore: "teststore", @@ -159,19 +161,19 @@ func TestConfig_saveFile(t *testing.T) { name string fileName string shouldCreateFile bool - oldCfg TestConfig - newCfg TestConfig - wantCfg TestConfig + oldCfg configtest.Config + newCfg configtest.Config + wantCfg configtest.Config }{ { name: "set credsStore in a non-existing file", fileName: "config.json", - oldCfg: TestConfig{}, - newCfg: TestConfig{ + oldCfg: configtest.Config{}, + newCfg: configtest.Config{ CredentialsStore: "teststore", }, - wantCfg: TestConfig{ - AuthConfigs: make(map[string]TestAuthConfig), + wantCfg: configtest.Config{ + AuthConfigs: make(map[string]configtest.AuthConfig), CredentialsStore: "teststore", }, shouldCreateFile: false, @@ -179,12 +181,12 @@ func TestConfig_saveFile(t *testing.T) { { name: "set credsStore in empty file", fileName: "empty.json", - oldCfg: TestConfig{}, - newCfg: TestConfig{ + oldCfg: configtest.Config{}, + newCfg: configtest.Config{ CredentialsStore: "teststore", }, - wantCfg: TestConfig{ - AuthConfigs: make(map[string]TestAuthConfig), + wantCfg: configtest.Config{ + AuthConfigs: make(map[string]configtest.AuthConfig), CredentialsStore: "teststore", }, shouldCreateFile: true, @@ -192,15 +194,15 @@ func TestConfig_saveFile(t *testing.T) { { name: "set credsStore in a no-auth-configured file", fileName: "empty.json", - oldCfg: TestConfig{ + oldCfg: configtest.Config{ SomeConfigField: 123, }, - newCfg: TestConfig{ + newCfg: configtest.Config{ CredentialsStore: "teststore", }, - wantCfg: TestConfig{ + wantCfg: configtest.Config{ SomeConfigField: 123, - AuthConfigs: make(map[string]TestAuthConfig), + AuthConfigs: make(map[string]configtest.AuthConfig), CredentialsStore: "teststore", }, shouldCreateFile: true, @@ -208,9 +210,9 @@ func TestConfig_saveFile(t *testing.T) { { name: "Set credsStore and credHelpers in an auth-configured file", fileName: "auth_configured.json", - oldCfg: TestConfig{ + oldCfg: configtest.Config{ SomeConfigField: 123, - AuthConfigs: map[string]TestAuthConfig{ + AuthConfigs: map[string]configtest.AuthConfig{ "registry1.example.com": { SomeAuthField: "something", Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", @@ -221,17 +223,17 @@ func TestConfig_saveFile(t *testing.T) { "registry2.example.com": "testhelper", }, }, - newCfg: TestConfig{ - AuthConfigs: make(map[string]TestAuthConfig), + newCfg: configtest.Config{ + AuthConfigs: make(map[string]configtest.AuthConfig), SomeConfigField: 123, CredentialsStore: "newstore", CredentialHelpers: map[string]string{ "xxx": "yyy", }, }, - wantCfg: TestConfig{ + wantCfg: configtest.Config{ SomeConfigField: 123, - AuthConfigs: map[string]TestAuthConfig{ + AuthConfigs: map[string]configtest.AuthConfig{ "registry1.example.com": { SomeAuthField: "something", Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", @@ -275,7 +277,7 @@ func TestConfig_saveFile(t *testing.T) { t.Fatalf("failed to open config file: %v", err) } defer configFile.Close() - var gotCfg TestConfig + var gotCfg configtest.Config if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { t.Fatalf("failed to decode config file: %v", err) } diff --git a/internal/config/testconfig.go b/internal/config/configtest/config.go similarity index 67% rename from internal/config/testconfig.go rename to internal/config/configtest/config.go index 8a8dc05..5945e12 100644 --- a/internal/config/testconfig.go +++ b/internal/config/configtest/config.go @@ -13,19 +13,19 @@ See the License for the specific language governing permissions and limitations under the License. */ -package config +package configtest -// TestConfig represents the structure of a config file for testing purpose. -type TestConfig struct { - AuthConfigs map[string]TestAuthConfig `json:"auths"` - CredentialsStore string `json:"credsStore,omitempty"` - CredentialHelpers map[string]string `json:"credHelpers,omitempty"` - SomeConfigField int `json:"some_config_field"` +// Config represents the structure of a config file for testing purpose. +type Config struct { + AuthConfigs map[string]AuthConfig `json:"auths"` + CredentialsStore string `json:"credsStore,omitempty"` + CredentialHelpers map[string]string `json:"credHelpers,omitempty"` + SomeConfigField int `json:"some_config_field"` } -// TestAuthConfig represents the structure of the "auths" field of a config file +// AuthConfig represents the structure of the "auths" field of a config file // for testing purpose. -type TestAuthConfig struct { +type AuthConfig struct { SomeAuthField string `json:"some_auth_field,omitempty"` Username string `json:"username,omitempty"` Password string `json:"password,omitempty"` diff --git a/store_test.go b/store_test.go index 3871eed..b59a80f 100644 --- a/store_test.go +++ b/store_test.go @@ -24,7 +24,7 @@ import ( "reflect" "testing" - "github.com/oras-project/oras-credentials-go/internal/config" + "github.com/oras-project/oras-credentials-go/internal/config/configtest" "oras.land/oras-go/v2/registry/remote/auth" ) @@ -32,8 +32,8 @@ func Test_dynamicStore_authConfigured(t *testing.T) { // prepare test content tempDir := t.TempDir() configPath := filepath.Join(tempDir, "auth_configured.json") - config := config.TestConfig{ - AuthConfigs: map[string]config.TestAuthConfig{ + config := configtest.Config{ + AuthConfigs: map[string]configtest.AuthConfig{ "xxx": {}, }, SomeConfigField: 123, @@ -92,7 +92,7 @@ func Test_dynamicStore_noAuthConfigured(t *testing.T) { // prepare test content tempDir := t.TempDir() configPath := filepath.Join(tempDir, "no_auth_configured.json") - cfg := config.TestConfig{ + cfg := configtest.Config{ SomeConfigField: 123, } jsonStr, err := json.Marshal(cfg) @@ -171,8 +171,8 @@ func Test_dynamicStore_fileStore_AllowPlainTextPut(t *testing.T) { } ctx := context.Background() - cfg := config.TestConfig{ - AuthConfigs: map[string]config.TestAuthConfig{ + cfg := configtest.Config{ + AuthConfigs: map[string]configtest.AuthConfig{ "test.example.com": {}, }, SomeConfigField: 123, @@ -210,12 +210,12 @@ func Test_dynamicStore_fileStore_AllowPlainTextPut(t *testing.T) { t.Fatalf("failed to open config file: %v", err) } defer configFile.Close() - var gotCfg config.TestConfig + var gotCfg configtest.Config if err := json.NewDecoder(configFile).Decode(&gotCfg); err != nil { t.Fatalf("failed to decode config file: %v", err) } - wantCfg := config.TestConfig{ - AuthConfigs: map[string]config.TestAuthConfig{ + wantCfg := configtest.Config{ + AuthConfigs: map[string]configtest.AuthConfig{ "test.example.com": {}, serverAddr: { Auth: "dXNlcm5hbWU6cGFzc3dvcmQ=", From 933f90db302a399488c3aba30c6b5fc69d6d3320 Mon Sep 17 00:00:00 2001 From: Sylvia Lei Date: Fri, 21 Apr 2023 20:31:34 +0800 Subject: [PATCH 34/34] address comments Signed-off-by: Sylvia Lei --- file_store.go | 2 +- internal/config/config.go | 72 +++++++++++++++++----------------- internal/config/config_test.go | 4 +- store.go | 26 ++++-------- store_test.go | 15 ++----- 5 files changed, 48 insertions(+), 71 deletions(-) diff --git a/file_store.go b/file_store.go index bbe61ba..b57b0b5 100644 --- a/file_store.go +++ b/file_store.go @@ -46,7 +46,7 @@ var ( // NewFileStore creates a new file credentials store. func NewFileStore(configPath string) (*FileStore, error) { - cfg, err := config.LoadConfigFile(configPath) + cfg, err := config.Load(configPath) if err != nil { return nil, err } diff --git a/internal/config/config.go b/internal/config/config.go index 82009d7..1758c5e 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -30,26 +30,18 @@ import ( "oras.land/oras-go/v2/registry/remote/auth" ) -// Config represents a docker configuration file. -// Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L17-L44 -type Config struct { - // path is the path to the config file. - path string - // rwLock is a read-write-lock for the file store. - rwLock sync.RWMutex - // content is the content of the config file. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L17-L44 - content map[string]json.RawMessage - // authsCache is a cache of the auths field of the config. +const ( + // configFieldAuths is the "auths" field in the config file. // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L19 - authsCache map[string]json.RawMessage - // credentialsStore is the credsStore field of the config. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L28 - credentialsStore string - // credentialHelpers is the credHelpers field of the config. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L29 - credentialHelpers map[string]string -} + configFieldAuths = "auths" + // configFieldCredentialsStore is the "credsStore" field in the config file. + configFieldCredentialsStore = "credsStore" + // configFieldCredentialHelpers is the "credHelpers" field in the config file. + configFieldCredentialHelpers = "credHelpers" +) + +// ErrInvalidConfigFormat is returned when the config format is invalid. +var ErrInvalidConfigFormat = errors.New("invalid config format") // AuthConfig contains authorization information for connecting to a Registry. // References: @@ -68,19 +60,6 @@ type AuthConfig struct { Password string `json:"password,omitempty"` // legacy field for compatibility } -const ( - // configFieldAuths is the "auths" field in the config file. - // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L19 - configFieldAuths = "auths" - // configFieldCredentialsStore is the "credsStore" field in the config file. - configFieldCredentialsStore = "credsStore" - // configFieldCredentialHelpers is the "credHelpers" field in the config file. - configFieldCredentialHelpers = "credHelpers" -) - -// ErrInvalidConfigFormat is returned when the config format is invalid. -var ErrInvalidConfigFormat = errors.New("invalid config format") - // NewAuthConfig creates an authConfig based on cred. func NewAuthConfig(cred auth.Credential) AuthConfig { return AuthConfig{ @@ -109,8 +88,29 @@ func (ac AuthConfig) Credential() (auth.Credential, error) { return cred, nil } -// LoadConfigFile loads Config from the given config path. -func LoadConfigFile(configPath string) (*Config, error) { +// Config represents a docker configuration file. +// Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L17-L44 +type Config struct { + // path is the path to the config file. + path string + // rwLock is a read-write-lock for the file store. + rwLock sync.RWMutex + // content is the content of the config file. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L17-L44 + content map[string]json.RawMessage + // authsCache is a cache of the auths field of the config. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L19 + authsCache map[string]json.RawMessage + // credentialsStore is the credsStore field of the config. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L28 + credentialsStore string + // credentialHelpers is the credHelpers field of the config. + // Reference: https://github.com/docker/cli/blob/v24.0.0-beta.2/cli/config/configfile/file.go#L29 + credentialHelpers map[string]string +} + +// Load loads Config from the given config path. +func Load(configPath string) (*Config, error) { cfg := &Config{path: configPath} configFile, err := os.Open(configPath) if err != nil { @@ -229,6 +229,7 @@ func (cfg *Config) IsAuthConfigured() bool { // saveFile saves Config into the file. func (cfg *Config) saveFile() (returnErr error) { // marshal content + // credentialHelpers is skipped as it's never set if cfg.credentialsStore != "" { credsStoreBytes, err := json.Marshal(cfg.credentialsStore) if err != nil { @@ -239,9 +240,6 @@ func (cfg *Config) saveFile() (returnErr error) { // omit empty delete(cfg.content, configFieldCredentialsStore) } - - // skip saving credentialHelpers as it's never set - authsBytes, err := json.Marshal(cfg.authsCache) if err != nil { return fmt.Errorf("failed to marshal credentials: %w", err) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index ee25a0a..a82971d 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -144,7 +144,7 @@ func TestConfig_IsAuthConfigured(t *testing.T) { } } - cfg, err := LoadConfigFile(configPath) + cfg, err := Load(configPath) if err != nil { t.Fatal("LoadConfigFile() error =", err) } @@ -261,7 +261,7 @@ func TestConfig_saveFile(t *testing.T) { } } - cfg, err := LoadConfigFile(configPath) + cfg, err := Load(configPath) if err != nil { t.Fatal("LoadConfigFile() error =", err) } diff --git a/store.go b/store.go index a903e38..87e8abb 100644 --- a/store.go +++ b/store.go @@ -71,7 +71,7 @@ type StoreOptions struct { // // Reference: https://docs.docker.com/engine/reference/commandline/login/#credentials-store func NewStore(configPath string, opts StoreOptions) (Store, error) { - cfg, err := config.LoadConfigFile(configPath) + cfg, err := config.Load(configPath) if err != nil { return nil, err } @@ -88,22 +88,14 @@ func NewStore(configPath string, opts StoreOptions) (Store, error) { // Get retrieves credentials from the store for the given server address. func (ds *dynamicStore) Get(ctx context.Context, serverAddress string) (auth.Credential, error) { - store, err := ds.getStore(serverAddress) - if err != nil { - return auth.EmptyCredential, err - } - return store.Get(ctx, serverAddress) + return ds.getStore(serverAddress).Get(ctx, serverAddress) } // Put saves credentials into the store for the given server address. // Returns ErrPlaintextPutDisabled if native store is not available and // StoreOptions.AllowPlaintextPut is set to false. func (ds *dynamicStore) Put(ctx context.Context, serverAddress string, cred auth.Credential) (returnErr error) { - store, err := ds.getStore(serverAddress) - if err != nil { - return err - } - if err := store.Put(ctx, serverAddress, cred); err != nil { + if err := ds.getStore(serverAddress).Put(ctx, serverAddress, cred); err != nil { return err } // save the detected creds store back to the config file on first put @@ -119,11 +111,7 @@ func (ds *dynamicStore) Put(ctx context.Context, serverAddress string, cred auth // Delete removes credentials from the store for the given server address. func (ds *dynamicStore) Delete(ctx context.Context, serverAddress string) error { - store, err := ds.getStore(serverAddress) - if err != nil { - return err - } - return store.Delete(ctx, serverAddress) + return ds.getStore(serverAddress).Delete(ctx, serverAddress) } // getHelperSuffix returns the credential helper suffix for the given server @@ -142,14 +130,14 @@ func (ds *dynamicStore) getHelperSuffix(serverAddress string) string { } // getStore returns a store for the given server address. -func (ds *dynamicStore) getStore(serverAddress string) (Store, error) { +func (ds *dynamicStore) getStore(serverAddress string) Store { if helper := ds.getHelperSuffix(serverAddress); helper != "" { - return NewNativeStore(helper), nil + return NewNativeStore(helper) } fs := newFileStore(ds.config) fs.DisablePut = !ds.options.AllowPlaintextPut - return fs, nil + return fs } // storeWithFallbacks is a store that has multiple fallback stores. diff --git a/store_test.go b/store_test.go index b59a80f..96cd0f7 100644 --- a/store_test.go +++ b/store_test.go @@ -320,10 +320,7 @@ func Test_dynamicStore_getStore_nativeStore(t *testing.T) { t.Fatal("NewStore() error =", err) } ds := store.(*dynamicStore) - gotStore, err := ds.getStore(tt.serverAddress) - if err != nil { - t.Fatal("dynamicStore.getStore() error =", err) - } + gotStore := ds.getStore(tt.serverAddress) if _, ok := gotStore.(*nativeStore); !ok { t.Errorf("gotStore is not a native store") } @@ -355,20 +352,14 @@ func Test_dynamicStore_getStore_fileStore(t *testing.T) { t.Fatal("NewStore() error =", err) } ds := store.(*dynamicStore) - gotStore, err := ds.getStore(tt.serverAddress) - if err != nil { - t.Fatal("dynamicStore.getStore() error =", err) - } + gotStore := ds.getStore(tt.serverAddress) gotFS1, ok := gotStore.(*FileStore) if !ok { t.Errorf("gotStore is not a file store") } // get again, the two file stores should be based on the same config instance - gotStore, err = ds.getStore(tt.serverAddress) - if err != nil { - t.Fatal("dynamicStore.getStore() error =", err) - } + gotStore = ds.getStore(tt.serverAddress) gotFS2, ok := gotStore.(*FileStore) if !ok { t.Errorf("gotStore is not a file store")