Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove additional features and add a feature flag instead #3663

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 0 additions & 30 deletions cmd/dex/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"log/slog"
"net/http"
"os"
"slices"
"strings"

"golang.org/x/crypto/bcrypt"
Expand Down Expand Up @@ -51,22 +50,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
Expand All @@ -85,7 +72,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"},
Expand All @@ -104,22 +90,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 {
Expand Down
18 changes: 0 additions & 18 deletions cmd/dex/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,15 @@ 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)
}
}

func TestInvalidConfiguration(t *testing.T) {
configuration := Config{}
configuration.Parse()
err := configuration.Validate()
if err == nil {
t.Fatal("this configuration should be invalid")
Expand Down Expand Up @@ -232,16 +228,13 @@ additionalFeatures: [
Level: slog.LevelDebug,
Format: "json",
},
AdditionalFeatures: server.ValidAdditionalFeatures,
}

var c Config
if err := yaml.Unmarshal(rawConfig, &c); err != nil {
t.Fatalf("failed to decode config: %v", err)
}

c.Parse()

if diff := pretty.Compare(c, want); diff != "" {
t.Errorf("got!=want: %s", diff)
}
Expand Down Expand Up @@ -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")
}
}
3 changes: 1 addition & 2 deletions cmd/dex/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -502,7 +501,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 {
Expand Down
5 changes: 0 additions & 5 deletions examples/config-dev.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,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"
3 changes: 3 additions & 0 deletions pkg/featureflags/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
29 changes: 13 additions & 16 deletions server/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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,
logger: logger.With("component", "api"),
version: version,
additionalFeatures: additionalFeatures,
s: s,
logger: logger.With("component", "api"),
version: version,
}
}

Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
return nil, fmt.Errorf("%v not provided in addtionalFeatures", ConnectorsCRUD)
if !featureflags.APIConnectorsCRUD.Enabled() {
return nil, fmt.Errorf("%s feature flag is not enabled", featureflags.APIConnectorsCRUD.Name)
}

if req.Connector.Id == "" {
Expand Down Expand Up @@ -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) {
return nil, fmt.Errorf("%v not provided in addtionalFeatures", ConnectorsCRUD)
if !featureflags.APIConnectorsCRUD.Enabled() {
return nil, fmt.Errorf("%s feature flag is not enabled", featureflags.APIConnectorsCRUD.Name)
}

if req.Id == "" {
Expand Down Expand Up @@ -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) {
return nil, fmt.Errorf("%v not provided in addtionalFeatures", ConnectorsCRUD)
if !featureflags.APIConnectorsCRUD.Enabled() {
return nil, fmt.Errorf("%s feature flag is not enabled", featureflags.APIConnectorsCRUD.Name)
}

if req.Id == "" {
Expand All @@ -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) {
return nil, fmt.Errorf("%v not provided in addtionalFeatures", ConnectorsCRUD)
if !featureflags.APIConnectorsCRUD.Enabled() {
return nil, fmt.Errorf("%s feature flag is not enabled", featureflags.APIConnectorsCRUD.Name)
}

connectorList, err := d.s.ListConnectors()
Expand Down
37 changes: 25 additions & 12 deletions server/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"io"
"log/slog"
"net"
"os"
"strings"
"testing"
"time"
Expand All @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
10 changes: 0 additions & 10 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,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"
Expand Down
Loading