*: pass algorithms a provider supports to the verifier

If a provider advertises support for algorithms like ES256 in its discovery, pass those algorithms to the verifier. Added a whitelist to ensure weird algorithms like "none" are thrown out.
coreos · Jan 24, 2020 · 40cd342 · 40cd342
1 parent 7bec05b
commit 40cd342
Show file tree

Hide file tree

Showing 3 changed files with 243 additions and 6 deletions.
diff --git a/oidc.go b/oidc.go
@@ -69,6 +69,7 @@ type Provider struct {
 	authURL     string
 	tokenURL    string
 	userInfoURL string
+	algorithms  []string
 
 	// Raw claims returned by the server.
 	rawClaims []byte
@@ -82,11 +83,27 @@ type cachedKeys struct {
 }
 
 type providerJSON struct {
-	Issuer      string `json:"issuer"`
-	AuthURL     string `json:"authorization_endpoint"`
-	TokenURL    string `json:"token_endpoint"`
-	JWKSURL     string `json:"jwks_uri"`
-	UserInfoURL string `json:"userinfo_endpoint"`
+	Issuer      string   `json:"issuer"`
+	AuthURL     string   `json:"authorization_endpoint"`
+	TokenURL    string   `json:"token_endpoint"`
+	JWKSURL     string   `json:"jwks_uri"`
+	UserInfoURL string   `json:"userinfo_endpoint"`
+	Algorithms  []string `json:"id_token_signing_alg_values_supported"`
+}
+
+// supportedAlgorithms is a list of algorithms explicitly supported by this
+// package. If a provider supports other algorithms, such as HS256 or none,
+// those values won't be passed to the IDTokenVerifier.
+var supportedAlgorithms = map[string]bool{
+	RS256: true,
+	RS384: true,
+	RS512: true,
+	ES256: true,
+	ES384: true,
+	ES512: true,
+	PS256: true,
+	PS384: true,
+	PS512: true,
 }
 
 // NewProvider uses the OpenID Connect discovery mechanism to construct a Provider.
@@ -123,11 +140,18 @@ func NewProvider(ctx context.Context, issuer string) (*Provider, error) {
 	if p.Issuer != issuer {
 		return nil, fmt.Errorf("oidc: issuer did not match the issuer returned by provider, expected %q got %q", issuer, p.Issuer)
 	}
+	var algs []string
+	for _, a := range p.Algorithms {
+		if supportedAlgorithms[a] {
+			algs = append(algs, a)
+		}
+	}
 	return &Provider{
 		issuer:       p.Issuer,
 		authURL:      p.AuthURL,
 		tokenURL:     p.TokenURL,
 		userInfoURL:  p.UserInfoURL,
+		algorithms:   algs,
 		rawClaims:    body,
 		remoteKeySet: NewRemoteKeySet(ctx, p.JWKSURL),
 	}, nil

diff --git a/oidc_test.go b/oidc_test.go
@@ -1,7 +1,13 @@
 package oidc
 
 import (
+	"context"
 	"fmt"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"reflect"
+	"strings"
 	"testing"
 )
 
@@ -93,3 +99,201 @@ func TestAccessTokenVerification(t *testing.T) {
 		t.Run(test.name, test.run)
 	}
 }
+
+func TestNewProvider(t *testing.T) {
+	tests := []struct {
+		name            string
+		data            string
+		trailingSlash   bool
+		wantAuthURL     string
+		wantTokenURL    string
+		wantUserInfoURL string
+		wantAlgorithms  []string
+		wantErr         bool
+	}{
+		{
+			name: "basic_case",
+			data: `{
+				"issuer": "ISSUER",
+				"authorization_endpoint": "https://example.com/auth",
+				"token_endpoint": "https://example.com/token",
+				"jwks_uri": "https://example.com/keys",
+				"id_token_signing_alg_values_supported": ["RS256"]
+			}`,
+			wantAuthURL:    "https://example.com/auth",
+			wantTokenURL:   "https://example.com/token",
+			wantAlgorithms: []string{"RS256"},
+		},
+		{
+			name: "additional_algorithms",
+			data: `{
+				"issuer": "ISSUER",
+				"authorization_endpoint": "https://example.com/auth",
+				"token_endpoint": "https://example.com/token",
+				"jwks_uri": "https://example.com/keys",
+				"id_token_signing_alg_values_supported": ["RS256", "RS384", "ES256"]
+			}`,
+			wantAuthURL:    "https://example.com/auth",
+			wantTokenURL:   "https://example.com/token",
+			wantAlgorithms: []string{"RS256", "RS384", "ES256"},
+		},
+		{
+			name: "unsupported_algorithms",
+			data: `{
+				"issuer": "ISSUER",
+				"authorization_endpoint": "https://example.com/auth",
+				"token_endpoint": "https://example.com/token",
+				"jwks_uri": "https://example.com/keys",
+				"id_token_signing_alg_values_supported": [
+					"RS256", "RS384", "ES256", "HS256", "none"
+				]
+			}`,
+			wantAuthURL:    "https://example.com/auth",
+			wantTokenURL:   "https://example.com/token",
+			wantAlgorithms: []string{"RS256", "RS384", "ES256"},
+		},
+		{
+			name: "mismatched_issuer",
+			data: `{
+				"issuer": "https://example.com",
+				"authorization_endpoint": "https://example.com/auth",
+				"token_endpoint": "https://example.com/token",
+				"jwks_uri": "https://example.com/keys",
+				"id_token_signing_alg_values_supported": ["RS256"]
+			}`,
+			wantErr: true,
+		},
+		{
+			name: "issuer_with_trailing_slash",
+			data: `{
+				"issuer": "ISSUER",
+				"authorization_endpoint": "https://example.com/auth",
+				"token_endpoint": "https://example.com/token",
+				"jwks_uri": "https://example.com/keys",
+				"id_token_signing_alg_values_supported": ["RS256"]
+			}`,
+			trailingSlash:  true,
+			wantAuthURL:    "https://example.com/auth",
+			wantTokenURL:   "https://example.com/token",
+			wantAlgorithms: []string{"RS256"},
+		},
+		{
+			// Test case taken directly from:
+			// https://accounts.google.com/.well-known/openid-configuration
+			name:            "google",
+			wantAuthURL:     "https://accounts.google.com/o/oauth2/v2/auth",
+			wantTokenURL:    "https://oauth2.googleapis.com/token",
+			wantUserInfoURL: "https://openidconnect.googleapis.com/v1/userinfo",
+			wantAlgorithms:  []string{"RS256"},
+			data: `{
+ "issuer": "ISSUER",
+ "authorization_endpoint": "https://accounts.google.com/o/oauth2/v2/auth",
+ "device_authorization_endpoint": "https://oauth2.googleapis.com/device/code",
+ "token_endpoint": "https://oauth2.googleapis.com/token",
+ "userinfo_endpoint": "https://openidconnect.googleapis.com/v1/userinfo",
+ "revocation_endpoint": "https://oauth2.googleapis.com/revoke",
+ "jwks_uri": "https://www.googleapis.com/oauth2/v3/certs",
+ "response_types_supported": [
+  "code",
+  "token",
+  "id_token",
+  "code token",
+  "code id_token",
+  "token id_token",
+  "code token id_token",
+  "none"
+ ],
+ "subject_types_supported": [
+  "public"
+ ],
+ "id_token_signing_alg_values_supported": [
+  "RS256"
+ ],
+ "scopes_supported": [
+  "openid",
+  "email",
+  "profile"
+ ],
+ "token_endpoint_auth_methods_supported": [
+  "client_secret_post",
+  "client_secret_basic"
+ ],
+ "claims_supported": [
+  "aud",
+  "email",
+  "email_verified",
+  "exp",
+  "family_name",
+  "given_name",
+  "iat",
+  "iss",
+  "locale",
+  "name",
+  "picture",
+  "sub"
+ ],
+ "code_challenge_methods_supported": [
+  "plain",
+  "S256"
+ ],
+ "grant_types_supported": [
+  "authorization_code",
+  "refresh_token",
+  "urn:ietf:params:oauth:grant-type:device_code",
+  "urn:ietf:params:oauth:grant-type:jwt-bearer"
+ ]
+}`,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+
+			var issuer string
+			hf := func(w http.ResponseWriter, r *http.Request) {
+				if r.URL.Path != "/.well-known/openid-configuration" {
+					http.NotFound(w, r)
+					return
+				}
+				w.Header().Set("Content-Type", "application/json")
+				io.WriteString(w, strings.ReplaceAll(test.data, "ISSUER", issuer))
+			}
+			s := httptest.NewServer(http.HandlerFunc(hf))
+			defer s.Close()
+
+			issuer = s.URL
+			if test.trailingSlash {
+				issuer += "/"
+			}
+
+			p, err := NewProvider(ctx, issuer)
+			if err != nil {
+				if !test.wantErr {
+					t.Errorf("NewProvider() failed: %v", err)
+				}
+				return
+			}
+			if test.wantErr {
+				t.Fatalf("NewProvider(): expected error")
+			}
+
+			if p.authURL != test.wantAuthURL {
+				t.Errorf("NewProvider() unexpected authURL value, got=%s, want=%s",
+					p.authURL, test.wantAuthURL)
+			}
+			if p.tokenURL != test.wantTokenURL {
+				t.Errorf("NewProvider() unexpected tokenURL value, got=%s, want=%s",
+					p.tokenURL, test.wantTokenURL)
+			}
+			if p.userInfoURL != test.wantUserInfoURL {
+				t.Errorf("NewProvider() unexpected userInfoURL value, got=%s, want=%s",
+					p.userInfoURL, test.wantUserInfoURL)
+			}
+			if !reflect.DeepEqual(p.algorithms, test.wantAlgorithms) {
+				t.Errorf("NewProvider() unexpected algorithms value, got=%s, want=%s",
+					p.algorithms, test.wantAlgorithms)
+			}
+		})
+	}
+}
diff --git a/verify.go b/verify.go
@@ -79,7 +79,9 @@ type Config struct {
 	ClientID string
 	// If specified, only this set of algorithms may be used to sign the JWT.
 	//
-	// Since many providers only support RS256, SupportedSigningAlgs defaults to this value.
+	// If the IDTokenVerifier is created from a provider with (*Provider).Verifier, this
+	// defaults to the set of algorithms the provider supports. Otherwise this values
+	// defaults to RS256.
 	SupportedSigningAlgs []string
 
 	// If true, no ClientID check performed. Must be true if ClientID field is empty.
@@ -105,6 +107,13 @@ type Config struct {
 // The returned IDTokenVerifier is tied to the Provider's context and its behavior is
 // undefined once the Provider's context is canceled.
 func (p *Provider) Verifier(config *Config) *IDTokenVerifier {
+	if len(config.SupportedSigningAlgs) == 0 && len(p.algorithms) > 0 {
+		// Make a copy so we don't modify the config values.
+		cp := &Config{}
+		*cp = *config
+		cp.SupportedSigningAlgs = p.algorithms
+		config = cp
+	}
 	return NewVerifier(p.issuer, p.remoteKeySet, config)
 }