mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-12 17:59:06 +02:00
Restrict embedded IdP connectors to a minimal grant allowlist
This commit is contained in:
+18
-11
@@ -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
|
||||
}
|
||||
|
||||
|
||||
+47
-5
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
}
|
||||
|
||||
+4
-1
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user