Skip to content

Commit

Permalink
authz: Change error code from 403 to 401 (#259)
Browse files Browse the repository at this point in the history
Closes #256
  • Loading branch information
ngrigoriev authored and aeneasr committed Sep 23, 2019
1 parent 72a2333 commit c17e564
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 9 deletions.
2 changes: 1 addition & 1 deletion pipeline/authn/authenticator_jwt.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (a *AuthenticatorJWT) Authenticate(r *http.Request, config json.RawMessage,
ScopeStrategy: a.c.ToScopeStrategy(cf.ScopeStrategy, "authenticators.jwt.Config.scope_strategy"),
})
if err != nil {
return nil, helper.ErrForbidden.WithReason(err.Error()).WithTrace(err)
return nil, helper.ErrUnauthorized.WithReason(err.Error()).WithTrace(err)
}

claims, ok := pt.Claims.(jwt.MapClaims)
Expand Down
20 changes: 14 additions & 6 deletions pipeline/authn/authenticator_jwt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (

"github.com/ory/x/urlx"

"github.com/ory/herodot"
"github.com/ory/oathkeeper/internal"
. "github.com/ory/oathkeeper/pipeline/authn"

Expand Down Expand Up @@ -71,6 +72,7 @@ func TestAuthenticatorJWT(t *testing.T) {
r *http.Request
config string
expectErr bool
expectCode int
expectSess *AuthenticationSession
}{
{
Expand Down Expand Up @@ -169,8 +171,9 @@ func TestAuthenticatorJWT(t *testing.T) {
"exp": now.Add(time.Hour).Unix(),
"nbf": now.Add(time.Hour).Unix(),
})}}},
config: `{}`,
expectErr: true,
config: `{}`,
expectErr: true,
expectCode: 401,
},
{
d: "should fail because JWT iat is in future",
Expand All @@ -179,8 +182,9 @@ func TestAuthenticatorJWT(t *testing.T) {
"exp": now.Add(time.Hour).Unix(),
"iat": now.Add(time.Hour).Unix(),
})}}},
config: `{}`,
expectErr: true,
config: `{}`,
expectErr: true,
expectCode: 401,
},
{
d: "should pass because JWT is missing scope",
Expand Down Expand Up @@ -218,8 +222,9 @@ func TestAuthenticatorJWT(t *testing.T) {
"sub": "sub",
"exp": now.Add(-time.Hour).Unix(),
})}}},
config: `{}`,
expectErr: true,
config: `{}`,
expectErr: true,
expectCode: 401,
},
} {
t.Run(fmt.Sprintf("case=%d/description=%s", k, tc.d), func(t *testing.T) {
Expand All @@ -230,6 +235,9 @@ func TestAuthenticatorJWT(t *testing.T) {
tc.config, _ = sjson.Set(tc.config, "jwks_urls", keys)
session, err := a.Authenticate(tc.r, json.RawMessage([]byte(tc.config)), nil)
if tc.expectErr {
if tc.expectCode != 0 {
assert.Equal(t, tc.expectCode, herodot.ToDefaultError(err, "").StatusCode(), "Status code mismatch")
}
require.Error(t, err)
} else {
require.NoError(t, err, "%#v", errors.Cause(err))
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/okclient/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func main() {
}

res, body = requestWithJWT("not.valid.token")
if res.StatusCode != 403 {
if res.StatusCode != 401 {
panic("proxy: expected 401: " + body)
}

Expand All @@ -68,7 +68,7 @@ func main() {
}

res, body = decisionWithJWT("not.valid.token")
if res.StatusCode != 403 {
if res.StatusCode != 401 {
panic("decision: expected 401: " + body)
}
}
Expand Down

0 comments on commit c17e564

Please sign in to comment.