Skip to content

Commit

Permalink
changes from code review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
kristinapathak committed May 4, 2021
1 parent 2dd4e0f commit 9a0108a
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 17 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Converted basculechecks to use touchstone metrics. [#95](https://github.com/xmidt-org/bascule/pull/95)
- Added method label to metric validator. [#96](https://github.com/xmidt-org/bascule/pull/96)
- Update errors to include reason used by metric validator. [#97](https://github.com/xmidt-org/bascule/pull/97)
- Made Capability Key comfigurable for CapabilitiesValidator and CapabilitiesMap. [#98](https://github.com/xmidt-org/bascule/pull/98)
- Made Capability Key configurable for CapabilitiesValidator and CapabilitiesMap. [#98](https://github.com/xmidt-org/bascule/pull/98)

## [v0.9.0]
- added helper function for building basic auth map [#59](https://github.com/xmidt-org/bascule/pull/59)
Expand Down
4 changes: 2 additions & 2 deletions basculechecks/capabilitiesmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ var (
type CapabilitiesMap struct {
Checkers map[string]EndpointChecker
DefaultChecker EndpointChecker
Key []string
KeyPath []string
}

// Check uses the parsed endpoint value to determine which EndpointChecker to
Expand All @@ -60,7 +60,7 @@ func (c CapabilitiesMap) CheckAuthentication(auth bascule.Authentication, vs Par
return ErrEmptyEndpoint
}

capabilities, err := getCapabilities(auth.Token.Attributes(), c.Key)
capabilities, err := getCapabilities(auth.Token.Attributes(), c.KeyPath)
if err != nil {
return err
}
Expand Down
14 changes: 7 additions & 7 deletions basculechecks/capabilitiesvalidator.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ type EndpointChecker interface {
// pulls the Authentication object from a context before checking it.
type CapabilitiesValidator struct {
Checker EndpointChecker
Key []string
KeyPath []string
ErrorOut bool
}

Expand Down Expand Up @@ -124,7 +124,7 @@ func (c CapabilitiesValidator) CheckAuthentication(auth bascule.Authentication,
if len(auth.Request.Method) == 0 {
return ErrNoMethod
}
vals, err := getCapabilities(auth.Token.Attributes(), c.Key)
vals, err := getCapabilities(auth.Token.Attributes(), c.KeyPath)
if err != nil {
return err
}
Expand Down Expand Up @@ -153,19 +153,19 @@ func (c CapabilitiesValidator) checkCapabilities(capabilities []string, reqURL s

// getCapabilities runs some error checks while getting the list of
// capabilities from the attributes.
func getCapabilities(attributes bascule.Attributes, key []string) ([]string, error) {
func getCapabilities(attributes bascule.Attributes, keyPath []string) ([]string, error) {
if attributes == nil {
return []string{}, ErrNilAttributes
}

if len(key) == 0 {
key = []string{CapabilityKey}
if len(keyPath) == 0 {
keyPath = []string{CapabilityKey}
}

val, ok := bascule.GetNestedAttribute(attributes, key...)
val, ok := bascule.GetNestedAttribute(attributes, keyPath...)
if !ok {
return []string{}, fmt.Errorf("%w using key path %v",
ErrGettingCapabilities, CapabilityKey)
ErrGettingCapabilities, keyPath)
}

vals, err := cast.ToStringSliceE(val)
Expand Down
21 changes: 14 additions & 7 deletions basculechecks/capabilitiesvalidator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,13 +310,7 @@ func TestGetCapabilities(t *testing.T) {
for _, tc := range tests {
t.Run(tc.description, func(t *testing.T) {
assert := assert.New(t)
m := map[string]interface{}{CapabilityKey: tc.keyValue}
if len(tc.key) > 0 {
m = map[string]interface{}{tc.key[len(tc.key)-1]: tc.keyValue}
for i := len(tc.key) - 2; i >= 0; i-- {
m = map[string]interface{}{tc.key[i]: m}
}
}
m := buildDummyAttributes(tc.key, tc.keyValue)
if tc.missingAttribute {
m = map[string]interface{}{}
}
Expand All @@ -340,3 +334,16 @@ func TestGetCapabilities(t *testing.T) {
})
}
}

func buildDummyAttributes(keyPath []string, val interface{}) map[string]interface{} {
keyLen := len(keyPath)
if keyLen == 0 {
return map[string]interface{}{CapabilityKey: val}
}
m := map[string]interface{}{keyPath[keyLen-1]: val}
// we want to move out from the inner most map.
for i := keyLen - 2; i >= 0; i-- {
m = map[string]interface{}{keyPath[i]: m}
}
return m
}

0 comments on commit 9a0108a

Please sign in to comment.