feat(proxy): add optional allow list of OIDC client ids for access tokens

pkg/oidc/client.go

@@ -8,6 +8,7 @@ import (
 	"io"
 	"mime"
 	"net/http"
+	"slices"
 	"strconv"
 	"strings"
 	"sync"
@@ -58,6 +59,7 @@ type oidcClient struct {
 	skipIssuerValidation    bool
 	accessTokenVerifyMethod string
 	accessTokenAudiences    []string
+	accessTokenClientIDs    []string
 	remoteKeySet            KeySet
 	algorithms              []string
 
@@ -95,13 +97,22 @@ func NewOIDCClient(opts ...Option) (OIDCClient, error) {
 			return nil, errors.New("access token audiences must not contain empty or whitespace-only entries")
 		}
 	}
+	if len(options.AccessTokenClientIDs) > 0 && options.AccessTokenVerifyMethod != config.AccessTokenVerificationJWT {
+		return nil, errors.New("access token client id validation requires the jwt verification method")
+	}
+	for _, clientID := range options.AccessTokenClientIDs {
+		if strings.TrimSpace(clientID) == "" {
+			return nil, errors.New("access token client ids must not contain empty or whitespace-only entries")
+		}
+	}
 
 	return &oidcClient{
 		Logger:                  options.Logger,
 		issuer:                  options.OIDCIssuer,
 		httpClient:              options.HTTPClient,
 		accessTokenVerifyMethod: options.AccessTokenVerifyMethod,
 		accessTokenAudiences:    options.AccessTokenAudiences,
+		accessTokenClientIDs:    options.AccessTokenClientIDs,
 		JWKSOptions:             options.JWKSOptions, // TODO I don't like that we pass down config options ...
 		JWKS:                    options.JWKS,
 		providerLock:            &sync.Mutex{},
@@ -323,10 +334,50 @@ func (c *oidcClient) verifyAccessTokenJWT(token string) (RegClaimsWithSID, jwt.M
 		c.Logger.Info().Err(err).Msg("Failed to parse/verify the access token.")
 		return claims, mapClaims, err
 	}
+	if err := c.verifyClientID(mapClaims); err != nil {
+		c.Logger.Info().Err(err).Msg("Rejected the access token's client.")
+		return RegClaimsWithSID{}, jwt.MapClaims{}, err
+	}
 
 	return claims, mapClaims, nil
 }
 
+// ErrTokenInvalidClientID is returned when an access token was issued to a
+// client that is not in the configured list of allowed client ids.
+var ErrTokenInvalidClientID = errors.New("token was issued to a client that is not allowed")
+
+// verifyClientID checks the client the access token was issued to against the
+// configured allow list. Audiences cannot do this: a client can usually ask for
+// any audience, so a token minted through another client of the same IDP, with
+// that client's weaker login policy (MFA, allowed users, networks), carries the
+// expected aud just as well. The client id is assigned by the IDP.
+//
+// The client is read from client_id (RFC 9068, JWT access tokens) and, if that
+// is absent, from azp (OpenID Connect, e.g. Keycloak).
+func (c *oidcClient) verifyClientID(claims jwt.MapClaims) error {
+	if len(c.accessTokenClientIDs) == 0 {
+		return nil
+	}
+	raw, ok := claims["client_id"]
+	if !ok {
+		raw, ok = claims["azp"]
+	}
+	if !ok || raw == nil {
+		return fmt.Errorf("%w: client_id/azp", jwt.ErrTokenRequiredClaimMissing)
+	}
+	clientID, ok := raw.(string)
+	if !ok {
+		return fmt.Errorf("%w: client_id/azp is not a string", jwt.ErrTokenMalformed)
+	}
+	if clientID == "" {
+		return fmt.Errorf("%w: client_id/azp", jwt.ErrTokenRequiredClaimMissing)
+	}
+	if !slices.Contains(c.accessTokenClientIDs, clientID) {
+		return ErrTokenInvalidClientID
+	}
+	return nil
+}
+
 func (c *oidcClient) VerifyLogoutToken(ctx context.Context, rawToken string) (*LogoutToken, error) {
 	var claims LogoutToken
 	if err := c.lookupWellKnownOpenidConfiguration(ctx); err != nil {

pkg/oidc/client_id_test.go

@@ -0,0 +1,148 @@
+package oidc_test
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/golang-jwt/jwt/v5"
+	"github.com/opencloud-eu/opencloud/pkg/log"
+	"github.com/opencloud-eu/opencloud/pkg/oidc"
+	"github.com/opencloud-eu/opencloud/services/proxy/pkg/config"
+	"github.com/stretchr/testify/require"
+)
+
+func TestAccessTokenClientIDs(t *testing.T) {
+	key := newRSAKey(t)
+	tests := []struct {
+		name      string
+		clientIDs []string
+		claims    jwt.MapClaims
+		wantErr   error
+	}{
+		{name: "disabled accepts any client", claims: jwt.MapClaims{"client_id": "other"}},
+		{name: "disabled accepts no client", claims: jwt.MapClaims{}},
+		{name: "explicitly empty configuration", clientIDs: []string{}, claims: jwt.MapClaims{"azp": "other"}},
+		{name: "client_id", clientIDs: []string{"web"}, claims: jwt.MapClaims{"client_id": "web"}},
+		{name: "azp", clientIDs: []string{"web"}, claims: jwt.MapClaims{"azp": "web"}},
+		{name: "any allowed client", clientIDs: []string{"web", "desktop"}, claims: jwt.MapClaims{"client_id": "desktop"}},
+		{name: "client_id wins over azp", clientIDs: []string{"web"}, claims: jwt.MapClaims{"client_id": "web", "azp": "other"}},
+		{name: "client_id not allowed despite allowed azp", clientIDs: []string{"web"}, claims: jwt.MapClaims{"client_id": "other", "azp": "web"}, wantErr: oidc.ErrTokenInvalidClientID},
+		{name: "foreign client", clientIDs: []string{"web"}, claims: jwt.MapClaims{"client_id": "other"}, wantErr: oidc.ErrTokenInvalidClientID},
+		{name: "foreign azp", clientIDs: []string{"web"}, claims: jwt.MapClaims{"azp": "other"}, wantErr: oidc.ErrTokenInvalidClientID},
+		{name: "allowed audience does not help", clientIDs: []string{"web"}, claims: jwt.MapClaims{"aud": "web", "azp": "other"}, wantErr: oidc.ErrTokenInvalidClientID},
+		{name: "case sensitive", clientIDs: []string{"web"}, claims: jwt.MapClaims{"client_id": "Web"}, wantErr: oidc.ErrTokenInvalidClientID},
+		{name: "exact match", clientIDs: []string{"web"}, claims: jwt.MapClaims{"client_id": "web-legacy"}, wantErr: oidc.ErrTokenInvalidClientID},
+		{name: "no wildcard matching", clientIDs: []string{"*"}, claims: jwt.MapClaims{"client_id": "web"}, wantErr: oidc.ErrTokenInvalidClientID},
+		{name: "missing", clientIDs: []string{"web"}, claims: jwt.MapClaims{}, wantErr: jwt.ErrTokenRequiredClaimMissing},
+		{name: "null", clientIDs: []string{"web"}, claims: jwt.MapClaims{"client_id": nil}, wantErr: jwt.ErrTokenRequiredClaimMissing},
+		{name: "empty", clientIDs: []string{"web"}, claims: jwt.MapClaims{"client_id": ""}, wantErr: jwt.ErrTokenRequiredClaimMissing},
+		{name: "number", clientIDs: []string{"web"}, claims: jwt.MapClaims{"client_id": 123}, wantErr: jwt.ErrTokenMalformed},
+		{name: "array", clientIDs: []string{"web"}, claims: jwt.MapClaims{"azp": []string{"web"}}, wantErr: jwt.ErrTokenMalformed},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			claims := jwt.MapClaims{
+				"iss": "https://issuer.example",
+				"sub": "alice",
+				"exp": time.Now().Add(time.Hour).Unix(),
+			}
+			for k, v := range tt.claims {
+				claims[k] = v
+			}
+			client := newClientIDTestClient(t, key, tt.clientIDs)
+			registered, all, err := client.VerifyAccessToken(context.Background(), signAccessToken(t, key, claims))
+			if tt.wantErr != nil {
+				require.ErrorIs(t, err, tt.wantErr)
+				require.Empty(t, all, "claims of a rejected token must not be returned")
+				require.Empty(t, registered.Subject, "claims of a rejected token must not be returned")
+				return
+			}
+			require.NoError(t, err)
+			require.Equal(t, "alice", registered.Subject)
+			require.Equal(t, "alice", all["sub"])
+		})
+	}
+}
+
+func TestAccessTokenClientIDsWithAudiences(t *testing.T) {
+	key := newRSAKey(t)
+	client, err := oidc.NewOIDCClient(
+		oidc.WithLogger(log.NopLogger()),
+		oidc.WithOidcIssuer("https://issuer.example"),
+		oidc.WithAccessTokenVerifyMethod(config.AccessTokenVerificationJWT),
+		oidc.WithAccessTokenAudiences([]string{"opencloud"}),
+		oidc.WithAccessTokenClientIDs([]string{"web"}),
+		oidc.WithJWKS(key.jwks),
+		oidc.WithProviderMetadata(&oidc.ProviderMetadata{}),
+	)
+	require.NoError(t, err)
+	verify := func(claims jwt.MapClaims) error {
+		claims["iss"] = "https://issuer.example"
+		_, _, err := client.VerifyAccessToken(context.Background(), signAccessToken(t, key, claims))
+		return err
+	}
+	require.NoError(t, verify(jwt.MapClaims{"aud": "opencloud", "azp": "web"}))
+	require.ErrorIs(t, verify(jwt.MapClaims{"aud": "opencloud", "azp": "other"}), oidc.ErrTokenInvalidClientID)
+	require.ErrorIs(t, verify(jwt.MapClaims{"aud": "other", "azp": "web"}), jwt.ErrTokenInvalidAudience)
+}
+
+func TestAccessTokenClientIDConfiguration(t *testing.T) {
+	for _, method := range []string{config.AccessTokenVerificationNone, ""} {
+		t.Run("incompatible method "+method, func(t *testing.T) {
+			// No HTTP client is supplied: invalid configuration must fail before discovery.
+			client, err := oidc.NewOIDCClient(
+				oidc.WithAccessTokenVerifyMethod(method),
+				oidc.WithAccessTokenClientIDs([]string{"web"}),
+			)
+			require.ErrorContains(t, err, "requires the jwt verification method")
+			require.Nil(t, client)
+		})
+	}
+	for _, clientIDs := range [][]string{{""}, {" \t"}, {"web", ""}} {
+		client, err := oidc.NewOIDCClient(
+			oidc.WithAccessTokenVerifyMethod(config.AccessTokenVerificationJWT),
+			oidc.WithAccessTokenClientIDs(clientIDs),
+		)
+		require.ErrorContains(t, err, "empty or whitespace-only")
+		require.Nil(t, client)
+	}
+	t.Run("caller cannot mutate the policy", func(t *testing.T) {
+		key := newRSAKey(t)
+		clientIDs := []string{"web"}
+		client := newClientIDTestClient(t, key, clientIDs)
+		clientIDs[0] = "other"
+		_, _, err := client.VerifyAccessToken(context.Background(), signAccessToken(t, key,
+			jwt.MapClaims{"iss": "https://issuer.example", "client_id": "other"}))
+		require.ErrorIs(t, err, oidc.ErrTokenInvalidClientID)
+	})
+}
+
+func TestAccessTokenClientIDsDoNotApplyToLogoutTokens(t *testing.T) {
+	key := newRSAKey(t)
+	client := newClientIDTestClient(t, key, []string{"web"})
+	token := signAccessToken(t, key, jwt.MapClaims{
+		"iss": "https://issuer.example",
+		"sub": "alice",
+		"aud": "other",
+		"events": map[string]any{
+			"http://schemas.openid.net/event/backchannel-logout": map[string]any{},
+		},
+	})
+	_, err := client.VerifyLogoutToken(context.Background(), token)
+	require.NoError(t, err)
+}
+
+func newClientIDTestClient(t *testing.T, key *signingKey, clientIDs []string) oidc.OIDCClient {
+	t.Helper()
+	client, err := oidc.NewOIDCClient(
+		oidc.WithLogger(log.NopLogger()),
+		oidc.WithOidcIssuer("https://issuer.example"),
+		oidc.WithAccessTokenVerifyMethod(config.AccessTokenVerificationJWT),
+		oidc.WithAccessTokenClientIDs(clientIDs),
+		oidc.WithJWKS(key.jwks),
+		oidc.WithProviderMetadata(&oidc.ProviderMetadata{}),
+	)
+	require.NoError(t, err)
+	return client
+}

