Merge pull request #19 from virtru/lint-fix

Add more linters, and fix lint errors #patch

.golangci.yaml

@@ -0,0 +1,9 @@
+linters:
+  # Enable specific linter
+  # https://golangci-lint.run/usage/linters/#enabled-by-default-linters
+  enable:
+    - godox
+    - bodyclose
+    - gosec
+    - misspell
+    - exportloopref

pdp/access-pdp.go

@@ -129,7 +129,7 @@ func (pdp *AccessPDP) DetermineAccess(dataAttributes []attrs.AttributeInstance,
 		//If GroupBy is set, determine which entities (out of the set of entities and their respective AttributeInstances)
 		//will be considered for evaluation under this rule definition.
 		//
-		//If GroupBy is not set, then we always consider all entities for evaulation under a rule definition
+		//If GroupBy is not set, then we always consider all entities for evaluation under a rule definition
 		//
 		//If this rule simply does not apply to a given entity ID as defined by the AttributeDefinition we have,
 		//and the entity AttributeInstances that entity ID has, then that entity ID passed (or skipped) this rule.
@@ -138,9 +138,6 @@ func (pdp *AccessPDP) DetermineAccess(dataAttributes []attrs.AttributeInstance,
 			pdp.logger.Debugf("Attribute Definition's GroupBy is set to %s, filtering entities that will be considered for rule %s", attrDefinition, canonicalName)
 			filteredEntities = pdp.groupByFilterEntityAttributeInstances(entityAttributeSets, attrDefinition.GroupBy)
 			pdp.logger.Debugf("For this definition, according to GroupBy, considering %d out of %d total entities", len(filteredEntities), len(entityAttributeSets))
-			//TODO I wonder if we should return a "Decision == skipped" for each of these entities that would get
-			//excluded by grouping, just to keep things consistent. On the other hand, caller can easily figure this out
-			//if they care, and I don't want to introduce trinary access states :D
 		}
 
 		var entityRuleDecision map[string]DataRuleResult
@@ -214,14 +211,14 @@ func (pdp *AccessPDP) allOfRule(dataAttrsBySingleCanonicalName []attrs.Attribute
 		entityAttrCluster := attrs.ClusterByCanonicalName(entityAttrs)
 
 		//For every unqiue data AttributeInstance (that is, unique data attribute value) in this set of data AttributeInstances sharing the same canonical name...
-		for _, dataAttrVal := range dataAttrsBySingleCanonicalName {
+		for dvIndex, dataAttrVal := range dataAttrsBySingleCanonicalName {
 			dvCanonicalName := dataAttrVal.GetCanonicalName()
 			pdp.logger.Debugf("Evaluating all-of decision for data attr %s with value %s", dvCanonicalName, dataAttrVal.Value)
 			//See if
 			// 1. there exists an entity AttributeInstance in the set of AttributeInstances
 			// with the same canonical name as the data AttributeInstance in question
 			// 2. It has the same VALUE as the data AttributeInstance in question
-			found := findInstanceValueInCluster(&dataAttrVal, entityAttrCluster[dvCanonicalName])
+			found := findInstanceValueInCluster(&dataAttrsBySingleCanonicalName[dvIndex], entityAttrCluster[dvCanonicalName])
 
 			denialMsg := ""
 			//If we did not find the data AttributeInstance canonical name + value in the entity AttributeInstance set,
@@ -231,7 +228,7 @@ func (pdp *AccessPDP) allOfRule(dataAttrsBySingleCanonicalName []attrs.Attribute
 				pdp.logger.Warn(denialMsg)
 				//Append the ValueFailure to the set of entity value failures
 				valueFailures = append(valueFailures, ValueFailure{
-					DataAttribute: &dataAttrVal,
+					DataAttribute: &dataAttrsBySingleCanonicalName[dvIndex],
 					Message:       denialMsg,
 				})
 			}
@@ -272,14 +269,14 @@ func (pdp *AccessPDP) anyOfRule(dataAttrsBySingleCanonicalName []attrs.Attribute
 		entityAttrCluster := attrs.ClusterByCanonicalName(entityAttrs)
 
 		//For every unqiue data AttributeInstance (that is, value) in this set of data AttributeInstance sharing the same canonical name...
-		for _, dataAttrVal := range dataAttrsBySingleCanonicalName {
+		for dvIndex, dataAttrVal := range dataAttrsBySingleCanonicalName {
 			pdp.logger.Debugf("Evaluating anyOf decision for data attr %s with value %s", dvCanonicalName, dataAttrVal.Value)
 
 			//See if
 			// 1. there exists an entity AttributeInstance in the set of AttributeInstances
 			// with the same canonical name as the data AttributeInstance in question
 			// 2. It has the same VALUE as the data AttributeInstance in question
-			found := findInstanceValueInCluster(&dataAttrVal, entityAttrCluster[dvCanonicalName])
+			found := findInstanceValueInCluster(&dataAttrsBySingleCanonicalName[dvIndex], entityAttrCluster[dvCanonicalName])
 
 			denialMsg := ""
 			//If we did not find the data AttributeInstance canonical name + value in the entity AttributeInstance set,
@@ -289,7 +286,7 @@ func (pdp *AccessPDP) anyOfRule(dataAttrsBySingleCanonicalName []attrs.Attribute
 				pdp.logger.Debug(denialMsg)
 
 				valueFailures = append(valueFailures, ValueFailure{
-					DataAttribute: &dataAttrVal,
+					DataAttribute: &dataAttrsBySingleCanonicalName[dvIndex],
 					Message:       denialMsg,
 				})
 			}
@@ -332,7 +329,6 @@ func (pdp *AccessPDP) hierarchyRule(dataAttrsBySingleCanonicalName []attrs.Attri
 	}
 	//All of the data AttributeInstances in the arg have the same canonical name.
 
-
 	//Go through every entity's AttributeInstance set...
 	for entityId, entityAttrs := range entityAttributes {
 		//Default to DENY
@@ -458,7 +454,7 @@ func findInstanceValueInCluster(instance *attrs.AttributeInstance, cluster []att
 //Given set of ordered/ranked values, a data singular AttributeInstance, and a set of entity AttributeInstances,
 //determine if the entity AttributeInstances include a ranked value that equals or exceeds
 //the rank of the data AttributeInstance value.
-//For heirarchy, convention is 0 == most privileged, 1 == less privileged, etc
+//For hierarchy, convention is 0 == most privileged, 1 == less privileged, etc
 func entityRankGreaterThanOrEqualToDataRank(order []string, dataAttribute *attrs.AttributeInstance, entityAttributeCluster []attrs.AttributeInstance) bool {
 	//default to least-perm
 	result := false

pdp/access-pdp_test.go

@@ -114,7 +114,7 @@ func Test_AccessPDP_AnyOf_FailMissingValue(t *testing.T) {
 	assert.Equal(t, 1, len(decisions[entityID].Results))
 	assert.False(t, decisions[entityID].Results[0].Passed)
 	assert.Equal(t, 2, len(decisions[entityID].Results[0].ValueFailures))
-	assert.Equal(t, &mockDataAttrs[1], decisions[entityID].Results[0].ValueFailures[0].DataAttribute)
+	assert.Equal(t, &mockDataAttrs[0], decisions[entityID].Results[0].ValueFailures[0].DataAttribute)
 	assert.Equal(t, &mockAttrDefinitions[0], decisions[entityID].Results[0].RuleDefinition)
 }
 
@@ -167,7 +167,7 @@ func Test_AccessPDP_AnyOf_FailMissingAttr(t *testing.T) {
 	assert.Equal(t, 1, len(decisions[entityID].Results))
 	assert.False(t, decisions[entityID].Results[0].Passed)
 	assert.Equal(t, 2, len(decisions[entityID].Results[0].ValueFailures))
-	assert.Equal(t, &mockDataAttrs[1], decisions[entityID].Results[0].ValueFailures[0].DataAttribute)
+	assert.Equal(t, &mockDataAttrs[0], decisions[entityID].Results[0].ValueFailures[0].DataAttribute)
 	assert.Equal(t, &mockAttrDefinitions[0], decisions[entityID].Results[0].RuleDefinition)
 }
 
@@ -220,7 +220,7 @@ func Test_AccessPDP_AnyOf_FailAttrWrongNamespace(t *testing.T) {
 	assert.Equal(t, 1, len(decisions[entityID].Results))
 	assert.False(t, decisions[entityID].Results[0].Passed)
 	assert.Equal(t, 2, len(decisions[entityID].Results[0].ValueFailures))
-	assert.Equal(t, &mockDataAttrs[1], decisions[entityID].Results[0].ValueFailures[0].DataAttribute)
+	assert.Equal(t, &mockDataAttrs[0], decisions[entityID].Results[0].ValueFailures[0].DataAttribute)
 	assert.Equal(t, &mockAttrDefinitions[0], decisions[entityID].Results[0].RuleDefinition)
 }
 
@@ -355,7 +355,7 @@ func Test_AccessPDP_AnyOf_NoEntityAttributes_Fails(t *testing.T) {
 	assert.Equal(t, 1, len(decisions[entityID].Results))
 	assert.False(t, decisions[entityID].Results[0].Passed)
 	assert.Equal(t, 2, len(decisions[entityID].Results[0].ValueFailures))
-	assert.Equal(t, &mockDataAttrs[1], decisions[entityID].Results[0].ValueFailures[0].DataAttribute)
+	assert.Equal(t, &mockDataAttrs[0], decisions[entityID].Results[0].ValueFailures[0].DataAttribute)
 	assert.Equal(t, &mockAttrDefinitions[0], decisions[entityID].Results[0].RuleDefinition)
 }
 
@@ -591,7 +591,7 @@ func Test_AccessPDP_AllOf_FailMissingValue(t *testing.T) {
 	assert.Equal(t, 1, len(decisions[entityID].Results))
 	assert.False(t, decisions[entityID].Results[0].Passed)
 	assert.Equal(t, 1, len(decisions[entityID].Results[0].ValueFailures))
-	assert.Equal(t, &mockDataAttrs[1], decisions[entityID].Results[0].ValueFailures[0].DataAttribute)
+	assert.Equal(t, &mockDataAttrs[0], decisions[entityID].Results[0].ValueFailures[0].DataAttribute)
 	assert.Equal(t, &mockAttrDefinitions[0], decisions[entityID].Results[0].RuleDefinition)
 }
 
@@ -644,7 +644,7 @@ func Test_AccessPDP_AllOf_FailMissingAttr(t *testing.T) {
 	assert.Equal(t, 1, len(decisions[entityID].Results))
 	assert.False(t, decisions[entityID].Results[0].Passed)
 	assert.Equal(t, 2, len(decisions[entityID].Results[0].ValueFailures))
-	assert.Equal(t, &mockDataAttrs[1], decisions[entityID].Results[0].ValueFailures[0].DataAttribute)
+	assert.Equal(t, &mockDataAttrs[0], decisions[entityID].Results[0].ValueFailures[0].DataAttribute)
 	assert.Equal(t, &mockAttrDefinitions[0], decisions[entityID].Results[0].RuleDefinition)
 }
 
@@ -702,7 +702,7 @@ func Test_AccessPDP_AllOf_FailAttrWrongNamespace(t *testing.T) {
 	assert.Equal(t, 1, len(decisions[entityID].Results))
 	assert.False(t, decisions[entityID].Results[0].Passed)
 	assert.Equal(t, 2, len(decisions[entityID].Results[0].ValueFailures))
-	assert.Equal(t, &mockDataAttrs[1], decisions[entityID].Results[0].ValueFailures[0].DataAttribute)
+	assert.Equal(t, &mockDataAttrs[0], decisions[entityID].Results[0].ValueFailures[0].DataAttribute)
 	assert.Equal(t, &mockAttrDefinitions[0], decisions[entityID].Results[0].RuleDefinition)
 }
 

protoconv/protobufconv.go

@@ -112,8 +112,8 @@ func DataRuleResultsToPb(results []pdp.DataRuleResult) []*pbPDP.DataRuleResult {
 
 	for _, v := range results {
 		var convFails []*pbPDP.ValueFailure
-		for _, fail := range v.ValueFailures {
-			convFails = append(convFails, ValueFailureToPb(&fail))
+		for fIdx := range v.ValueFailures {
+			convFails = append(convFails, ValueFailureToPb(&v.ValueFailures[fIdx]))
 		}
 		pbresults = append(pbresults, &pbPDP.DataRuleResult{Passed: v.Passed, RuleDefinition: AttributeDefinitionToPb(v.RuleDefinition), ValueFailures: convFails})
 	}