util/wrapper: fix noop auth nil account bug (#1721)

* util/wrapper: fix noop nil account

* util/wrapper: improve comments

* util/wrapper: update tests
This commit is contained in:
ben-toogood 2020-06-19 12:16:39 +01:00 committed by GitHub
parent 58c6bbbf6b
commit ece02a6d21
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 10 additions and 23 deletions

View File

@ -189,19 +189,22 @@ func AuthHandler(fn func() auth.Auth) server.HandlerWrapper {
return h(ctx, req, rsp) return h(ctx, req, rsp)
} }
// Extract the token if present. Note: if noop is being used // Extract the token if the header is present. We will inspect the token regardless of if it's
// then the token can be blank without erroring // present or not since noop auth will return a blank account upon Inspecting a blank token.
var account *auth.Account var token string
if header, ok := metadata.Get(ctx, "Authorization"); ok { if header, ok := metadata.Get(ctx, "Authorization"); ok {
// Ensure the correct scheme is being used // Ensure the correct scheme is being used
if !strings.HasPrefix(header, auth.BearerScheme) { if !strings.HasPrefix(header, auth.BearerScheme) {
return errors.Unauthorized(req.Service(), "invalid authorization header. expected Bearer schema") return errors.Unauthorized(req.Service(), "invalid authorization header. expected Bearer schema")
} }
// Strip the prefix and inspect the resulting token // Strip the bearer scheme prefix
account, _ = a.Inspect(strings.TrimPrefix(header, auth.BearerScheme)) token = strings.TrimPrefix(header, auth.BearerScheme)
} }
// Inspect the token and decode an account
account, _ := a.Inspect(token)
// Extract the namespace header // Extract the namespace header
ns, ok := metadata.Get(ctx, "Micro-Namespace") ns, ok := metadata.Get(ctx, "Micro-Namespace")
if !ok { if !ok {

View File

@ -121,22 +121,6 @@ func TestAuthHandler(t *testing.T) {
} }
}) })
// If the Authorization header is blank, no error should be returned and verify not called
t.Run("BlankAuthorizationHeader", func(t *testing.T) {
a := testAuth{}
handler := AuthHandler(func() auth.Auth {
return &a
})
err := handler(h)(context.TODO(), serviceReq, nil)
if err != nil {
t.Errorf("Expected nil error but got %v", err)
}
if a.inspectCount != 0 {
t.Errorf("Did not expect inspect to be called")
}
})
// If the Authorization header is invalid, an error should be returned and verify not called // If the Authorization header is invalid, an error should be returned and verify not called
t.Run("InvalidAuthorizationHeader", func(t *testing.T) { t.Run("InvalidAuthorizationHeader", func(t *testing.T) {
a := testAuth{} a := testAuth{}
@ -173,7 +157,7 @@ func TestAuthHandler(t *testing.T) {
// If the issuer header was not set on the request, the wrapper should set it to the auths // If the issuer header was not set on the request, the wrapper should set it to the auths
// own issuer // own issuer
t.Run("BlankissuerHeader", func(t *testing.T) { t.Run("BlankIssuerHeader", func(t *testing.T) {
a := testAuth{issuer: "myissuer"} a := testAuth{issuer: "myissuer"}
handler := AuthHandler(func() auth.Auth { handler := AuthHandler(func() auth.Auth {
return &a return &a
@ -193,7 +177,7 @@ func TestAuthHandler(t *testing.T) {
t.Errorf("Expected issuer to be set to %v but was %v", a.issuer, ns) t.Errorf("Expected issuer to be set to %v but was %v", a.issuer, ns)
} }
}) })
t.Run("ValidissuerHeader", func(t *testing.T) { t.Run("ValidIssuerHeader", func(t *testing.T) {
a := testAuth{issuer: "myissuer"} a := testAuth{issuer: "myissuer"}
handler := AuthHandler(func() auth.Auth { handler := AuthHandler(func() auth.Auth {
return &a return &a