pkg/oidc/options.go

@@ -38,6 +38,9 @@ type Options struct {
 	// AccessTokenAudiences requires at least one matching audience in access tokens.
 	// An empty list disables audience validation.
 	AccessTokenAudiences []string
+	// AccessTokenClientIDs requires the client an access token was issued to
+	// (client_id, else azp) to be one of these. An empty list disables the check.
+	AccessTokenClientIDs []string
 	// Config to use
 	Config *goidc.Config
 
@@ -85,6 +88,14 @@ func WithAccessTokenAudiences(val []string) Option {
 	}
 }
 
+// WithAccessTokenClientIDs sets the clients whose access tokens are accepted.
+// An empty list disables client validation.
+func WithAccessTokenClientIDs(val []string) Option {
+	return func(o *Options) {
+		o.AccessTokenClientIDs = append([]string(nil), val...)
+	}
+}
+
 // WithHTTPClient provides a function to set the httpClient option.
 func WithHTTPClient(val *http.Client) Option {
 	return func(o *Options) {

services/proxy/README.md

@@ -79,6 +79,42 @@ The disabled default is a compatibility decision. It does not relax the
 a resource server following that JWT access token profile must reject tokens
 whose audience does not identify the resource server.
 
+### OIDC Allowed Client IDs
+
+An audience check alone does not keep out tokens minted through *other clients*
+of the same identity provider. Many IDPs let a client request any audience, and
+with dynamic client registration anyone can create such a client. Identity
+providers attach their login policy to the client: who may sign in, MFA,
+allowed networks and so on. A token obtained through a weaker client carries
+the expected audience just as well and bypasses that policy.
+
+To accept only tokens issued to your OpenCloud clients, list their client ids:
+
+```console
+PROXY_OIDC_ALLOWED_CLIENT_IDS=web,OpenCloudDesktop,OpenCloudAndroid,OpenCloudIOS
+PROXY_OIDC_ACCESS_TOKEN_VERIFY_METHOD=jwt
+```
+
+or in `proxy.yaml`:
+
+```yaml
+oidc:
+  allowed_client_ids:
+    - web
+    - OpenCloudDesktop
+  access_token_verify_method: jwt
+```
+
+The client is read from the access token's `client_id` claim
+([RFC 9068](https://www.rfc-editor.org/rfc/rfc9068.html#name-data-structure)) and,
+if that is absent, from `azp` (OpenID Connect; Keycloak, for example). The value
+must match one entry exactly and case-sensitively. Tokens without a client, with
+a malformed one or with a client that is not listed receive HTTP 401 on
+protected routes. Like audiences, this requires JWT verification, entries must
+not be blank, an empty list disables the check, and it is evaluated on Userinfo
+cache misses. The check works together with `PROXY_OIDC_AUDIENCES`; both must
+pass when both are configured.
+
 ## Configuring Routes
 
 The proxy handles routing to all endpoints that OpenCloud offers. The currently availabe default routes can be found [in the code](https://github.com/opencloud-eu/opencloud/blob/main/services/proxy/pkg/config/defaults/defaultconfig.go). Changing or adding routes can be necessary when writing own OpenCloud extensions.

services/proxy/pkg/command/server.go

@@ -265,6 +265,7 @@ func newOIDCAuthenticator(logger log.Logger, cfg *config.Config, userInfoCache m
 	oidcClient, err := oidc.NewOIDCClient(
 		oidc.WithAccessTokenVerifyMethod(cfg.OIDC.AccessTokenVerifyMethod),
 		oidc.WithAccessTokenAudiences(cfg.OIDC.Audiences),
+		oidc.WithAccessTokenClientIDs(cfg.OIDC.AllowedClientIDs),
 		oidc.WithLogger(logger),
 		oidc.WithHTTPClient(httpClient),
 		oidc.WithOidcIssuer(cfg.OIDC.Issuer),

services/proxy/pkg/config/config.go

@@ -118,6 +118,7 @@ const (
 // with the configured oidc-provider
 type OIDC struct {
 	Audiences               []string `yaml:"audiences" env:"PROXY_OIDC_AUDIENCES" desc:"Optional comma-separated list of allowed audiences for OIDC access tokens. Empty disables audience validation for compatibility. Configuring audiences is recommended for production and requires PROXY_OIDC_ACCESS_TOKEN_VERIFY_METHOD=jwt. Tokens must contain at least one exactly matching, case-sensitive audience in their aud claim." introductionVersion:"8.1.0"`
+	AllowedClientIDs        []string `yaml:"allowed_client_ids" env:"PROXY_OIDC_ALLOWED_CLIENT_IDS" desc:"Optional comma-separated list of OIDC client ids whose access tokens are accepted. The client is read from the token's client_id claim, or azp if client_id is absent. Empty disables client validation for compatibility. Requires PROXY_OIDC_ACCESS_TOKEN_VERIFY_METHOD=jwt. Unlike audiences, the client id is assigned by the IDP, so this keeps tokens minted through other clients of the same IDP (with their own login policies) out." introductionVersion:"%%NEXT%%"`
 	Issuer                  string   `yaml:"issuer" env:"OC_URL;OC_OIDC_ISSUER;PROXY_OIDC_ISSUER" desc:"URL of the OIDC issuer. It defaults to URL of the builtin IDP." introductionVersion:"1.0.0"`
 	Insecure                bool     `yaml:"insecure" env:"OC_INSECURE;PROXY_OIDC_INSECURE" desc:"Disable TLS certificate validation for connections to the IDP. Note that this is not recommended for production environments." introductionVersion:"1.0.0"`
 	AccessTokenVerifyMethod string   `yaml:"access_token_verify_method" env:"PROXY_OIDC_ACCESS_TOKEN_VERIFY_METHOD" desc:"Sets how OIDC access tokens should be verified. Possible values are 'none' and 'jwt'. When using 'none', no special validation apart from using it for accessing the IDP's userinfo endpoint will be done. When using 'jwt', it tries to parse the access token as a jwt token and verifies the signature using the keys published on the IDP's 'jwks_uri'." introductionVersion:"1.0.0"`

services/proxy/pkg/config/parser/parse.go

@@ -65,6 +65,14 @@ func Validate(cfg *config.Config) error {
 			return fmt.Errorf("OIDC audiences (PROXY_OIDC_AUDIENCES) in service %s must not contain empty or whitespace-only entries", cfg.Service.Name)
 		}
 	}
+	if len(cfg.OIDC.AllowedClientIDs) > 0 && cfg.OIDC.AccessTokenVerifyMethod != config.AccessTokenVerificationJWT {
+		return fmt.Errorf("OIDC allowed client ids (PROXY_OIDC_ALLOWED_CLIENT_IDS) in service %s require access_token_verify_method to be 'jwt'", cfg.Service.Name)
+	}
+	for _, clientID := range cfg.OIDC.AllowedClientIDs {
+		if strings.TrimSpace(clientID) == "" {
+			return fmt.Errorf("OIDC allowed client ids (PROXY_OIDC_ALLOWED_CLIENT_IDS) in service %s must not contain empty or whitespace-only entries", cfg.Service.Name)
+		}
+	}
 
 	if cfg.ServiceAccount.ServiceAccountID == "" {
 		return shared.MissingServiceAccountID(cfg.Service.Name)