Skip to content

Commit

Permalink
Accept web.Request instead of http.Request in Authenticator interface (
Browse files Browse the repository at this point in the history
  • Loading branch information
NickyMateev authored Feb 24, 2020
1 parent 3684099 commit 4623a49
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 27 deletions.
4 changes: 1 addition & 3 deletions pkg/security/authenticators/basic_authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
13 changes: 7 additions & 6 deletions pkg/security/authenticators/basic_authentication_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand Down Expand Up @@ -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))
Expand All @@ -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())
Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion pkg/security/authenticators/oidc_authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions pkg/security/authenticators/oidc_authenticator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand All @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion pkg/security/filters/middlewares/authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 1 addition & 3 deletions pkg/security/http/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ package http

import (
"context"
"net/http"

"github.com/Peripli/service-manager/pkg/web"
)

Expand Down Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions pkg/security/http/authn/or_authenticator.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package authn

import (
"net/http"
"strings"

"github.com/Peripli/service-manager/pkg/web"
Expand All @@ -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
Expand Down
13 changes: 6 additions & 7 deletions pkg/security/http/httpfakes/fake_authenticator.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 4623a49

Please sign in to comment.