From 295ca2c7f6f0cda395fb17b606b91abcfd574fe0 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Mon, 7 Sep 2026 21:25:18 +0300 Subject: [PATCH] Restrict embedded IdP connectors to a minimal grant allowlist --- idp/dex/config.go | 29 ++++++++----- idp/dex/connector.go | 52 +++++++++++++++++++--- idp/dex/connector_test.go | 91 +++++++++++++++++++++++++++++++++++++++ idp/dex/provider.go | 5 ++- idp/dex/provider_test.go | 55 +++++++++++++++++++---- 5 files changed, 207 insertions(+), 25 deletions(-) diff --git a/idp/dex/config.go b/idp/dex/config.go index 00b5ce745..499cae1c1 100644 --- a/idp/dex/config.go +++ b/idp/dex/config.go @@ -7,16 +7,16 @@ import ( "log/slog" "net/url" "os" + "slices" "strconv" "strings" "time" - "golang.org/x/crypto/bcrypt" - "gopkg.in/yaml.v3" - "github.com/dexidp/dex/server" "github.com/dexidp/dex/storage" "github.com/dexidp/dex/storage/sql" + "golang.org/x/crypto/bcrypt" + "gopkg.in/yaml.v3" "github.com/netbirdio/netbird/idp/dex/web" ) @@ -226,10 +226,11 @@ func (p *Password) UnmarshalYAML(node *yaml.Node) error { // Connector is a connector configuration that can unmarshal YAML dynamically. type Connector struct { - Type string `yaml:"type" json:"type"` - Name string `yaml:"name" json:"name"` - ID string `yaml:"id" json:"id"` - Config map[string]interface{} `yaml:"config" json:"config"` + Type string `yaml:"type" json:"type"` + Name string `yaml:"name" json:"name"` + ID string `yaml:"id" json:"id"` + Config map[string]interface{} `yaml:"config" json:"config"` + GrantTypes []string `yaml:"grantTypes" json:"grantTypes"` } // ToStorageConnector converts a Connector to storage.Connector type. @@ -243,11 +244,17 @@ func (c *Connector) ToStorageConnector() (storage.Connector, error) { return storage.Connector{}, fmt.Errorf("failed to marshal connector config: %v", err) } + grantTypes := c.GrantTypes + if len(grantTypes) == 0 { + grantTypes = slices.Clone(DefaultGrantTypes) + } + return storage.Connector{ - ID: c.ID, - Type: dexType, - Name: c.Name, - Config: data, + ID: c.ID, + Type: dexType, + Name: c.Name, + Config: data, + GrantTypes: grantTypes, }, nil } diff --git a/idp/dex/connector.go b/idp/dex/connector.go index fb20fdcc3..0c0f93ea1 100644 --- a/idp/dex/connector.go +++ b/idp/dex/connector.go @@ -6,11 +6,20 @@ import ( "encoding/json" "errors" "fmt" + "slices" "strings" "github.com/dexidp/dex/storage" ) +// DefaultGrantTypes is the minimal set of OAuth2 grants in use. Dex enables every +// grant it supports when the list is empty. +var DefaultGrantTypes = []string{ + "authorization_code", // dashboard login + "refresh_token", // session renewal + "urn:ietf:params:oauth:grant-type:device_code", // CLI login +} + // ConnectorConfig represents the configuration for an identity provider connector type ConnectorConfig struct { // ID is the unique identifier for the connector @@ -111,10 +120,11 @@ func (p *Provider) UpdateConnector(ctx context.Context, cfg *ConnectorConfig) er } return storage.Connector{ - ID: cfg.ID, - Type: old.Type, - Name: name, - Config: configData, + ID: cfg.ID, + Type: old.Type, + Name: name, + Config: configData, + GrantTypes: old.GrantTypes, }, nil }); err != nil { return fmt.Errorf("failed to update connector: %w", err) @@ -200,7 +210,13 @@ func (p *Provider) buildStorageConnector(cfg *ConnectorConfig) (storage.Connecto return storage.Connector{}, err } - return storage.Connector{ID: cfg.ID, Type: dexType, Name: cfg.Name, Config: configData}, nil + return storage.Connector{ + ID: cfg.ID, + Type: dexType, + Name: cfg.Name, + Config: configData, + GrantTypes: slices.Clone(DefaultGrantTypes), + }, nil } // resolveRedirectURI returns the redirect URI, using a default if not provided @@ -423,6 +439,7 @@ func ensureStaticConnectors(ctx context.Context, stor storage.Storage, connector if err := stor.UpdateConnector(ctx, conn.ID, func(old storage.Connector) (storage.Connector, error) { old.Name = storConn.Name old.Config = storConn.Config + old.GrantTypes = storConn.GrantTypes return old, nil }); err != nil { return fmt.Errorf("failed to update connector %s: %w", conn.ID, err) @@ -430,3 +447,28 @@ func ensureStaticConnectors(ctx context.Context, stor storage.Storage, connector } return nil } + +// ensureConnectorGrantTypes backfills DefaultGrantTypes onto stored connectors +// that have no grants set. An explicit allowlist is left untouched. +func ensureConnectorGrantTypes(ctx context.Context, stor storage.Storage) error { + connectors, err := stor.ListConnectors(ctx) + if err != nil { + return fmt.Errorf("failed to list connectors: %w", err) + } + + for _, conn := range connectors { + if len(conn.GrantTypes) > 0 { + continue + } + if err := stor.UpdateConnector(ctx, conn.ID, func(old storage.Connector) (storage.Connector, error) { + if len(old.GrantTypes) == 0 { + old.GrantTypes = slices.Clone(DefaultGrantTypes) + } + return old, nil + }); err != nil { + return fmt.Errorf("failed to set grant types on connector %s: %w", conn.ID, err) + } + } + + return nil +} diff --git a/idp/dex/connector_test.go b/idp/dex/connector_test.go index 4253e02b7..56a177388 100644 --- a/idp/dex/connector_test.go +++ b/idp/dex/connector_test.go @@ -203,3 +203,94 @@ func TestUpdateConnector_AllowsSameTypeUpdate(t *testing.T) { require.NoError(t, json.Unmarshal(conn.Config, &m)) assert.Equal(t, "https://login.microsoftonline.com/new/v2.0", m["issuer"]) } + +func TestConnectorGrantTypes(t *testing.T) { + ctx := context.Background() + + t.Run("created connectors get the default allowlist", func(t *testing.T) { + p, cleanup := newTestProvider(t) + defer cleanup() + + _, err := p.CreateConnector(ctx, &ConnectorConfig{ + ID: "entra-test", + Name: "Entra", + Type: "entra", + Issuer: "https://login.microsoftonline.com/tid/v2.0", + ClientID: "client-id", + }) + require.NoError(t, err) + + conn, err := p.storage.GetConnector(ctx, "entra-test") + require.NoError(t, err) + assert.Equal(t, DefaultGrantTypes, conn.GrantTypes) + assert.NotContains(t, conn.GrantTypes, "urn:ietf:params:oauth:grant-type:token-exchange") + }) + + t.Run("updates do not reset the allowlist", func(t *testing.T) { + p, cleanup := newTestProvider(t) + defer cleanup() + + _, err := p.CreateConnector(ctx, &ConnectorConfig{ + ID: "entra-test", + Name: "Entra", + Type: "entra", + Issuer: "https://login.microsoftonline.com/tid/v2.0", + ClientID: "client-id", + ClientSecret: "old-secret", + }) + require.NoError(t, err) + + require.NoError(t, p.UpdateConnector(ctx, &ConnectorConfig{ + ID: "entra-test", + Type: "entra", + ClientSecret: "new-secret", + })) + + conn, err := p.storage.GetConnector(ctx, "entra-test") + require.NoError(t, err) + assert.Equal(t, DefaultGrantTypes, conn.GrantTypes, + "an empty list would re-enable token exchange for this connector") + }) + + t.Run("backfills connectors stored without an allowlist", func(t *testing.T) { + p, cleanup := newTestProvider(t) + defer cleanup() + + // Mimic a connector written by an older release. + require.NoError(t, p.storage.CreateConnector(ctx, storage.Connector{ + ID: "legacy-oidc", + Type: "oidc", + Name: "Legacy", + Config: []byte(`{"issuer":"https://accounts.example.com"}`), + })) + require.NoError(t, p.storage.CreateConnector(ctx, storage.Connector{ + ID: "opted-in", + Type: "oidc", + Name: "Opted In", + Config: []byte(`{"issuer":"https://accounts.example.com"}`), + GrantTypes: []string{"urn:ietf:params:oauth:grant-type:token-exchange"}, + })) + + require.NoError(t, ensureConnectorGrantTypes(ctx, p.storage)) + + legacy, err := p.storage.GetConnector(ctx, "legacy-oidc") + require.NoError(t, err) + assert.Equal(t, DefaultGrantTypes, legacy.GrantTypes) + + optedIn, err := p.storage.GetConnector(ctx, "opted-in") + require.NoError(t, err) + assert.Equal(t, []string{"urn:ietf:params:oauth:grant-type:token-exchange"}, optedIn.GrantTypes, + "an explicit operator allowlist must not be overwritten") + }) + + t.Run("static connectors default and honour an explicit allowlist", func(t *testing.T) { + conn, err := (&Connector{ID: "static-oidc", Type: "oidc", Name: "Static"}).ToStorageConnector() + require.NoError(t, err) + assert.Equal(t, DefaultGrantTypes, conn.GrantTypes) + + explicit := []string{"authorization_code", "urn:ietf:params:oauth:grant-type:token-exchange"} + conn, err = (&Connector{ID: "static-oidc", Type: "oidc", Name: "Static", GrantTypes: explicit}).ToStorageConnector() + require.NoError(t, err) + assert.Equal(t, explicit, conn.GrantTypes) + }) +} diff --git a/idp/dex/provider.go b/idp/dex/provider.go index f40b96a58..db33928eb 100644 --- a/idp/dex/provider.go +++ b/idp/dex/provider.go @@ -240,7 +240,10 @@ func initializeStorage(ctx context.Context, stor storage.Storage, cfg *YAMLConfi if err := ensureStaticClients(ctx, stor, cfg.StaticClients); err != nil { return err } - return ensureStaticConnectors(ctx, stor, cfg.StaticConnectors) + if err := ensureStaticConnectors(ctx, stor, cfg.StaticConnectors); err != nil { + return err + } + return ensureConnectorGrantTypes(ctx, stor) } // ensureStaticPasswords creates or updates static passwords in storage diff --git a/idp/dex/provider_test.go b/idp/dex/provider_test.go index 5e132d544..afd5a3e98 100644 --- a/idp/dex/provider_test.go +++ b/idp/dex/provider_test.go @@ -245,7 +245,7 @@ web: enablePasswordDB: true ` configPath := filepath.Join(tmpDir, "config.yaml") - err = os.WriteFile(configPath, []byte(yamlContent), 0644) + err = os.WriteFile(configPath, []byte(yamlContent), 0o644) require.NoError(t, err) // Load config and create provider @@ -317,7 +317,7 @@ connectors: redirectURI: http://localhost:5556/dex/callback ` configPath := filepath.Join(tmpDir, "config.yaml") - err = os.WriteFile(configPath, []byte(yamlContent), 0644) + err = os.WriteFile(configPath, []byte(yamlContent), 0o644) require.NoError(t, err) yamlConfig, err := LoadConfig(configPath) @@ -375,7 +375,7 @@ connectors: clientSecret: original-secret ` configPath := filepath.Join(tmpDir, "config.yaml") - err = os.WriteFile(configPath, []byte(yamlContent1), 0644) + err = os.WriteFile(configPath, []byte(yamlContent1), 0o644) require.NoError(t, err) yamlConfig1, err := LoadConfig(configPath) @@ -417,7 +417,7 @@ connectors: clientID: updated-client-id clientSecret: updated-secret ` - err = os.WriteFile(configPath, []byte(yamlContent2), 0644) + err = os.WriteFile(configPath, []byte(yamlContent2), 0o644) require.NoError(t, err) yamlConfig2, err := LoadConfig(configPath) @@ -483,7 +483,7 @@ connectors: clientSecret: google-secret ` configPath := filepath.Join(tmpDir, "config.yaml") - err = os.WriteFile(configPath, []byte(yamlContent), 0644) + err = os.WriteFile(configPath, []byte(yamlContent), 0o644) require.NoError(t, err) yamlConfig, err := LoadConfig(configPath) @@ -549,7 +549,7 @@ web: enablePasswordDB: true ` configPath := filepath.Join(tmpDir, "config.yaml") - err = os.WriteFile(configPath, []byte(yamlContent), 0644) + err = os.WriteFile(configPath, []byte(yamlContent), 0o644) require.NoError(t, err) yamlConfig, err := LoadConfig(configPath) @@ -610,7 +610,7 @@ web: enablePasswordDB: true ` configPath := filepath.Join(tmpDir, "config.yaml") - err = os.WriteFile(configPath, []byte(yamlContent), 0644) + err = os.WriteFile(configPath, []byte(yamlContent), 0o644) require.NoError(t, err) yamlConfig, err := LoadConfig(configPath) @@ -668,7 +668,7 @@ enablePasswordDB: true ` + grantTypesYAML configPath := filepath.Join(tmpDir, "config.yaml") - require.NoError(t, os.WriteFile(configPath, []byte(yamlContent), 0644)) + require.NoError(t, os.WriteFile(configPath, []byte(yamlContent), 0o644)) yamlConfig, err := LoadConfig(configPath) require.NoError(t, err) @@ -717,3 +717,42 @@ func TestHandler_AllowsDeviceEndpointsWhenGrantsDefault(t *testing.T) { provider.Handler().ServeHTTP(rec, req) assert.NotEqual(t, http.StatusNotFound, rec.Code) } + +func TestInitializeStorage_SetsConnectorGrantTypes(t *testing.T) { + ctx := context.Background() + tmpDir := t.TempDir() + + yamlContent := ` +issuer: http://localhost:5556/dex +storage: + type: sqlite3 + config: + file: ` + filepath.Join(tmpDir, "dex.db") + ` +web: + http: 127.0.0.1:5556 +enablePasswordDB: true +connectors: +- type: oidc + id: my-oidc + name: My OIDC Provider + config: + issuer: https://accounts.example.com + clientID: test-client-id +` + configPath := filepath.Join(tmpDir, "config.yaml") + require.NoError(t, os.WriteFile(configPath, []byte(yamlContent), 0o644)) + + yamlConfig, err := LoadConfig(configPath) + require.NoError(t, err) + + stor := openTestStorage(t, tmpDir) + defer stor.Close() + require.NoError(t, initializeStorage(ctx, stor, yamlConfig)) + + connectors, err := stor.ListConnectors(ctx) + require.NoError(t, err) + require.Len(t, connectors, 2) + for _, conn := range connectors { + assert.Equal(t, DefaultGrantTypes, conn.GrantTypes, "connector %s", conn.ID) + } +}