Skip to content

Commit

Permalink
Prepare for merge
Browse files Browse the repository at this point in the history
- Split long lines on multiple lines
- Remove unnecessary checks
- Move schema to new location in `spec` directory
- Replace ioutil with io packages
- Add manual close to the original request body stream.
  • Loading branch information
vladr11 committed Sep 29, 2022
1 parent 854e659 commit 53ead8f
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 17 deletions.
File renamed without changes.
43 changes: 27 additions & 16 deletions pipeline/authn/authenticator_remote_json.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"encoding/json"
"io"
"io/ioutil"
"net/http"
"net/url"

Expand Down Expand Up @@ -67,14 +66,6 @@ func (a *AuthenticatorRemoteJSON) Config(config json.RawMessage) (*Authenticator
return nil, NewErrAuthenticatorMisconfigured(a, err)

Check warning on line 66 in pipeline/authn/authenticator_remote_json.go

View check run for this annotation

Codecov / codecov/patch

pipeline/authn/authenticator_remote_json.go#L66

Added line #L66 was not covered by tests
}

if len(c.ExtraFrom) == 0 {
c.ExtraFrom = "extra"
}

if len(c.SubjectFrom) == 0 {
c.SubjectFrom = "subject"
}

return &c, nil
}

Expand All @@ -100,11 +91,19 @@ func (a *AuthenticatorRemoteJSON) Authenticate(r *http.Request, session *Authent
)

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", cfg.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)

Check warning on line 98 in pipeline/authn/authenticator_remote_json.go

View check run for this annotation

Codecov / codecov/patch

pipeline/authn/authenticator_remote_json.go#L94-L98

Added lines #L94 - L98 were not covered by tests
}

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", cfg.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)

Check warning on line 106 in pipeline/authn/authenticator_remote_json.go

View check run for this annotation

Codecov / codecov/patch

pipeline/authn/authenticator_remote_json.go#L102-L106

Added lines #L102 - L106 were not covered by tests
}

session.Subject = subject
Expand All @@ -127,7 +126,10 @@ func forwardMethod(r *http.Request, cfg *AuthenticatorRemoteJSONConfiguration) s
func forwardRequestToAuthenticator(r *http.Request, method string, serviceURL string, preservePath bool) (json.RawMessage, error) {
reqUrl, err := url.Parse(serviceURL)
if err != nil {
return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to parse remote URL: %s", err))
return nil, errors.WithStack(
herodot.
ErrInternalServerError.WithReasonf("Unable to parse remote URL: %s", err),
)

Check warning on line 132 in pipeline/authn/authenticator_remote_json.go

View check run for this annotation

Codecov / codecov/patch

pipeline/authn/authenticator_remote_json.go#L129-L132

Added lines #L129 - L132 were not covered by tests
}

if !preservePath {
Expand All @@ -136,15 +138,24 @@ func forwardRequestToAuthenticator(r *http.Request, method string, serviceURL st

var forwardRequestBody io.ReadCloser = nil
if r.Body != nil {
body, err := ioutil.ReadAll(r.Body)
body, err := io.ReadAll(r.Body)
if err != nil {
return nil, helper.ErrBadRequest.WithReason(err.Error()).WithTrace(err)

Check warning on line 143 in pipeline/authn/authenticator_remote_json.go

View check run for this annotation

Codecov / codecov/patch

pipeline/authn/authenticator_remote_json.go#L143

Added line #L143 was not covered by tests
}

err = r.Body.Close()
if err != nil {
return nil, errors.WithStack(
herodot.
ErrInternalServerError.
WithReasonf("Could not close body reader: %s\n", err),
)

Check warning on line 152 in pipeline/authn/authenticator_remote_json.go

View check run for this annotation

Codecov / codecov/patch

pipeline/authn/authenticator_remote_json.go#L148-L152

Added lines #L148 - L152 were not covered by tests
}

// Unfortunately the body reader needs to be read once to forward the request,
// thus the upstream request will fail miserably without recreating a fresh ReaderCloser
forwardRequestBody = ioutil.NopCloser(bytes.NewReader(body))
r.Body = ioutil.NopCloser(bytes.NewReader(body))
forwardRequestBody = io.NopCloser(bytes.NewReader(body))
r.Body = io.NopCloser(bytes.NewReader(body))
}

req := http.Request{
Expand All @@ -163,7 +174,7 @@ func forwardRequestToAuthenticator(r *http.Request, method string, serviceURL st

func handleResponse(r *http.Response) (json.RawMessage, error) {
if r.StatusCode == http.StatusOK {
body, err := ioutil.ReadAll(r.Body)
body, err := io.ReadAll(r.Body)
if err != nil {
return json.RawMessage{}, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Remote server returned error: %+v", err))

Check warning on line 179 in pipeline/authn/authenticator_remote_json.go

View check run for this annotation

Codecov / codecov/patch

pipeline/authn/authenticator_remote_json.go#L179

Added line #L179 was not covered by tests
}
Expand Down
2 changes: 1 addition & 1 deletion pipeline/authn/authenticator_remote_json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestAuthenticatorRemoteJSON(t *testing.T) {
err := pipelineAuthenticator.Authenticate(
makeRemoteJSONRequest("GET", "/", map[string]string{"sessionid": "zyx"}, ""),
session,
json.RawMessage(fmt.Sprintf(`{"check_session_url": "%s"}`, testServer.URL)),
json.RawMessage(fmt.Sprintf(`{"service_url": "%s"}`, testServer.URL)),
nil,
)
require.Error(t, err, "%#v", errors.Cause(err))
Expand Down

0 comments on commit 53ead8f

Please sign in to comment.