From 4caaa079cad3721a1e795a90ddcf3fdd199165fa Mon Sep 17 00:00:00 2001 From: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com> Date: Fri, 17 Jul 2026 19:12:51 -0700 Subject: [PATCH] Actor is done --- backend/go.mod | 4 +- backend/go.sum | 2 - backend/internal/appconfig/appconfig_actor.go | 38 +- backend/internal/appconfig/model.go | 45 +++ backend/internal/appconfig/model_test.go | 20 +- backend/internal/appconfig/service.go | 149 ++------ backend/internal/appconfig/service_test.go | 343 ++++++++++++++++++ .../controller/app_config_controller.go | 12 +- 8 files changed, 473 insertions(+), 140 deletions(-) create mode 100644 backend/internal/appconfig/service_test.go diff --git a/backend/go.mod b/backend/go.mod index 98f4c1a6..0ec16f87 100644 --- a/backend/go.mod +++ b/backend/go.mod @@ -24,7 +24,7 @@ require ( github.com/go-webauthn/webauthn v0.17.4 github.com/golang-migrate/migrate/v4 v4.19.1 github.com/google/uuid v1.6.0 - github.com/italypaleale/francis v0.1.0-beta.10 + github.com/italypaleale/francis v0.1.0-beta.11 github.com/italypaleale/go-kit v0.0.0-20260708054611-e276b65dd3be github.com/italypaleale/go-sql-utils v0.2.4 github.com/jackc/pgx/v5 v5.10.0 @@ -249,3 +249,5 @@ require ( ) replace github.com/ory/fosite => github.com/pocket-id/fosite v0.0.0-20260708083902-56a3c0f378d6 + +replace github.com/italypaleale/francis => /Users/alessandro/Desktop/Code/actors diff --git a/backend/go.sum b/backend/go.sum index ca9a4b07..5033530a 100644 --- a/backend/go.sum +++ b/backend/go.sum @@ -263,8 +263,6 @@ github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= -github.com/italypaleale/francis v0.1.0-beta.10 h1:LCYVwkZAkakv7g5ZS6TVIRH7hozr1A0eg24LXWWBJYE= -github.com/italypaleale/francis v0.1.0-beta.10/go.mod h1:vqKhwdLs5Sx+n6JCNknEKAODtEU51E9/LC1q9JAG3zk= github.com/italypaleale/go-kit v0.0.0-20260708054611-e276b65dd3be h1:jgu+Mdsda++LqPxz8cj8vvgiFINQ8PhFB4Q1VZpyPjs= github.com/italypaleale/go-kit v0.0.0-20260708054611-e276b65dd3be/go.mod h1:pl0r3F+thZIyDsyDo8aOUsAIVcsRuAeP1bB4GuAHLoY= github.com/italypaleale/go-sql-utils v0.2.4 h1:6CN8y3qEdNzvYlS/JK6N65E8cL9F8a6OBCJjzaQIv3c= diff --git a/backend/internal/appconfig/appconfig_actor.go b/backend/internal/appconfig/appconfig_actor.go index 903da950..ab678991 100644 --- a/backend/internal/appconfig/appconfig_actor.go +++ b/backend/internal/appconfig/appconfig_actor.go @@ -2,6 +2,7 @@ package appconfig import ( "context" + "errors" "fmt" "log/slog" "time" @@ -139,52 +140,59 @@ func (a *appConfigActor) Invoke(parentCtx context.Context, method string, data a case "replace": // Replace the entire config // The input data must be a dto.AppConfigUpdateDto - payload := dto.AppConfigUpdateDto{} if data == nil { - return nil, fmt.Errorf("request body is empty for method 'replace': %w", err) + return nil, errors.New("request body is empty for method 'replace'") } + payload := dto.AppConfigUpdateDto{} err = data.Decode(&payload) if err != nil { return nil, fmt.Errorf("request body is not valid for method 'replace': %w", err) } // Update the in-memory data - // Work on a clone to avoid touching the cached object in case of errors (we'll update after we've committed the state) + // Work on a clone to avoid touching the cached object in case of errors newState := state.Clone() newState.Replace(payload) - // Save the updated state + // Save the updated state, which also updates the cached object ctx, cancel = context.WithTimeout(parentCtx, 10*time.Second) defer cancel() - err = a.client.SetState(ctx, state, nil) + err = a.client.SetState(ctx, newState, nil) + if err != nil { + return nil, fmt.Errorf("error saving actor state: %w", err) + } - // Update the cached state too - *state = *newState + return newState, nil case "update": // Update the config // The input data must be a map[string]string - var payload map[string]string if data == nil { - return nil, fmt.Errorf("request body is empty for method 'update': %w", err) + return nil, errors.New("request body is empty for method 'update'") } + payload := map[string]string{} err = data.Decode(&payload) if err != nil { return nil, fmt.Errorf("request body is not valid for method 'update': %w", err) } // Update the in-memory data - // Work on a clone to avoid touching the cached object in case of errors (we'll update after we've committed the state) + // Work on a clone to avoid touching the cached object in case of errors newState := state.Clone() - newState.Update(payload) + err = newState.Update(payload) + if err != nil { + return nil, fmt.Errorf("request body is not valid for method 'update': %w", err) + } - // Save the updated state + // Save the updated state, which also updates the cached object ctx, cancel = context.WithTimeout(parentCtx, 10*time.Second) defer cancel() - err = a.client.SetState(ctx, state, nil) + err = a.client.SetState(ctx, newState, nil) + if err != nil { + return nil, fmt.Errorf("error saving actor state: %w", err) + } - // Update the cached state too - *state = *newState + return newState, nil } // Return the state diff --git a/backend/internal/appconfig/model.go b/backend/internal/appconfig/model.go index 798e26b6..116edec6 100644 --- a/backend/internal/appconfig/model.go +++ b/backend/internal/appconfig/model.go @@ -10,6 +10,7 @@ import ( "github.com/italypaleale/go-kit/utils" + "github.com/pocket-id/pocket-id/backend/internal/common" "github.com/pocket-id/pocket-id/backend/internal/dto" ) @@ -256,6 +257,50 @@ func (m *AppConfigModel) Update(values map[string]string) error { return nil } +// AppConfigVariable is a single application configuration property, as a key/value pair +type AppConfigVariable struct { + Key string + Value string +} + +// ToAppConfigVariableSlice returns the configuration as a slice of key/value pairs +// If showAll is false, only properties marked as public are included +// If redactSensitiveValues is true, sensitive values are redacted when the UI config is disabled +func (m *AppConfigModel) ToAppConfigVariableSlice(showAll bool, redactSensitiveValues bool) []AppConfigVariable { + // Iterate through all fields + cfgValue := reflect.ValueOf(m).Elem() + cfgType := cfgValue.Type() + + res := make([]AppConfigVariable, 0, cfgType.NumField()) + for i := range cfgType.NumField() { + field := cfgType.Field(i) + + key, _, _ := strings.Cut(field.Tag.Get("json"), ",") + if key == "" { + continue + } + + // If we're only showing public variables and this is not public, skip it + if !showAll && field.Tag.Get("public") != "true" { + continue + } + + value := cfgValue.Field(i).String() + + // Redact sensitive values if the value isn't empty, the UI config is disabled, and redactSensitiveValues is true + if value != "" && common.EnvConfig.UiConfigDisabled && redactSensitiveValues && field.Tag.Get("sensitive") == "true" { + value = "XXXXXXXXXX" + } + + res = append(res, AppConfigVariable{ + Key: key, + Value: value, + }) + } + + return res +} + type AppConfigKeyNotFoundError struct { field string } diff --git a/backend/internal/appconfig/model_test.go b/backend/internal/appconfig/model_test.go index b992f230..d5e68984 100644 --- a/backend/internal/appconfig/model_test.go +++ b/backend/internal/appconfig/model_test.go @@ -61,7 +61,7 @@ func TestAppConfigModel_Replace(t *testing.T) { assert.Equal(t, defaults.LdapUserSearchFilter, m.LdapUserSearchFilter) // A property that was provided keeps the provided value - assert.Equal(t, "marker-homePageUrl", m.HomePageURL) + assert.Equal(t, AppConfigValue("marker-homePageUrl"), m.HomePageURL) }) t.Run("an empty DTO resets every property to its default", func(t *testing.T) { @@ -88,7 +88,7 @@ func TestAppConfigModel_Replace(t *testing.T) { m.Replace(input) // Explicitly provided value wins - assert.Equal(t, "New Name", m.AppName) + assert.Equal(t, AppConfigValue("New Name"), m.AppName) // Everything else in the DTO was empty, so it is reset to the default assert.Equal(t, getDefaultConfig().LdapEnabled, m.LdapEnabled) }) @@ -101,8 +101,8 @@ func TestAppConfigModel_Replace(t *testing.T) { var m AppConfigModel m.Replace(input) - assert.Equal(t, "120", m.SessionDuration) - assert.Equal(t, "true", m.LdapEnabled) + assert.Equal(t, AppConfigValue("120"), m.SessionDuration) + assert.Equal(t, AppConfigValue("true"), m.LdapEnabled) }) } @@ -163,7 +163,7 @@ func TestAppConfigModel_Update(t *testing.T) { err := m.Update(map[string]string{"appName": "My App"}) require.NoError(t, err) - assert.Equal(t, "My App", m.AppName) + assert.Equal(t, AppConfigValue("My App"), m.AppName) }) t.Run("updates multiple properties and leaves others untouched", func(t *testing.T) { @@ -172,9 +172,9 @@ func TestAppConfigModel_Update(t *testing.T) { err := m.Update(map[string]string{"appName": "My App", "homePageUrl": "/home", "ldapEnabled": "true"}) require.NoError(t, err) - assert.Equal(t, "My App", m.AppName) - assert.Equal(t, "/home", m.HomePageURL) - assert.Equal(t, "true", m.LdapEnabled) + assert.Equal(t, AppConfigValue("My App"), m.AppName) + assert.Equal(t, AppConfigValue("/home"), m.HomePageURL) + assert.Equal(t, AppConfigValue("true"), m.LdapEnabled) // A property that was not part of the update keeps its previous value assert.Equal(t, getDefaultConfig().SessionDuration, m.SessionDuration) }) @@ -197,8 +197,8 @@ func TestAppConfigModel_Update(t *testing.T) { err := m.Update(map[string]string{"sessionDuration": "120", "disableAnimations": "true"}) require.NoError(t, err) - assert.Equal(t, "120", m.SessionDuration) - assert.Equal(t, "true", m.DisableAnimations) + assert.Equal(t, AppConfigValue("120"), m.SessionDuration) + assert.Equal(t, AppConfigValue("true"), m.DisableAnimations) }) t.Run("an empty map is a no-op", func(t *testing.T) { diff --git a/backend/internal/appconfig/service.go b/backend/internal/appconfig/service.go index 7d58d716..29c9a255 100644 --- a/backend/internal/appconfig/service.go +++ b/backend/internal/appconfig/service.go @@ -15,7 +15,6 @@ import ( "github.com/pocket-id/pocket-id/backend/internal/common" "github.com/pocket-id/pocket-id/backend/internal/dto" - "github.com/pocket-id/pocket-id/backend/internal/model" "github.com/pocket-id/pocket-id/backend/internal/tracing" "github.com/pocket-id/pocket-id/backend/internal/utils" ) @@ -95,73 +94,26 @@ func (s *AppConfigService) GetConfig(parentCtx context.Context) (*AppConfigModel return &cfg, nil } -func (s *AppConfigService) UpdateAppConfig(ctx context.Context, input dto.AppConfigUpdateDto) ([]model.AppConfigVariable, error) { +// UpdateAppConfig replaces the entire application configuration with the values from the input DTO. +func (s *AppConfigService) UpdateAppConfig(ctx context.Context, input dto.AppConfigUpdateDto) ([]AppConfigVariable, error) { // If the UI config is disabled, we cannot continue if common.EnvConfig.UiConfigDisabled { return nil, &common.UiConfigDisabledError{} } - // From here onwards, we know we are the only process/goroutine with exclusive access to the config - // Re-load the config from the database to be sure we have the correct data - cfg, err := s.loadDbConfigInternal(ctx, tx) - if err != nil { - return nil, fmt.Errorf("failed to reload config from database: %w", err) - } - - defaultCfg := getDefaultConfig() - - // Iterate through all the fields to update - // We update the in-memory data (in the cfg struct) and collect values to update in the database - rt := reflect.ValueOf(input).Type() - rv := reflect.ValueOf(input) - dbUpdate := make([]model.AppConfigVariable, 0, rt.NumField()) - for field := range rt.Fields() { - value := rv.FieldByName(field.Name).String() - - // Get the value of the json tag, taking only what's before the comma - key, _, _ := strings.Cut(field.Tag.Get("json"), ",") - - // Update the in-memory config value - // If the new value is an empty string, then we set the in-memory value to the default one - if value == "" { - // Ignore errors here as we know the key exists - defaultValue, _ := defaultCfg.FieldByKey(key) - err = cfg.UpdateField(key, defaultValue) - } else { - err = cfg.UpdateField(key, value) - } - - if err != nil { - return nil, fmt.Errorf("failed to update in-memory config for key '%s': %w", key, err) - } - - // We always save "value" which can be an empty string - dbUpdate = append(dbUpdate, model.AppConfigVariable{ - Key: key, - Value: value, - }) - } - - // Update the values in the database - err = s.updateAppConfigUpdateDatabase(ctx, tx, &dbUpdate) + // Replace the entire config by invoking the actor + cfg, err := s.invokeConfigActor(ctx, "replace", input) if err != nil { return nil, err } - // Commit the changes to the DB, then finally save the updated config in the object - err = tx.Commit().Error - if err != nil { - return nil, fmt.Errorf("failed to commit transaction: %w", err) - } - - s.dbConfig.Store(cfg) - // Return the updated config - res := cfg.ToAppConfigVariableSlice(true, false) - return res, nil + return cfg.ToAppConfigVariableSlice(true, false), nil } -// UpdateAppConfigValues updates the application configuration values in the database. +// UpdateAppConfigValues updates the provided application configuration values. +// Keys correspond to the "json" tags on the config model. +// An empty string value resets the property to its default value. func (s *AppConfigService) UpdateAppConfigValues(ctx context.Context, keysAndValues ...string) error { // Count of keysAndValues must be even if len(keysAndValues)%2 != 0 { @@ -173,71 +125,48 @@ func (s *AppConfigService) UpdateAppConfigValues(ctx context.Context, keysAndVal return &common.UiConfigDisabledError{} } - // Start the transaction - tx, err := s.updateAppConfigStartTransaction(ctx) - if err != nil { - return err - } - defer tx.Rollback() - - // From here onwards, we know we are the only process/goroutine with exclusive access to the config - // Re-load the config from the database to be sure we have the correct data - cfg, err := s.loadDbConfigInternal(ctx, tx) - if err != nil { - return fmt.Errorf("failed to reload config from database: %w", err) - } - - defaultCfg := getDefaultDbConfig() - - // Iterate through all the fields to update - // We update the in-memory data (in the cfg struct) and collect values to update in the database + // Collect the key-value pairs into a map for the actor // (Note the += 2, as we are iterating through key-value pairs) - dbUpdate := make([]model.AppConfigVariable, 0, len(keysAndValues)/2) + values := make(map[string]string, len(keysAndValues)/2) for i := 1; i < len(keysAndValues); i += 2 { - key := keysAndValues[i-1] - value := keysAndValues[i] - - // Ensure that the field is valid - // We do this by grabbing the default value - var defaultValue string - defaultValue, err := defaultCfg.FieldByKey(key) - if err != nil { - return fmt.Errorf("invalid configuration key '%s': %w", key, err) - } - - // Update the in-memory config value - // If the new value is an empty string, then we set the in-memory value to the default one - if value == "" { - err = cfg.UpdateField(key, defaultValue) - } else { - err = cfg.UpdateField(key, value) - } - if err != nil { - return fmt.Errorf("failed to update in-memory config for key '%s': %w", key, err) - } - - // We always save "value" which can be an empty string - dbUpdate = append(dbUpdate, model.AppConfigVariable{ - Key: key, - Value: value, - }) + values[keysAndValues[i-1]] = keysAndValues[i] } - // Update the values in the database - err = s.updateAppConfigUpdateDatabase(ctx, tx, &dbUpdate) + // Update the config by invoking the actor + _, err := s.invokeConfigActor(ctx, "update", values) + return err +} + +// ListAppConfig returns the application configuration as a slice of key/value pairs. +// If showAll is false, only properties marked as public are included. +func (s *AppConfigService) ListAppConfig(ctx context.Context, showAll bool) ([]AppConfigVariable, error) { + cfg, err := s.GetConfig(ctx) if err != nil { - return err + return nil, err } - // Commit the changes to the DB, then finally save the updated config in the object - err = tx.Commit().Error + return cfg.ToAppConfigVariableSlice(showAll, true), nil +} + +// invokeConfigActor invokes a method on the AppConfig actor and decodes the returned state. +func (s *AppConfigService) invokeConfigActor(parentCtx context.Context, method string, data any) (*AppConfigModel, error) { + ctx, cancel := context.WithTimeout(parentCtx, 10*time.Second) + defer cancel() + res, err := s.actSvc.Invoke(ctx, AppConfigActorType, actor.SingletonActorID, method, data) if err != nil { - return fmt.Errorf("failed to commit transaction: %w", err) + return nil, fmt.Errorf("error invoking config actor method '%s': %w", method, err) + } + if res == nil { + return nil, errors.New("config actor response was empty") } - s.dbConfig.Store(cfg) + var cfg AppConfigModel + err = res.Decode(&cfg) + if err != nil { + return nil, fmt.Errorf("error decoding config actor response: %w", err) + } - return nil + return &cfg, nil } func (s *AppConfigService) loadDbConfigFromEnv() (*AppConfigModel, error) { diff --git a/backend/internal/appconfig/service_test.go b/backend/internal/appconfig/service_test.go new file mode 100644 index 00000000..4c5e2e2f --- /dev/null +++ b/backend/internal/appconfig/service_test.go @@ -0,0 +1,343 @@ +package appconfig + +import ( + "encoding/json" + "testing" + "time" + + "github.com/italypaleale/francis/host/local" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm" + + "github.com/pocket-id/pocket-id/backend/internal/common" + "github.com/pocket-id/pocket-id/backend/internal/dto" + "github.com/pocket-id/pocket-id/backend/internal/model" + testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing" +) + +// setUIConfigDisabled sets common.EnvConfig.UiConfigDisabled for the duration of the test, restoring the previous global afterwards +func setUIConfigDisabled(t *testing.T, disabled bool) { + t.Helper() + + original := common.EnvConfig + t.Cleanup(func() { + common.EnvConfig = original + }) + common.EnvConfig.UiConfigDisabled = disabled +} + +// newActorBackedService creates an AppConfigService wired to an in-memory test actor host. +// The AppConfig singleton actor is registered and bootstrapped from db, which is also used to load any legacy config. +func newActorBackedService(t *testing.T, db *gorm.DB) *AppConfigService { + t.Helper() + + var svc *AppConfigService + testutils.NewActorHostForTest(t, func(t *testing.T, h *local.Host) { + var err error + svc, err = NewService(t.Context(), h, db) + require.NoError(t, err) + }) + require.NotNil(t, svc) + + // The singleton actor is bootstrapped asynchronously once the host is ready. + // Before bootstrap runs, the actor has no state and GetConfig decodes it into a non-nil but zero config, so wait until a non-zero (bootstrapped) config is available before returning. + require.Eventually(t, func() bool { + cfg, err := svc.GetConfig(t.Context()) + return err == nil && cfg != nil && *cfg != (AppConfigModel{}) + }, 10*time.Second, 20*time.Millisecond, "config actor was not bootstrapped in time") + + return svc +} + +// seedLegacyConfig writes a legacy config blob to the kv table so the AppConfig actor bootstraps from it. +func seedLegacyConfig(t *testing.T, db *gorm.DB, values map[string]string) { + t.Helper() + + blob, err := json.Marshal(values) + require.NoError(t, err) + + value := string(blob) + err = db.Create(&model.KV{Key: "config_migrated", Value: &value}).Error + require.NoError(t, err) +} + +// findConfigValue returns the value for key in a slice of AppConfigVariable, and whether it was found. +func findConfigValue(vars []AppConfigVariable, key string) (string, bool) { + for _, v := range vars { + if v.Key == key { + return v.Value, true + } + } + return "", false +} + +func TestService_NewService(t *testing.T) { + t.Run("bootstraps the default config when the database is empty", func(t *testing.T) { + setUIConfigDisabled(t, false) + db := testutils.NewDatabaseForTest(t) + svc := newActorBackedService(t, db) + + cfg, err := svc.GetConfig(t.Context()) + require.NoError(t, err) + require.NotNil(t, cfg) + assert.Equal(t, *getDefaultConfig(), *cfg) + }) + + t.Run("bootstraps from the legacy config in the database", func(t *testing.T) { + setUIConfigDisabled(t, false) + db := testutils.NewDatabaseForTest(t) + seedLegacyConfig(t, db, map[string]string{ + "appName": "Legacy App", + "ldapEnabled": "true", + }) + + svc := newActorBackedService(t, db) + + cfg, err := svc.GetConfig(t.Context()) + require.NoError(t, err) + assert.Equal(t, AppConfigValue("Legacy App"), cfg.AppName) + assert.Equal(t, AppConfigValue("true"), cfg.LdapEnabled) + // Keys not present in the legacy config keep their defaults + assert.Equal(t, getDefaultConfig().SessionDuration, cfg.SessionDuration) + }) + + t.Run("loads config from the environment when the UI config is disabled", func(t *testing.T) { + setUIConfigDisabled(t, true) + + // No actor host or database is needed when the UI config is disabled + svc, err := NewService(t.Context(), nil, nil) + require.NoError(t, err) + require.NotNil(t, svc) + + cfg, err := svc.GetConfig(t.Context()) + require.NoError(t, err) + assert.Equal(t, *getDefaultConfig(), *cfg) + }) +} + +func TestService_GetConfig(t *testing.T) { + t.Run("returns a fresh copy on each call", func(t *testing.T) { + setUIConfigDisabled(t, false) + db := testutils.NewDatabaseForTest(t) + svc := newActorBackedService(t, db) + + first, err := svc.GetConfig(t.Context()) + require.NoError(t, err) + + // Mutating the returned config must not affect what the service returns later + first.AppName = "Mutated" + + second, err := svc.GetConfig(t.Context()) + require.NoError(t, err) + assert.Equal(t, getDefaultConfig().AppName, second.AppName) + }) + + t.Run("returns the env config when the UI config is disabled", func(t *testing.T) { + setUIConfigDisabled(t, true) + svc := NewTestAppConfigService(&AppConfigModel{AppName: "From Env"}) + + cfg, err := svc.GetConfig(t.Context()) + require.NoError(t, err) + assert.Equal(t, AppConfigValue("From Env"), cfg.AppName) + }) +} + +func TestService_UpdateAppConfig(t *testing.T) { + t.Run("replaces the configuration and returns all variables", func(t *testing.T) { + setUIConfigDisabled(t, false) + db := testutils.NewDatabaseForTest(t) + svc := newActorBackedService(t, db) + + input := dto.AppConfigUpdateDto{ + AppName: "Replaced App", + SessionDuration: "120", + LdapEnabled: "true", + SmtpTls: "tls", + } + res, err := svc.UpdateAppConfig(t.Context(), input) + require.NoError(t, err) + + // The returned slice includes all variables, both public and private + got, ok := findConfigValue(res, "appName") + require.True(t, ok) + assert.Equal(t, "Replaced App", got) + got, ok = findConfigValue(res, "smtpTls") + require.True(t, ok, "the returned slice should include private variables") + assert.Equal(t, "tls", got) + + // The change is persisted and visible on subsequent reads + cfg, err := svc.GetConfig(t.Context()) + require.NoError(t, err) + assert.Equal(t, AppConfigValue("Replaced App"), cfg.AppName) + assert.Equal(t, AppConfigValue("120"), cfg.SessionDuration) + assert.Equal(t, AppConfigValue("true"), cfg.LdapEnabled) + }) + + t.Run("resets fields omitted from the DTO to their defaults", func(t *testing.T) { + setUIConfigDisabled(t, false) + db := testutils.NewDatabaseForTest(t) + svc := newActorBackedService(t, db) + + // First set some non-default values + _, err := svc.UpdateAppConfig(t.Context(), dto.AppConfigUpdateDto{ + AppName: "First", + LdapEnabled: "true", + SmtpTls: "tls", + }) + require.NoError(t, err) + + // Replace again with only AppName set: the rest must reset to their defaults + _, err = svc.UpdateAppConfig(t.Context(), dto.AppConfigUpdateDto{AppName: "Second"}) + require.NoError(t, err) + + cfg, err := svc.GetConfig(t.Context()) + require.NoError(t, err) + assert.Equal(t, AppConfigValue("Second"), cfg.AppName) + assert.Equal(t, getDefaultConfig().LdapEnabled, cfg.LdapEnabled) + assert.Equal(t, getDefaultConfig().SmtpTls, cfg.SmtpTls) + }) + + t.Run("returns UiConfigDisabledError when the UI config is disabled", func(t *testing.T) { + setUIConfigDisabled(t, true) + svc := NewTestAppConfigService(nil) + + _, err := svc.UpdateAppConfig(t.Context(), dto.AppConfigUpdateDto{AppName: "X"}) + require.Error(t, err) + var target *common.UiConfigDisabledError + assert.ErrorAs(t, err, &target) + }) +} + +func TestService_UpdateAppConfigValues(t *testing.T) { + t.Run("updates a subset of keys and leaves the rest unchanged", func(t *testing.T) { + setUIConfigDisabled(t, false) + db := testutils.NewDatabaseForTest(t) + svc := newActorBackedService(t, db) + + err := svc.UpdateAppConfigValues(t.Context(), "appName", "Updated", "sessionDuration", "120") + require.NoError(t, err) + + cfg, err := svc.GetConfig(t.Context()) + require.NoError(t, err) + assert.Equal(t, AppConfigValue("Updated"), cfg.AppName) + assert.Equal(t, AppConfigValue("120"), cfg.SessionDuration) + // A key that was not part of the update keeps its default + assert.Equal(t, getDefaultConfig().LdapEnabled, cfg.LdapEnabled) + }) + + t.Run("an empty value resets the property to its default", func(t *testing.T) { + setUIConfigDisabled(t, false) + db := testutils.NewDatabaseForTest(t) + svc := newActorBackedService(t, db) + + // Set a non-default value first + err := svc.UpdateAppConfigValues(t.Context(), "sessionDuration", "120") + require.NoError(t, err) + + // Then reset it with an empty value + err = svc.UpdateAppConfigValues(t.Context(), "sessionDuration", "") + require.NoError(t, err) + + cfg, err := svc.GetConfig(t.Context()) + require.NoError(t, err) + assert.Equal(t, getDefaultConfig().SessionDuration, cfg.SessionDuration) + }) + + t.Run("an odd number of arguments returns an error", func(t *testing.T) { + setUIConfigDisabled(t, false) + db := testutils.NewDatabaseForTest(t) + svc := newActorBackedService(t, db) + + err := svc.UpdateAppConfigValues(t.Context(), "appName") + require.Error(t, err) + assert.ErrorContains(t, err, "invalid number of arguments received") + }) + + t.Run("an unknown key returns an error and does not change the config", func(t *testing.T) { + setUIConfigDisabled(t, false) + db := testutils.NewDatabaseForTest(t) + svc := newActorBackedService(t, db) + + err := svc.UpdateAppConfigValues(t.Context(), "thisKeyDoesNotExist", "value") + require.Error(t, err) + + // The config must not have been modified + cfg, err := svc.GetConfig(t.Context()) + require.NoError(t, err) + assert.Equal(t, *getDefaultConfig(), *cfg) + }) + + t.Run("returns UiConfigDisabledError when the UI config is disabled", func(t *testing.T) { + setUIConfigDisabled(t, true) + svc := NewTestAppConfigService(nil) + + // An even number of arguments so the count check passes and we reach the UI-config check + err := svc.UpdateAppConfigValues(t.Context(), "appName", "X") + require.Error(t, err) + var target *common.UiConfigDisabledError + assert.ErrorAs(t, err, &target) + }) +} + +func TestService_ListAppConfig(t *testing.T) { + t.Run("returns only public variables when showAll is false", func(t *testing.T) { + setUIConfigDisabled(t, false) + db := testutils.NewDatabaseForTest(t) + svc := newActorBackedService(t, db) + + vars, err := svc.ListAppConfig(t.Context(), false) + require.NoError(t, err) + + // appName is public and must be present + _, ok := findConfigValue(vars, "appName") + assert.True(t, ok, "public variable appName should be present") + // smtpHost is not public and must be excluded + _, ok = findConfigValue(vars, "smtpHost") + assert.False(t, ok, "private variable smtpHost should be excluded") + }) + + t.Run("returns all variables when showAll is true", func(t *testing.T) { + setUIConfigDisabled(t, false) + db := testutils.NewDatabaseForTest(t) + svc := newActorBackedService(t, db) + + vars, err := svc.ListAppConfig(t.Context(), true) + require.NoError(t, err) + + _, ok := findConfigValue(vars, "appName") + assert.True(t, ok) + _, ok = findConfigValue(vars, "smtpHost") + assert.True(t, ok, "private variables should be included when showAll is true") + }) + + t.Run("reflects updates made through the service", func(t *testing.T) { + setUIConfigDisabled(t, false) + db := testutils.NewDatabaseForTest(t) + svc := newActorBackedService(t, db) + + err := svc.UpdateAppConfigValues(t.Context(), "appName", "Listed App") + require.NoError(t, err) + + vars, err := svc.ListAppConfig(t.Context(), true) + require.NoError(t, err) + + got, ok := findConfigValue(vars, "appName") + require.True(t, ok) + assert.Equal(t, "Listed App", got) + }) + + t.Run("redacts sensitive values when the UI config is disabled", func(t *testing.T) { + setUIConfigDisabled(t, true) + svc := NewTestAppConfigService(&AppConfigModel{ + SmtpPassword: "super-secret", + }) + + vars, err := svc.ListAppConfig(t.Context(), true) + require.NoError(t, err) + + got, ok := findConfigValue(vars, "smtpPassword") + require.True(t, ok) + assert.Equal(t, "XXXXXXXXXX", got) + }) +} diff --git a/backend/internal/controller/app_config_controller.go b/backend/internal/controller/app_config_controller.go index 9c7442e3..d852d2a1 100644 --- a/backend/internal/controller/app_config_controller.go +++ b/backend/internal/controller/app_config_controller.go @@ -53,7 +53,11 @@ type AppConfigController struct { // @Success 200 {array} dto.PublicAppConfigVariableDto // @Router /api/application-configuration [get] func (acc *AppConfigController) listAppConfigHandler(c *gin.Context) { - configuration := acc.appConfigService.ListAppConfig(false) + configuration, err := acc.appConfigService.ListAppConfig(c.Request.Context(), false) + if err != nil { + _ = c.Error(err) + return + } var configVariablesDto []dto.PublicAppConfigVariableDto if err := dto.MapStructList(configuration, &configVariablesDto); err != nil { @@ -87,7 +91,11 @@ func (acc *AppConfigController) listAppConfigHandler(c *gin.Context) { // @Success 200 {array} dto.AppConfigVariableDto // @Router /api/application-configuration/all [get] func (acc *AppConfigController) listAllAppConfigHandler(c *gin.Context) { - configuration := acc.appConfigService.ListAppConfig(true) + configuration, err := acc.appConfigService.ListAppConfig(c.Request.Context(), true) + if err != nil { + _ = c.Error(err) + return + } var configVariablesDto []dto.AppConfigVariableDto if err := dto.MapStructList(configuration, &configVariablesDto); err != nil {