Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/master' into logout-notify-idp
Browse files Browse the repository at this point in the history
  • Loading branch information
hperl committed Aug 14, 2023
2 parents 4f63d4f + dc878b8 commit 0a241c5
Show file tree
Hide file tree
Showing 10 changed files with 396 additions and 428 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,7 @@
* Deflake ttl test ([6741a49](https://github.com/ory/hydra/commit/6741a49f7b4d55a270f3eb968921894b1e5f2989))
* Only query access tokens by hashed signature ([a21e945](https://github.com/ory/hydra/commit/a21e94519416cc7801995b0804696348b18fa844))
* Racy random string generation ([#3555](https://github.com/ory/hydra/issues/3555)) ([1b26c4c](https://github.com/ory/hydra/commit/1b26c4cb96400b333fe214d2da892fc045bbc69f))
* Reject invalid JWKS in client configuration / dependency cleanup and bump ([#3603](https://github.com/ory/hydra/issues/3603)) ([1d73d83](https://github.com/ory/hydra/commit/1d73d83eb03e4ceef6edb4bd0738959007053118))
* Restore ability to override auth and token urls for exemplary app ([#3590](https://github.com/ory/hydra/issues/3590)) ([dfb129a](https://github.com/ory/hydra/commit/dfb129a5b7c8ae01e1c490fce1a127697abc7bee))
* Return proper error when the grant request cannot be parsed ([#3558](https://github.com/ory/hydra/issues/3558)) ([26f2d34](https://github.com/ory/hydra/commit/26f2d34459f55444e880e6e27e081c002d630246))
* Use correct tracer in middleware ([#3567](https://github.com/ory/hydra/issues/3567)) ([807cbd2](https://github.com/ory/hydra/commit/807cbd209af376b9b2d18c278cc927d1c43e6865))
Expand Down
4 changes: 2 additions & 2 deletions client/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ type createOAuth2Client struct {
func (h *Handler) createOAuth2Client(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
c, err := h.CreateClient(r, h.r.ClientValidator().Validate, false)
if err != nil {
h.r.Writer().WriteError(w, r, errorsx.WithStack(err))
h.r.Writer().WriteError(w, r, err)
return
}

Expand Down Expand Up @@ -164,7 +164,7 @@ func (h *Handler) createOidcDynamicClient(w http.ResponseWriter, r *http.Request
func (h *Handler) CreateClient(r *http.Request, validator func(context.Context, *Client) error, isDynamic bool) (*Client, error) {
var c Client
if err := json.NewDecoder(r.Body).Decode(&c); err != nil {
return nil, err
return nil, errorsx.WithStack(herodot.ErrBadRequest.WithReasonf("Unable to decode the request body: %s", err))
}

if isDynamic {
Expand Down
20 changes: 16 additions & 4 deletions client/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ import (
"net/url"
"strings"

"github.com/hashicorp/go-retryablehttp"

"github.com/ory/herodot"
"github.com/ory/hydra/v2/driver/config"
"github.com/ory/hydra/v2/x"
"github.com/ory/x/ipx"

"github.com/ory/x/errorsx"

"github.com/ory/x/ipx"
"github.com/ory/x/stringslice"
)

Expand Down Expand Up @@ -65,6 +65,14 @@ func (v *Validator) Validate(ctx context.Context, c *Client) error {
return errorsx.WithStack(ErrInvalidClientMetadata.WithHint("Fields jwks and jwks_uri can not both be set, you must choose one."))
}

if c.JSONWebKeys != nil && c.JSONWebKeys.JSONWebKeySet != nil {
for _, k := range c.JSONWebKeys.Keys {
if !k.Valid() {
return errorsx.WithStack(ErrInvalidClientMetadata.WithHint("Invalid JSON web key in set."))
}
}
}

if v.r.Config().ClientHTTPNoPrivateIPRanges() {
values := map[string]string{
"jwks_uri": c.JSONWebKeysURI,
Expand Down Expand Up @@ -213,7 +221,11 @@ func (v *Validator) ValidateSectorIdentifierURL(ctx context.Context, location st
return errorsx.WithStack(ErrInvalidClientMetadata.WithDebug("Value sector_identifier_uri must be an HTTPS URL but it is not."))
}

response, err := v.r.HTTPClient(ctx).Get(location)
req, err := retryablehttp.NewRequestWithContext(ctx, "GET", location, nil)
if err != nil {
return errorsx.WithStack(ErrInvalidClientMetadata.WithDebugf("Value sector_identifier_uri must be an HTTPS URL but it is not: %s", err.Error()))
}
response, err := v.r.HTTPClient(ctx).Do(req)
if err != nil {
return errorsx.WithStack(ErrInvalidClientMetadata.WithDebug(fmt.Sprintf("Unable to connect to URL set by sector_identifier_uri: %s", err)))
}
Expand Down
88 changes: 75 additions & 13 deletions client/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@ package client_test

import (
"context"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"strings"
"testing"

"github.com/hashicorp/go-retryablehttp"

"github.com/ory/fosite"
"github.com/ory/hydra/v2/driver"
"github.com/ory/x/httpx"

Expand All @@ -38,11 +41,16 @@ func TestValidate(t *testing.T) {

testCtx := context.TODO()

dec := json.NewDecoder(strings.NewReader(validJWKS))
dec.DisallowUnknownFields()
var goodJWKS jose.JSONWebKeySet
require.NoError(t, dec.Decode(&goodJWKS))

for k, tc := range []struct {
in *Client
check func(t *testing.T, c *Client)
expectErr bool
v func(t *testing.T) *Validator
check func(*testing.T, *Client)
assertErr func(t assert.TestingT, err error, msg ...interface{}) bool
v func(*testing.T) *Validator
}{
{
in: new(Client),
Expand All @@ -66,31 +74,57 @@ func TestValidate(t *testing.T) {
},
{
in: &Client{LegacyClientID: "foo", UserinfoSignedResponseAlg: "foo"},
expectErr: true,
assertErr: assert.Error,
},
{
in: &Client{LegacyClientID: "foo", TokenEndpointAuthMethod: "private_key_jwt"},
expectErr: true,
assertErr: assert.Error,
},
{
in: &Client{LegacyClientID: "foo", JSONWebKeys: &x.JoseJSONWebKeySet{JSONWebKeySet: new(jose.JSONWebKeySet)}, JSONWebKeysURI: "asdf", TokenEndpointAuthMethod: "private_key_jwt"},
expectErr: true,
assertErr: assert.Error,
},
{
in: &Client{LegacyClientID: "foo", JSONWebKeys: &x.JoseJSONWebKeySet{JSONWebKeySet: new(jose.JSONWebKeySet)}, TokenEndpointAuthMethod: "private_key_jwt", TokenEndpointAuthSigningAlgorithm: "HS256"},
expectErr: true,
assertErr: assert.Error,
},
{
in: &Client{LegacyClientID: "foo", JSONWebKeys: &x.JoseJSONWebKeySet{JSONWebKeySet: new(jose.JSONWebKeySet)}, JSONWebKeysURI: "https://example.org/jwks.json"},
assertErr: func(_ assert.TestingT, err error, msg ...interface{}) bool {
e := new(fosite.RFC6749Error)
assert.ErrorAs(t, err, &e)
assert.Contains(t, e.HintField, "jwks and jwks_uri can not both be set")
return true
},
},
{
in: &Client{LegacyClientID: "foo", JSONWebKeys: &x.JoseJSONWebKeySet{JSONWebKeySet: &goodJWKS}},
check: func(t *testing.T, c *Client) {
assert.Len(t, c.JSONWebKeys.Keys, 2)
assert.Equal(t, c.JSONWebKeys.Keys[0].KeyID, "1")
assert.Equal(t, c.JSONWebKeys.Keys[1].KeyID, "2011-04-29")
},
},
{
in: &Client{LegacyClientID: "foo", JSONWebKeys: &x.JoseJSONWebKeySet{JSONWebKeySet: &jose.JSONWebKeySet{Keys: []jose.JSONWebKey{{}}}}},
assertErr: func(_ assert.TestingT, err error, msg ...interface{}) bool {
e := new(fosite.RFC6749Error)
assert.ErrorAs(t, err, &e)
assert.Contains(t, e.HintField, "Invalid JSON web key in set")
return true
},
},
{
in: &Client{LegacyClientID: "foo", PostLogoutRedirectURIs: []string{"https://bar/"}, RedirectURIs: []string{"https://foo/"}},
expectErr: true,
assertErr: assert.Error,
},
{
in: &Client{LegacyClientID: "foo", PostLogoutRedirectURIs: []string{"http://foo/"}, RedirectURIs: []string{"https://foo/"}},
expectErr: true,
assertErr: assert.Error,
},
{
in: &Client{LegacyClientID: "foo", PostLogoutRedirectURIs: []string{"https://foo:1234/"}, RedirectURIs: []string{"https://foo/"}},
expectErr: true,
assertErr: assert.Error,
},
{
in: &Client{LegacyClientID: "foo", PostLogoutRedirectURIs: []string{"https://foo/"}, RedirectURIs: []string{"https://foo/"}},
Expand Down Expand Up @@ -122,18 +156,19 @@ func TestValidate(t *testing.T) {
},
{
in: &Client{LegacyClientID: "foo", SubjectType: "foo"},
expectErr: true,
assertErr: assert.Error,
},
} {
tc := tc
t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
if tc.v == nil {
tc.v = func(t *testing.T) *Validator {
return v
}
}
err := tc.v(t).Validate(testCtx, tc.in)
if tc.expectErr {
require.Error(t, err)
if tc.assertErr != nil {
tc.assertErr(t, err)
} else {
require.NoError(t, err)
tc.check(t, tc.in)
Expand Down Expand Up @@ -205,6 +240,33 @@ func TestValidateSectorIdentifierURL(t *testing.T) {
}
}

// from https://datatracker.ietf.org/doc/html/rfc7517#appendix-A.2
const validJWKS = `
{"keys":
[
{"kty":"EC",
"crv":"P-256",
"x":"MKBCTNIcKUSDii11ySs3526iDZ8AiTo7Tu6KPAqv7D4",
"y":"4Etl6SRW2YiLUrN5vfvVHuhp7x8PxltmWWlbbM4IFyM",
"d":"870MB6gfuTJ4HtUnUvYMyJpr5eUZNP4Bk43bVdj3eAE",
"use":"enc",
"kid":"1"},
{"kty":"RSA",
"n":"0vx7agoebGcQSuuPiLJXZptN9nndrQmbXEps2aiAFbWhM78LhWx4cbbfAAtVT86zwu1RK7aPFFxuhDR1L6tSoc_BJECPebWKRXjBZCiFV4n3oknjhMstn64tZ_2W-5JsGY4Hc5n9yBXArwl93lqt7_RN5w6Cf0h4QyQ5v-65YGjQR0_FDW2QvzqY368QQMicAtaSqzs8KJZgnYb9c7d0zgdAZHzu6qMQvRL5hajrn1n91CbOpbISD08qNLyrdkt-bFTWhAI4vMQFh6WeZu0fM4lFd2NcRwr3XPksINHaQ-G_xBniIqbw0Ls1jF44-csFCur-kEgU8awapJzKnqDKgw",
"e":"AQAB",
"d":"X4cTteJY_gn4FYPsXB8rdXix5vwsg1FLN5E3EaG6RJoVH-HLLKD9M7dx5oo7GURknchnrRweUkC7hT5fJLM0WbFAKNLWY2vv7B6NqXSzUvxT0_YSfqijwp3RTzlBaCxWp4doFk5N2o8Gy_nHNKroADIkJ46pRUohsXywbReAdYaMwFs9tv8d_cPVY3i07a3t8MN6TNwm0dSawm9v47UiCl3Sk5ZiG7xojPLu4sbg1U2jx4IBTNBznbJSzFHK66jT8bgkuqsk0GjskDJk19Z4qwjwbsnn4j2WBii3RL-Us2lGVkY8fkFzme1z0HbIkfz0Y6mqnOYtqc0X4jfcKoAC8Q",
"p":"83i-7IvMGXoMXCskv73TKr8637FiO7Z27zv8oj6pbWUQyLPQBQxtPVnwD20R-60eTDmD2ujnMt5PoqMrm8RfmNhVWDtjjMmCMjOpSXicFHj7XOuVIYQyqVWlWEh6dN36GVZYk93N8Bc9vY41xy8B9RzzOGVQzXvNEvn7O0nVbfs",
"q":"3dfOR9cuYq-0S-mkFLzgItgMEfFzB2q3hWehMuG0oCuqnb3vobLyumqjVZQO1dIrdwgTnCdpYzBcOfW5r370AFXjiWft_NGEiovonizhKpo9VVS78TzFgxkIdrecRezsZ-1kYd_s1qDbxtkDEgfAITAG9LUnADun4vIcb6yelxk",
"dp":"G4sPXkc6Ya9y8oJW9_ILj4xuppu0lzi_H7VTkS8xj5SdX3coE0oimYwxIi2emTAue0UOa5dpgFGyBJ4c8tQ2VF402XRugKDTP8akYhFo5tAA77Qe_NmtuYZc3C3m3I24G2GvR5sSDxUyAN2zq8Lfn9EUms6rY3Ob8YeiKkTiBj0",
"dq":"s9lAH9fggBsoFR8Oac2R_E2gw282rT2kGOAhvIllETE1efrA6huUUvMfBcMpn8lqeW6vzznYY5SSQF7pMdC_agI3nG8Ibp1BUb0JUiraRNqUfLhcQb_d9GF4Dh7e74WbRsobRonujTYN1xCaP6TO61jvWrX-L18txXw494Q_cgk",
"qi":"GyM_p6JrXySiz1toFgKbWV-JdI3jQ4ypu9rbMWx3rQJBfmt0FoYzgUIZEVFEcOqwemRN81zoDAaa-Bk0KWNGDjJHZDdDmFhW3AN7lI-puxk_mHZGJ11rxyR8O55XLSe3SPmRfKwZI6yU24ZxvQKFYItdldUKGzO6Ia6zTKhAVRU",
"alg":"RS256",
"kid":"2011-04-29"}
]
}
`

func TestValidateIPRanges(t *testing.T) {
ctx := context.Background()
c := internal.NewConfigurationWithDefaults()
Expand Down
14 changes: 12 additions & 2 deletions consent/strategy_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"time"

"github.com/gorilla/sessions"
"github.com/hashicorp/go-retryablehttp"
"github.com/pborman/uuid"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -777,12 +778,21 @@ func (s *DefaultStrategy) executeBackChannelLogout(r *http.Request, subject, sid
tasks = append(tasks, task{url: c.BackChannelLogoutURI, clientID: c.GetID(), token: t})
}

var execute = func(t task) {
span := trace.SpanFromContext(ctx)
cl := s.r.HTTPClient(ctx)
execute := func(t task) {
log := s.r.Logger().WithRequest(r).
WithField("client_id", t.clientID).
WithField("backchannel_logout_url", t.url)

res, err := s.r.HTTPClient(ctx).PostForm(t.url, url.Values{"logout_token": {t.token}})
body := url.Values{"logout_token": {t.token}}.Encode()
req, err := retryablehttp.NewRequestWithContext(trace.ContextWithSpan(context.Background(), span), "POST", t.url, []byte(body))
if err != nil {
log.WithError(err).Error("Unable to construct OpenID Connect Back-Channel Logout Request")
return
}
req.Header.Add("Content-Type", "application/x-www-form-urlencoded")
res, err := cl.Do(req)
if err != nil {
log.WithError(err).Error("Unable to execute OpenID Connect Back-Channel Logout Request")
return
Expand Down
Loading

0 comments on commit 0a241c5

Please sign in to comment.