From 854e659e92a1f2dd1e68531d996e8602412d3f9c Mon Sep 17 00:00:00 2001 From: Vlad Rusu Date: Tue, 14 Jun 2022 16:37:40 +0300 Subject: [PATCH] fix: change behaviour of auth remote_json to send POST request by default to the auth service --- .schemas/authenticators.forward.schema.json | 8 ++--- pipeline/authn/authenticator_remote_json.go | 36 +++++++++++-------- .../authn/authenticator_remote_json_test.go | 25 +++++++++++-- spec/config.schema.json | 26 +++++++------- 4 files changed, 58 insertions(+), 37 deletions(-) diff --git a/.schemas/authenticators.forward.schema.json b/.schemas/authenticators.forward.schema.json index 6451b8cad5..a3ccdb1476 100644 --- a/.schemas/authenticators.forward.schema.json +++ b/.schemas/authenticators.forward.schema.json @@ -10,9 +10,7 @@ "type": "string", "format": "uri", "description": "The origin to proxy requests to. If the response is a 200 with body `{ \"subject\": \"...\", \"extra\": {} }`. The request will pass the subject through successfully, otherwise it will be marked as unauthorized.\n\n>If this authenticator is enabled, this value is required.", - "examples": [ - "https://service-host" - ] + "examples": ["https://service-host"] }, "preserve_path": { "title": "Preserve Path", @@ -20,8 +18,6 @@ "description": "When set to true, any path specified in `check_session_url` will be preserved instead of overwriting the path with the path from the original request" } }, - "required": [ - "service_url" - ], + "required": ["service_url"], "additionalProperties": false } diff --git a/pipeline/authn/authenticator_remote_json.go b/pipeline/authn/authenticator_remote_json.go index 6b9b7bf534..5206e8640c 100644 --- a/pipeline/authn/authenticator_remote_json.go +++ b/pipeline/authn/authenticator_remote_json.go @@ -30,11 +30,12 @@ type AuthenticatorRemoteJSONFilter struct { } type AuthenticatorRemoteJSONConfiguration struct { - ServiceURL string `json:"service_url"` - PreservePath bool `json:"preserve_path"` - ExtraFrom string `json:"extra_from"` - SubjectFrom string `json:"subject_from"` - Method string `json:"method"` + ServiceURL string `json:"service_url"` + PreservePath bool `json:"preserve_path"` + ExtraFrom string `json:"extra_from"` + SubjectFrom string `json:"subject_from"` + Method string `json:"method"` + UseOriginalMethod bool `json:"use_original_method"` } type AuthenticatorRemoteJSON struct { @@ -78,14 +79,14 @@ func (a *AuthenticatorRemoteJSON) Config(config json.RawMessage) (*Authenticator } func (a *AuthenticatorRemoteJSON) Authenticate(r *http.Request, session *AuthenticationSession, config json.RawMessage, _ pipeline.Rule) error { - cf, err := a.Config(config) + cfg, err := a.Config(config) if err != nil { return err } - method := forwardMethod(r, cf.Method) + method := forwardMethod(r, cfg) - body, err := forwardRequestToAuthenticator(r, method, cf.ServiceURL, cf.PreservePath) + body, err := forwardRequestToAuthenticator(r, method, cfg.ServiceURL, cfg.PreservePath) if err != nil { return err } @@ -94,16 +95,16 @@ func (a *AuthenticatorRemoteJSON) Authenticate(r *http.Request, session *Authent subject string extra map[string]interface{} - subjectRaw = []byte(stringsx.Coalesce(gjson.GetBytes(body, cf.SubjectFrom).Raw, "null")) - extraRaw = []byte(stringsx.Coalesce(gjson.GetBytes(body, cf.ExtraFrom).Raw, "null")) + subjectRaw = []byte(stringsx.Coalesce(gjson.GetBytes(body, cfg.SubjectFrom).Raw, "null")) + extraRaw = []byte(stringsx.Coalesce(gjson.GetBytes(body, cfg.ExtraFrom).Raw, "null")) ) if err = json.Unmarshal(subjectRaw, &subject); err != nil { - return helper.ErrForbidden.WithReasonf("The configured subject_from GJSON path returned an error on JSON output: %s", err.Error()).WithDebugf("GJSON path: %s\nBody: %s\nResult: %s", cf.SubjectFrom, body, subjectRaw).WithTrace(err) + return helper.ErrForbidden.WithReasonf("The configured subject_from GJSON path returned an error on JSON output: %s", err.Error()).WithDebugf("GJSON path: %s\nBody: %s\nResult: %s", cfg.SubjectFrom, body, subjectRaw).WithTrace(err) } if err = json.Unmarshal(extraRaw, &extra); err != nil { - return helper.ErrForbidden.WithReasonf("The configured extra_from GJSON path returned an error on JSON output: %s", err.Error()).WithDebugf("GJSON path: %s\nBody: %s\nResult: %s", cf.ExtraFrom, body, extraRaw).WithTrace(err) + return helper.ErrForbidden.WithReasonf("The configured extra_from GJSON path returned an error on JSON output: %s", err.Error()).WithDebugf("GJSON path: %s\nBody: %s\nResult: %s", cfg.ExtraFrom, body, extraRaw).WithTrace(err) } session.Subject = subject @@ -111,11 +112,16 @@ func (a *AuthenticatorRemoteJSON) Authenticate(r *http.Request, session *Authent return nil } -func forwardMethod(r *http.Request, method string) string { +func forwardMethod(r *http.Request, cfg *AuthenticatorRemoteJSONConfiguration) string { + method := cfg.Method if len(method) == 0 { - return r.Method + if cfg.UseOriginalMethod { + return r.Method + } else { + return http.MethodPost + } } - return method + return cfg.Method } func forwardRequestToAuthenticator(r *http.Request, method string, serviceURL string, preservePath bool) (json.RawMessage, error) { diff --git a/pipeline/authn/authenticator_remote_json_test.go b/pipeline/authn/authenticator_remote_json_test.go index 0b66b6c6f7..185a096907 100644 --- a/pipeline/authn/authenticator_remote_json_test.go +++ b/pipeline/authn/authenticator_remote_json_test.go @@ -63,7 +63,7 @@ func TestAuthenticatorRemoteJSON(t *testing.T) { err := pipelineAuthenticator.Authenticate( makeRemoteJSONRequest("PUT", "/users/123?query=string", map[string]string{"sessionid": "zyx"}, "Test body"), session, - json.RawMessage(fmt.Sprintf(`{"service_url": "%s"}`, testServer.URL)), + json.RawMessage(fmt.Sprintf(`{"service_url": "%s", "use_original_method": true}`, testServer.URL)), nil, ) require.NoError(t, err, "%#v", errors.Cause(err)) @@ -78,6 +78,27 @@ func TestAuthenticatorRemoteJSON(t *testing.T) { assert.Equal(t, &AuthenticationSession{Subject: "123"}, session) }) + t.Run("description=should pass through path, headers, and body to auth server, and use POST method", func(t *testing.T) { + testServer, requestRecorder := makeServiceServer(200, `{"subject": "123"}`) + defer testServer.Close() + err := pipelineAuthenticator.Authenticate( + makeRemoteJSONRequest("PUT", "/users/123?query=string", map[string]string{"sessionid": "zyx"}, "Test body"), + session, + json.RawMessage(fmt.Sprintf(`{"service_url": "%s"}`, testServer.URL)), + nil, + ) + require.NoError(t, err, "%#v", errors.Cause(err)) + assert.Len(t, requestRecorder.requests, 1) + + r := requestRecorder.requests[0] + + assert.Equal(t, r.Method, "POST") + assert.Equal(t, r.URL.Path, "/users/123?query=string") + assert.Equal(t, r.Header.Get("sessionid"), "zyx") + assert.Equal(t, requestRecorder.bodies[0], []byte("Test body")) + assert.Equal(t, &AuthenticationSession{Subject: "123"}, session) + }) + t.Run("description=should pass through path, headers, and body and use custom method to auth server", func(t *testing.T) { testServer, requestRecorder := makeServiceServer(200, `{"subject": "123"}`) defer testServer.Close() @@ -103,7 +124,7 @@ func TestAuthenticatorRemoteJSON(t *testing.T) { err := pipelineAuthenticator.Authenticate( makeRemoteJSONRequest("PUT", "/users/123?query=string", map[string]string{"sessionid": "zyx"}, ""), session, - json.RawMessage(fmt.Sprintf(`{"service_url": "%s", "preserve_path": true}`, testServer.URL)), + json.RawMessage(fmt.Sprintf(`{"service_url": "%s", "preserve_path": true, "use_original_method": true}`, testServer.URL)), nil, ) require.NoError(t, err, "%#v", errors.Cause(err)) diff --git a/spec/config.schema.json b/spec/config.schema.json index 855fef0f94..26e110f8c7 100644 --- a/spec/config.schema.json +++ b/spec/config.schema.json @@ -439,9 +439,7 @@ "type": "string", "format": "uri", "description": "The origin to proxy requests to. If the response is a 200 with body `{ \"subject\": \"...\", \"extra\": {} }`. The request will pass the subject through successfully, otherwise it will be marked as unauthorized.\n\n>If this authenticator is enabled, this value is required.", - "examples": [ - "https://service-host" - ] + "examples": ["https://service-host"] }, "preserve_path": { "title": "Preserve Path", @@ -462,16 +460,18 @@ }, "method": { "title": "Method to pass to the auth server", - "description": "The method to pass to the auth server. Defaults to the original request method.", + "description": "The method to pass to the auth server. Defaults to the original request method or to POST (see `use_original_method`).", "type": "string", - "examples": [ - "GET" - ] + "examples": ["GET"] + }, + "use_original_method": { + "title": "Use Original Method", + "description": "When set to true, the original request method is sent to the authentication service. If set to false, the POST method is used as a fallback. This flag is ignored when the `method` field is set.", + "type": "boolean", + "default": false } }, - "required": [ - "service_url" - ], + "required": ["service_url"], "additionalProperties": false }, "configAuthenticatorsBearerToken": { @@ -1389,7 +1389,7 @@ }, "remote_json": { "title": "Remote JSON Request", - "description": "The custom forward request authenticator.", + "description": "The [`remote_json` authenticator](https://www.ory.sh/oathkeeper/docs/pipeline/authn#remote_json).", "type": "object", "properties": { "enabled": { @@ -1406,9 +1406,7 @@ "$ref": "#/definitions/configAuthenticatorsRemoteJSON" } }, - "required": [ - "config" - ] + "required": ["config"] }, { "properties": {