fix: issuer missing from netid claims (#3080)

The NetID provider omits the issuer claim in the userinfo response. To resolve this issue, the ID token returned by NetID is now validated and its `sub` and `iss` values are used.
ory · Feb 9, 2023 · dec7cbc · dec7cbc
1 parent 3b8f426
commit dec7cbc
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 11 deletions.
diff --git a/selfservice/strategy/oidc/provider_netid.go b/selfservice/strategy/oidc/provider_netid.go
@@ -6,8 +6,13 @@ package oidc
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"net/url"
 
+	gooidc "github.com/coreos/go-oidc"
+
+	"github.com/ory/x/stringslice"
+
 	"github.com/hashicorp/go-retryablehttp"
 	"github.com/pkg/errors"
 	"golang.org/x/oauth2"
@@ -31,6 +36,11 @@ func NewProviderNetID(
 	config *Configuration,
 	reg dependencies,
 ) *ProviderNetID {
+	config.IssuerURL = fmt.Sprintf("%s://%s/", defaultBrokerScheme, defaultBrokerHost)
+	if !stringslice.Has(config.Scope, gooidc.ScopeOpenID) {
+		config.Scope = append(config.Scope, gooidc.ScopeOpenID)
+	}
+
 	return &ProviderNetID{
 		ProviderGenericOIDC: &ProviderGenericOIDC{
 			config: config,
@@ -68,18 +78,13 @@ func (n *ProviderNetID) Claims(ctx context.Context, exchange *oauth2.Token, _ ur
 		return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("%s", err))
 	}
 
-	client := n.reg.HTTPClient(ctx, httpx.ResilientClientDisallowInternalIPs(), httpx.ResilientClientWithClient(o.Client(ctx, exchange)))
-
-	u := n.brokerURL()
-	if err != nil {
-		return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("%s", err))
-	}
-	userInfoURL := urlx.AppendPaths(u, "/userinfo")
-	req, err := retryablehttp.NewRequest("GET", userInfoURL.String(), nil)
+	req, err := retryablehttp.NewRequestWithContext(ctx, "GET", urlx.AppendPaths(n.brokerURL(), "/userinfo").String(), nil)
 	if err != nil {
 		return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("%s", err))
 	}
 
+	client := n.reg.HTTPClient(ctx, httpx.ResilientClientDisallowInternalIPs(), httpx.ResilientClientWithClient(o.Client(ctx, exchange)))
+
 	resp, err := client.Do(req)
 	if err != nil {
 		return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("%s", err))
@@ -90,12 +95,29 @@ func (n *ProviderNetID) Claims(ctx context.Context, exchange *oauth2.Token, _ ur
 		return nil, err
 	}
 
-	var claims Claims
-	if err := json.NewDecoder(resp.Body).Decode(&claims); err != nil {
+	p, err := n.provider(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	raw, ok := exchange.Extra("id_token").(string)
+	if !ok || len(raw) == 0 {
+		return nil, errors.WithStack(ErrIDTokenMissing)
+	}
+
+	claims, err := n.verifyAndDecodeClaimsWithProvider(ctx, p, raw)
+	if err != nil {
+		return nil, err
+	}
+
+	var userinfo Claims
+	if err := json.NewDecoder(resp.Body).Decode(&userinfo); err != nil {
 		return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("%s", err))
 	}
 
-	return &claims, nil
+	userinfo.Issuer = claims.Issuer
+	userinfo.Subject = claims.Subject
+	return &userinfo, nil
 }
 
 func (n *ProviderNetID) brokerURL() *url.URL {

diff --git a/selfservice/strategy/oidc/provider_userinfo_test.go b/selfservice/strategy/oidc/provider_userinfo_test.go
@@ -84,7 +84,35 @@ func TestProviderClaimsRespectsErrorCodes(t *testing.T) {
 				IssuerURL: "https://broker.netid.de",
 				ID:        "netid",
 				Provider:  "netid",
+				ClientID:  "foo",
 			}, reg),
+			useToken: token.WithExtra(map[string]interface{}{"id_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiZW1haWwiOiJqb2huLmRvZUBleGFtcGxlLmNvbSIsImlhdCI6MTUxNjIzOTAyMiwidGlkIjoiYTliODYzODUtZjMyYy00ODAzLWFmYzgtNGIyMzEyZmJkZjI0IiwiaXNzIjoiaHR0cHM6Ly9icm9rZXIubmV0aWQuZGUvIiwiYXVkIjpbImZvbyJdLCJleHAiOjQwNzE3Mjg1MDR9.Zt_-9jULoENQ7pq6rKrevhecBlWKR2rzNti42EJti_OelCrGbl5zyHYRfIg264VzEY0Zp9BAZTWmcF6Z-cpuD05RddTXZDrrC_47bJeYDL-bjDfKjoSZUt_1JnNFgqeyMA1Ji6HCIsEf_g8QnuSELAO0c-qb2ANmDLVL8_dY6oUmCN5oWLJS2q6xO-Iaz-AuKaGTbLZBcjCe2NB_--kIx4IrgLm78U90X3ePOV0CcYZLNvDgzEsVJ2M4ixKp87bYUaZZ3JJEjuxgnHqKRMExDKron3mvfQtC1L-dZQyeDo3-mCFJL_JhEw9zLmoWFBD7aARrVL_yAe31o26FB3S-Dg"}),
+			hook: func(t *testing.T) {
+				httpmock.RegisterResponder("GET", "https://broker.netid.de/.well-known/openid-configuration",
+					func(req *http.Request) (*http.Response, error) {
+						return httpmock.NewJsonResponse(200, map[string]interface{}{
+							"issuer":   "https://broker.netid.de/",
+							"jwks_uri": "https://broker.netid.de/.well-known/jwks.json",
+						})
+					},
+				)
+				httpmock.RegisterResponder("GET", "https://broker.netid.de/.well-known/jwks.json",
+					func(req *http.Request) (*http.Response, error) {
+						return httpmock.NewJsonResponse(200, json.RawMessage(`{
+  "keys": [
+    {
+      "kty": "RSA",
+      "e": "AQAB",
+      "alg": "RS256",
+      "use": "sig",
+      "n": "uw0hrmZpA5CH5lDnjbCK6ZQD-I0BGxde-ChDzUz5eK4r7VtyMGvAxwD-k-mWw4FJ2NgYmT_T89sAtE6NQqT5u9HAe-CI22lf5LMvmqvMzekcmBAvXNw8VeTV_N6CbS9INJrxf20cObf-kpTxVxlYtYxYwIhYdOw3DwX8y31vI38qHQ4_OzTRo4KFVLCr68MzIHHRI4d5EHrFv1VFjiaa_ATwuwCMUfg0RMnK09FpMCgvp0E6bQeptXhBBNQVQkoC5whT1GzikfSxyeugjQ_XuTt1MKoyYsN2pmfrBdcfWrPYvV_JPgO1MkEtqErvtCByairINfXrHTMOxNWe3sYlXQ"
+    }
+  ]
+}`))
+					},
+				)
+			},
+			expectedClaims: &oidc.Claims{Issuer: "https://broker.netid.de/", Subject: "1234567890", Name: "John Doe", GivenName: "John", FamilyName: "Doe", LastName: "", MiddleName: "", Nickname: "John Doe", PreferredUsername: "John Doe", Profile: "", Picture: "", Website: "", Email: "john.doe@example.com", EmailVerified: true, Gender: "", Birthdate: "01/01/1990", Zoneinfo: "", Locale: "", PhoneNumber: "", PhoneNumberVerified: false, UpdatedAt: 0, HD: "", Team: ""},
 		},
 		{
 			name:             "vk",