Skip to content

Commit

Permalink
fix(handlers): send status 303 auth requests that are not get/head (#…
Browse files Browse the repository at this point in the history
…2184)

When a request occurs, if the browser is not performing a HTTP GET/HEAD request, the 302 status code is not valid. This commit resolves this. MDN: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/302.
  • Loading branch information
james-d-elliott committed Jul 16, 2021
1 parent 596346d commit ddeb46b
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 2 deletions.
11 changes: 9 additions & 2 deletions internal/handlers/handler_verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,15 @@ func handleUnauthorized(ctx *middlewares.AutheliaCtx, targetURL fmt.Stringer, is
}

ctx.Logger.Infof("Access to %s (method %s) is not authorized to user %s, redirecting to %s", targetURL.String(), friendlyMethod, friendlyUsername, redirectionURL)
ctx.Redirect(redirectionURL, 302)
ctx.SetBodyString(fmt.Sprintf("Found. Redirecting to %s", redirectionURL))

switch rm {
case fasthttp.MethodGet, fasthttp.MethodHead, "":
ctx.Redirect(redirectionURL, 302)
ctx.SetBodyString(fmt.Sprintf("Found. Redirecting to %s", redirectionURL))
default:
ctx.Redirect(redirectionURL, 303)
ctx.SetBodyString(fmt.Sprintf("See Other. Redirecting to %s", redirectionURL))
}
} else {
ctx.Logger.Infof("Access to %s (method %s) is not authorized to user %s, sending 401 response", targetURL.String(), friendlyMethod, friendlyUsername)
ctx.ReplyUnauthorized()
Expand Down
25 changes: 25 additions & 0 deletions internal/handlers/handler_verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,31 @@ func TestShouldRedirectWhenSessionInactiveForTooLongAndRDParamProvided(t *testin
assert.Equal(t, clock.Now().Unix(), newUserSession.LastActivity)
}

func TestShouldRedirectWithCorrectStatusCodeBasedOnRequestMethod(t *testing.T) {
mock := mocks.NewMockAutheliaCtx(t)
defer mock.Close()

mock.Ctx.QueryArgs().Add("rd", "https://login.example.com")
mock.Ctx.Request.Header.Set("X-Original-URL", "https://two-factor.example.com")
mock.Ctx.Request.Header.Set("X-Forwarded-Method", "GET")

VerifyGet(verifyGetCfg)(mock.Ctx)

assert.Equal(t, "Found. Redirecting to https://login.example.com?rd=https%3A%2F%2Ftwo-factor.example.com&rm=GET",
string(mock.Ctx.Response.Body()))
assert.Equal(t, 302, mock.Ctx.Response.StatusCode())

mock.Ctx.QueryArgs().Add("rd", "https://login.example.com")
mock.Ctx.Request.Header.Set("X-Original-URL", "https://two-factor.example.com")
mock.Ctx.Request.Header.Set("X-Forwarded-Method", "POST")

VerifyGet(verifyGetCfg)(mock.Ctx)

assert.Equal(t, "See Other. Redirecting to https://login.example.com?rd=https%3A%2F%2Ftwo-factor.example.com&rm=POST",
string(mock.Ctx.Response.Body()))
assert.Equal(t, 303, mock.Ctx.Response.StatusCode())
}

func TestShouldUpdateInactivityTimestampEvenWhenHittingForbiddenResources(t *testing.T) {
mock := mocks.NewMockAutheliaCtx(t)
defer mock.Close()
Expand Down
30 changes: 30 additions & 0 deletions internal/suites/suite_standalone_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,36 @@ func (s *StandaloneSuite) TestShouldRespectMethodsACL() {
s.Assert().Equal(res.StatusCode, 200)
}

func (s *StandaloneSuite) TestShouldRespondWithCorrectStatusCode() {
req, err := http.NewRequest("GET", fmt.Sprintf("%s/api/verify?rd=%s", AutheliaBaseURL, GetLoginBaseURL()), nil)
s.Assert().NoError(err)
req.Header.Set("X-Forwarded-Method", "GET")
req.Header.Set("X-Forwarded-Proto", "https")
req.Header.Set("X-Forwarded-Host", fmt.Sprintf("secure.%s", BaseDomain))
req.Header.Set("X-Forwarded-URI", "/")

client := NewHTTPClient()
res, err := client.Do(req)
s.Assert().NoError(err)
s.Assert().Equal(res.StatusCode, 302)
body, err := ioutil.ReadAll(res.Body)
s.Assert().NoError(err)

urlEncodedAdminURL := url.QueryEscape(SecureBaseURL + "/")
s.Assert().Equal(fmt.Sprintf("Found. Redirecting to %s?rd=%s&rm=GET", GetLoginBaseURL(), urlEncodedAdminURL), string(body))

req.Header.Set("X-Forwarded-Method", "POST")

res, err = client.Do(req)
s.Assert().NoError(err)
s.Assert().Equal(res.StatusCode, 303)
body, err = ioutil.ReadAll(res.Body)
s.Assert().NoError(err)

urlEncodedAdminURL = url.QueryEscape(SecureBaseURL + "/")
s.Assert().Equal(fmt.Sprintf("See Other. Redirecting to %s?rd=%s&rm=POST", GetLoginBaseURL(), urlEncodedAdminURL), string(body))
}

// Standard case using nginx.
func (s *StandaloneSuite) TestShouldVerifyAPIVerifyUnauthorize() {
req, err := http.NewRequest("GET", fmt.Sprintf("%s/api/verify", AutheliaBaseURL), nil)
Expand Down

0 comments on commit ddeb46b

Please sign in to comment.