Merge pull request #276 from coveooss/fix/DT-6821-extra-data-crash

Fail Credentials validation if it contains extra data
coveooss · Mar 11, 2024 · f1bb1ee · f1bb1ee
2 parents d0dfc75 + 861c50d
commit f1bb1ee
Show file tree

Hide file tree

Showing 8 changed files with 73 additions and 13 deletions.
diff --git a/credentials/credentials.go b/credentials/credentials.go
@@ -158,11 +158,20 @@ func ParseSingleCredentials(credentialsMap map[string]interface{}) (Credentials,
 	default:
 		return nil, fmt.Errorf("entry %s: unknown credentials type: %s", id, credentialsType)
 	}
-	err := mapstructure.Decode(credentialsMap, credentials)
+	var validationErrors error
+	delete(credentialsMap, "type")
+	decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
+		ErrorUnused: true,
+		Metadata:    nil,
+		Result:      credentials})
 	if err != nil {
-		return nil, fmt.Errorf("entry %s: invalid credentials data: %v", id, err)
+		validationErrors = multierror.Append(validationErrors, fmt.Errorf("entry %s: unable to create a decoder: %v", id, err))
 	}
-	var validationErrors error
+	if err := decoder.Decode(credentialsMap); err != nil {
+		return nil, fmt.Errorf("entry %s: invalid credentials data: %w", id, err)
+	}
+	credentialsMap["type"] = credentialsType
+
 	if err := credentials.BaseValidate(); err != nil {
 		validationErrors = multierror.Append(validationErrors, err)
 	}

diff --git a/credentials/credentials_aws_test.go b/credentials/credentials_aws_test.go
@@ -15,7 +15,7 @@ func TestParseAwsCredentialsFromValue(t *testing.T) {
 		"value": "key:secret",
 	})
 
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	cred := credInterface.(*AmazonWebServicesCredentials)
 
 	assert.Equal(t, "key", cred.AccessKey)
@@ -50,12 +50,17 @@ func TestAwsCredentialsValidationErrors(t *testing.T) {
 	// All OK
 	credMap["secret_key"] = "secret"
 	_, err = ParseSingleCredentials(credMap)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	// No access key
 	delete(credMap, "access_key")
 	_, err = ParseSingleCredentials(credMap)
 	assert.Error(t, err)
+
+	// Extra data
+	credMap["extra"] = "data"
+	_, err = ParseSingleCredentials(credMap)
+	assert.Error(t, err)
 }
 
 func TestAwsCredentialsToString(t *testing.T) {
@@ -66,3 +71,49 @@ func TestAwsCredentialsToString(t *testing.T) {
 	assert.Equal(t, "test -> Type: Amazon Web Services - key:********", cred.ToString(false))
 	assert.Equal(t, "test -> Type: Amazon Web Services - key:secret", cred.ToString(true))
 }
+
+func TestCredentialWithTargetTags(t *testing.T) {
+	credMap := map[string]interface{}{
+		"id":          "test",
+		"type":        "aws",
+		"description": "test-desc",
+		"access_key":  "key",
+		"secret_key":  "secret_key",
+		"target_tags": map[string]interface{}{
+			"do_match": map[string]interface{}{
+				"tag1": "value1",
+			},
+		},
+	}
+
+	cred, err := ParseSingleCredentials(credMap)
+	assert.NoError(t, err)
+
+	assert.Equal(t, &AmazonWebServicesCredentials{
+		Base: Base{
+			ID:          "test",
+			CredType:    "Amazon Web Services",
+			Description: "test-desc",
+			TargetTags:  targetTagsMatcher{DoMatch: map[string]interface{}{"tag1": "value1"}},
+		},
+		AccessKey: "key",
+		SecretKey: "secret_key",
+	}, cred)
+}
+
+func TestCredentialWithTargetTagsMalformed(t *testing.T) {
+	credMap := map[string]interface{}{
+		"id":         "test",
+		"type":       "aws",
+		"access_key": "key",
+		"secret_key": "secret_key",
+		"target_tags": map[string]interface{}{
+			"match": map[string]interface{}{
+				"tag1": "value1",
+			},
+		},
+	}
+
+	_, err := ParseSingleCredentials(credMap)
+	assert.Error(t, err)
+}
diff --git a/credentials/credentials_secret_test.go b/credentials/credentials_secret_test.go
@@ -15,7 +15,7 @@ func TestParsSecretCredentialsFromValue(t *testing.T) {
 		"value": "my secret",
 	})
 
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	cred := credInterface.(*SecretTextCredentials)
 
 	assert.Equal(t, "my secret", cred.Secret)

diff --git a/credentials/credentials_userpass_test.go b/credentials/credentials_userpass_test.go
@@ -15,7 +15,7 @@ func TestParseUserPassCredentialsFromValue(t *testing.T) {
 		"value": "user:pass",
 	})
 
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	cred := credInterface.(*UsernamePasswordCredentials)
 
 	assert.Equal(t, "user", cred.Username)

diff --git a/credentials/source_local_test.go b/credentials/source_local_test.go
@@ -35,6 +35,6 @@ func TestGetCredentialsFromLocalSource(t *testing.T) {
 	expectedCred.Description = "a credential"
 	expectedCred.Username = "user"
 	expectedCred.Password = "pass"
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.Equal(t, []Credentials{expectedCred}, credentials)
 }
diff --git a/credentials/source_s3_test.go b/credentials/source_s3_test.go
@@ -89,6 +89,6 @@ func TestGetCredentialsFromS3Source(t *testing.T) {
 	expectedCred.Description = "a credential"
 	expectedCred.Username = "user"
 	expectedCred.Password = "pass"
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.Equal(t, []Credentials{expectedCred}, credentials)
 }
diff --git a/credentials/source_secretsmanager_test.go b/credentials/source_secretsmanager_test.go
@@ -123,7 +123,7 @@ func TestGetCredentialsFromSecretsManagerSourceWithPrefix(t *testing.T) {
 	credentials, err := secretsManagerSource.Credentials()
 	sort.Slice(credentials, func(i, j int) bool { return credentials[i].GetID() < credentials[j].GetID() })
 
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.Equal(t, expectedSecretsManagerCredentials, credentials)
 }
 
@@ -138,7 +138,7 @@ func TestGetCredentialsFromSecretsManagerSourceWithID(t *testing.T) {
 	credentials, err := secretsManagerSource.Credentials()
 	sort.Slice(credentials, func(i, j int) bool { return credentials[i].GetID() < credentials[j].GetID() })
 
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.Equal(t, testCredentials, credentials)
 }
 

diff --git a/credentials/sources_test.go b/credentials/sources_test.go
@@ -26,7 +26,7 @@ func TestSourcesConfigWithLocalSource(t *testing.T) {
 	sourcesConfig := SourcesConfiguration{LocalSources: []*LocalSource{localSource}}
 
 	credentials, err := sourcesConfig.Credentials()
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.Equal(t, []Credentials{testCredentials[0]}, credentials)
 }
 
@@ -110,7 +110,7 @@ func TestGetCredentialsFromBytes(t *testing.T) {
 			if tt.wantErr {
 				assert.Error(t, err)
 			} else {
-				assert.Nil(t, err)
+				assert.NoError(t, err)
 			}
 			var expectedAsMaps []map[string]interface{}
 			var gottenAsMaps []map[string]interface{}