Add validation of iss claim parameter (#3552)

Co-authored-by: Don Browne <dmjb@users.noreply.github.com>

cmd/server/app/serve.go

@@ -87,11 +87,19 @@ var serveCmd = &cobra.Command{
 		}
 
 		// Identity
-		jwksUrl, err := cfg.Identity.Server.Path("realms/stacklok/protocol/openid-connect/certs")
+		// TODO: cfg.Identity.Server.IssuerUrl _should_ be a URL to an issuer that has an
+		// .../.well-known/jwks.json or .../.well-known/openid-configuration endpoint.  Right
+		// now it's just a hostname.  When we have this, we can consolidate the jwksUrl and issUrl,
+		// and remove the Keycloak-specific paths.
+		jwksUrl, err := cfg.Identity.Server.Path("/realms/stacklok/protocol/openid-connect/certs")
 		if err != nil {
 			return fmt.Errorf("failed to create JWKS URL: %w\n", err)
 		}
-		jwt, err := auth.NewJwtValidator(ctx, jwksUrl.String(), cfg.Identity.Server.Audience)
+		issUrl, err := cfg.Identity.Server.JwtUrl()
+		if err != nil {
+			return fmt.Errorf("failed to create issuer URL: %w\n", err)
+		}
+		jwt, err := auth.NewJwtValidator(ctx, jwksUrl.String(), issUrl.String(), cfg.Identity.Server.Audience)
 		if err != nil {
 			return fmt.Errorf("failed to fetch and cache identity provider JWKS: %w\n", err)
 		}

config/server-config.yaml.example

@@ -46,7 +46,8 @@ database:
 
 identity:
   server:
-    issuer_url: http://keycloak:8080 # Use http://localhost:8081 instead for running minder outside of docker compose
+    issuer_url: http://localhost:8081
+    issuer_claim: http://localhost:8081/realms/stacklok
     client_id: minder-server
     client_secret: secret
     audience: minder

docker-compose.yaml

@@ -78,6 +78,7 @@ services:
         condition: service_completed_successfully
       keycloak-config:
         condition: service_completed_successfully
+
   migrate:
     container_name: minder_migrate_up
     build:

internal/auth/jwauth_test.go

@@ -50,6 +50,8 @@ func TestParseAndValidate(t *testing.T) {
 	err = jwks.AddKey(publicJwk)
 	require.NoError(t, err, "failed to setup JWK set")
 
+	issUrl := "https://localhost/realm/foo"
+
 	testCases := []struct {
 		name       string
 		buildToken func() string
@@ -58,7 +60,7 @@ func TestParseAndValidate(t *testing.T) {
 		{
 			name: "Valid token",
 			buildToken: func() string {
-				token, _ := jwt.NewBuilder().Subject("123").Audience([]string{"minder"}).Expiration(time.Now().Add(time.Duration(1) * time.Minute)).Build()
+				token, _ := jwtBuilder("123", issUrl, "minder").Expiration(time.Now().Add(time.Duration(1) * time.Minute)).Build()
 				signed, _ := jwt.Sign(token, jwt.WithKey(jwa.RS256, privateJwk))
 				return string(signed)
 			},
@@ -71,7 +73,7 @@ func TestParseAndValidate(t *testing.T) {
 		{
 			name: "Expired token",
 			buildToken: func() string {
-				token, _ := jwt.NewBuilder().Subject("123").Audience([]string{"minder"}).Expiration(time.Now().Add(-time.Duration(1) * time.Minute)).Build()
+				token, _ := jwtBuilder("123", issUrl, "minder").Expiration(time.Now().Add(-time.Duration(1) * time.Minute)).Build()
 				signed, _ := jwt.Sign(token, jwt.WithKey(jwa.RS256, privateJwk))
 				return string(signed)
 			},
@@ -88,7 +90,7 @@ func TestParseAndValidate(t *testing.T) {
 				otherJwk, _ := jwk.FromRaw(otherKey)
 				err = otherJwk.Set(jwk.KeyIDKey, `otherKey`)
 				require.NoError(t, err, "failed to setup signing key ID")
-				token, _ := jwt.NewBuilder().Subject("123").Expiration(time.Now().Add(time.Duration(1) * time.Minute)).Build()
+				token, _ := jwtBuilder("123", issUrl, "minder").Expiration(time.Now().Add(time.Duration(1) * time.Minute)).Build()
 				signed, _ := jwt.Sign(token, jwt.WithKey(jwa.RS256, otherJwk))
 				return string(signed)
 			},
@@ -112,7 +114,20 @@ func TestParseAndValidate(t *testing.T) {
 		{
 			name: "Missing subject claim",
 			buildToken: func() string {
-				token, _ := jwt.NewBuilder().Expiration(time.Now().Add(-time.Duration(1) * time.Minute)).Build()
+				token, _ := jwtBuilder("", issUrl, "minder").Expiration(time.Now().Add(-time.Duration(1) * time.Minute)).Build()
+				signed, _ := jwt.Sign(token, jwt.WithKey(jwa.RS256, privateJwk))
+				return string(signed)
+			},
+			checkError: func(t *testing.T, err error) {
+				t.Helper()
+
+				assert.Error(t, err)
+			},
+		},
+		{
+			name: "Missing issuer claim",
+			buildToken: func() string {
+				token, _ := jwtBuilder("123", "", "minder").Expiration(time.Now().Add(-time.Duration(1) * time.Minute)).Build()
 				signed, _ := jwt.Sign(token, jwt.WithKey(jwa.RS256, privateJwk))
 				return string(signed)
 			},
@@ -125,7 +140,7 @@ func TestParseAndValidate(t *testing.T) {
 		{
 			name: "Missing audience claim",
 			buildToken: func() string {
-				token, _ := jwt.NewBuilder().Subject("123").Expiration(time.Now().Add(-time.Duration(1) * time.Minute)).Build()
+				token, _ := jwtBuilder("123", issUrl, "").Expiration(time.Now().Add(-time.Duration(1) * time.Minute)).Build()
 				signed, _ := jwt.Sign(token, jwt.WithKey(jwa.RS256, privateJwk))
 				return string(signed)
 			},
@@ -149,6 +164,7 @@ func TestParseAndValidate(t *testing.T) {
 
 			jwtValidator := JwkSetJwtValidator{
 				jwksFetcher: mockKeyFetcher,
+				iss:         issUrl,
 				aud:         "minder",
 			}
 			_, err := jwtValidator.ParseAndValidate(tc.buildToken())
@@ -167,3 +183,19 @@ func randomKeypair(length int) (*rsa.PrivateKey, *rsa.PublicKey) {
 
 	return privateKey, publicKey
 }
+
+func jwtBuilder(sub, iss, aud string) *jwt.Builder {
+	r := jwt.NewBuilder()
+
+	if sub != "" {
+		r = r.Subject(sub)
+	}
+	if iss != "" {
+		r = r.Issuer(iss)
+	}
+	if aud != "" {
+		r = r.Audience([]string{aud})
+	}
+
+	return r
+}

internal/auth/jwtauth.go

@@ -34,6 +34,7 @@ type JwtValidator interface {
 // JwkSetJwtValidator is a JWT validator that uses a JWK set URL to validate the tokens
 type JwkSetJwtValidator struct {
 	jwksFetcher KeySetFetcher
+	iss         string
 	aud         string
 }
 
@@ -64,6 +65,7 @@ func (j *JwkSetJwtValidator) ParseAndValidate(tokenString string) (openid.Token,
 	token, err := jwt.ParseString(
 		tokenString,
 		jwt.WithKeySet(set),
+		jwt.WithIssuer(j.iss),
 		jwt.WithValidate(true),
 		jwt.WithToken(openid.New()),
 		jwt.WithAudience(j.aud))
@@ -84,7 +86,7 @@ func (j *JwkSetJwtValidator) ParseAndValidate(tokenString string) (openid.Token,
 }
 
 // NewJwtValidator creates a new JWT validator that uses a JWK set URL to validate the tokens
-func NewJwtValidator(ctx context.Context, jwksUrl string, aud string) (JwtValidator, error) {
+func NewJwtValidator(ctx context.Context, jwksUrl string, issUrl string, aud string) (JwtValidator, error) {
 	// Cache the JWK set
 	// The cache will refresh every 15 minutes by default
 	jwks := jwk.NewCache(ctx)
@@ -106,6 +108,7 @@ func NewJwtValidator(ctx context.Context, jwksUrl string, aud string) (JwtValida
 	}
 	return &JwkSetJwtValidator{
 		jwksFetcher: &keySetCache,
+		iss:         issUrl,
 		aud:         aud,
 	}, nil
 }

internal/config/server/identity.go

@@ -36,8 +36,12 @@ type IdentityConfigWrapper struct {
 
 // IdentityConfig is the configuration for the identity provider in minder server
 type IdentityConfig struct {
-	// IssuerUrl is the base URL where the identity server is running
+	// IssuerUrl is the base URL for calling APIs on the identity server.  Note that this URL
+	// ised for direct communication with the identity server, and is not the URL that
+	// is included in the JWT tokens.  It is named 'issuer_url' for historical compatibility.
 	IssuerUrl string `mapstructure:"issuer_url" default:"http://localhost:8081"`
+	// IssuerClaim is the claim in the JWT token that identifies the issuer
+	IssuerClaim string `mapstructure:"issuer_claim" default:"http://localhost:8081/realms/stacklok"`
 	// ClientId is the client ID that identifies the minder server
 	ClientId string `mapstructure:"client_id" default:"minder-server"`
 	// ClientSecret is the client secret for the minder server
@@ -59,6 +63,15 @@ func RegisterIdentityFlags(v *viper.Viper, flags *pflag.FlagSet) error {
 		"The base URL where the identity server is running", flags.String)
 }
 
+// JwtUrl returns the base `iss` claim as a URL.
+func (sic *IdentityConfig) JwtUrl(elem ...string) (*url.URL, error) {
+	parsedUrl, err := url.Parse(sic.IssuerClaim)
+	if err != nil {
+		return nil, err
+	}
+	return parsedUrl.JoinPath(elem...), nil
+}
+
 // Path returns a URL for the given path on the identity server
 func (sic *IdentityConfig) Path(path string) (*url.URL, error) {
 	parsedUrl, err := url.Parse(sic.IssuerUrl)