Remove additional features and add a feature flag instead (#3663)

Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>

cmd/dex/config.go

@@ -8,7 +8,6 @@ import (
 	"net/http"
 	"net/netip"
 	"os"
-	"slices"
 	"strings"
 
 	"golang.org/x/crypto/bcrypt"
@@ -52,22 +51,10 @@ type Config struct {
 	// querying the storage. Cannot be specified without enabling a passwords
 	// database.
 	StaticPasswords []password `json:"staticPasswords"`
-
-	// AdditionalFeature allow the extension of Dex functionalities
-	AdditionalFeatures []server.AdditionalFeature `json:"additionalFeatures"`
-}
-
-// Parse the configuration
-func (c *Config) Parse() {
-	if c.AdditionalFeatures == nil {
-		c.AdditionalFeatures = []server.AdditionalFeature{}
-	}
 }
 
 // Validate the configuration
 func (c Config) Validate() error {
-	invalidFeatures := c.findInvalidAdditionalFeatures()
-
 	// Fast checks. Perform these first for a more responsive CLI.
 	checks := []struct {
 		bad    bool
@@ -86,7 +73,6 @@ func (c Config) Validate() error {
 		{c.GRPC.TLSKey != "" && c.GRPC.Addr == "", "no address specified for gRPC"},
 		{(c.GRPC.TLSCert == "") != (c.GRPC.TLSKey == ""), "must specific both a gRPC TLS cert and key"},
 		{c.GRPC.TLSCert == "" && c.GRPC.TLSClientCA != "", "cannot specify gRPC TLS client CA without a gRPC TLS cert"},
-		{len(invalidFeatures) > 0, fmt.Sprintf("invalid additionalFeatures supplied: %v. Valid entries: %s", invalidFeatures, server.ValidAdditionalFeatures)},
 		{c.GRPC.TLSMinVersion != "" && c.GRPC.TLSMinVersion != "1.2" && c.GRPC.TLSMinVersion != "1.3", "supported TLS versions are: 1.2, 1.3"},
 		{c.GRPC.TLSMaxVersion != "" && c.GRPC.TLSMaxVersion != "1.2" && c.GRPC.TLSMaxVersion != "1.3", "supported TLS versions are: 1.2, 1.3"},
 		{c.GRPC.TLSMaxVersion != "" && c.GRPC.TLSMinVersion != "" && c.GRPC.TLSMinVersion > c.GRPC.TLSMaxVersion, "TLSMinVersion greater than TLSMaxVersion"},
@@ -105,22 +91,6 @@ func (c Config) Validate() error {
 	return nil
 }
 
-// findInvalidAdditionalFeatures returns additional features that are not considered valid
-func (c Config) findInvalidAdditionalFeatures() []server.AdditionalFeature {
-	if c.AdditionalFeatures == nil {
-		return []server.AdditionalFeature{}
-	}
-
-	badFeatures := []server.AdditionalFeature{}
-	for _, feature := range c.AdditionalFeatures {
-		if !slices.Contains(server.ValidAdditionalFeatures, feature) {
-			badFeatures = append(badFeatures, feature)
-		}
-	}
-
-	return badFeatures
-}
-
 type password storage.Password
 
 func (p *password) UnmarshalJSON(b []byte) error {

cmd/dex/config_test.go

@@ -37,11 +37,8 @@ func TestValidConfiguration(t *testing.T) {
 				Config: &mock.CallbackConfig{},
 			},
 		},
-		AdditionalFeatures: server.ValidAdditionalFeatures,
 	}
 
-	configuration.Parse()
-
 	if err := configuration.Validate(); err != nil {
 		t.Fatalf("this configuration should have been valid: %v", err)
 	}
@@ -49,7 +46,6 @@ func TestValidConfiguration(t *testing.T) {
 
 func TestInvalidConfiguration(t *testing.T) {
 	configuration := Config{}
-	configuration.Parse()
 	err := configuration.Validate()
 	if err == nil {
 		t.Fatal("this configuration should be invalid")
@@ -232,7 +228,6 @@ additionalFeatures: [
 			Level:  slog.LevelDebug,
 			Format: "json",
 		},
-		AdditionalFeatures: server.ValidAdditionalFeatures,
 	}
 
 	var c Config
@@ -240,8 +235,6 @@ additionalFeatures: [
 		t.Fatalf("failed to decode config: %v", err)
 	}
 
-	c.Parse()
-
 	if diff := pretty.Compare(c, want); diff != "" {
 		t.Errorf("got!=want: %s", diff)
 	}
@@ -450,18 +443,7 @@ logger:
 		t.Fatalf("failed to decode config: %v", err)
 	}
 
-	c.Parse()
-
 	if diff := pretty.Compare(c, want); diff != "" {
 		t.Errorf("got!=want: %s", diff)
 	}
 }
-
-func TestParseConfig(t *testing.T) {
-	configuration := Config{}
-	configuration.Parse()
-
-	if configuration.AdditionalFeatures == nil || len(configuration.AdditionalFeatures) != 0 {
-		t.Fatal("AdditionalFeatures should be an empty slice")
-	}
-}

cmd/dex/serve.go

@@ -99,7 +99,6 @@ func runServe(options serveOptions) error {
 		return fmt.Errorf("error parse config file %s: %v", configFile, err)
 	}
 
-	c.Parse()
 	applyConfigOverrides(options, &c)
 
 	logger, err := newLogger(c.Logger.Level, c.Logger.Format)
@@ -509,7 +508,7 @@ func runServe(options serveOptions) error {
 		}
 
 		grpcSrv := grpc.NewServer(grpcOptions...)
-		api.RegisterDexServer(grpcSrv, server.NewAPI(serverConfig.Storage, logger, version, c.AdditionalFeatures))
+		api.RegisterDexServer(grpcSrv, server.NewAPI(serverConfig.Storage, logger, version))
 
 		grpcMetrics.InitializeMetrics(grpcSrv)
 		if c.GRPC.Reflection {

examples/config-dev.yaml

@@ -165,8 +165,3 @@ staticPasswords:
   hash: "$2a$10$2b2cU8CPhOTaGrs1HRQuAueS7JTT5ZHsHSzYiFPm1leZck7Mc8T4W"
   username: "admin"
   userID: "08a8684b-db88-4b73-90a9-3cd1661f5466"
-
-# A list of features that extend Dex functionalities
-# additionalFeatures:
-#   # allows CRUD operations on connectors through the gRPC API
-#   - "ConnectorsCRUD"

pkg/featureflags/set.go

@@ -8,4 +8,7 @@ var (
 	// ExpandEnv can enable or disable env expansion in the config which can be useful in environments where, e.g.,
 	// $ sign is a part of the password for LDAP user.
 	ExpandEnv = newFlag("expand_env", true)
+
+	// APIConnectorsCRUD allows CRUD operations on connectors through the gRPC API
+	APIConnectorsCRUD = newFlag("api_connectors_crud", false)
 )

server/api.go

@@ -6,11 +6,11 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
-	"slices"
 
 	"golang.org/x/crypto/bcrypt"
 
 	"github.com/dexidp/dex/api/v2"
+	"github.com/dexidp/dex/pkg/featureflags"
 	"github.com/dexidp/dex/server/internal"
 	"github.com/dexidp/dex/storage"
 )
@@ -31,12 +31,11 @@ const (
 )
 
 // NewAPI returns a server which implements the gRPC API interface.
-func NewAPI(s storage.Storage, logger *slog.Logger, version string, additionalFeatures []AdditionalFeature) api.DexServer {
+func NewAPI(s storage.Storage, logger *slog.Logger, version string) api.DexServer {
 	return dexAPI{
-		s:                  s,
+		s:       s,
-		logger:             logger.With("component", "api"),
+		logger:  logger.With("component", "api"),
-		version:            version,
+		version: version,
-		additionalFeatures: additionalFeatures,
 	}
 }
 
@@ -46,8 +45,6 @@ type dexAPI struct {
 	s       storage.Storage
 	logger  *slog.Logger
 	version string
-
-	additionalFeatures []AdditionalFeature
 }
 
 func (d dexAPI) GetClient(ctx context.Context, req *api.GetClientReq) (*api.GetClientResp, error) {
@@ -392,8 +389,8 @@ func (d dexAPI) RevokeRefresh(ctx context.Context, req *api.RevokeRefreshReq) (*
 }
 
 func (d dexAPI) CreateConnector(ctx context.Context, req *api.CreateConnectorReq) (*api.CreateConnectorResp, error) {
-	if !slices.Contains(d.additionalFeatures, ConnectorsCRUD) {
+	if !featureflags.APIConnectorsCRUD.Enabled() {
-		return nil, fmt.Errorf("%v not provided in addtionalFeatures", ConnectorsCRUD)
+		return nil, fmt.Errorf("%s feature flag is not enabled", featureflags.APIConnectorsCRUD.Name)
 	}
 
 	if req.Connector.Id == "" {
@@ -434,8 +431,8 @@ func (d dexAPI) CreateConnector(ctx context.Context, req *api.CreateConnectorReq
 }
 
 func (d dexAPI) UpdateConnector(ctx context.Context, req *api.UpdateConnectorReq) (*api.UpdateConnectorResp, error) {
-	if !slices.Contains(d.additionalFeatures, ConnectorsCRUD) {
+	if !featureflags.APIConnectorsCRUD.Enabled() {
-		return nil, fmt.Errorf("%v not provided in addtionalFeatures", ConnectorsCRUD)
+		return nil, fmt.Errorf("%s feature flag is not enabled", featureflags.APIConnectorsCRUD.Name)
 	}
 
 	if req.Id == "" {
@@ -478,8 +475,8 @@ func (d dexAPI) UpdateConnector(ctx context.Context, req *api.UpdateConnectorReq
 }
 
 func (d dexAPI) DeleteConnector(ctx context.Context, req *api.DeleteConnectorReq) (*api.DeleteConnectorResp, error) {
-	if !slices.Contains(d.additionalFeatures, ConnectorsCRUD) {
+	if !featureflags.APIConnectorsCRUD.Enabled() {
-		return nil, fmt.Errorf("%v not provided in addtionalFeatures", ConnectorsCRUD)
+		return nil, fmt.Errorf("%s feature flag is not enabled", featureflags.APIConnectorsCRUD.Name)
 	}
 
 	if req.Id == "" {
@@ -498,8 +495,8 @@ func (d dexAPI) DeleteConnector(ctx context.Context, req *api.DeleteConnectorReq
 }
 
 func (d dexAPI) ListConnectors(ctx context.Context, req *api.ListConnectorReq) (*api.ListConnectorResp, error) {
-	if !slices.Contains(d.additionalFeatures, ConnectorsCRUD) {
+	if !featureflags.APIConnectorsCRUD.Enabled() {
-		return nil, fmt.Errorf("%v not provided in addtionalFeatures", ConnectorsCRUD)
+		return nil, fmt.Errorf("%s feature flag is not enabled", featureflags.APIConnectorsCRUD.Name)
 	}
 
 	connectorList, err := d.s.ListConnectors()

server/api_test.go

@@ -5,6 +5,7 @@ import (
 	"io"
 	"log/slog"
 	"net"
+	"os"
 	"strings"
 	"testing"
 	"time"
@@ -30,14 +31,14 @@ type apiClient struct {
 }
 
 // newAPI constructs a gRCP client connected to a backing server.
-func newAPI(s storage.Storage, logger *slog.Logger, t *testing.T, addtionalFeatures []AdditionalFeature) *apiClient {
+func newAPI(s storage.Storage, logger *slog.Logger, t *testing.T) *apiClient {
 	l, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	serv := grpc.NewServer()
-	api.RegisterDexServer(serv, NewAPI(s, logger, "test", addtionalFeatures))
+	api.RegisterDexServer(serv, NewAPI(s, logger, "test"))
 	go serv.Serve(l)
 
 	// NewClient will retry automatically if the serv.Serve() goroutine
@@ -62,7 +63,7 @@ func TestPassword(t *testing.T) {
 	logger := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{}))
 
 	s := memory.New(logger)
-	client := newAPI(s, logger, t, []AdditionalFeature{})
+	client := newAPI(s, logger, t)
 	defer client.Close()
 
 	ctx := context.Background()
@@ -171,7 +172,7 @@ func TestCheckCost(t *testing.T) {
 	logger := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{}))
 
 	s := memory.New(logger)
-	client := newAPI(s, logger, t, []AdditionalFeature{})
+	client := newAPI(s, logger, t)
 	defer client.Close()
 
 	tests := []struct {
@@ -224,7 +225,7 @@ func TestRefreshToken(t *testing.T) {
 	logger := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{}))
 
 	s := memory.New(logger)
-	client := newAPI(s, logger, t, []AdditionalFeature{})
+	client := newAPI(s, logger, t)
 	defer client.Close()
 
 	ctx := context.Background()
@@ -333,7 +334,7 @@ func TestUpdateClient(t *testing.T) {
 	logger := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{}))
 
 	s := memory.New(logger)
-	client := newAPI(s, logger, t, []AdditionalFeature{})
+	client := newAPI(s, logger, t)
 	defer client.Close()
 	ctx := context.Background()
 
@@ -493,10 +494,13 @@ func find(item string, items []string) bool {
 }
 
 func TestCreateConnector(t *testing.T) {
+	os.Setenv("DEX_API_CONNECTORS_CRUD", "true")
+	defer os.Unsetenv("DEX_API_CONNECTORS_CRUD")
+
 	logger := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{}))
 
 	s := memory.New(logger)
-	client := newAPI(s, logger, t, []AdditionalFeature{ConnectorsCRUD})
+	client := newAPI(s, logger, t)
 	defer client.Close()
 
 	ctx := context.Background()
@@ -540,10 +544,13 @@ func TestCreateConnector(t *testing.T) {
 }
 
 func TestUpdateConnector(t *testing.T) {
+	os.Setenv("DEX_API_CONNECTORS_CRUD", "true")
+	defer os.Unsetenv("DEX_API_CONNECTORS_CRUD")
+
 	logger := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{}))
 
 	s := memory.New(logger)
-	client := newAPI(s, logger, t, []AdditionalFeature{ConnectorsCRUD})
+	client := newAPI(s, logger, t)
 	defer client.Close()
 
 	ctx := context.Background()
@@ -605,10 +612,13 @@ func TestUpdateConnector(t *testing.T) {
 }
 
 func TestDeleteConnector(t *testing.T) {
+	os.Setenv("DEX_API_CONNECTORS_CRUD", "true")
+	defer os.Unsetenv("DEX_API_CONNECTORS_CRUD")
+
 	logger := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{}))
 
 	s := memory.New(logger)
-	client := newAPI(s, logger, t, []AdditionalFeature{ConnectorsCRUD})
+	client := newAPI(s, logger, t)
 	defer client.Close()
 
 	ctx := context.Background()
@@ -646,10 +656,13 @@ func TestDeleteConnector(t *testing.T) {
 }
 
 func TestListConnectors(t *testing.T) {
+	os.Setenv("DEX_API_CONNECTORS_CRUD", "true")
+	defer os.Unsetenv("DEX_API_CONNECTORS_CRUD")
+
 	logger := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{}))
 
 	s := memory.New(logger)
-	client := newAPI(s, logger, t, []AdditionalFeature{ConnectorsCRUD})
+	client := newAPI(s, logger, t)
 	defer client.Close()
 
 	ctx := context.Background()
@@ -685,11 +698,11 @@ func TestListConnectors(t *testing.T) {
 	}
 }
 
-func TestMissingAdditionalFeature(t *testing.T) {
+func TestMissingConnectorsCRUDFeatureFlag(t *testing.T) {
 	logger := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{}))
 
 	s := memory.New(logger)
-	client := newAPI(s, logger, t, []AdditionalFeature{})
+	client := newAPI(s, logger, t)
 	defer client.Close()
 
 	ctx := context.Background()

server/server.go

@@ -50,16 +50,6 @@ import (
 	"github.com/dexidp/dex/web"
 )
 
-// AdditionalFeature allows the extension of Dex server functionalities
-type AdditionalFeature string
-
-// ConnectorsCRUD is an additional feature that allows CRUD operations on connectors
-var ConnectorsCRUD AdditionalFeature = "ConnectorsCRUD"
-
-var ValidAdditionalFeatures []AdditionalFeature = []AdditionalFeature{
-	ConnectorsCRUD,
-}
-
 // LocalConnector is the local passwordDB connector which is an internal
 // connector maintained by the server.
 const LocalConnector = "local"