Skip to content

Commit

Permalink
fix some suggestions from the bot - error messages, test coverage, ma…
Browse files Browse the repository at this point in the history
…rk purely defensive code

Signed-off-by: Dave Lee <dave@gray101.com>
  • Loading branch information
dave-gray101 committed Jun 17, 2024
1 parent 26bc132 commit 9588706
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 4 deletions.
3 changes: 2 additions & 1 deletion middleware/keyauth/keyauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ func MultipleKeySourceLookup(keyLookups []string, authScheme string) (KeyLookupF
return res, nil
}
if !errors.Is(err, ErrMissingOrMalformedAPIKey) {
// Defensive Code - not currently possible to hit
return "", fmt.Errorf("[%s] %w", keyLookup, err)

Check warning on line 98 in middleware/keyauth/keyauth.go

View check run for this annotation

Codecov / codecov/patch

middleware/keyauth/keyauth.go#L98

Added line #L98 was not covered by tests
}
}
Expand All @@ -104,7 +105,7 @@ func MultipleKeySourceLookup(keyLookups []string, authScheme string) (KeyLookupF
func DefaultKeyLookup(keyLookup, authScheme string) (KeyLookupFunc, error) {
parts := strings.Split(keyLookup, ":")
if len(parts) <= 1 {
return nil, fmt.Errorf("invalid keyLookup: %s", keyLookup)
return nil, fmt.Errorf("invalid keyLookup: %q, expected format 'source:name'", keyLookup)
}
extractor := KeyFromHeader(parts[1], authScheme) // in the event of an invalid prefix, it is interpreted as header:
switch parts[0] {
Expand Down
22 changes: 19 additions & 3 deletions middleware/keyauth/keyauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,23 @@ func TestPanicOnInvalidConfiguration(t *testing.T) {
require.NoError(t, err)
}()
app.Use(authMiddleware)
})
}, "should panic if Validator is missing")

require.Panics(t, func() {
authMiddleware := New(Config{
KeyLookup: "invalid",
Validator: func(_ fiber.Ctx, _ string) (bool, error) {
return true, nil
},
})
// We shouldn't even make it this far, but these next two lines prevent authMiddleware from being an unused variable.
app := fiber.New()
defer func() { // testing panics, defer block to ensure cleanup
err := app.Shutdown()
require.NoError(t, err)
}()
app.Use(authMiddleware)
}, "should panic if CustomKeyLookup is not set AND KeyLookup has an invalid value")
}

func TestCustomKeyUtilityFunctionErrors(t *testing.T) {
Expand All @@ -152,10 +168,10 @@ func TestCustomKeyUtilityFunctionErrors(t *testing.T) {

// Invalid element while parsing
_, err := DefaultKeyLookup("invalid", scheme)
require.Error(t, err)
require.Error(t, err, "DefaultKeyLookup should fail for 'invalid' keyLookup")

_, err = MultipleKeySourceLookup([]string{"header:key", "invalid"}, scheme)
require.Error(t, err)
require.Error(t, err, "MultipleKeySourceLookup should fail for 'invalid' keyLookup")
}

func TestMultipleKeyLookup(t *testing.T) {
Expand Down

0 comments on commit 9588706

Please sign in to comment.