Skip to content

Commit

Permalink
fix: strip sensitive values from the hook payload
Browse files Browse the repository at this point in the history
  • Loading branch information
sgal committed Mar 23, 2023
1 parent fb5bd4b commit 3f75efd
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 20 deletions.
24 changes: 11 additions & 13 deletions oauth2/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,6 @@ type AccessRequestHook func(ctx context.Context, requester fosite.AccessRequeste
//
// swagger:ignore
type Requester struct {
// ClientID is the identifier of the OAuth 2.0 client.
ClientID string `json:"client_id"`
// GrantedScopes is the list of scopes granted to the OAuth 2.0 client.
GrantedScopes []string `json:"granted_scopes"`
// GrantedAudience is the list of audiences granted to the OAuth 2.0 client.
GrantedAudience []string `json:"granted_audience"`
// GrantTypes is the requests grant types.
GrantTypes []string `json:"grant_types"`
// Payload is the requests payload.
Expand Down Expand Up @@ -126,6 +120,13 @@ func JWTBearerHook(reg interface {
}
}

func getSafePayload(requester fosite.AccessRequester) url.Values {
payload := requester.GetRequestForm()
payload.Del("client_secret")

return payload
}

func callHook(ctx context.Context, reg x.HTTPClientProvider, requester fosite.AccessRequester, grantType string, hookURL *url.URL) error {
if !requester.GetGrantTypes().ExactOne(grantType) {
return nil
Expand All @@ -138,16 +139,13 @@ func callHook(ctx context.Context, reg x.HTTPClientProvider, requester fosite.Ac

payload := map[string][]string{}

if grantType == "urn:ietf:params:oauth:grant-type:jwt-bearer" || grantType == "client_credentials" {
payload = requester.GetRequestForm()
if grantType == "urn:ietf:params:oauth:grant-type:jwt-bearer" {
payload = getSafePayload(requester)
}

requesterInfo := Requester{
ClientID: requester.GetClient().GetID(),
GrantedScopes: requester.GetGrantedScopes(),
GrantedAudience: requester.GetGrantedAudience(),
GrantTypes: requester.GetGrantTypes(),
Payload: payload,
GrantTypes: requester.GetGrantTypes(),
Payload: payload,
}

reqBody := TokenHookRequest{
Expand Down
2 changes: 0 additions & 2 deletions oauth2/oauth2_auth_code_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1227,8 +1227,6 @@ func TestAuthCodeWithMockStrategy(t *testing.T) {
require.Equal(t, hookReq.Session.ClientID, oauthConfig.ClientID)
require.Equal(t, hookReq.Session.Extra, map[string]interface{}{})
require.NotEmpty(t, hookReq.Requester)
require.Equal(t, hookReq.Requester.ClientID, oauthConfig.ClientID)
require.ElementsMatch(t, hookReq.Requester.GrantedScopes, expectedGrantedScopes)
require.Equal(t, hookReq.Requester.Payload, map[string][]string{})

except := []string{
Expand Down
4 changes: 1 addition & 3 deletions oauth2/oauth2_client_credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,6 @@ func TestClientCredentials(t *testing.T) {

expectedGrantedScopes := []string{"foobar"}
expectedGrantedAudience := []string{"https://api.ory.sh/"}
expectedPayload := map[string][]string(map[string][]string{"audience": audience, "grant_type": {"client_credentials"}, "scope": {scope}})

var hookReq hydraoauth2.TokenHookRequest
require.NoError(t, json.NewDecoder(r.Body).Decode(&hookReq))
Expand All @@ -270,8 +269,7 @@ func TestClientCredentials(t *testing.T) {
require.NotEmpty(t, hookReq.Session)
require.Equal(t, hookReq.Session.Extra, map[string]interface{}{})
require.NotEmpty(t, hookReq.Requester)
require.ElementsMatch(t, hookReq.Requester.GrantedScopes, expectedGrantedScopes)
require.Equal(t, hookReq.Requester.Payload, expectedPayload)
require.Equal(t, hookReq.Requester.Payload, map[string][]string{})

claims := map[string]interface{}{
"hooked": true,
Expand Down
81 changes: 79 additions & 2 deletions oauth2/oauth2_jwt_bearer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ func TestJWTBearer(t *testing.T) {
}

var getToken = func(t *testing.T, conf *clientcredentials.Config) (*goauth2.Token, error) {
conf.AuthStyle = goauth2.AuthStyleInHeader
if conf.AuthStyle == goauth2.AuthStyleAutoDetect {
conf.AuthStyle = goauth2.AuthStyleInHeader
}
return conf.Token(context.Background())
}

Expand Down Expand Up @@ -333,7 +335,6 @@ func TestJWTBearer(t *testing.T) {
require.NotEmpty(t, hookReq.Session)
require.Equal(t, hookReq.Session.Extra, map[string]interface{}{})
require.NotEmpty(t, hookReq.Requester)
require.ElementsMatch(t, hookReq.Requester.GrantedScopes, expectedGrantedScopes)
require.Equal(t, hookReq.Requester.Payload, expectedPayload)

claims := map[string]interface{}{
Expand Down Expand Up @@ -371,6 +372,82 @@ func TestJWTBearer(t *testing.T) {
t.Run("strategy=jwt", run("jwt"))
})

t.Run("should call token hook if configured and omit client_secret from payload", func(t *testing.T) {
run := func(strategy string) func(t *testing.T) {
return func(t *testing.T) {
audience := reg.Config().OAuth2TokenURL(ctx).String()
grantType := "urn:ietf:params:oauth:grant-type:jwt-bearer"

token, _, err := signer.Generate(ctx, jwt.MapClaims{
"jti": uuid.NewString(),
"iss": trustGrant.Issuer,
"sub": trustGrant.Subject,
"aud": audience,
"exp": time.Now().Add(time.Hour).Unix(),
"iat": time.Now().Add(-time.Minute).Unix(),
}, &jwt.Headers{Extra: map[string]interface{}{"kid": kid}})
require.NoError(t, err)

client := &hc.Client{
Secret: secret,
GrantTypes: []string{"urn:ietf:params:oauth:grant-type:jwt-bearer"},
Scope: "offline_access",
TokenEndpointAuthMethod: "client_secret_post",
}
require.NoError(t, reg.ClientManager().CreateClient(ctx, client))

hs := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, r.Header.Get("Content-Type"), "application/json; charset=UTF-8")

expectedGrantedScopes := []string{client.Scope}
expectedGrantedAudience := []string{audience}
expectedPayload := map[string][]string(map[string][]string{"client_id": {client.GetID()}, "grant_type": {grantType}, "assertion": {token}, "scope": {client.Scope}})

var hookReq hydraoauth2.TokenHookRequest
require.NoError(t, json.NewDecoder(r.Body).Decode(&hookReq))
require.ElementsMatch(t, hookReq.GrantedScopes, expectedGrantedScopes)
require.ElementsMatch(t, hookReq.GrantedAudience, expectedGrantedAudience)
require.NotEmpty(t, hookReq.Session)
require.Equal(t, hookReq.Session.Extra, map[string]interface{}{})
require.NotEmpty(t, hookReq.Requester)
require.Equal(t, hookReq.Requester.Payload, expectedPayload)

claims := map[string]interface{}{
"hooked": true,
}

hookResp := hydraoauth2.TokenHookResponse{
Session: consent.AcceptOAuth2ConsentRequestSession{
AccessToken: claims,
IDToken: claims,
},
}

w.WriteHeader(http.StatusOK)
require.NoError(t, json.NewEncoder(w).Encode(&hookResp))
}))
defer hs.Close()

reg.Config().MustSet(ctx, config.KeyAccessTokenStrategy, strategy)
reg.Config().MustSet(ctx, config.KeyJWTBearerHookURL, hs.URL)

defer reg.Config().MustSet(ctx, config.KeyJWTBearerHookURL, nil)

conf := newConf(client)
conf.AuthStyle = goauth2.AuthStyleInParams
conf.EndpointParams = url.Values{"grant_type": {grantType}, "assertion": {token}}

result, err := getToken(t, conf)
require.NoError(t, err)

inspectToken(t, result, client, strategy, trustGrant, true)
}
}

t.Run("strategy=opaque", run("opaque"))
t.Run("strategy=jwt", run("jwt"))
})

t.Run("should fail token if hook fails", func(t *testing.T) {
run := func(strategy string) func(t *testing.T) {
return func(t *testing.T) {
Expand Down

0 comments on commit 3f75efd

Please sign in to comment.