Skip to content

Commit

Permalink
fix: change behaviour of auth remote_json to send POST request by def…
Browse files Browse the repository at this point in the history
…ault to the auth service
  • Loading branch information
vladr11 committed Sep 28, 2022
1 parent 1ccc89d commit 854e659
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 37 deletions.
8 changes: 2 additions & 6 deletions .schemas/authenticators.forward.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,14 @@
"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",
"type": "boolean",
"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
}
36 changes: 21 additions & 15 deletions pipeline/authn/authenticator_remote_json.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand All @@ -94,28 +95,33 @@ 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
session.Extra = extra
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) {
Expand Down
25 changes: 23 additions & 2 deletions pipeline/authn/authenticator_remote_json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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()
Expand All @@ -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))
Expand Down
26 changes: 12 additions & 14 deletions spec/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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": {
Expand Down Expand Up @@ -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": {
Expand All @@ -1406,9 +1406,7 @@
"$ref": "#/definitions/configAuthenticatorsRemoteJSON"
}
},
"required": [
"config"
]
"required": ["config"]
},
{
"properties": {
Expand Down

0 comments on commit 854e659

Please sign in to comment.