Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
elikatsis committed Sep 6, 2023
1 parent 71cf82b commit 1d7794e
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 12 deletions.
2 changes: 2 additions & 0 deletions extension/oauth2clientauthextension/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,11 @@ Following are the configuration fields
- [**token_url**](https://datatracker.ietf.org/doc/html/rfc6749#section-3.2) - The resource server's token endpoint URLs.
- [**client_id**](https://datatracker.ietf.org/doc/html/rfc6749#section-2.2) - The client identifier issued to the client.
- **client_id_file** - The file path to retrieve the client identifier issued to the client.
The extension reads this file and updates the client ID used whenever it needs to issue a new token. This enables dynamically changing the client credentials by modifying the file contents when, for example, they need to rotate. <!-- Intended whitespace for compact new line -->
This setting takes precedence over `client_id`.
- [**client_secret**](https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1) - The secret string associated with above identifier.
- **client_secret_file** - The file path to retrieve the secret string associated with above identifier.
The extension reads this file and updates the client secret used whenever it needs to issue a new token. This enables dynamically changing the client credentials by modifying the file contents when, for example, they need to rotate. <!-- Intended whitespace for compact new line -->
This setting takes precedence over `client_secret`.
- [**endpoint_params**](https://github.com/golang/oauth2/blob/master/clientcredentials/clientcredentials.go#L44) - Additional parameters that are sent to the token endpoint.
- [**scopes**](https://datatracker.ietf.org/doc/html/rfc6749#section-3.3) - **Optional** optional requested permissions associated for the client.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,15 @@ func readCredentialsFile(path string) (string, error) {
}

func getActualValue(value, filepath string) (string, error) {
actualValue := value
var err error

if len(filepath) > 0 {
actualValue, err = readCredentialsFile(filepath)
return readCredentialsFile(filepath)
}

return actualValue, err
return value, nil
}

// createConfig creates a proper clientcredentials.Config with values retrieved
// from files, if the user has specified a "file:" prefix
// from files, if the user has specified '*_file' values
func (c *clientCredentialsConfig) createConfig() (*clientcredentials.Config, error) {
clientID, err := getActualValue(c.ClientID, c.ClientIDFile)
if err != nil {
Expand Down
12 changes: 6 additions & 6 deletions extension/oauth2clientauthextension/extension_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func TestOAuthClientSettingsCredsConfig(t *testing.T) {
settings *Config
expectedClientConfig *clientcredentials.Config
shouldError bool
expectedError string
expectedError error
}{
{
name: "client_id_file",
Expand All @@ -163,7 +163,7 @@ func TestOAuthClientSettingsCredsConfig(t *testing.T) {
ClientSecret: "testsecret",
},
shouldError: false,
expectedError: "",
expectedError: nil,
},
{
name: "client_secret_file",
Expand All @@ -178,7 +178,7 @@ func TestOAuthClientSettingsCredsConfig(t *testing.T) {
ClientSecret: "testcreds",
},
shouldError: false,
expectedError: "",
expectedError: nil,
},
{
name: "empty_client_creds_file",
Expand All @@ -189,7 +189,7 @@ func TestOAuthClientSettingsCredsConfig(t *testing.T) {
Scopes: []string{"resource.read"},
},
shouldError: true,
expectedError: errNoClientIDProvided.Error(),
expectedError: errNoClientIDProvided,
},
{
name: "missing_client_creds_file",
Expand All @@ -200,7 +200,7 @@ func TestOAuthClientSettingsCredsConfig(t *testing.T) {
Scopes: []string{"resource.read"},
},
shouldError: true,
expectedError: errNoClientSecretProvided.Error(),
expectedError: errNoClientSecretProvided,
},
}

Expand All @@ -210,7 +210,7 @@ func TestOAuthClientSettingsCredsConfig(t *testing.T) {
cfg, err := rc.clientCredentials.createConfig()
if test.shouldError {
assert.NotNil(t, err)
assert.Contains(t, err.Error(), test.expectedError)
assert.ErrorAs(t, err, &test.expectedError)
return
}
assert.NoError(t, err)
Expand Down

0 comments on commit 1d7794e

Please sign in to comment.