From 4623a495d1d96fe95b3c73dc78749dc277d806db Mon Sep 17 00:00:00 2001 From: Nikolay Mateev Date: Mon, 24 Feb 2020 14:24:51 +0200 Subject: [PATCH] Accept web.Request instead of http.Request in Authenticator interface (#442) --- pkg/security/authenticators/basic_authentication.go | 4 +--- .../authenticators/basic_authentication_test.go | 13 +++++++------ pkg/security/authenticators/oidc_authenticator.go | 2 +- .../authenticators/oidc_authenticator_test.go | 8 ++++---- pkg/security/filters/middlewares/authentication.go | 2 +- pkg/security/http/auth.go | 4 +--- pkg/security/http/authn/or_authenticator.go | 3 +-- pkg/security/http/httpfakes/fake_authenticator.go | 13 ++++++------- 8 files changed, 22 insertions(+), 27 deletions(-) diff --git a/pkg/security/authenticators/basic_authentication.go b/pkg/security/authenticators/basic_authentication.go index 74d8ed39d..9f81f545a 100644 --- a/pkg/security/authenticators/basic_authentication.go +++ b/pkg/security/authenticators/basic_authentication.go @@ -19,8 +19,6 @@ package authenticators import ( "encoding/json" "fmt" - "net/http" - "github.com/Peripli/service-manager/pkg/types" "github.com/Peripli/service-manager/pkg/query" @@ -37,7 +35,7 @@ type Basic struct { } // Authenticate authenticates by using the provided Basic credentials -func (a *Basic) Authenticate(request *http.Request) (*web.UserContext, httpsec.Decision, error) { +func (a *Basic) Authenticate(request *web.Request) (*web.UserContext, httpsec.Decision, error) { username, password, ok := request.BasicAuth() if !ok { return nil, httpsec.Abstain, nil diff --git a/pkg/security/authenticators/basic_authentication_test.go b/pkg/security/authenticators/basic_authentication_test.go index c05929b6c..85acd2e1f 100644 --- a/pkg/security/authenticators/basic_authentication_test.go +++ b/pkg/security/authenticators/basic_authentication_test.go @@ -19,6 +19,7 @@ package authenticators_test import ( "encoding/base64" "fmt" + "github.com/Peripli/service-manager/pkg/web" "net/http" "github.com/Peripli/service-manager/storage/storagefakes" @@ -61,7 +62,7 @@ var _ = Describe("Basic Authenticator", func() { Context("When authorization is not basic", func() { It("Should abstain", func() { request.Header.Add("Authorization", "Bearer token") - user, decision, err := authenticator.Authenticate(request) + user, decision, err := authenticator.Authenticate(&web.Request{Request: request}) Expect(err).ToNot(HaveOccurred()) Expect(user).To(BeNil()) Expect(decision).To(Equal(httpsec.Abstain)) @@ -79,7 +80,7 @@ var _ = Describe("Basic Authenticator", func() { }) It("Should deny", func() { - user, decision, err := authenticator.Authenticate(request) + user, decision, err := authenticator.Authenticate(&web.Request{Request: request}) Expect(err).To(HaveOccurred()) Expect(user).To(BeNil()) Expect(decision).To(Equal(httpsec.Deny)) @@ -117,7 +118,7 @@ var _ = Describe("Basic Authenticator", func() { }) It("Should deny", func() { - user, decision, err := authenticator.Authenticate(request) + user, decision, err := authenticator.Authenticate(&web.Request{Request: request}) Expect(err).To(HaveOccurred()) Expect(user).To(BeNil()) Expect(decision).To(Equal(httpsec.Deny)) @@ -132,7 +133,7 @@ var _ = Describe("Basic Authenticator", func() { }) It("Should abstain with error", func() { - user, decision, err := authenticator.Authenticate(request) + user, decision, err := authenticator.Authenticate(&web.Request{Request: request}) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring(expectedError.Error())) Expect(user).To(BeNil()) @@ -160,7 +161,7 @@ var _ = Describe("Basic Authenticator", func() { }) It("Should deny", func() { - user, decision, err := authenticator.Authenticate(request) + user, decision, err := authenticator.Authenticate(&web.Request{Request: request}) Expect(err).To(HaveOccurred()) Expect(user).To(BeNil()) Expect(decision).To(Equal(httpsec.Deny)) @@ -187,7 +188,7 @@ var _ = Describe("Basic Authenticator", func() { }) It("Should allow", func() { - user, decision, err := authenticator.Authenticate(request) + user, decision, err := authenticator.Authenticate(&web.Request{Request: request}) Expect(err).ToNot(HaveOccurred()) Expect(user).To(Not(BeNil())) Expect(decision).To(Equal(httpsec.Allow)) diff --git a/pkg/security/authenticators/oidc_authenticator.go b/pkg/security/authenticators/oidc_authenticator.go index 517437d26..07a6fbfcb 100644 --- a/pkg/security/authenticators/oidc_authenticator.go +++ b/pkg/security/authenticators/oidc_authenticator.go @@ -106,7 +106,7 @@ func newOIDCConfig(options *OIDCOptions) *goidc.Config { } // Authenticate returns information about the user by obtaining it from the bearer token, or an error if security is unsuccessful -func (a *OauthAuthenticator) Authenticate(request *http.Request) (*web.UserContext, httpsec.Decision, error) { +func (a *OauthAuthenticator) Authenticate(request *web.Request) (*web.UserContext, httpsec.Decision, error) { authorizationHeader := request.Header.Get("Authorization") if authorizationHeader == "" || !strings.HasPrefix(strings.ToLower(authorizationHeader), "bearer ") { return nil, httpsec.Abstain, nil diff --git a/pkg/security/authenticators/oidc_authenticator_test.go b/pkg/security/authenticators/oidc_authenticator_test.go index 6f31a9f0d..12038755d 100644 --- a/pkg/security/authenticators/oidc_authenticator_test.go +++ b/pkg/security/authenticators/oidc_authenticator_test.go @@ -259,7 +259,7 @@ var _ = Describe("OIDC Authenticator", func() { validateAuthenticationReturns := func(expectedUser *web.UserContext, expectedDecision httpsec.Decision, expectedErr error) { authenticator, _, _ := NewOIDCAuthenticator(ctx, oauthOptions) - user, decision, err := authenticator.Authenticate(request) + user, decision, err := authenticator.Authenticate(&web.Request{Request: request}) if expectedUser != nil { Expect(user).To(Equal(expectedUser)) @@ -337,7 +337,7 @@ var _ = Describe("OIDC Authenticator", func() { }) It("should deny with an error", func() { - user, decision, err := authenticator.Authenticate(request) + user, decision, err := authenticator.Authenticate(&web.Request{Request: request}) Expect(user).To(BeNil()) Expect(decision).To(Equal(httpsec.Deny)) @@ -359,7 +359,7 @@ var _ = Describe("OIDC Authenticator", func() { }) It("should deny with an error", func() { - user, decision, err := authenticator.Authenticate(request) + user, decision, err := authenticator.Authenticate(&web.Request{Request: request}) Expect(user).To(BeNil()) Expect(decision).To(Equal(httpsec.Deny)) @@ -380,7 +380,7 @@ var _ = Describe("OIDC Authenticator", func() { }) It("should allow authentication and return user", func() { - user, decision, err := authenticator.Authenticate(request) + user, decision, err := authenticator.Authenticate(&web.Request{Request: request}) Expect(user).To(Not(BeNil())) Expect(user.Name).To(Equal(expectedUserName)) diff --git a/pkg/security/filters/middlewares/authentication.go b/pkg/security/filters/middlewares/authentication.go index 07893547c..ea738b87d 100644 --- a/pkg/security/filters/middlewares/authentication.go +++ b/pkg/security/filters/middlewares/authentication.go @@ -36,7 +36,7 @@ func (m *Authentication) Run(request *web.Request, next web.Handler) (*web.Respo return next.Handle(request) } - user, decision, err := m.Authenticator.Authenticate(request.Request) + user, decision, err := m.Authenticator.Authenticate(request) if err != nil { if decision == http.Deny { log.C(ctx).Debug(err) diff --git a/pkg/security/http/auth.go b/pkg/security/http/auth.go index 337da4b50..e7f8cdd50 100644 --- a/pkg/security/http/auth.go +++ b/pkg/security/http/auth.go @@ -18,8 +18,6 @@ package http import ( "context" - "net/http" - "github.com/Peripli/service-manager/pkg/web" ) @@ -51,7 +49,7 @@ func (a Decision) String() string { type Authenticator interface { // Authenticate returns information about the user if security is successful, a bool specifying // whether the authenticator ran or not and an error if one occurs - Authenticate(req *http.Request) (*web.UserContext, Decision, error) + Authenticate(req *web.Request) (*web.UserContext, Decision, error) } // Authorizer extracts the information from the authenticated user and diff --git a/pkg/security/http/authn/or_authenticator.go b/pkg/security/http/authn/or_authenticator.go index 45f2a1865..f4f1df560 100644 --- a/pkg/security/http/authn/or_authenticator.go +++ b/pkg/security/http/authn/or_authenticator.go @@ -1,7 +1,6 @@ package authn import ( - "net/http" "strings" "github.com/Peripli/service-manager/pkg/web" @@ -21,7 +20,7 @@ func NewOrAuthenticator(authenticators ...httpsec.Authenticator) httpsec.Authent } // Authenticate allows the request if at least one of the nested authenticators allows it -func (a *orAuthenticator) Authenticate(request *http.Request) (*web.UserContext, httpsec.Decision, error) { +func (a *orAuthenticator) Authenticate(request *web.Request) (*web.UserContext, httpsec.Decision, error) { ctx := request.Context() logger := log.C(ctx) finalDecision := httpsec.Abstain diff --git a/pkg/security/http/httpfakes/fake_authenticator.go b/pkg/security/http/httpfakes/fake_authenticator.go index d8db12051..99dfd548e 100644 --- a/pkg/security/http/httpfakes/fake_authenticator.go +++ b/pkg/security/http/httpfakes/fake_authenticator.go @@ -2,7 +2,6 @@ package httpfakes import ( - httpa "net/http" "sync" "github.com/Peripli/service-manager/pkg/security/http" @@ -10,10 +9,10 @@ import ( ) type FakeAuthenticator struct { - AuthenticateStub func(*httpa.Request) (*web.UserContext, http.Decision, error) + AuthenticateStub func(*web.Request) (*web.UserContext, http.Decision, error) authenticateMutex sync.RWMutex authenticateArgsForCall []struct { - arg1 *httpa.Request + arg1 *web.Request } authenticateReturns struct { result1 *web.UserContext @@ -29,11 +28,11 @@ type FakeAuthenticator struct { invocationsMutex sync.RWMutex } -func (fake *FakeAuthenticator) Authenticate(arg1 *httpa.Request) (*web.UserContext, http.Decision, error) { +func (fake *FakeAuthenticator) Authenticate(arg1 *web.Request) (*web.UserContext, http.Decision, error) { fake.authenticateMutex.Lock() ret, specificReturn := fake.authenticateReturnsOnCall[len(fake.authenticateArgsForCall)] fake.authenticateArgsForCall = append(fake.authenticateArgsForCall, struct { - arg1 *httpa.Request + arg1 *web.Request }{arg1}) fake.recordInvocation("Authenticate", []interface{}{arg1}) fake.authenticateMutex.Unlock() @@ -53,13 +52,13 @@ func (fake *FakeAuthenticator) AuthenticateCallCount() int { return len(fake.authenticateArgsForCall) } -func (fake *FakeAuthenticator) AuthenticateCalls(stub func(*httpa.Request) (*web.UserContext, http.Decision, error)) { +func (fake *FakeAuthenticator) AuthenticateCalls(stub func(*web.Request) (*web.UserContext, http.Decision, error)) { fake.authenticateMutex.Lock() defer fake.authenticateMutex.Unlock() fake.AuthenticateStub = stub } -func (fake *FakeAuthenticator) AuthenticateArgsForCall(i int) *httpa.Request { +func (fake *FakeAuthenticator) AuthenticateArgsForCall(i int) *web.Request { fake.authenticateMutex.RLock() defer fake.authenticateMutex.RUnlock() argsForCall := fake.authenticateArgsForCall[i]