From 2a3e0a4828ca2cccf5363f322607cfd018a3249a Mon Sep 17 00:00:00 2001 From: robertotting Date: Tue, 30 Aug 2022 11:34:09 -0500 Subject: [PATCH 1/2] feat(app): Updated the request handler to skip other parts of the pipeline when the noop authenticator is ran. --- api/decision_test.go | 19 ++++++++++++ proxy/request_handler.go | 13 +++++++++ proxy/request_handler_test.go | 54 +++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+) diff --git a/api/decision_test.go b/api/decision_test.go index d4c6d07236..eea55bea5f 100644 --- a/api/decision_test.go +++ b/api/decision_test.go @@ -51,6 +51,14 @@ func TestDecisionAPI(t *testing.T) { ts := httptest.NewServer(n) defer ts.Close() + ruleNoOpAuthenticatorWithMutator := rule.Rule{ + Match: &rule.Match{Methods: []string{"GET"}, URL: ts.URL + "/authn-noop-mutator/<[0-9]+>"}, + Authenticators: []rule.Handler{{Handler: "noop"}}, + Authorizer: rule.Handler{Handler: "allow"}, + Mutators: []rule.Handler{{Handler: "id_token"}}, + Upstream: rule.Upstream{URL: ""}, + } + ruleNoOpAuthenticator := rule.Rule{ Match: &rule.Match{Methods: []string{"GET"}, URL: ts.URL + "/authn-noop/<[0-9]+>"}, Authenticators: []rule.Handler{{Handler: "noop"}}, @@ -105,6 +113,17 @@ func TestDecisionAPI(t *testing.T) { rulesGlob: []rule.Rule{ruleNoOpAuthenticatorGLOB, ruleNoOpAuthenticatorGLOB}, code: http.StatusInternalServerError, }, + { + d: "should pass and skip mutator with noop authenticator", + url: ts.URL + "/decisions" + "/authn-noop-mutator/1234", + rulesRegexp: []rule.Rule{ruleNoOpAuthenticatorWithMutator}, + rulesGlob: []rule.Rule{ruleNoOpAuthenticatorWithMutator}, + code: http.StatusOK, + transform: func(r *http.Request) { + r.Header.Add("Authorization", "basic user:pass") + }, + authz: "basic user:pass", + }, { d: "should pass", url: ts.URL + "/decisions" + "/authn-noop/1234", diff --git a/proxy/request_handler.go b/proxy/request_handler.go index 7c2aaae9f3..9abab5594f 100644 --- a/proxy/request_handler.go +++ b/proxy/request_handler.go @@ -180,6 +180,7 @@ func (d *requestHandler) HandleRequest(r *http.Request, rl *rule.Rule) (session return nil, err } + useNoopAuthenticator := false for _, a := range rl.Authenticators { anh, err := d.r.PipelineAuthenticator(a.Handler) if err != nil { @@ -225,6 +226,10 @@ func (d *requestHandler) HandleRequest(r *http.Request, rl *rule.Rule) (session return nil, err } } else { + // If we hit the no-op authenticator then make a note of it + if anh.GetID() == "noop" { + useNoopAuthenticator = true + } // The first authenticator that matches must return the session found = true fields["subject"] = session.Subject @@ -242,6 +247,14 @@ func (d *requestHandler) HandleRequest(r *http.Request, rl *rule.Rule) (session return nil, err } + if useNoopAuthenticator { + // This is essentially what the noop mutator does so if we choose to + // use noop authentication and skip other parts of the pipeline then + // we need to set the session from the request headers + session.Header = r.Header + return session, nil + } + azh, err := d.r.PipelineAuthorizer(rl.Authorizer.Handler) if err != nil { d.r.Logger().WithError(err). diff --git a/proxy/request_handler_test.go b/proxy/request_handler_test.go index 4fe5a0ea6f..9c55e7ba74 100644 --- a/proxy/request_handler_test.go +++ b/proxy/request_handler_test.go @@ -32,6 +32,12 @@ func newTestRequest(u string) *http.Request { return &http.Request{URL: x.ParseURLOrPanic(u), Method: "GET", Header: TestHeader} } +func newTestRequestWithBasicAuth(u string) *http.Request { + headers := TestHeader.Clone() + headers.Add("Authorization", "Basic dXNlcm5hbWU6c2VjcmV0Cg==") + return &http.Request{URL: x.ParseURLOrPanic(u), Method: "GET", Header: headers} +} + func TestHandleError(t *testing.T) { for k, tc := range []struct { d string @@ -443,6 +449,54 @@ func TestRequestHandler(t *testing.T) { } } +func TestRequestHandlerWithNoopAuthorizer(t *testing.T) { + for k, tc := range []struct { + d string + setup func() + rule rule.Rule + r *http.Request + expectErr bool + }{ + { + d: "should skip other authorizers / mutators when noop is enabled", + setup: func() { + viper.Set(configuration.ViperKeyAuthenticatorNoopIsEnabled, true) + viper.Set(configuration.ViperKeyAuthorizerAllowIsEnabled, false) + viper.Set(configuration.ViperKeyMutatorNoopIsEnabled, false) + }, + // TODO: Test for Basic Auth header to come through... + r: newTestRequestWithBasicAuth("http://localhost"), + rule: rule.Rule{ + Authenticators: []rule.Handler{{Handler: "noop"}}, + Authorizer: rule.Handler{Handler: "allow"}, + Mutators: []rule.Handler{{Handler: "invalid-id"}}, + }, + }, + // TODO: Could we create a test that asserts if a given handler was NOT evaluated + // not sure based on the current setup... + } { + t.Run(fmt.Sprintf("case=%d/description=%s", k, tc.d), func(t *testing.T) { + + conf := internal.NewConfigurationWithDefaults() + reg := internal.NewRegistry(conf) + + if tc.setup != nil { + tc.setup() + } + + session, err := reg.ProxyRequestHandler().HandleRequest(tc.r, &tc.rule) + t.Logf("Found the session: %+v", session) + user, pass, ok := tc.r.BasicAuth() + t.Logf("Request headers: %+v, %s:%s %v", tc.r.Header, user, pass, ok) + if tc.expectErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} + func TestInitializeSession(t *testing.T) { for k, tc := range []struct { d string From 287ea3a7ad447d9659ebb3da8eaa519329b28256 Mon Sep 17 00:00:00 2001 From: Matthew Ellison Date: Thu, 23 Mar 2023 09:18:47 -0400 Subject: [PATCH 2/2] Fixup Test for ruleNoOpAuthenticatorWithMutator to with GLOB --- api/decision_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/api/decision_test.go b/api/decision_test.go index eea55bea5f..aeb85e3d8a 100644 --- a/api/decision_test.go +++ b/api/decision_test.go @@ -74,6 +74,13 @@ func TestDecisionAPI(t *testing.T) { Upstream: rule.Upstream{URL: "", StripPath: "/strip-path/", PreserveHost: true}, } + ruleNoOpAuthenticatorWithMutatorGLOB := rule.Rule{ + Match: &rule.Match{Methods: []string{"GET"}, URL: ts.URL + "/authn-noop-mutator/<[0-9]*>"}, + Authenticators: []rule.Handler{{Handler: "noop"}}, + Authorizer: rule.Handler{Handler: "allow"}, + Mutators: []rule.Handler{{Handler: "id_token"}}, + Upstream: rule.Upstream{URL: ""}, + } ruleNoOpAuthenticatorGLOB := rule.Rule{ Match: &rule.Match{Methods: []string{"GET"}, URL: ts.URL + "/authn-noop/<[0-9]*>"}, Authenticators: []rule.Handler{{Handler: "noop"}}, @@ -117,7 +124,7 @@ func TestDecisionAPI(t *testing.T) { d: "should pass and skip mutator with noop authenticator", url: ts.URL + "/decisions" + "/authn-noop-mutator/1234", rulesRegexp: []rule.Rule{ruleNoOpAuthenticatorWithMutator}, - rulesGlob: []rule.Rule{ruleNoOpAuthenticatorWithMutator}, + rulesGlob: []rule.Rule{ruleNoOpAuthenticatorWithMutatorGLOB}, code: http.StatusOK, transform: func(r *http.Request) { r.Header.Add("Authorization", "basic user:pass")