diff --git a/UPGRADE.md b/UPGRADE.md index 2826e7bfef..ce19208994 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -16,6 +16,15 @@ before finalizing the upgrade process. +## 1.0.0-beta.8 + +### `noop` authenticator no longer bypasses authorizers/credentials issuers + +The `noop` authenticator is now very similar to `anonymous` with the difference that no anonymous subject is being +set. + +Previously, the `noop` authenticator bypassed the authorizer and credential issuers. This patch changes that. + ## 1.0.0-beta.2 This release introduces serious breaking changes. If you are upgrading, you will - unfortunately - need to diff --git a/judge/handler_test.go b/judge/handler_test.go index ddf6a04615..930de6b188 100644 --- a/judge/handler_test.go +++ b/judge/handler_test.go @@ -34,7 +34,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestProxy(t *testing.T) { +func TestJudge(t *testing.T) { matcher := &rule.CachedMatcher{Rules: map[string]rule.Rule{}} rh := proxy.NewRequestHandler( nil, @@ -50,14 +50,18 @@ func TestProxy(t *testing.T) { defer ts.Close() ruleNoOpAuthenticator := rule.Rule{ - Match: rule.RuleMatch{Methods: []string{"GET"}, URL: ts.URL + "/authn-noop/<[0-9]+>"}, - Authenticators: []rule.RuleHandler{{Handler: "noop"}}, - Upstream: rule.Upstream{URL: ""}, + Match: rule.RuleMatch{Methods: []string{"GET"}, URL: ts.URL + "/authn-noop/<[0-9]+>"}, + Authenticators: []rule.RuleHandler{{Handler: "noop"}}, + Authorizer: rule.RuleHandler{Handler: proxy.NewAuthorizerAllow().GetID()}, + CredentialsIssuer: rule.RuleHandler{Handler: proxy.NewCredentialsIssuerNoOp().GetID()}, + Upstream: rule.Upstream{URL: ""}, } ruleNoOpAuthenticatorModifyUpstream := rule.Rule{ - Match: rule.RuleMatch{Methods: []string{"GET"}, URL: ts.URL + "/strip-path/authn-noop/<[0-9]+>"}, - Authenticators: []rule.RuleHandler{{Handler: "noop"}}, - Upstream: rule.Upstream{URL: "", StripPath: "/strip-path/", PreserveHost: true}, + Match: rule.RuleMatch{Methods: []string{"GET"}, URL: ts.URL + "/strip-path/authn-noop/<[0-9]+>"}, + Authenticators: []rule.RuleHandler{{Handler: "noop"}}, + Authorizer: rule.RuleHandler{Handler: proxy.NewAuthorizerAllow().GetID()}, + CredentialsIssuer: rule.RuleHandler{Handler: proxy.NewCredentialsIssuerNoOp().GetID()}, + Upstream: rule.Upstream{URL: "", StripPath: "/strip-path/", PreserveHost: true}, } for k, tc := range []struct { diff --git a/proxy/authenticator_noop.go b/proxy/authenticator_noop.go index 425c386c18..8aab49e461 100644 --- a/proxy/authenticator_noop.go +++ b/proxy/authenticator_noop.go @@ -5,7 +5,6 @@ import ( "net/http" "github.com/ory/oathkeeper/rule" - "github.com/pkg/errors" ) type AuthenticatorNoOp struct{} @@ -19,5 +18,5 @@ func (a *AuthenticatorNoOp) GetID() string { } func (a *AuthenticatorNoOp) Authenticate(r *http.Request, config json.RawMessage, rl *rule.Rule) (*AuthenticationSession, error) { - return nil, errors.WithStack(ErrAuthenticatorBypassed) + return &AuthenticationSession{Subject: ""}, nil } diff --git a/proxy/authenticator_noop_test.go b/proxy/authenticator_noop_test.go index ae51a8a48d..f57d535844 100644 --- a/proxy/authenticator_noop_test.go +++ b/proxy/authenticator_noop_test.go @@ -32,5 +32,5 @@ func TestAuthenticatorNoop(t *testing.T) { assert.NotEmpty(t, NewAuthenticatorNoOp().GetID()) _, err := NewAuthenticatorNoOp().Authenticate(nil, nil, nil) - require.Error(t, err) + require.NoError(t, err) } diff --git a/proxy/proxy_test.go b/proxy/proxy_test.go index 4fa67ae4c9..f6fee64c4c 100644 --- a/proxy/proxy_test.go +++ b/proxy/proxy_test.go @@ -87,14 +87,18 @@ func TestProxy(t *testing.T) { defer proxy.Close() ruleNoOpAuthenticator := rule.Rule{ - Match: rule.RuleMatch{Methods: []string{"GET"}, URL: proxy.URL + "/authn-noop/<[0-9]+>"}, - Authenticators: []rule.RuleHandler{{Handler: "noop"}}, - Upstream: rule.Upstream{URL: backend.URL}, + Match: rule.RuleMatch{Methods: []string{"GET"}, URL: proxy.URL + "/authn-noop/<[0-9]+>"}, + Authenticators: []rule.RuleHandler{{Handler: "noop"}}, + Authorizer: rule.RuleHandler{Handler: "allow"}, + CredentialsIssuer: rule.RuleHandler{Handler: "noop"}, + Upstream: rule.Upstream{URL: backend.URL}, } ruleNoOpAuthenticatorModifyUpstream := rule.Rule{ - Match: rule.RuleMatch{Methods: []string{"GET"}, URL: proxy.URL + "/strip-path/authn-noop/<[0-9]+>"}, - Authenticators: []rule.RuleHandler{{Handler: "noop"}}, - Upstream: rule.Upstream{URL: backend.URL, StripPath: "/strip-path/", PreserveHost: true}, + Match: rule.RuleMatch{Methods: []string{"GET"}, URL: proxy.URL + "/strip-path/authn-noop/<[0-9]+>"}, + Authenticators: []rule.RuleHandler{{Handler: "noop"}}, + Authorizer: rule.RuleHandler{Handler: "allow"}, + CredentialsIssuer: rule.RuleHandler{Handler: "noop"}, + Upstream: rule.Upstream{URL: backend.URL, StripPath: "/strip-path/", PreserveHost: true}, } //acceptRuleStripHost := rule.Rule{MatchesMethods: []string{"GET"}, MatchesURLCompiled: mustCompileRegex(t, proxy.URL+"/users/<[0-9]+>"), Mode: "pass_through_accept", Upstream: rule.Upstream{URLParsed: u, StripPath: "/users/", PreserveHost: true}} diff --git a/proxy/request_handler.go b/proxy/request_handler.go index 47e4e88406..8522a9f9b9 100644 --- a/proxy/request_handler.go +++ b/proxy/request_handler.go @@ -102,10 +102,10 @@ func (d *RequestHandler) HandleRequest(r *http.Request, rl *rule.Rule) error { case ErrAuthenticatorNotResponsible.Error(): // The authentication handler is not responsible for handling this request, skip to the next handler break - case ErrAuthenticatorBypassed.Error(): - // The authentication handler says that no further authentication/authorization is required, and the request should - // be forwarded to its final destination. - return nil + //case ErrAuthenticatorBypassed.Error(): + // The authentication handler says that no further authentication/authorization is required, and the request should + // be forwarded to its final destination. + //return nil default: d.Logger.WithError(err). WithField("granted", false). diff --git a/proxy/request_handler_test.go b/proxy/request_handler_test.go index c466566f8a..79eaf2dfc1 100644 --- a/proxy/request_handler_test.go +++ b/proxy/request_handler_test.go @@ -73,7 +73,7 @@ func TestRequestHandler(t *testing.T) { { expectErr: true, r: newTestRequest(t, "http://localhost"), - j: NewRequestHandler(nil, []Authenticator{NewAuthenticatorNoOp()}, []Authorizer{}, []CredentialsIssuer{}), + j: NewRequestHandler(nil, []Authenticator{NewAuthenticatorNoOp()}, []Authorizer{NewAuthorizerAllow()}, []CredentialsIssuer{NewCredentialsIssuerNoOp()}), rule: rule.Rule{ Authenticators: []rule.RuleHandler{}, Authorizer: rule.RuleHandler{}, @@ -83,11 +83,11 @@ func TestRequestHandler(t *testing.T) { { expectErr: false, r: newTestRequest(t, "http://localhost"), - j: NewRequestHandler(nil, []Authenticator{NewAuthenticatorNoOp()}, []Authorizer{}, []CredentialsIssuer{}), + j: NewRequestHandler(nil, []Authenticator{NewAuthenticatorNoOp()}, []Authorizer{NewAuthorizerAllow()}, []CredentialsIssuer{NewCredentialsIssuerNoOp()}), rule: rule.Rule{ Authenticators: []rule.RuleHandler{{Handler: NewAuthenticatorNoOp().GetID()}}, - Authorizer: rule.RuleHandler{}, - CredentialsIssuer: rule.RuleHandler{}, + Authorizer: rule.RuleHandler{Handler: NewAuthorizerAllow().GetID()}, + CredentialsIssuer: rule.RuleHandler{Handler: NewCredentialsIssuerNoOp().GetID()}, }, }, {