Skip to content

Commit

Permalink
missing userinfo endpoint (#709)
Browse files Browse the repository at this point in the history
* test for missing userinfo_endpoint

* do not try to request userinfo if no userinfo_endpoint is available

* refactor: early exit

* docu: userinfo depends on availability of recommended userinfo endpoint

* go fmt

* changelog entry
  • Loading branch information
johakoch authored Feb 9, 2023
1 parent 6cf3cc4 commit 53238ca
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 26 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Unreleased changes are available as `avenga/couper:edge` container.

* **Fixed**
* url scheme while using the [`tls` block](https://docs.couper.io/configuration/block/server_tls) ([#703](https://github.com/avenga/couper/issues/703))
* For [OIDC](https://docs.couper.io/configuration/block/oidc), trying to request userinfo from a non-existing (not required, though recommended) userinfo endpoint ([#709](https://github.com/avenga/couper/pull/709))

---

Expand Down
2 changes: 1 addition & 1 deletion docs/website/content/2.configuration/5.variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ and for an [`oidc` block](/configuration/block/oidc) additionally:

- `id_token`: The ID token.
- `id_token_claims`: A map of claims from the ID token.
- `userinfo`: A map of claims retrieved from the userinfo endpoint.
- `userinfo`: A map of properties retrieved from the userinfo endpoint (if the recommended endpoint is available).

## `beta_token_response`

Expand Down
61 changes: 36 additions & 25 deletions oauth2/oidc_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,27 +52,31 @@ func NewOidcClient(evalCtx *hcl.EvalContext, oidcConfig *oidc.Config) (*OidcClie

// validateTokenResponseData validates the token response data
func (o *OidcClient) validateTokenResponseData(ctx context.Context, tokenResponseData map[string]interface{}, hashedVerifierValue, verifierValue, accessToken string) error {
if idTokenString, ok := tokenResponseData["id_token"].(string); ok {
idTokenClaims := jwt.MapClaims{}
_, err := o.jwtParser.ParseWithClaims(idTokenString, idTokenClaims, o.Keyfunc)
if err != nil {
return err
}
idTokenString, ok := tokenResponseData["id_token"].(string)
if !ok {
return errors.Oauth2.Message("missing id_token in token response")
}

var userinfo map[string]interface{}
userinfo, err = o.validateIDTokenClaims(ctx, idTokenClaims, hashedVerifierValue, verifierValue, accessToken)
if err != nil {
return err
}
idTokenClaims := jwt.MapClaims{}
_, err := o.jwtParser.ParseWithClaims(idTokenString, idTokenClaims, o.Keyfunc)
if err != nil {
return err
}

// treat token claims as map for context
tokenResponseData["id_token_claims"] = map[string]interface{}(idTokenClaims)
tokenResponseData["userinfo"] = userinfo
// treat token claims as map for context
tokenResponseData["id_token_claims"] = map[string]interface{}(idTokenClaims)

var userinfo map[string]interface{}
userinfo, err = o.validateIDTokenClaims(ctx, idTokenClaims, hashedVerifierValue, verifierValue, accessToken)
if err != nil {
return err
}

return nil
if userinfo != nil {
tokenResponseData["userinfo"] = userinfo
}

return errors.Oauth2.Message("missing id_token in token response")
return nil
}

func (o *OidcClient) Keyfunc(token *jwt.Token) (interface{}, error) {
Expand Down Expand Up @@ -165,7 +169,19 @@ func (o *OidcClient) validateIDTokenClaims(ctx context.Context, idTokenClaims jw
}
}

userinfoData, subUserinfo, err := o.getUserinfo(ctx, accessToken)
userinfoEndpoint, err := o.config.GetUserinfoEndpoint()
if err != nil {
return nil, err
}

// https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata
// userinfo_endpoint
// RECOMMENDED. URL of the OP's UserInfo Endpoint
if userinfoEndpoint == "" {
return nil, nil
}

userinfoData, subUserinfo, err := o.getUserinfo(ctx, userinfoEndpoint, accessToken)
if err != nil {
return nil, errors.Oauth2.Message("userinfo request error").With(err)
}
Expand All @@ -181,8 +197,8 @@ func (o *OidcClient) validateIDTokenClaims(ctx context.Context, idTokenClaims jw
return userinfoData, nil
}

func (o *OidcClient) getUserinfo(ctx context.Context, accessToken string) (map[string]interface{}, string, error) {
userinfoReq, err := o.newUserinfoRequest(ctx, accessToken)
func (o *OidcClient) getUserinfo(ctx context.Context, userinfoEndpoint, accessToken string) (map[string]interface{}, string, error) {
userinfoReq, err := o.newUserinfoRequest(ctx, userinfoEndpoint, accessToken)
if err != nil {
return nil, "", err
}
Expand Down Expand Up @@ -232,12 +248,7 @@ func parseUserinfoResponse(userinfoResponse []byte) (map[string]interface{}, str
return userinfoData, sub, nil
}

func (o *OidcClient) newUserinfoRequest(ctx context.Context, accessToken string) (*http.Request, error) {
userinfoEndpoint, err := o.config.GetUserinfoEndpoint()
if err != nil {
return nil, err
}

func (o *OidcClient) newUserinfoRequest(ctx context.Context, userinfoEndpoint, accessToken string) (*http.Request, error) {
outreq, err := http.NewRequest(http.MethodGet, userinfoEndpoint, nil)
if err != nil {
return nil, err
Expand Down
15 changes: 15 additions & 0 deletions server/http_oauth2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,8 @@ func TestOAuth2_AccessControl(t *testing.T) {
if !strings.HasSuffix(code, "-miss-id") {
if strings.HasSuffix(code, "-wiss-id") {
mapClaims["iss"] = "https://malicious.authorization.server"
} else if strings.HasSuffix(code, "-wuiss-id") {
mapClaims["iss"] = "https://authorization.server/without/userinfo"
} else {
mapClaims["iss"] = "https://authorization.server"
}
Expand Down Expand Up @@ -1271,6 +1273,18 @@ func TestOAuth2_AccessControl(t *testing.T) {
t.Log(werr)
}
return
} else if req.URL.Path == "/without/userinfo/.well-known/openid-configuration" {
body := []byte(`{
"issuer": "https://authorization.server/without/userinfo",
"authorization_endpoint": "https://authorization.server/oauth2/authorize",
"jwks_uri": "http://` + req.Host + `/jwks",
"token_endpoint": "http://` + req.Host + `/token"
}`)
_, werr := rw.Write(body)
if werr != nil {
t.Log(werr)
}
return
}
rw.WriteHeader(http.StatusBadRequest)
}))
Expand Down Expand Up @@ -1324,6 +1338,7 @@ func TestOAuth2_AccessControl(t *testing.T) {
{"code, nonce param", "07_couper.hcl", http.MethodGet, "/cb?code=qeuboub-id", http.Header{"Cookie": []string{"nnc=" + st}}, http.StatusOK, "code=qeuboub-id&grant_type=authorization_code&redirect_uri=http%3A%2F%2Flocalhost%3A8080%2Fcb", "Basic Zm9vOmV0YmluYnA0aW4=", ""},
{"code; client_secret_basic; PKCE; relative redirect_uri", "08_couper.hcl", http.MethodGet, "/cb?code=qeuboub", http.Header{"Cookie": []string{"pkcecv=qerbnr"}, "X-Forwarded-Proto": []string{"https"}, "X-Forwarded-Host": []string{"www.example.com"}}, http.StatusOK, "code=qeuboub&code_verifier=qerbnr&grant_type=authorization_code&redirect_uri=https%3A%2F%2Fwww.example.com%2Fcb", "Basic Zm9vOmV0YmluYnA0aW4=", ""},
{"code; nonce param; relative redirect_uri", "09_couper.hcl", http.MethodGet, "/cb?code=qeuboub-id", http.Header{"Cookie": []string{"nnc=" + st}, "X-Forwarded-Proto": []string{"https"}, "X-Forwarded-Host": []string{"www.example.com"}}, http.StatusOK, "code=qeuboub-id&grant_type=authorization_code&redirect_uri=https%3A%2F%2Fwww.example.com%2Fcb", "Basic Zm9vOmV0YmluYnA0aW4=", ""},
{"code; without userinfo", "24_couper.hcl", http.MethodGet, "/cb?code=qeuboub-wuiss-id", http.Header{"Cookie": []string{"nnc=" + st}, "X-Forwarded-Proto": []string{"https"}, "X-Forwarded-Host": []string{"www.example.com"}}, http.StatusOK, "code=qeuboub-wuiss-id&grant_type=authorization_code&redirect_uri=http%3A%2F%2Fwww.example.com%2Fcb", "Basic Zm9vOmV0YmluYnA0aW4=", ""},
} {
t.Run(tc.path[1:], func(subT *testing.T) {
h := test.New(subT)
Expand Down
21 changes: 21 additions & 0 deletions server/testdata/oauth2/24_couper.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
server "client" {
api {
endpoint "/cb" {
access_control = ["ac"]
response {
json_body = request.context.ac
}
}
}
}

definitions {
oidc "ac" {
configuration_url = "{{.asOrigin}}/without/userinfo/.well-known/openid-configuration"
configuration_ttl = "1h"
client_id = "foo"
client_secret = "etbinbp4in"
redirect_uri = "http://www.example.com/cb" # value is not checked
verifier_value = request.cookies.nnc
}
}

0 comments on commit 53238ca

Please sign in to comment.