Skip to content

Commit

Permalink
fix(IamAuthenticator): tweak Validate() method to be more lenient (#158)
Browse files Browse the repository at this point in the history
  • Loading branch information
@padamstx
padamstx authored Mar 23, 2022
1 parent e46d057 commit 8f002d6
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 30 deletions.
18 changes: 9 additions & 9 deletions .secrets.baseline
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"files": "go.sum|package-lock.json|^.secrets.baseline$",
"lines": null
},
"generated_at": "2022-03-17T20:39:01Z",
"generated_at": "2022-03-23T14:25:50Z",
"plugins_used": [
{
"name": "AWSKeyDetector"
Expand Down Expand Up @@ -116,7 +116,7 @@
"hashed_secret": "bc2f74c22f98f7b6ffbc2f67453dbfa99bce9a32",
"is_secret": false,
"is_verified": false,
"line_number": 617,
"line_number": 624,
"type": "Secret Keyword",
"verified_result": null
}
Expand Down Expand Up @@ -550,55 +550,55 @@
"hashed_secret": "1f5e25be9b575e9f5d39c82dfd1d9f4d73f1975c",
"is_secret": false,
"is_verified": false,
"line_number": 195,
"line_number": 224,
"type": "Secret Keyword",
"verified_result": null
},
{
"hashed_secret": "ffc168ba60490856fec503b911fab745e277370b",
"is_secret": false,
"is_verified": false,
"line_number": 210,
"line_number": 239,
"type": "Secret Keyword",
"verified_result": null
},
{
"hashed_secret": "84de897bbaa1dac9c7e13b27ab2afc2a233a5e4e",
"is_secret": false,
"is_verified": false,
"line_number": 231,
"line_number": 260,
"type": "Secret Keyword",
"verified_result": null
},
{
"hashed_secret": "e952fd77963a8d1e995a104cfee55565780dffed",
"is_secret": false,
"is_verified": false,
"line_number": 247,
"line_number": 276,
"type": "Secret Keyword",
"verified_result": null
},
{
"hashed_secret": "7480f0b7140317bd82ade3c7a9526408304d5a7f",
"is_secret": false,
"is_verified": false,
"line_number": 515,
"line_number": 544,
"type": "Secret Keyword",
"verified_result": null
},
{
"hashed_secret": "6a0a3e8036180c23da91ede4f9d7bbfefd56e1a9",
"is_secret": false,
"is_verified": false,
"line_number": 1074,
"line_number": 1103,
"type": "Secret Keyword",
"verified_result": null
},
{
"hashed_secret": "32e8612d8ca77c7ea8374aa7918db8e5df9252ed",
"is_secret": false,
"is_verified": false,
"line_number": 1096,
"line_number": 1125,
"type": "Secret Keyword",
"verified_result": null
}
Expand Down
28 changes: 21 additions & 7 deletions v5/core/iam_authenticator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,9 +275,24 @@ func (authenticator *IamAuthenticator) setTokenData(tokenData *iamTokenData) {
// and that the ClientId and ClientSecret properties are mutually inclusive.
func (this *IamAuthenticator) Validate() error {

// The user should specify exactly one of ApiKey or RefreshToken.
if this.ApiKey == "" && this.RefreshToken == "" ||
this.ApiKey != "" && this.RefreshToken != "" {
// The user should specify at least one of ApiKey or RefreshToken.
// Note: We'll allow both ApiKey and RefreshToken to be specified,
// in which case we'd use ApiKey in the RequestToken() method.
// Consider this scenario...
// - An IamAuthenticator instance is configured with an apikey and is initially
// declared to be "valid" by the Validate() method.
// - The authenticator is used to construct a service, then an operation is
// invoked which then triggers the very first call to RequestToken().
// - The authenticator invokes the IAM get_token operation and then receives
// the response. The authenticator copies the refresh_token value from the response
// to the authenticator's RefreshToken field.
// - At this point, the authenticator would have non-empty values in both the
// ApiKey and RefreshToken fields.
// This all means that we must try to make sure that a previously-validated
// instance of the authenticator doesn't become invalidated simply through
// normal use.
//
if this.ApiKey == "" && this.RefreshToken == "" {
return fmt.Errorf(ERRORMSG_EXCLUSIVE_PROPS_ERROR, "ApiKey", "RefreshToken")
}

Expand All @@ -286,10 +301,9 @@ func (this *IamAuthenticator) Validate() error {
}

// Validate ClientId and ClientSecret.
// If RefreshToken is not specified, then both or neither should be specified.
// If RefreshToken is specified, then both must be specified.
if this.ClientId == "" && this.ClientSecret == "" && this.RefreshToken == "" {
// Do nothing as this is the valid scenario
// Either both or neither should be specified.
if this.ClientId == "" && this.ClientSecret == "" {
// Do nothing as this is the valid scenario.
} else {
// Since it is NOT the case that both properties are empty, make sure BOTH are specified.
if this.ClientId == "" {
Expand Down
57 changes: 43 additions & 14 deletions v5/core/iam_authenticator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,6 @@ func TestIamAuthBuilderErrors(t *testing.T) {
assert.Nil(t, auth)
t.Logf("Expected error: %s", err.Error())

// Error: specify both apikey and refresh token
auth, err = NewIamAuthenticatorBuilder().
SetApiKey(iamAuthMockApiKey).
SetRefreshToken(iamAuthMockRefreshToken).
Build()
assert.NotNil(t, err)
assert.Nil(t, auth)
t.Logf("Expected error: %s", err.Error())

// Error: invalid apikey
auth, err = NewIamAuthenticatorBuilder().
SetApiKey("{invalid-apikey}").
Expand All @@ -76,22 +67,24 @@ func TestIamAuthBuilderErrors(t *testing.T) {
assert.Nil(t, auth)
t.Logf("Expected error: %s", err.Error())

// Error: refresh token without client id
// Error: apikey and client-id set, but no client-secret
auth, err = NewIamAuthenticatorBuilder().
SetRefreshToken(iamAuthMockRefreshToken).
SetApiKey(iamAuthMockApiKey).
SetClientIDSecret(iamAuthMockClientID, "").
Build()
assert.NotNil(t, err)
assert.Nil(t, auth)
t.Logf("Expected error: %s", err.Error())

// Error: refresh token without client secret
// Error: apikey and client-secret set, but no client-id
auth, err = NewIamAuthenticatorBuilder().
SetRefreshToken(iamAuthMockRefreshToken).
SetClientIDSecret(iamAuthMockClientID, "").
SetApiKey(iamAuthMockApiKey).
SetClientIDSecret("", iamAuthMockClientSecret).
Build()
assert.NotNil(t, err)
assert.Nil(t, auth)
t.Logf("Expected error: %s", err.Error())

}

func TestIamAuthBuilderSuccess(t *testing.T) {
Expand Down Expand Up @@ -135,6 +128,14 @@ func TestIamAuthBuilderSuccess(t *testing.T) {
assert.Nil(t, auth.Headers)
assert.Equal(t, AUTHTYPE_IAM, auth.AuthenticationType())

// Success: specify both apikey and refresh token
auth, err = NewIamAuthenticatorBuilder().
SetApiKey(iamAuthMockApiKey).
SetRefreshToken(iamAuthMockRefreshToken).
Build()
assert.Nil(t, err)
assert.NotNil(t, auth)

// Specify apikey with other properties.
auth, err = NewIamAuthenticatorBuilder().
SetURL(iamAuthMockURL).
Expand Down Expand Up @@ -178,6 +179,34 @@ func TestIamAuthBuilderSuccess(t *testing.T) {
assert.Equal(t, AUTHTYPE_IAM, auth.AuthenticationType())
}

func TestIamAuthReuseAuthenticator(t *testing.T) {
auth, err := NewIamAuthenticatorBuilder().
SetApiKey(iamAuthMockApiKey).
Build()
assert.Nil(t, err)
assert.NotNil(t, auth)

// Use the authenticator to construct a service.
service, err := NewBaseService(&ServiceOptions{
URL: "don't care",
Authenticator: auth,
})
assert.Nil(t, err)
assert.NotNil(t, service)

// Simulate use of the authenticator by setting the RefreshToken
// field (this will be set when processing an IAM get-token response).
auth.RefreshToken = iamAuthMockRefreshToken

// Now re-use the authenticator with a new service.
service, err = NewBaseService(&ServiceOptions{
URL: "don't care",
Authenticator: auth,
})
assert.Nil(t, err)
assert.NotNil(t, service)
}

//
// Tests that construct an authenticator via map properties.
//
Expand Down

0 comments on commit 8f002d6

Please sign in to comment.