Address feedback from eleftherias

cmd/server/app/serve.go

@@ -96,7 +96,9 @@ var serveCmd = &cobra.Command{
 		if err != nil {
 			return fmt.Errorf("failed to fetch and cache identity provider JWKS: %w\n", err)
 		}
-		dynamicJwt := dynamic.NewDynamicValidator(ctx, cfg.Identity.Server.Audience)
+		allowedIssuers := []string{issUrl.String()}
+		allowedIssuers = append(allowedIssuers, cfg.Identity.AdditionalIssuers...)
+		dynamicJwt := dynamic.NewDynamicValidator(ctx, cfg.Identity.Server.Audience, allowedIssuers)
 		jwt := merged.Validator{Validators: []jwt.Validator{staticJwt, dynamicJwt}}
 
 		authzc, err := authz.NewAuthzClient(&cfg.Authz, l)

internal/auth/context.go

@@ -1,4 +1,4 @@
-// SPDX-FileCopyrightText: Copyright 2023 The Minder Authors
+// SPDX-FileCopyrightText: Copyright 2025 The Minder Authors
 // SPDX-License-Identifier: Apache-2.0
 
 package auth

internal/auth/githubactions/githubactions.go

@@ -1,17 +1,5 @@
-//
-// Copyright 2024 Stacklok, Inc.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
+// SPDX-FileCopyrightText: Copyright 2025 The Minder Authors
+// SPDX-License-Identifier: Apache-2.0
 
 // Package githubactions provides an implementation of the GitHub IdentityProvider.
 package githubactions

internal/auth/githubactions/githubactions_test.go

@@ -1,17 +1,5 @@
-//
-// Copyright 2024 Stacklok, Inc.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
+// SPDX-FileCopyrightText: Copyright 2025 The Minder Authors
+// SPDX-License-Identifier: Apache-2.0
 
 // Package githubactions provides an implementation of the GitHub IdentityProvider.
 package githubactions

internal/auth/jwt/dynamic/dynamic_fetch.go

@@ -1,20 +1,8 @@
-//
-// Copyright 2024 Stacklok, Inc.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
+// SPDX-FileCopyrightText: Copyright 2025 The Minder Authors
+// SPDX-License-Identifier: Apache-2.0
 
 // Package dynamic provides the logic for reading and validating JWT tokens
-// using a JWKS URL from the token's
+// using a JWKS URL from the token's `iss` claim.
 package dynamic
 
 import (
@@ -24,6 +12,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"slices"
 	"strings"
 	"sync"
 	"time"
@@ -45,19 +34,21 @@ type openIdConfig struct {
 }
 
 var cachedIssuers metric.Int64Counter
+var deniedIssuers metric.Int64Counter
 var dynamicAuths metric.Int64Counter
 var metricsInit sync.Once
 
 // Validator dynamically validates JWTs by fetching the key from the well-known OIDC issuer URL.
 type Validator struct {
-	jwks *jwk.Cache
-	aud  string
+	jwks           *jwk.Cache
+	aud            string
+	allowedIssuers []string
 }
 
 var _ stacklok_jwt.Validator = (*Validator)(nil)
 
 // NewDynamicValidator creates a new instance of the dynamic JWT validator
-func NewDynamicValidator(ctx context.Context, aud string) *Validator {
+func NewDynamicValidator(ctx context.Context, aud string, issuers []string) *Validator {
 	metricsInit.Do(func() {
 		meter := otel.Meter("minder")
 		var err error
@@ -68,6 +59,13 @@ func NewDynamicValidator(ctx context.Context, aud string) *Validator {
 		if err != nil {
 			zerolog.Ctx(context.Background()).Warn().Err(err).Msg("Creating gauge for cached issuers failed")
 		}
+		deniedIssuers, err = meter.Int64Counter("dynamic_jwt.denied_issuers",
+			metric.WithDescription("Number of denied issuers for dynamic JWT validation"),
+			metric.WithUnit("count"),
+		)
+		if err != nil {
+			zerolog.Ctx(context.Background()).Warn().Err(err).Msg("Creating gauge for denied issuers failed")
+		}
 		dynamicAuths, err = meter.Int64Counter("dynamic_jwt.auths",
 			metric.WithDescription("Number of dynamic JWT authentications"),
 			metric.WithUnit("count"),
@@ -77,8 +75,9 @@ func NewDynamicValidator(ctx context.Context, aud string) *Validator {
 		}
 	})
 	return &Validator{
-		jwks: jwk.NewCache(ctx),
-		aud:  aud,
+		jwks:           jwk.NewCache(ctx),
+		aud:            aud,
+		allowedIssuers: issuers,
 	}
 }
 
@@ -122,6 +121,12 @@ func (m Validator) ParseAndValidate(tokenString string) (openid.Token, error) {
 }
 
 func (m Validator) getKeySet(issuer string) (jwk.Set, error) {
+	if !slices.Contains(m.allowedIssuers, issuer) {
+		if deniedIssuers != nil {
+			deniedIssuers.Add(context.Background(), 1)
+		}
+		return nil, fmt.Errorf("issuer %s is not allowed", issuer)
+	}
 	jwksUrl, err := getJWKSUrlForOpenId(issuer)
 	if err != nil {
 		return nil, fmt.Errorf("failed to fetch JWKS URL from openid: %w", err)

internal/auth/jwt/dynamic/dynamic_fetch_test.go

@@ -1,20 +1,8 @@
-//
-// Copyright 2024 Stacklok, Inc.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
+// SPDX-FileCopyrightText: Copyright 2025 The Minder Authors
+// SPDX-License-Identifier: Apache-2.0
 
 // Package dynamic provides the logic for reading and validating JWT tokens
-// using a JWKS URL from the token's
+// using a JWKS URL from the token's `iss` claim.
 package dynamic
 
 import (
@@ -95,10 +83,12 @@ func TestValidator_ParseAndValidate(t *testing.T) {
 
 	tests := []struct {
 		name     string
+		issuers  []string
 		getToken func(t *testing.T) (string, openid.Token)
 		wantErr  string
 	}{{
 		name: "valid token",
+		issuers: []string{server.URL},
 		getToken: func(t *testing.T) (string, openid.Token) {
 			t.Helper()
 			token, err := openid.NewBuilder().
@@ -115,6 +105,7 @@ func TestValidator_ParseAndValidate(t *testing.T) {
 		},
 	}, {
 		name: "valid token, other issuer",
+		issuers: []string{server.URL + "/other"},
 		getToken: func(t *testing.T) (string, openid.Token) {
 			t.Helper()
 			token, err := openid.NewBuilder().
@@ -131,17 +122,19 @@ func TestValidator_ParseAndValidate(t *testing.T) {
 		},
 	}, {
 		name: "invalid signature",
+		issuers: []string{server.URL},
 		getToken: func(_ *testing.T) (string, openid.Token) {
 			t.Helper()
 			return "invalid", nil
 		},
 		wantErr: `failed to split compact JWT: invalid number of segments`,
 	}, {
 		name: "expired jwt",
+		issuers: []string{server.URL},
 		getToken: func(_ *testing.T) (string, openid.Token) {
 			t.Helper()
 			token, err := openid.NewBuilder().
-				Issuer(server.URL + "/elsewhere").
+				Issuer(server.URL).
 				Subject("test").
 				Audience([]string{"minder"}).
 				Expiration(time.Now().Add(-1 * time.Minute)).
@@ -155,6 +148,7 @@ func TestValidator_ParseAndValidate(t *testing.T) {
 		wantErr: `failed to parse JWT payload: "exp" not satisfied`,
 	}, {
 		name: "bad well-known URL",
+		issuers: []string{server.URL, server.URL + "/elsewhere"},
 		getToken: func(t *testing.T) (string, openid.Token) {
 			t.Helper()
 			token, err := openid.NewBuilder().
@@ -172,6 +166,7 @@ func TestValidator_ParseAndValidate(t *testing.T) {
 		wantErr: `non-200 response code "404 Not Found"`,
 	}, {
 		name: "bad issuer",
+		issuers: []string{server.URL, server.URL + "/nothing"},
 		getToken: func(t *testing.T) (string, openid.Token) {
 			t.Helper()
 			token, err := openid.NewBuilder().
@@ -187,12 +182,30 @@ func TestValidator_ParseAndValidate(t *testing.T) {
 			return string(signed), token
 		},
 		wantErr: `failed to fetch JWKS URL`,
+	}, {
+		name: "prohibited issuer",
+		issuers: []string{server.URL},
+		getToken: func(t *testing.T) (string, openid.Token) {
+			t.Helper()
+			token, err := openid.NewBuilder().
+				Issuer(server.URL + "/nothing").
+				Subject("test").
+				Audience([]string{"minder"}).
+				Expiration(time.Now().Add(time.Minute)).
+				IssuedAt(time.Now()).
+				Build()
+			require.NoError(t, err)
+			signed, err := jwt.Sign(token, jwt.WithKey(jwa.RS256, jwkKey))
+			require.NoError(t, err)
+			return string(signed), token
+		},
+		wantErr: `is not allowed`,
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 			ctx := context.Background()
-			validator := NewDynamicValidator(ctx, "minder")
+			validator := NewDynamicValidator(ctx, "minder", tt.issuers)
 			token, want := tt.getToken(t)
 
 			got, err := validator.ParseAndValidate(string(token))

internal/auth/jwt/merged/merged_jwt.go

@@ -1,17 +1,5 @@
-//
-// Copyright 2024 Stacklok, Inc.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
+// SPDX-FileCopyrightText: Copyright 2025 The Minder Authors
+// SPDX-License-Identifier: Apache-2.0
 
 // Package merged provides the logic for reading and validating JWT tokens
 package merged

internal/controlplane/handlers_authz.go

@@ -467,7 +467,7 @@ func (s *Server) UpdateRole(ctx context.Context, req *minder.UpdateRoleRequest)
 // isUserSelfUpdating is used to prevent if the user is trying to update their own role
 func isUserSelfUpdating(ctx context.Context, subject, inviteeEmail string) error {
 	if subject != "" {
-		if auth.IdentityFromContext(ctx).Human() == subject {
+		if auth.IdentityFromContext(ctx).String() == subject {
 			return util.UserVisibleError(codes.InvalidArgument, "cannot update your own role")
 		}
 	}

pkg/config/server/identity.go

@@ -19,7 +19,8 @@ import (
 
 // IdentityConfigWrapper is the configuration for the identity provider
 type IdentityConfigWrapper struct {
-	Server IdentityConfig `mapstructure:"server"`
+	Server            IdentityConfig `mapstructure:"server"`
+	AdditionalIssuers []string       `mapstructure:"additional_issuers"`
 }
 
 // IdentityConfig is the configuration for the identity provider in minder server