From 2cfbcb4b67d194ddc378199b4fa0bdc1e81c00a1 Mon Sep 17 00:00:00 2001 From: "Alessandro (Ale) Segala" <43508+ItalyPaleAle@users.noreply.github.com> Date: Sun, 19 Jul 2026 20:48:05 -1000 Subject: [PATCH] refactor: use actors for db configuration (#1604) Co-authored-by: Claude --- backend/go.mod | 2 +- backend/go.sum | 4 +- backend/internal/appconfig/appconfig_actor.go | 200 ++++++++ backend/internal/appconfig/migration.go | 97 ++++ backend/internal/appconfig/model.go | 272 ++++++++++ backend/internal/appconfig/model_test.go | 225 +++++++++ backend/internal/appconfig/service.go | 210 ++++++++ backend/internal/appconfig/service_test.go | 344 +++++++++++++ backend/internal/appconfig/testing_unit.go | 27 + backend/internal/bootstrap/bootstrap.go | 2 +- .../internal/bootstrap/router_bootstrap.go | 4 +- .../internal/bootstrap/services_bootstrap.go | 40 +- .../internal/cmds/encryption_key_rotate.go | 2 +- .../cmds/encryption_key_rotate_test.go | 2 +- backend/internal/common/internal_errors.go | 20 + .../controller/app_config_controller.go | 35 +- .../internal/controller/user_controller.go | 69 ++- .../controller/user_group_controller.go | 22 +- .../internal/instanceid/instanceid_test.go | 18 +- backend/internal/job/api_key_expiry_job.go | 14 +- backend/internal/job/ldap_job.go | 15 +- .../middleware/auth_middleware_test.go | 9 +- .../internal/middleware/rate_limit_test.go | 1 + backend/internal/model/app_config.go | 203 -------- backend/internal/model/app_config_test.go | 126 ----- .../internal/service/app_config_service.go | 416 --------------- .../service/app_config_service_test.go | 473 ------------------ backend/internal/service/audit_log_service.go | 23 +- backend/internal/service/e2etest_service.go | 79 ++- backend/internal/service/email_service.go | 45 +- .../internal/service/import_service_test.go | 6 +- backend/internal/service/jwt_service.go | 25 +- backend/internal/service/jwt_service_test.go | 65 +-- backend/internal/service/ldap_service.go | 154 +++--- backend/internal/service/ldap_service_test.go | 74 +-- backend/internal/service/oidc_service.go | 23 +- backend/internal/service/oidc_service_test.go | 6 +- .../service/one_time_access_service.go | 63 +-- .../service/one_time_access_service_test.go | 10 +- .../internal/service/user_group_service.go | 35 +- backend/internal/service/user_service.go | 66 +-- backend/internal/service/user_service_test.go | 25 +- backend/internal/usersignup/handler.go | 25 +- backend/internal/usersignup/module.go | 15 +- backend/internal/usersignup/service.go | 20 +- backend/internal/webauthn/handler.go | 23 +- backend/internal/webauthn/module.go | 12 +- backend/internal/webauthn/service.go | 40 +- backend/internal/webauthn/service_test.go | 21 +- .../20260718000000_freeze_config.down.sql | 16 + .../20260718000000_freeze_config.up.sql | 12 + .../20260718000000_freeze_config.down.sql | 22 + .../20260718000000_freeze_config.up.sql | 18 + 53 files changed, 2061 insertions(+), 1714 deletions(-) create mode 100644 backend/internal/appconfig/appconfig_actor.go create mode 100644 backend/internal/appconfig/migration.go create mode 100644 backend/internal/appconfig/model.go create mode 100644 backend/internal/appconfig/model_test.go create mode 100644 backend/internal/appconfig/service.go create mode 100644 backend/internal/appconfig/service_test.go create mode 100644 backend/internal/appconfig/testing_unit.go create mode 100644 backend/internal/common/internal_errors.go delete mode 100644 backend/internal/model/app_config.go delete mode 100644 backend/internal/model/app_config_test.go delete mode 100644 backend/internal/service/app_config_service.go delete mode 100644 backend/internal/service/app_config_service_test.go create mode 100644 backend/resources/migrations/postgres/20260718000000_freeze_config.down.sql create mode 100644 backend/resources/migrations/postgres/20260718000000_freeze_config.up.sql create mode 100644 backend/resources/migrations/sqlite/20260718000000_freeze_config.down.sql create mode 100644 backend/resources/migrations/sqlite/20260718000000_freeze_config.up.sql diff --git a/backend/go.mod b/backend/go.mod index 522c38a3..fa3d4e29 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 diff --git a/backend/go.sum b/backend/go.sum index 1c7844e2..101b5802 100644 --- a/backend/go.sum +++ b/backend/go.sum @@ -263,8 +263,8 @@ 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/francis v0.1.0-beta.11 h1:FurXV2vMkRzJRFldQ6Z/bhLSJz8YXHm84uiASGeyWWU= +github.com/italypaleale/francis v0.1.0-beta.11/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 new file mode 100644 index 00000000..ab678991 --- /dev/null +++ b/backend/internal/appconfig/appconfig_actor.go @@ -0,0 +1,200 @@ +package appconfig + +import ( + "context" + "errors" + "fmt" + "log/slog" + "time" + + "github.com/italypaleale/francis/actor" + + "github.com/pocket-id/pocket-id/backend/internal/common" + "github.com/pocket-id/pocket-id/backend/internal/dto" +) + +// The AppConfig singleton actor maintains the dynamic configuration for the Pocket ID cluster +// Instances of Pocket ID should bootstrap the AppConfig's actor upon startup to ensure the config is loaded (and migrated if needed) +// After startup, Peek can be used for read-only operations such as retrieving the config or listing it + +// AppConfigActorType is the actor type for the AppConfig actor +const AppConfigActorType = "AppConfig" + +// appConfigActor is a singleton actor that manages the dynamic app configuration +type appConfigActor struct { + log *slog.Logger + client actor.Client[*AppConfigModel] +} + +// appConfigActorBootstrap is the type for the payload of the init method +type appConfigActorBootstrap struct { + LegacyConfig map[string]string +} + +// NewAppConfigActor allocates a new AppConfig actor +// It satisfies actor.Factory +func NewAppConfigActor(actorID string, service *actor.Service) actor.Actor { + log := slog. + With( + slog.String("scope", "actor"), + slog.String("actorType", AppConfigActorType), + slog.String("actorID", actorID), + ) + + log.Info("AppConfig actor created") + + return &appConfigActor{ + log: log, + client: actor.NewActorClient[*AppConfigModel](AppConfigActorType, actorID, service), + } +} + +// Bootstrap implements actor.ActorBootstrapper for the singleton actor +func (a *appConfigActor) Bootstrap(parentCtx context.Context, data actor.Envelope) error { + // Load the actor state + ctx, cancel := context.WithTimeout(parentCtx, 10*time.Second) + defer cancel() + state, err := a.client.GetState(ctx) + if err != nil { + return fmt.Errorf("error retrieving actor state: %w", err) + } + + // If we already have a state, nothing else to do + if state != nil { + return nil + } + + // Check if the request data contains legacy config to init from + if data != nil { + payload := appConfigActorBootstrap{} + err = data.Decode(&payload) + if err != nil { + return fmt.Errorf("request body is not valid for method 'init': %w", err) + } + + if len(payload.LegacyConfig) > 0 { + state, err = fromLegacyConfig(payload.LegacyConfig) + if err != nil { + return fmt.Errorf("request body is not valid for method 'init': LegacyConfig property could not be parsed: %w", err) + } + } + } + + // If we still have no state, generate a new default config + if state == nil { + state = getDefaultConfig() + } + + // Save the updated state + ctx, cancel = context.WithTimeout(parentCtx, 10*time.Second) + defer cancel() + err = a.client.SetState(ctx, state, nil) + if err != nil { + return fmt.Errorf("error saving actor state: %w", err) + } + + return nil +} + +func (a *appConfigActor) Peek(parentCtx context.Context, method string, data actor.Envelope) (any, error) { + // Only supported method is "get" + if method != "get" { + return nil, common.ErrUnsupportedActorMethod{Method: method} + } + + // Load the actor state + ctx, cancel := context.WithTimeout(parentCtx, 10*time.Second) + defer cancel() + state, err := a.client.GetState(ctx) + if err != nil { + return nil, fmt.Errorf("error retrieving actor state: %w", err) + } + + // Return the state + return state, nil +} + +func (a *appConfigActor) Invoke(parentCtx context.Context, method string, data actor.Envelope) (any, error) { + // Check the method first + switch method { + case "get", "update", "replace": + // All good + // Note: we support "get" also via Invoke and not just Peek + default: + return nil, common.ErrUnsupportedActorMethod{Method: method} + } + + // Load the actor state + ctx, cancel := context.WithTimeout(parentCtx, 10*time.Second) + defer cancel() + state, err := a.client.GetState(ctx) + if err != nil { + return nil, fmt.Errorf("error retrieving actor state: %w", err) + } + + switch method { + case "get": + // If the method is "get", just return the actor state, we're done + // This switch case is a no-op + + case "replace": + // Replace the entire config + // The input data must be a dto.AppConfigUpdateDto + if data == nil { + 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 + newState := state.Clone() + newState.Replace(payload) + + // 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, newState, nil) + if err != nil { + return nil, fmt.Errorf("error saving actor state: %w", err) + } + + return newState, nil + + case "update": + // Update the config + // The input data must be a map[string]string + if data == nil { + 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 + newState := state.Clone() + 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, which also updates the cached object + ctx, cancel = context.WithTimeout(parentCtx, 10*time.Second) + defer cancel() + err = a.client.SetState(ctx, newState, nil) + if err != nil { + return nil, fmt.Errorf("error saving actor state: %w", err) + } + + return newState, nil + } + + // Return the state + return state, nil +} diff --git a/backend/internal/appconfig/migration.go b/backend/internal/appconfig/migration.go new file mode 100644 index 00000000..0b2086ae --- /dev/null +++ b/backend/internal/appconfig/migration.go @@ -0,0 +1,97 @@ +package appconfig + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "reflect" + "strconv" + "strings" + "time" + + "github.com/italypaleale/go-kit/utils" + "gorm.io/gorm" + + "github.com/pocket-id/pocket-id/backend/internal/model" +) + +// This file holds the one-time migration of the legacy (pre-actor) app config +// The legacy config was stored in the "config_migrated" key of the kv table, and it's loaded here to bootstrap the AppConfig actor on first startup + +// LoadLegacyConfig loads the legacy config from the database +// This was migrated to the "config_migrated" key in the kv table +func LoadLegacyConfig(ctx context.Context, db *gorm.DB) (map[string]string, error) { + // Retrieve the migrated config from the kv table + row := model.KV{ + Key: "config_migrated", + } + ctx, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + err := db.WithContext(ctx).First(&row).Error + switch { + case errors.Is(err, gorm.ErrRecordNotFound): + // There's no migrated config in the database, nothing to do + return nil, nil + case err != nil: + return nil, fmt.Errorf("failed to load migrated config from the database: %w", err) + case row.Value == nil || len(*row.Value) == 0: + // Also no migrated config, nothing to do + return nil, nil + } + + // The value is a JSON-encoded dictionary + res := map[string]string{} + err = json.Unmarshal([]byte(*row.Value), &res) + if err != nil { + return nil, fmt.Errorf("error parsing migrated config: %w", err) + } + + if len(res) == 0 { + return nil, nil + } + return res, nil +} + +// fromLegacyConfig builds an appConfigModel from a legacy config map +// The map's keys correspond to the "json" tags on appConfigModel, and all values are strings that are cast to each field's type +// Keys that are missing (or have an empty value) retain the default value +func fromLegacyConfig(legacyCfg map[string]string) (*AppConfigModel, error) { + // Start from the default configuration, then override with the values from the legacy config + dest := getDefaultConfig() + + rt := reflect.ValueOf(dest).Elem().Type() + rv := reflect.ValueOf(dest).Elem() + for i := range rt.NumField() { + field := rt.Field(i) + + // Get the value of the json tag, taking only what's before the comma + key, _, _ := strings.Cut(field.Tag.Get("json"), ",") + + // Look up the value in the legacy config + // If the key is missing or the value is empty, we keep the default value + value, ok := legacyCfg[key] + if !ok || value == "" { + continue + } + + // Cast the string value to the field's type + fv := rv.Field(i) + switch fv.Kind() { //nolint:exhaustive + case reflect.String: + fv.SetString(value) + case reflect.Bool: + fv.SetBool(utils.IsTruthy(value)) + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + n, err := strconv.ParseInt(value, 10, 64) + if err != nil { + return nil, fmt.Errorf("failed to parse integer value for key '%s': %w", key, err) + } + fv.SetInt(n) + default: + return nil, fmt.Errorf("unsupported field type '%s' for key '%s'", fv.Kind(), key) + } + } + + return dest, nil +} diff --git a/backend/internal/appconfig/model.go b/backend/internal/appconfig/model.go new file mode 100644 index 00000000..47436522 --- /dev/null +++ b/backend/internal/appconfig/model.go @@ -0,0 +1,272 @@ +package appconfig + +import ( + "errors" + "reflect" + "strconv" + "strings" + "time" + + "github.com/italypaleale/go-kit/utils" + + "github.com/pocket-id/pocket-id/backend/internal/common" + "github.com/pocket-id/pocket-id/backend/internal/dto" +) + +type AppConfigModel struct { + // General + AppName AppConfigValue `json:"appName" public:"true"` + SessionDuration AppConfigValue `json:"sessionDuration" type:"int"` // In minutes + HomePageURL AppConfigValue `json:"homePageUrl" public:"true"` + EmailsVerified AppConfigValue `json:"emailsVerified" type:"bool"` + AccentColor AppConfigValue `json:"accentColor" public:"true"` + DisableAnimations AppConfigValue `json:"disableAnimations" type:"bool" public:"true"` + AllowOwnAccountEdit AppConfigValue `json:"allowOwnAccountEdit" type:"bool" public:"true"` + AllowUserSignups AppConfigValue `json:"allowUserSignups" public:"true"` + + SignupDefaultUserGroupIDs AppConfigValue `json:"signupDefaultUserGroupIDs"` // JSON-encoded array of strings + SignupDefaultCustomClaims AppConfigValue `json:"signupDefaultCustomClaims"` // JSON-encoded array of {key:string,value:string} + // Email + RequireUserEmail AppConfigValue `json:"requireUserEmail" type:"bool" public:"true"` + SmtpHost AppConfigValue `json:"smtpHost"` + SmtpPort AppConfigValue `json:"smtpPort"` + SmtpFrom AppConfigValue `json:"smtpFrom"` + SmtpUser AppConfigValue `json:"smtpUser"` + SmtpPassword AppConfigValue `json:"smtpPassword" sensitive:"true"` + SmtpTls AppConfigValue `json:"smtpTls"` + SmtpSkipCertVerify AppConfigValue `json:"smtpSkipCertVerify" type:"bool"` + EmailLoginNotificationEnabled AppConfigValue `json:"emailLoginNotificationEnabled" type:"bool"` + EmailOneTimeAccessAsUnauthenticatedEnabled AppConfigValue `json:"emailOneTimeAccessAsUnauthenticatedEnabled" type:"bool" public:"true"` + EmailOneTimeAccessAsAdminEnabled AppConfigValue `json:"emailOneTimeAccessAsAdminEnabled" type:"bool" public:"true"` + EmailApiKeyExpirationEnabled AppConfigValue `json:"emailApiKeyExpirationEnabled" type:"bool"` + EmailVerificationEnabled AppConfigValue `json:"emailVerificationEnabled" type:"bool" public:"true"` + // LDAP + LdapEnabled AppConfigValue `json:"ldapEnabled" type:"bool" public:"true"` + LdapUrl AppConfigValue `json:"ldapUrl"` + LdapBindDn AppConfigValue `json:"ldapBindDn"` + LdapBindPassword AppConfigValue `json:"ldapBindPassword" sensitive:"true"` + LdapBase AppConfigValue `json:"ldapBase"` + LdapUserSearchFilter AppConfigValue `json:"ldapUserSearchFilter"` + LdapUserGroupSearchFilter AppConfigValue `json:"ldapUserGroupSearchFilter"` + LdapSkipCertVerify AppConfigValue `json:"ldapSkipCertVerify" type:"bool"` + LdapAttributeUserUniqueIdentifier AppConfigValue `json:"ldapAttributeUserUniqueIdentifier"` + LdapAttributeUserUsername AppConfigValue `json:"ldapAttributeUserUsername"` + LdapAttributeUserEmail AppConfigValue `json:"ldapAttributeUserEmail"` + LdapAttributeUserFirstName AppConfigValue `json:"ldapAttributeUserFirstName"` + LdapAttributeUserLastName AppConfigValue `json:"ldapAttributeUserLastName"` + LdapAttributeUserDisplayName AppConfigValue `json:"ldapAttributeUserDisplayName"` + LdapAttributeUserProfilePicture AppConfigValue `json:"ldapAttributeUserProfilePicture"` + LdapAttributeGroupMember AppConfigValue `json:"ldapAttributeGroupMember"` + LdapAttributeGroupUniqueIdentifier AppConfigValue `json:"ldapAttributeGroupUniqueIdentifier"` + LdapAttributeGroupName AppConfigValue `json:"ldapAttributeGroupName"` + LdapAdminGroupName AppConfigValue `json:"ldapAdminGroupName"` + LdapSoftDeleteUsers AppConfigValue `json:"ldapSoftDeleteUsers" type:"bool"` +} + +// Clone returns a deep copy of the AppConfigModel. +func (m *AppConfigModel) Clone() *AppConfigModel { + if m == nil { + return nil + } + + // All fields are value types (AppConfigValue is a string), so copying the struct is sufficient for a deep copy. + clone := *m + return &clone +} + +// AppConfigValue holds a value +type AppConfigValue string + +// IsTrue returns true if the value is a truthy string, such as "true", "t", "yes", "1", etc. +func (a AppConfigValue) IsTrue() bool { + return utils.IsTruthy(string(a)) +} + +// AsDurationMinutes returns the value as a time.Duration, interpreting the string as a whole number of minutes. +func (a AppConfigValue) AsDurationMinutes() time.Duration { + val, err := strconv.Atoi(string(a)) + if err != nil { + return 0 + } + return time.Duration(val) * time.Minute +} + +// String implements fmt.Stringer +func (a AppConfigValue) String() string { + return string(a) +} + +func getDefaultConfig() *AppConfigModel { + // Values are the default ones + return &AppConfigModel{ + // General + AppName: "Pocket ID", + SessionDuration: "60", + HomePageURL: "/settings/account", + EmailsVerified: "false", + DisableAnimations: "false", + AllowOwnAccountEdit: "true", + AllowUserSignups: "disabled", + SignupDefaultUserGroupIDs: "[]", + SignupDefaultCustomClaims: "[]", + AccentColor: "default", + // Email + RequireUserEmail: "true", + SmtpHost: "", + SmtpPort: "", + SmtpFrom: "", + SmtpUser: "", + SmtpPassword: "", + SmtpTls: "none", + SmtpSkipCertVerify: "false", + EmailLoginNotificationEnabled: "false", + EmailOneTimeAccessAsUnauthenticatedEnabled: "false", + EmailOneTimeAccessAsAdminEnabled: "false", + EmailApiKeyExpirationEnabled: "false", + EmailVerificationEnabled: "false", + // LDAP + LdapEnabled: "false", + LdapUrl: "", + LdapBindDn: "", + LdapBindPassword: "", + LdapBase: "", + LdapUserSearchFilter: "(objectClass=person)", + LdapUserGroupSearchFilter: "(objectClass=groupOfNames)", + LdapSkipCertVerify: "false", + LdapAttributeUserUniqueIdentifier: "", + LdapAttributeUserUsername: "", + LdapAttributeUserEmail: "", + LdapAttributeUserFirstName: "", + LdapAttributeUserLastName: "", + LdapAttributeUserDisplayName: "cn", + LdapAttributeUserProfilePicture: "", + LdapAttributeGroupMember: "member", + LdapAttributeGroupUniqueIdentifier: "", + LdapAttributeGroupName: "", + LdapAdminGroupName: "", + LdapSoftDeleteUsers: "true", + } +} + +// Replace updates every configuration property with the values from the input DTO +// An empty string value resets the corresponding property to its default value +func (m *AppConfigModel) Replace(input dto.AppConfigUpdateDto) { + // Collect the values from the input DTO into a map, keyed by the "json" tag + inRv := reflect.ValueOf(input) + inRt := inRv.Type() + values := make(map[string]string, inRt.NumField()) + for i := range inRt.NumField() { + // Get the value of the json tag, taking only what's before the comma + key, _, _ := strings.Cut(inRt.Field(i).Tag.Get("json"), ",") + values[key] = inRv.Field(i).String() + } + + // Iterate through all the properties, setting each one from the input + // Properties that are missing from the input or have an empty value are reset to their default + defaults := reflect.ValueOf(getDefaultConfig()).Elem() + rv := reflect.ValueOf(m).Elem() + rt := rv.Type() + for i := range rt.NumField() { + key, _, _ := strings.Cut(rt.Field(i).Tag.Get("json"), ",") + + value, ok := values[key] + if !ok || value == "" { + value = defaults.Field(i).String() + } + + rv.Field(i).SetString(value) + } +} + +// Update sets configuration properties from the provided key-value pairs +// Keys correspond to the "json" tags on the model +// An empty string value resets the property to its default value +func (m *AppConfigModel) Update(values map[string]string) error { + rv := reflect.ValueOf(m).Elem() + rt := rv.Type() + defaults := reflect.ValueOf(getDefaultConfig()).Elem() + + // Iterate through the key-value pairs + for key, value := range values { + // Find the field in the struct whose "json" tag matches + fieldIdx := -1 + for j := range rt.NumField() { + // Separate the key (before the comma) from any optional attributes after + tagValue, _, _ := strings.Cut(rt.Field(j).Tag.Get("json"), ",") + if tagValue == key { + fieldIdx = j + break + } + } + if fieldIdx < 0 { + return AppConfigKeyNotFoundError{field: key} + } + + // An empty string means we use the default value for the property + if value == "" { + value = defaults.Field(fieldIdx).String() + } + + rv.Field(fieldIdx).SetString(value) + } + + 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 +} + +func (e AppConfigKeyNotFoundError) Error() string { + return "cannot find config key '" + e.field + "'" +} + +func (e AppConfigKeyNotFoundError) Is(target error) bool { + // Ignore the field property when checking if an error is of the type AppConfigKeyNotFoundError + _, ok := errors.AsType[*AppConfigKeyNotFoundError](target) + return ok +} diff --git a/backend/internal/appconfig/model_test.go b/backend/internal/appconfig/model_test.go new file mode 100644 index 00000000..961760f3 --- /dev/null +++ b/backend/internal/appconfig/model_test.go @@ -0,0 +1,225 @@ +package appconfig + +import ( + "errors" + "reflect" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/pocket-id/pocket-id/backend/internal/dto" +) + +// dtoWithMarkerValues returns a DTO where every field is set to a unique, non-empty marker derived from its "json" key, so we can assert each value lands in the right place. +func dtoWithMarkerValues() dto.AppConfigUpdateDto { + var input dto.AppConfigUpdateDto + rv := reflect.ValueOf(&input).Elem() + rt := rv.Type() + for i := range rt.NumField() { + key, _, _ := strings.Cut(rt.Field(i).Tag.Get("json"), ",") + rv.Field(i).SetString("marker-" + key) + } + return input +} + +func TestAppConfigModel_Replace(t *testing.T) { + t.Run("populates every property from the DTO", func(t *testing.T) { + input := dtoWithMarkerValues() + + var m AppConfigModel + m.Replace(input) + + // Each model property must hold the marker built from its own "json" key. + // This also asserts that the model and the DTO share the same set of keys. + rv := reflect.ValueOf(&m).Elem() + rt := rv.Type() + for i := range rt.NumField() { + key, _, _ := strings.Cut(rt.Field(i).Tag.Get("json"), ",") + assert.Equalf(t, "marker-"+key, rv.Field(i).String(), "property %s (key %q)", rt.Field(i).Name, key) + } + }) + + t.Run("empty values fall back to their default", func(t *testing.T) { + defaults := getDefaultConfig() + + // Start from all-markers, then blank out a few properties whose default is non-empty + input := dtoWithMarkerValues() + input.AppName = "" + input.SessionDuration = "" + input.SmtpTls = "" + input.LdapUserSearchFilter = "" + + var m AppConfigModel + m.Replace(input) + + // Blanked properties are reset to their default + assert.Equal(t, defaults.AppName, m.AppName) + assert.Equal(t, defaults.SessionDuration, m.SessionDuration) + assert.Equal(t, defaults.SmtpTls, m.SmtpTls) + assert.Equal(t, defaults.LdapUserSearchFilter, m.LdapUserSearchFilter) + + // A property that was provided keeps the provided value + assert.Equal(t, AppConfigValue("marker-homePageUrl"), m.HomePageURL) + }) + + t.Run("an empty DTO resets every property to its default", func(t *testing.T) { + // Pre-populate with junk to prove Replace overwrites existing state + m := AppConfigModel{ + AppName: "Custom Name", + LdapEnabled: "true", + SmtpHost: "smtp.example.com", + } + + m.Replace(dto.AppConfigUpdateDto{}) + + assert.Equal(t, *getDefaultConfig(), m) + }) + + t.Run("provided values overwrite existing non-default values", func(t *testing.T) { + m := getDefaultConfig() + m.AppName = "Old Name" + m.LdapEnabled = "true" + + input := dto.AppConfigUpdateDto{} + input.AppName = "New Name" + + m.Replace(input) + + // Explicitly provided value wins + 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) + }) + + t.Run("stores raw string values without type coercion", func(t *testing.T) { + input := dto.AppConfigUpdateDto{} + input.SessionDuration = "120" // int-tagged property + input.LdapEnabled = "true" // bool-tagged property + + var m AppConfigModel + m.Replace(input) + + assert.Equal(t, AppConfigValue("120"), m.SessionDuration) + assert.Equal(t, AppConfigValue("true"), m.LdapEnabled) + }) +} + +func TestAppConfigModel_Clone(t *testing.T) { + t.Run("clones every property", func(t *testing.T) { + // Populate every property with a unique marker so we can assert each one is copied + var original AppConfigModel + rv := reflect.ValueOf(&original).Elem() + rt := rv.Type() + for i := range rt.NumField() { + key, _, _ := strings.Cut(rt.Field(i).Tag.Get("json"), ",") + rv.Field(i).SetString("marker-" + key) + } + + clone := original.Clone() + + require.NotNil(t, clone) + // The clone must be a distinct object with equal contents + assert.NotSame(t, &original, clone) + assert.Equal(t, original, *clone) + }) + + t.Run("mutating the clone does not affect the original", func(t *testing.T) { + original := getDefaultConfig() + + clone := original.Clone() + clone.AppName = "Changed" + clone.LdapEnabled = "true" + + // The original keeps its values + assert.Equal(t, getDefaultConfig().AppName, original.AppName) + assert.Equal(t, getDefaultConfig().LdapEnabled, original.LdapEnabled) + + // The clone holds the new values + assert.Equal(t, AppConfigValue("Changed"), clone.AppName) + assert.Equal(t, AppConfigValue("true"), clone.LdapEnabled) + }) + + t.Run("mutating the original does not affect the clone", func(t *testing.T) { + original := getDefaultConfig() + + clone := original.Clone() + original.AppName = "Changed" + + assert.Equal(t, getDefaultConfig().AppName, clone.AppName) + }) + + t.Run("cloning a nil receiver returns nil", func(t *testing.T) { + var m *AppConfigModel + assert.Nil(t, m.Clone()) + }) +} + +func TestAppConfigModel_Update(t *testing.T) { + t.Run("updates a single property", func(t *testing.T) { + m := getDefaultConfig() + + err := m.Update(map[string]string{"appName": "My App"}) + require.NoError(t, err) + + assert.Equal(t, AppConfigValue("My App"), m.AppName) + }) + + t.Run("updates multiple properties and leaves others untouched", func(t *testing.T) { + m := getDefaultConfig() + + err := m.Update(map[string]string{"appName": "My App", "homePageUrl": "/home", "ldapEnabled": "true"}) + require.NoError(t, err) + + 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) + }) + + t.Run("an empty value resets the property to its default", func(t *testing.T) { + m := getDefaultConfig() + m.SmtpTls = "tls" // default is "none" + m.SessionDuration = "120" // default is "60" + + err := m.Update(map[string]string{"smtpTls": "", "sessionDuration": ""}) + require.NoError(t, err) + + assert.Equal(t, getDefaultConfig().SmtpTls, m.SmtpTls) + assert.Equal(t, getDefaultConfig().SessionDuration, m.SessionDuration) + }) + + t.Run("stores raw string values without type coercion", func(t *testing.T) { + m := getDefaultConfig() + + err := m.Update(map[string]string{"sessionDuration": "120", "disableAnimations": "true"}) + require.NoError(t, err) + + 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) { + m := getDefaultConfig() + before := *m + + err := m.Update(nil) + require.NoError(t, err) + + assert.Equal(t, before, *m) + }) + + t.Run("an unknown key returns AppConfigKeyNotFoundError", func(t *testing.T) { + m := getDefaultConfig() + + err := m.Update(map[string]string{"thisKeyDoesNotExist": "value"}) + require.Error(t, err) + require.EqualError(t, err, "cannot find config key 'thisKeyDoesNotExist'") + + notFound, ok := errors.AsType[AppConfigKeyNotFoundError](err) + require.True(t, ok) + assert.Equal(t, "thisKeyDoesNotExist", notFound.field) + }) +} diff --git a/backend/internal/appconfig/service.go b/backend/internal/appconfig/service.go new file mode 100644 index 00000000..3eb92f4a --- /dev/null +++ b/backend/internal/appconfig/service.go @@ -0,0 +1,210 @@ +package appconfig + +import ( + "context" + "errors" + "fmt" + "os" + "reflect" + "strings" + "time" + + "github.com/italypaleale/francis/actor" + "github.com/italypaleale/francis/host/local" + "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/tracing" + "github.com/pocket-id/pocket-id/backend/internal/utils" +) + +type AppConfigService struct { + actSvc *actor.Service + envConfig *AppConfigModel +} + +func NewService(ctx context.Context, actors *local.Host, db *gorm.DB) (service *AppConfigService, err error) { + service = &AppConfigService{} + + // If the UI config is disabled, we do not need to init the config actor + if common.EnvConfig.UiConfigDisabled { + service.envConfig, err = service.loadDbConfigFromEnv() + if err != nil { + return nil, fmt.Errorf("error loading app config from the env: %w", err) + } + + return service, nil + } + + // Note: we need to assign to the "err" variable in this method (for tracing), do not inline this into the "if" + ctx, span := tracing.Start(ctx, "pocketid.appconfig.init") + defer tracing.End(span, err) + + // Load the legacy config if any, which we need to send to the actor as bootstrap data + legacyCfg, err := LoadLegacyConfig(ctx, db) + if err != nil { + return nil, fmt.Errorf("error loading legacy config: %w", err) + } + + // Register the AppConfig actor + // This is a singleton actor and it's bootstrapped with the legacy config if present + bootstrapData := &appConfigActorBootstrap{ + LegacyConfig: legacyCfg, + } + err = actors.RegisterSingletonActor( + AppConfigActorType, NewAppConfigActor, + local.WithBootstrapData(bootstrapData), + local.WithIdleTimeout(-1), // Disable idle timeout for this actor + ) + if err != nil { + return nil, fmt.Errorf("error registering the %s actor: %w", AppConfigActorType, err) + } + + service.actSvc = actors.Service() + + return service, nil +} + +// GetConfig returns the application configuration +// Important: Treat the object as read-only: do not modify its properties directly! +func (s *AppConfigService) GetConfig(parentCtx context.Context) (*AppConfigModel, error) { + // If the UI config is disabled, only load from the env + if common.EnvConfig.UiConfigDisabled { + return s.envConfig, nil + } + + // Retrieve the config from the actor + ctx, cancel := context.WithTimeout(parentCtx, 10*time.Second) + defer cancel() + res, err := s.actSvc.Peek(ctx, AppConfigActorType, actor.SingletonActorID, "get", nil) + if err != nil { + return nil, fmt.Errorf("error retrieving config from actor: %w", err) + } + if res == nil { + return nil, errors.New("config actor response was empty") + } + + var cfg AppConfigModel + err = res.Decode(&cfg) + if err != nil { + return nil, fmt.Errorf("error decoding config actor response: %w", err) + } + + return &cfg, nil +} + +// 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{} + } + + // Replace the entire config by invoking the actor + cfg, err := s.invokeConfigActor(ctx, "replace", input) + if err != nil { + return nil, err + } + + // Return the updated config + return cfg.ToAppConfigVariableSlice(true, false), nil +} + +// 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 { + return errors.New("invalid number of arguments received") + } + + // If the UI config is disabled, we cannot continue + if common.EnvConfig.UiConfigDisabled { + return &common.UiConfigDisabledError{} + } + + // Collect the key-value pairs into a map for the actor + // (Note the += 2, as we are iterating through key-value pairs) + values := make(map[string]string, len(keysAndValues)/2) + for i := 1; i < len(keysAndValues); i += 2 { + values[keysAndValues[i-1]] = keysAndValues[i] + } + + // 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 nil, err + } + + 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 nil, fmt.Errorf("error invoking config actor method '%s': %w", method, err) + } + if res == nil { + return nil, errors.New("config actor response was empty") + } + + var cfg AppConfigModel + err = res.Decode(&cfg) + if err != nil { + return nil, fmt.Errorf("error decoding config actor response: %w", err) + } + + return &cfg, nil +} + +func (s *AppConfigService) loadDbConfigFromEnv() (*AppConfigModel, error) { + // First, start from the default configuration + dest := getDefaultConfig() + + // Iterate through each field + rt := reflect.ValueOf(dest).Elem().Type() + rv := reflect.ValueOf(dest).Elem() + for i := range rt.NumField() { + field := rt.Field(i) + + // Derive the environment variable name from the configuration's JSON key + key, _, _ := strings.Cut(field.Tag.Get("json"), ",") + envVarName := utils.CamelCaseToScreamingSnakeCase(key) + + // Set the value if it's set + value, ok := os.LookupEnv(envVarName) + if ok { + rv.Field(i).SetString(value) + continue + } + + // If it's sensitive, we also allow reading from file + if field.Tag.Get("sensitive") == "true" { + fileName := os.Getenv(envVarName + "_FILE") + if fileName != "" { + // #nosec G703 - Value is provided by admin + b, err := os.ReadFile(fileName) + if err != nil { + return nil, fmt.Errorf("failed to read secret '%s' from file '%s': %w", envVarName, fileName, err) + } + + rv.Field(i).SetString(string(b)) + continue + } + } + } + + return dest, nil +} diff --git a/backend/internal/appconfig/service_test.go b/backend/internal/appconfig/service_test.go new file mode 100644 index 00000000..1e3b08ad --- /dev/null +++ b/backend/internal/appconfig/service_test.go @@ -0,0 +1,344 @@ +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) + t.Setenv("APP_NAME", "Environment App") + + // 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, AppConfigValue("Environment App"), cfg.AppName) + }) +} + +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/appconfig/testing_unit.go b/backend/internal/appconfig/testing_unit.go new file mode 100644 index 00000000..3df1fa6c --- /dev/null +++ b/backend/internal/appconfig/testing_unit.go @@ -0,0 +1,27 @@ +//go:build unit + +// This file contains utils for unit tests and it's only built when the "unit" tag is set +package appconfig + +// NewTestAppConfigService is a function used by tests to create AppConfigService objects with pre-defined configuration values +func NewTestAppConfigService(config *AppConfigModel) *AppConfigService { + if config == nil { + // If there's no config, set the default one + config = getDefaultConfig() + } + + service := &AppConfigService{ + envConfig: config, + } + + return service +} + +// NewTestConfig returns an application configuration for use in tests, falling back to the default configuration when none is provided +func NewTestConfig(config *AppConfigModel) *AppConfigModel { + if config == nil { + config = getDefaultConfig() + } + + return config +} diff --git a/backend/internal/bootstrap/bootstrap.go b/backend/internal/bootstrap/bootstrap.go index 53505426..8fd171e8 100644 --- a/backend/internal/bootstrap/bootstrap.go +++ b/backend/internal/bootstrap/bootstrap.go @@ -100,7 +100,7 @@ func Bootstrap(ctx context.Context) error { services = append(services, actorsRun) // Create all services - svc, err := initServices(ctx, db, instanceID, httpClient, imageExtensions, fileStorage, scheduler) + svc, err := initServices(ctx, db, instanceID, actors, httpClient, imageExtensions, fileStorage, scheduler) if err != nil { return fmt.Errorf("failed to initialize services: %w", err) } diff --git a/backend/internal/bootstrap/router_bootstrap.go b/backend/internal/bootstrap/router_bootstrap.go index 9969045f..11a71b96 100644 --- a/backend/internal/bootstrap/router_bootstrap.go +++ b/backend/internal/bootstrap/router_bootstrap.go @@ -158,11 +158,11 @@ func registerRoutes(r *gin.Engine, db *gorm.DB, svc *services, rateLimitServices rateLimitMiddleware.Add(middleware.RateLimitWebauthnReauthenticate), ) controller.NewOidcController(apiGroup, authMiddleware, fileSizeLimitMiddleware, svc.oidcService) - controller.NewUserController(apiGroup, authMiddleware, rateLimitMiddleware, svc.userService, svc.oneTimeAccessService, svc.webauthnModule, svc.appConfigService) + controller.NewUserController(apiGroup, authMiddleware, rateLimitMiddleware, svc.appConfigService, svc.userService, svc.oneTimeAccessService, svc.webauthnModule) controller.NewAppConfigController(apiGroup, authMiddleware, svc.appConfigService, svc.emailService, svc.ldapService) controller.NewAppImagesController(apiGroup, authMiddleware, svc.appImagesService) controller.NewAuditLogController(apiGroup, svc.auditLogService, authMiddleware) - controller.NewUserGroupController(apiGroup, authMiddleware, svc.userGroupService) + controller.NewUserGroupController(apiGroup, authMiddleware, svc.appConfigService, svc.userGroupService) svc.apiModule.RegisterRoutes(apiGroup, authMiddleware.Add()) controller.NewCustomClaimController(apiGroup, authMiddleware, svc.customClaimService) controller.NewVersionController(apiGroup, authMiddleware, svc.versionService) diff --git a/backend/internal/bootstrap/services_bootstrap.go b/backend/internal/bootstrap/services_bootstrap.go index d05edf06..c3600f4f 100644 --- a/backend/internal/bootstrap/services_bootstrap.go +++ b/backend/internal/bootstrap/services_bootstrap.go @@ -5,12 +5,14 @@ import ( "fmt" "net/http" - "github.com/pocket-id/pocket-id/backend/internal/apikey" - "github.com/pocket-id/pocket-id/backend/internal/job" + "github.com/italypaleale/francis/host/local" "gorm.io/gorm" "github.com/pocket-id/pocket-id/backend/internal/api" + "github.com/pocket-id/pocket-id/backend/internal/apikey" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/common" + "github.com/pocket-id/pocket-id/backend/internal/job" "github.com/pocket-id/pocket-id/backend/internal/oidc" "github.com/pocket-id/pocket-id/backend/internal/service" "github.com/pocket-id/pocket-id/backend/internal/storage" @@ -19,7 +21,7 @@ import ( ) type services struct { - appConfigService *service.AppConfigService + appConfigService *appconfig.AppConfigService appImagesService *service.AppImagesService emailService *service.EmailService geoLiteService *service.GeoLiteService @@ -44,10 +46,20 @@ type services struct { } // Initializes all services -func initServices(ctx context.Context, db *gorm.DB, instanceID string, httpClient *http.Client, imageExtensions map[string]string, fileStorage storage.FileStorage, scheduler *job.Scheduler) (svc *services, err error) { +func initServices( + ctx context.Context, + db *gorm.DB, + instanceID string, + actors *local.Host, + httpClient *http.Client, + imageExtensions map[string]string, + fileStorage storage.FileStorage, + scheduler *job.Scheduler, +) (svc *services, err error) { svc = &services{} - svc.appConfigService, err = service.NewAppConfigService(ctx, db) + // Init the app config service + svc.appConfigService, err = appconfig.NewService(ctx, actors, db) if err != nil { return nil, fmt.Errorf("failed to create app config service: %w", err) } @@ -56,14 +68,14 @@ func initServices(ctx context.Context, db *gorm.DB, instanceID string, httpClien svc.appImagesService = service.NewAppImagesService(imageExtensions, fileStorage) svc.appLockService = service.NewAppLockService(db) - svc.emailService, err = service.NewEmailService(db, svc.appConfigService) + svc.emailService, err = service.NewEmailService(db) if err != nil { return nil, fmt.Errorf("failed to create email service: %w", err) } svc.geoLiteService = service.NewGeoLiteService(httpClient) - svc.auditLogService = service.NewAuditLogService(db, svc.appConfigService, svc.emailService, svc.geoLiteService) - svc.jwtService, err = service.NewJwtService(ctx, db, instanceID, svc.appConfigService) + svc.auditLogService = service.NewAuditLogService(db, svc.emailService, svc.geoLiteService, svc.appConfigService) + svc.jwtService, err = service.NewJwtService(ctx, db, instanceID) if err != nil { return nil, fmt.Errorf("failed to create JWT service: %w", err) } @@ -103,14 +115,14 @@ func initServices(ctx context.Context, db *gorm.DB, instanceID string, httpClien return nil, fmt.Errorf("failed to create OIDC module: %w", err) } - svc.oidcService, err = service.NewOidcService(db, svc.jwtService, svc.appConfigService, svc.oidcModule.Preview, svc.scimService, httpClient, fileStorage) + svc.oidcService, err = service.NewOidcService(db, svc.jwtService, svc.oidcModule.Preview, svc.scimService, httpClient, fileStorage) if err != nil { return nil, fmt.Errorf("failed to create OIDC service: %w", err) } - svc.userGroupService = service.NewUserGroupService(db, svc.appConfigService, svc.scimService) - svc.userService = service.NewUserService(db, svc.jwtService, svc.auditLogService, svc.emailService, svc.appConfigService, svc.customClaimService, svc.appImagesService, svc.scimService, fileStorage) - svc.ldapService = service.NewLdapService(db, httpClient, svc.appConfigService, svc.userService, svc.userGroupService, fileStorage) + svc.userGroupService = service.NewUserGroupService(db, svc.scimService) + svc.userService = service.NewUserService(db, svc.jwtService, svc.auditLogService, svc.emailService, svc.customClaimService, svc.appImagesService, svc.scimService, fileStorage) + svc.ldapService = service.NewLdapService(db, httpClient, svc.userService, svc.userGroupService, fileStorage) svc.apiKeyModule, err = apikey.New(ctx, apikey.Dependencies{ DB: db, @@ -124,10 +136,10 @@ func initServices(ctx context.Context, db *gorm.DB, instanceID string, httpClien DB: db, Signer: svc.jwtService, AuditLog: svc.auditLogService, - AppConfig: svc.appConfigService, UserCreator: svc.userService, + AppConfig: svc.appConfigService, }) - svc.oneTimeAccessService = service.NewOneTimeAccessService(db, svc.userService, svc.jwtService, svc.auditLogService, svc.emailService, svc.appConfigService) + svc.oneTimeAccessService = service.NewOneTimeAccessService(db, svc.userService, svc.jwtService, svc.auditLogService, svc.emailService) svc.versionService = service.NewVersionService(httpClient) diff --git a/backend/internal/cmds/encryption_key_rotate.go b/backend/internal/cmds/encryption_key_rotate.go index d28a535f..e620ac3c 100644 --- a/backend/internal/cmds/encryption_key_rotate.go +++ b/backend/internal/cmds/encryption_key_rotate.go @@ -6,13 +6,13 @@ import ( "fmt" "os" - "github.com/pocket-id/pocket-id/backend/internal/model" "github.com/spf13/cobra" "gorm.io/gorm" "github.com/pocket-id/pocket-id/backend/internal/bootstrap" "github.com/pocket-id/pocket-id/backend/internal/common" "github.com/pocket-id/pocket-id/backend/internal/instanceid" + "github.com/pocket-id/pocket-id/backend/internal/model" datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" "github.com/pocket-id/pocket-id/backend/internal/utils" jwkutils "github.com/pocket-id/pocket-id/backend/internal/utils/jwk" diff --git a/backend/internal/cmds/encryption_key_rotate_test.go b/backend/internal/cmds/encryption_key_rotate_test.go index a90749ee..a22f0e46 100644 --- a/backend/internal/cmds/encryption_key_rotate_test.go +++ b/backend/internal/cmds/encryption_key_rotate_test.go @@ -4,12 +4,12 @@ import ( "testing" "time" - "github.com/pocket-id/pocket-id/backend/internal/model" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/pocket-id/pocket-id/backend/internal/common" "github.com/pocket-id/pocket-id/backend/internal/instanceid" + "github.com/pocket-id/pocket-id/backend/internal/model" datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" jwkutils "github.com/pocket-id/pocket-id/backend/internal/utils/jwk" testingutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing" diff --git a/backend/internal/common/internal_errors.go b/backend/internal/common/internal_errors.go new file mode 100644 index 00000000..26765d3e --- /dev/null +++ b/backend/internal/common/internal_errors.go @@ -0,0 +1,20 @@ +package common + +import ( + "errors" +) + +// ErrUnsupportedActorMethod is returned by custom actors when the invoked method isn't supported +type ErrUnsupportedActorMethod struct { + Method string +} + +func (e ErrUnsupportedActorMethod) Error() string { + return "method '" + e.Method + "' unsupported for actor invocation" +} + +func (e ErrUnsupportedActorMethod) Is(target error) bool { + // Ignore the field method when checking if an error is of the type ErrUnsupportedActorMethod + _, ok := errors.AsType[ErrUnsupportedActorMethod](target) + return ok +} diff --git a/backend/internal/controller/app_config_controller.go b/backend/internal/controller/app_config_controller.go index c52d8b1c..289cd765 100644 --- a/backend/internal/controller/app_config_controller.go +++ b/backend/internal/controller/app_config_controller.go @@ -5,6 +5,7 @@ import ( "strconv" "github.com/gin-gonic/gin" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "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/middleware" @@ -19,7 +20,7 @@ import ( func NewAppConfigController( group *gin.RouterGroup, authMiddleware *middleware.AuthMiddleware, - appConfigService *service.AppConfigService, + appConfigService *appconfig.AppConfigService, emailService *service.EmailService, ldapService *service.LdapService, ) { @@ -38,7 +39,7 @@ func NewAppConfigController( } type AppConfigController struct { - appConfigService *service.AppConfigService + appConfigService *appconfig.AppConfigService emailService *service.EmailService ldapService *service.LdapService } @@ -52,7 +53,12 @@ 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) + dbConfig, err := acc.appConfigService.GetConfig(c.Request.Context()) + if err != nil { + _ = c.Error(err) + return + } + configuration := dbConfig.ToAppConfigVariableSlice(false, true) var configVariablesDto []dto.PublicAppConfigVariableDto if err := dto.MapStructList(configuration, &configVariablesDto); err != nil { @@ -86,7 +92,12 @@ 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) + dbConfig, err := acc.appConfigService.GetConfig(c.Request.Context()) + if err != nil { + _ = c.Error(err) + return + } + configuration := dbConfig.ToAppConfigVariableSlice(true, true) var configVariablesDto []dto.AppConfigVariableDto if err := dto.MapStructList(configuration, &configVariablesDto); err != nil { @@ -135,7 +146,13 @@ func (acc *AppConfigController) updateAppConfigHandler(c *gin.Context) { // @Success 204 "No Content" // @Router /api/application-configuration/sync-ldap [post] func (acc *AppConfigController) syncLdapHandler(c *gin.Context) { - err := acc.ldapService.SyncAll(c.Request.Context()) + dbConfig, err := acc.appConfigService.GetConfig(c.Request.Context()) + if err != nil { + _ = c.Error(err) + return + } + + err = acc.ldapService.SyncAll(c.Request.Context(), dbConfig) if err != nil { _ = c.Error(err) return @@ -151,9 +168,15 @@ func (acc *AppConfigController) syncLdapHandler(c *gin.Context) { // @Success 204 "No Content" // @Router /api/application-configuration/test-email [post] func (acc *AppConfigController) testEmailHandler(c *gin.Context) { + dbConfig, err := acc.appConfigService.GetConfig(c.Request.Context()) + if err != nil { + _ = c.Error(err) + return + } + userID := c.GetString("userID") - err := acc.emailService.SendTestEmail(c.Request.Context(), userID) + err = acc.emailService.SendTestEmail(c.Request.Context(), dbConfig, userID) if err != nil { _ = c.Error(err) return diff --git a/backend/internal/controller/user_controller.go b/backend/internal/controller/user_controller.go index 615aa21e..57ea9095 100644 --- a/backend/internal/controller/user_controller.go +++ b/backend/internal/controller/user_controller.go @@ -1,9 +1,11 @@ package controller import ( + "fmt" "net/http" "time" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/common" "github.com/pocket-id/pocket-id/backend/internal/utils/cookie" @@ -21,12 +23,12 @@ const defaultOneTimeAccessTokenDuration = 15 * time.Minute // @Summary User management controller // @Description Initializes all user-related API endpoints // @Tags Users -func NewUserController(group *gin.RouterGroup, authMiddleware *middleware.AuthMiddleware, rateLimitMiddleware *middleware.RateLimitMiddleware, userService *service.UserService, oneTimeAccessService *service.OneTimeAccessService, webAuthnService *webauthn.Module, appConfigService *service.AppConfigService) { +func NewUserController(group *gin.RouterGroup, authMiddleware *middleware.AuthMiddleware, rateLimitMiddleware *middleware.RateLimitMiddleware, appConfigService *appconfig.AppConfigService, userService *service.UserService, oneTimeAccessService *service.OneTimeAccessService, webAuthnService *webauthn.Module) { uc := UserController{ + appConfigService: appConfigService, userService: userService, oneTimeAccessService: oneTimeAccessService, webAuthnService: webAuthnService, - appConfigService: appConfigService, } group.GET("/users", authMiddleware.Add(), uc.listUsersHandler) @@ -61,10 +63,10 @@ func NewUserController(group *gin.RouterGroup, authMiddleware *middleware.AuthMi } type UserController struct { + appConfigService *appconfig.AppConfigService userService *service.UserService oneTimeAccessService *service.OneTimeAccessService webAuthnService *webauthn.Module - appConfigService *service.AppConfigService } // getUserGroupsHandler godoc @@ -207,7 +209,13 @@ func (uc *UserController) getCurrentUserHandler(c *gin.Context) { // @Success 204 "No Content" // @Router /api/users/{id} [delete] func (uc *UserController) deleteUserHandler(c *gin.Context) { - if err := uc.userService.DeleteUser(c.Request.Context(), c.Param("id"), false); err != nil { + dbConfig, err := uc.appConfigService.GetConfig(c.Request.Context()) + if err != nil { + _ = c.Error(fmt.Errorf("error loading app configuration: %w", err)) + return + } + + if err := uc.userService.DeleteUser(c.Request.Context(), dbConfig, c.Param("id"), false); err != nil { _ = c.Error(err) return } @@ -248,13 +256,19 @@ func (uc *UserController) deleteUserWebauthnCredentialHandler(c *gin.Context) { // @Success 201 {object} dto.UserDto // @Router /api/users [post] func (uc *UserController) createUserHandler(c *gin.Context) { + dbConfig, err := uc.appConfigService.GetConfig(c.Request.Context()) + if err != nil { + _ = c.Error(fmt.Errorf("error loading app configuration: %w", err)) + return + } + var input dto.UserCreateDto if err := dto.ShouldBindWithNormalizedJSON(c, &input); err != nil { _ = c.Error(err) return } - user, err := uc.userService.CreateUser(c.Request.Context(), input) + user, err := uc.userService.CreateUser(c.Request.Context(), dbConfig, input) if err != nil { _ = c.Error(err) return @@ -452,13 +466,19 @@ func (uc *UserController) createAdminOneTimeAccessTokenHandler(c *gin.Context) { // @Success 204 "No Content" // @Router /api/one-time-access-email [post] func (uc *UserController) RequestOneTimeAccessEmailAsUnauthenticatedUserHandler(c *gin.Context) { + dbConfig, err := uc.appConfigService.GetConfig(c.Request.Context()) + if err != nil { + _ = c.Error(fmt.Errorf("error loading app configuration: %w", err)) + return + } + var input dto.OneTimeAccessEmailAsUnauthenticatedUserDto if err := dto.ShouldBindWithNormalizedJSON(c, &input); err != nil { _ = c.Error(err) return } - deviceToken, err := uc.oneTimeAccessService.RequestOneTimeAccessEmailAsUnauthenticatedUser(c.Request.Context(), input.Email, input.RedirectPath) + deviceToken, err := uc.oneTimeAccessService.RequestOneTimeAccessEmailAsUnauthenticatedUser(c.Request.Context(), dbConfig, input.Email, input.RedirectPath) if err != nil { _ = c.Error(err) return @@ -479,6 +499,12 @@ func (uc *UserController) RequestOneTimeAccessEmailAsUnauthenticatedUserHandler( // @Success 204 "No Content" // @Router /api/users/{id}/one-time-access-email [post] func (uc *UserController) RequestOneTimeAccessEmailAsAdminHandler(c *gin.Context) { + dbConfig, err := uc.appConfigService.GetConfig(c.Request.Context()) + if err != nil { + _ = c.Error(fmt.Errorf("error loading app configuration: %w", err)) + return + } + var input dto.OneTimeAccessEmailAsAdminDto if err := c.ShouldBindJSON(&input); err != nil { _ = c.Error(err) @@ -491,7 +517,7 @@ func (uc *UserController) RequestOneTimeAccessEmailAsAdminHandler(c *gin.Context if ttl <= 0 { ttl = defaultOneTimeAccessTokenDuration } - err := uc.oneTimeAccessService.RequestOneTimeAccessEmailAsAdmin(c.Request.Context(), userID, ttl) + err = uc.oneTimeAccessService.RequestOneTimeAccessEmailAsAdmin(c.Request.Context(), dbConfig, userID, ttl) if err != nil { _ = c.Error(err) return @@ -508,6 +534,12 @@ func (uc *UserController) RequestOneTimeAccessEmailAsAdminHandler(c *gin.Context // @Success 200 {object} dto.UserDto // @Router /api/one-time-access-token/{token} [post] func (uc *UserController) exchangeOneTimeAccessTokenHandler(c *gin.Context) { + cfg, err := uc.appConfigService.GetConfig(c.Request.Context()) + if err != nil { + _ = c.Error(fmt.Errorf("error loading app configuration: %w", err)) + return + } + loginCode := c.Param("token") // reject invalid length login codes if len(loginCode) != 6 && len(loginCode) != 16 { @@ -516,19 +548,20 @@ func (uc *UserController) exchangeOneTimeAccessTokenHandler(c *gin.Context) { } deviceToken, _ := c.Cookie(cookie.DeviceTokenCookieName) - user, token, err := uc.oneTimeAccessService.ExchangeOneTimeAccessToken(c.Request.Context(), loginCode, deviceToken, c.ClientIP(), c.Request.UserAgent()) + user, token, err := uc.oneTimeAccessService.ExchangeOneTimeAccessToken(c.Request.Context(), cfg, loginCode, deviceToken, c.ClientIP(), c.Request.UserAgent()) if err != nil { _ = c.Error(err) return } var userDto dto.UserDto - if err := dto.MapStruct(user, &userDto); err != nil { + err = dto.MapStruct(user, &userDto) + if err != nil { _ = c.Error(err) return } - maxAge := int(uc.appConfigService.GetDbConfig().SessionDuration.AsDurationMinutes().Seconds()) + maxAge := int(cfg.SessionDuration.AsDurationMinutes().Seconds()) cookie.AddAccessTokenCookie(c, maxAge, token) c.JSON(http.StatusOK, userDto) @@ -566,6 +599,12 @@ func (uc *UserController) updateUserGroups(c *gin.Context) { // updateUser is an internal helper method, not exposed as an API endpoint func (uc *UserController) updateUser(c *gin.Context, updateOwnUser bool) { + dbConfig, err := uc.appConfigService.GetConfig(c.Request.Context()) + if err != nil { + _ = c.Error(fmt.Errorf("error loading app configuration: %w", err)) + return + } + var input dto.UserCreateDto if err := dto.ShouldBindWithNormalizedJSON(c, &input); err != nil { _ = c.Error(err) @@ -579,7 +618,7 @@ func (uc *UserController) updateUser(c *gin.Context, updateOwnUser bool) { userID = c.Param("id") } - user, err := uc.userService.UpdateUser(c.Request.Context(), userID, input, updateOwnUser, false) + user, err := uc.userService.UpdateUser(c.Request.Context(), dbConfig, userID, input, updateOwnUser, false) if err != nil { _ = c.Error(err) return @@ -639,9 +678,15 @@ func (uc *UserController) resetCurrentUserProfilePictureHandler(c *gin.Context) // @Success 204 "No Content" // @Router /api/users/me/send-email-verification [post] func (uc *UserController) sendEmailVerificationHandler(c *gin.Context) { + dbConfig, err := uc.appConfigService.GetConfig(c.Request.Context()) + if err != nil { + _ = c.Error(fmt.Errorf("error loading app configuration: %w", err)) + return + } + userID := c.GetString("userID") - if err := uc.userService.SendEmailVerification(c.Request.Context(), userID); err != nil { + if err := uc.userService.SendEmailVerification(c.Request.Context(), dbConfig, userID); err != nil { _ = c.Error(err) return } diff --git a/backend/internal/controller/user_group_controller.go b/backend/internal/controller/user_group_controller.go index a0567752..5ecfd319 100644 --- a/backend/internal/controller/user_group_controller.go +++ b/backend/internal/controller/user_group_controller.go @@ -1,9 +1,11 @@ package controller import ( + "fmt" "net/http" "github.com/gin-gonic/gin" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/dto" "github.com/pocket-id/pocket-id/backend/internal/middleware" "github.com/pocket-id/pocket-id/backend/internal/service" @@ -14,8 +16,9 @@ import ( // @Summary User group management controller // @Description Initializes all user group-related API endpoints // @Tags User Groups -func NewUserGroupController(group *gin.RouterGroup, authMiddleware *middleware.AuthMiddleware, userGroupService *service.UserGroupService) { +func NewUserGroupController(group *gin.RouterGroup, authMiddleware *middleware.AuthMiddleware, appConfigService *appconfig.AppConfigService, userGroupService *service.UserGroupService) { ugc := UserGroupController{ + appConfigService: appConfigService, UserGroupService: userGroupService, } @@ -33,6 +36,7 @@ func NewUserGroupController(group *gin.RouterGroup, authMiddleware *middleware.A } type UserGroupController struct { + appConfigService *appconfig.AppConfigService UserGroupService *service.UserGroupService } @@ -146,13 +150,19 @@ func (ugc *UserGroupController) create(c *gin.Context) { // @Success 200 {object} dto.UserGroupDto "Updated user group" // @Router /api/user-groups/{id} [put] func (ugc *UserGroupController) update(c *gin.Context) { + dbConfig, err := ugc.appConfigService.GetConfig(c.Request.Context()) + if err != nil { + _ = c.Error(fmt.Errorf("error loading app configuration: %w", err)) + return + } + var input dto.UserGroupCreateDto if err := dto.ShouldBindWithNormalizedJSON(c, &input); err != nil { _ = c.Error(err) return } - group, err := ugc.UserGroupService.Update(c.Request.Context(), c.Param("id"), input) + group, err := ugc.UserGroupService.Update(c.Request.Context(), dbConfig, c.Param("id"), input) if err != nil { _ = c.Error(err) return @@ -177,7 +187,13 @@ func (ugc *UserGroupController) update(c *gin.Context) { // @Success 204 "No Content" // @Router /api/user-groups/{id} [delete] func (ugc *UserGroupController) delete(c *gin.Context) { - if err := ugc.UserGroupService.Delete(c.Request.Context(), c.Param("id")); err != nil { + dbConfig, err := ugc.appConfigService.GetConfig(c.Request.Context()) + if err != nil { + _ = c.Error(fmt.Errorf("error loading app configuration: %w", err)) + return + } + + if err := ugc.UserGroupService.Delete(c.Request.Context(), dbConfig, c.Param("id")); err != nil { _ = c.Error(err) return } diff --git a/backend/internal/instanceid/instanceid_test.go b/backend/internal/instanceid/instanceid_test.go index b850efd2..c335a0d7 100644 --- a/backend/internal/instanceid/instanceid_test.go +++ b/backend/internal/instanceid/instanceid_test.go @@ -137,15 +137,6 @@ func TestMigrateFromAppConfig(t *testing.T) { } } - // countLegacyInstanceID returns the number of "instanceId" rows left in the app_config_variables table - countLegacyInstanceID := func(t *testing.T, db *gorm.DB) int64 { - t.Helper() - var count int64 - err := db.Table("app_config_variables").Where(`"key" = ?`, "instanceId").Count(&count).Error - require.NoError(t, err) - return count - } - t.Run("moves an existing instance ID from app_config_variables into the kv table", func(t *testing.T) { legacyID := uuid.NewString() db := testutils.NewDatabaseForTestWithMigrationSeed(t, versionBeforeMove, seedAppConfigInstanceID(legacyID)) @@ -155,8 +146,8 @@ func TestMigrateFromAppConfig(t *testing.T) { require.Equal(t, 1, count) require.Equal(t, legacyID, stored) - // The legacy row must have been removed from app_config_variables - require.Zero(t, countLegacyInstanceID(t, db)) + // The final config migration removes the legacy table after freezing its remaining values + require.False(t, db.Migrator().HasTable("app_config_variables")) // Load must return the migrated value without generating a new one id, err := Load(t.Context(), db) @@ -184,7 +175,8 @@ func TestMigrateFromAppConfig(t *testing.T) { require.Equal(t, 1, count) require.Equal(t, "kv-instance-id", stored) - // The legacy row must still be removed regardless of the conflict - require.Zero(t, countLegacyInstanceID(t, db)) + // The final config migration removes the legacy table regardless of the conflict + ok := db.Migrator().HasTable("app_config_variables") + require.False(t, ok) }) } diff --git a/backend/internal/job/api_key_expiry_job.go b/backend/internal/job/api_key_expiry_job.go index eccdeb98..4717f0a2 100644 --- a/backend/internal/job/api_key_expiry_job.go +++ b/backend/internal/job/api_key_expiry_job.go @@ -8,17 +8,18 @@ import ( "github.com/go-co-op/gocron/v2" "github.com/pocket-id/pocket-id/backend/internal/apikey" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/service" "github.com/pocket-id/pocket-id/backend/internal/utils/email" ) type ApiKeyEmailJobs struct { apiKeyModule *apikey.Module - appConfigService *service.AppConfigService + appConfigService *appconfig.AppConfigService emailService *service.EmailService } -func (s *Scheduler) RegisterApiKeyExpiryJob(ctx context.Context, apiKeyModule *apikey.Module, appConfigService *service.AppConfigService, emailService *service.EmailService) error { +func (s *Scheduler) RegisterApiKeyExpiryJob(ctx context.Context, apiKeyModule *apikey.Module, appConfigService *appconfig.AppConfigService, emailService *service.EmailService) error { jobs := &ApiKeyEmailJobs{ apiKeyModule: apiKeyModule, appConfigService: appConfigService, @@ -30,8 +31,13 @@ func (s *Scheduler) RegisterApiKeyExpiryJob(ctx context.Context, apiKeyModule *a } func (j *ApiKeyEmailJobs) checkAndNotifyExpiringApiKeys(ctx context.Context) error { + dbConfig, err := j.appConfigService.GetConfig(ctx) + if err != nil { + return fmt.Errorf("error load app config: %w", err) + } + // Skip if the feature is disabled - if !j.appConfigService.GetDbConfig().EmailApiKeyExpirationEnabled.IsTrue() { + if !dbConfig.EmailApiKeyExpirationEnabled.IsTrue() { return nil } @@ -45,7 +51,7 @@ func (j *ApiKeyEmailJobs) checkAndNotifyExpiringApiKeys(ctx context.Context) err continue } - err = service.SendEmail(ctx, j.emailService, email.Address{ + err = service.SendEmail(ctx, j.emailService, dbConfig, email.Address{ Name: key.User.FullName(), Email: *key.User.Email, }, service.ApiKeyExpiringSoonTemplate, &service.ApiKeyExpiringSoonTemplateData{ diff --git a/backend/internal/job/ldap_job.go b/backend/internal/job/ldap_job.go index 1547d954..4c450bcb 100644 --- a/backend/internal/job/ldap_job.go +++ b/backend/internal/job/ldap_job.go @@ -2,17 +2,19 @@ package job import ( "context" + "fmt" "time" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/service" ) type LdapJobs struct { ldapService *service.LdapService - appConfigService *service.AppConfigService + appConfigService *appconfig.AppConfigService } -func (s *Scheduler) RegisterLdapJobs(ctx context.Context, ldapService *service.LdapService, appConfigService *service.AppConfigService) error { +func (s *Scheduler) RegisterLdapJobs(ctx context.Context, ldapService *service.LdapService, appConfigService *appconfig.AppConfigService) error { jobs := &LdapJobs{ldapService: ldapService, appConfigService: appConfigService} // Register the job to run every hour (with some jitter) @@ -20,9 +22,14 @@ func (s *Scheduler) RegisterLdapJobs(ctx context.Context, ldapService *service.L } func (j *LdapJobs) syncLdap(ctx context.Context) error { - if !j.appConfigService.GetDbConfig().LdapEnabled.IsTrue() { + dbConfig, err := j.appConfigService.GetConfig(ctx) + if err != nil { + return fmt.Errorf("error load app config: %w", err) + } + + if !dbConfig.LdapEnabled.IsTrue() { return nil } - return j.ldapService.SyncAll(ctx) + return j.ldapService.SyncAll(ctx, dbConfig) } diff --git a/backend/internal/middleware/auth_middleware_test.go b/backend/internal/middleware/auth_middleware_test.go index 08379af2..22c98b43 100644 --- a/backend/internal/middleware/auth_middleware_test.go +++ b/backend/internal/middleware/auth_middleware_test.go @@ -33,23 +33,20 @@ func TestWithApiKeyAuthDisabled(t *testing.T) { db := testutils.NewDatabaseForTest(t) - appConfigService, err := service.NewAppConfigService(t.Context(), db) - require.NoError(t, err) - instanceID, err := instanceid.Load(t.Context(), db) require.NoError(t, err) - jwtService, err := service.NewJwtService(t.Context(), db, instanceID, appConfigService) + jwtService, err := service.NewJwtService(t.Context(), db, instanceID) require.NoError(t, err) - userService := service.NewUserService(db, jwtService, nil, nil, appConfigService, nil, nil, nil, nil) + userService := service.NewUserService(db, jwtService, nil, nil, nil, nil, nil, nil) apiKeyModule, err := apikey.New(t.Context(), apikey.Dependencies{DB: db}) require.NoError(t, err) authMiddleware := NewAuthMiddleware(apiKeyModule, userService, jwtService) user := createUserForAuthMiddlewareTest(t, db) - jwtToken, err := jwtService.GenerateAccessToken(user, "") + jwtToken, err := jwtService.GenerateAccessToken(user, "", time.Hour) require.NoError(t, err) apiKeyToken := "middleware-test-api-key-raw-token" diff --git a/backend/internal/middleware/rate_limit_test.go b/backend/internal/middleware/rate_limit_test.go index 863b6204..42a567bc 100644 --- a/backend/internal/middleware/rate_limit_test.go +++ b/backend/internal/middleware/rate_limit_test.go @@ -39,6 +39,7 @@ func startRateLimitServices(t *testing.T, policies ...RateLimitPolicy) map[strin for name, rl := range limiters { services[name] = rl.Service(svc) } + return services } diff --git a/backend/internal/model/app_config.go b/backend/internal/model/app_config.go deleted file mode 100644 index 76d508d5..00000000 --- a/backend/internal/model/app_config.go +++ /dev/null @@ -1,203 +0,0 @@ -package model - -import ( - "errors" - "fmt" - "reflect" - "slices" - "strconv" - "strings" - "time" - - "github.com/pocket-id/pocket-id/backend/internal/common" -) - -type AppConfigVariable struct { - Key string `gorm:"primaryKey;not null"` - Value string -} - -// IsTrue returns true if the value is a truthy string, such as "true", "t", "yes", "1", etc. -func (a *AppConfigVariable) IsTrue() bool { - ok, _ := strconv.ParseBool(a.Value) - return ok -} - -// AsDurationMinutes returns the value as a time.Duration, interpreting the string as a whole number of minutes. -func (a *AppConfigVariable) AsDurationMinutes() time.Duration { - val, err := strconv.Atoi(a.Value) - if err != nil { - return 0 - } - return time.Duration(val) * time.Minute -} - -type AppConfig struct { - // General - AppName AppConfigVariable `key:"appName,public"` // Public - SessionDuration AppConfigVariable `key:"sessionDuration"` - HomePageURL AppConfigVariable `key:"homePageUrl,public"` // Public - EmailsVerified AppConfigVariable `key:"emailsVerified"` - AccentColor AppConfigVariable `key:"accentColor,public"` // Public - DisableAnimations AppConfigVariable `key:"disableAnimations,public"` // Public - AllowOwnAccountEdit AppConfigVariable `key:"allowOwnAccountEdit,public"` // Public - AllowUserSignups AppConfigVariable `key:"allowUserSignups,public"` // Public - SignupDefaultUserGroupIDs AppConfigVariable `key:"signupDefaultUserGroupIDs"` - SignupDefaultCustomClaims AppConfigVariable `key:"signupDefaultCustomClaims"` - // Email - RequireUserEmail AppConfigVariable `key:"requireUserEmail,public"` // Public - SmtpHost AppConfigVariable `key:"smtpHost"` - SmtpPort AppConfigVariable `key:"smtpPort"` - SmtpFrom AppConfigVariable `key:"smtpFrom"` - SmtpUser AppConfigVariable `key:"smtpUser"` - SmtpPassword AppConfigVariable `key:"smtpPassword,sensitive"` - SmtpTls AppConfigVariable `key:"smtpTls"` - SmtpSkipCertVerify AppConfigVariable `key:"smtpSkipCertVerify"` - EmailLoginNotificationEnabled AppConfigVariable `key:"emailLoginNotificationEnabled"` - EmailOneTimeAccessAsUnauthenticatedEnabled AppConfigVariable `key:"emailOneTimeAccessAsUnauthenticatedEnabled,public"` // Public - EmailOneTimeAccessAsAdminEnabled AppConfigVariable `key:"emailOneTimeAccessAsAdminEnabled,public"` // Public - EmailApiKeyExpirationEnabled AppConfigVariable `key:"emailApiKeyExpirationEnabled"` - EmailVerificationEnabled AppConfigVariable `key:"emailVerificationEnabled,public"` // Public - // LDAP - LdapEnabled AppConfigVariable `key:"ldapEnabled,public"` // Public - LdapUrl AppConfigVariable `key:"ldapUrl"` - LdapBindDn AppConfigVariable `key:"ldapBindDn"` - LdapBindPassword AppConfigVariable `key:"ldapBindPassword,sensitive"` - LdapBase AppConfigVariable `key:"ldapBase"` - LdapUserSearchFilter AppConfigVariable `key:"ldapUserSearchFilter"` - LdapUserGroupSearchFilter AppConfigVariable `key:"ldapUserGroupSearchFilter"` - LdapSkipCertVerify AppConfigVariable `key:"ldapSkipCertVerify"` - LdapAttributeUserUniqueIdentifier AppConfigVariable `key:"ldapAttributeUserUniqueIdentifier"` - LdapAttributeUserUsername AppConfigVariable `key:"ldapAttributeUserUsername"` - LdapAttributeUserEmail AppConfigVariable `key:"ldapAttributeUserEmail"` - LdapAttributeUserFirstName AppConfigVariable `key:"ldapAttributeUserFirstName"` - LdapAttributeUserLastName AppConfigVariable `key:"ldapAttributeUserLastName"` - LdapAttributeUserDisplayName AppConfigVariable `key:"ldapAttributeUserDisplayName"` - LdapAttributeUserProfilePicture AppConfigVariable `key:"ldapAttributeUserProfilePicture"` - LdapAttributeGroupMember AppConfigVariable `key:"ldapAttributeGroupMember"` - LdapAttributeGroupUniqueIdentifier AppConfigVariable `key:"ldapAttributeGroupUniqueIdentifier"` - LdapAttributeGroupName AppConfigVariable `key:"ldapAttributeGroupName"` - LdapAdminGroupName AppConfigVariable `key:"ldapAdminGroupName"` - LdapSoftDeleteUsers AppConfigVariable `key:"ldapSoftDeleteUsers"` -} - -func (c *AppConfig) ToAppConfigVariableSlice(showAll bool, redactSensitiveValues bool) []AppConfigVariable { - // Use reflection to iterate through all fields - cfgValue := reflect.ValueOf(c).Elem() - cfgType := cfgValue.Type() - - var res []AppConfigVariable - - for i := range cfgType.NumField() { - field := cfgType.Field(i) - - key, attrs, _ := strings.Cut(field.Tag.Get("key"), ",") - if key == "" { - continue - } - - // If we're only showing public variables and this is not public, skip it - if !showAll && attrs != "public" { - continue - } - - value := cfgValue.Field(i).FieldByName("Value").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 && attrs == "sensitive" { - value = "XXXXXXXXXX" - } - - appConfigVariable := AppConfigVariable{ - Key: key, - Value: value, - } - - res = append(res, appConfigVariable) - } - - return res -} - -func (c *AppConfig) FieldByKey(key string) (defaultValue string, isInternal bool, err error) { - rv := reflect.ValueOf(c).Elem() - rt := rv.Type() - - // Find the field in the struct whose "key" tag matches - for i := range rt.NumField() { - // Grab only the first part of the key, if there's a comma with additional properties - tagValue := strings.Split(rt.Field(i).Tag.Get("key"), ",") - keyFromTag := tagValue[0] - isInternal = slices.Contains(tagValue, "internal") - if keyFromTag != key { - continue - } - - valueField := rv.Field(i).FieldByName("Value") - return valueField.String(), isInternal, nil - } - - // If we are here, the config key was not found - return "", false, AppConfigKeyNotFoundError{field: key} -} - -func (c *AppConfig) UpdateField(key string, value string, noInternal bool) error { - rv := reflect.ValueOf(c).Elem() - rt := rv.Type() - - // Find the field in the struct whose "key" tag matches, then update that - for i := range rt.NumField() { - // Separate the key (before the comma) from any optional attributes after - tagValue, attrs, _ := strings.Cut(rt.Field(i).Tag.Get("key"), ",") - if tagValue != key { - continue - } - - // If the field is internal and noInternal is true, we skip that - if noInternal && attrs == "internal" { - return AppConfigInternalForbiddenError{field: key} - } - - valueField := rv.Field(i).FieldByName("Value") - if !valueField.CanSet() { - return fmt.Errorf("field Value in AppConfigVariable is not settable for config key '%s'", key) - } - - // Update the value - valueField.SetString(value) - - // Return once updated - return nil - } - - // If we're here, we have not found the right field to update - return AppConfigKeyNotFoundError{field: key} -} - -type AppConfigKeyNotFoundError struct { - field string -} - -func (e AppConfigKeyNotFoundError) Error() string { - return "cannot find config key '" + e.field + "'" -} - -func (e AppConfigKeyNotFoundError) Is(target error) bool { - // Ignore the field property when checking if an error is of the type AppConfigKeyNotFoundError - x := AppConfigKeyNotFoundError{} - return errors.As(target, &x) -} - -type AppConfigInternalForbiddenError struct { - field string -} - -func (e AppConfigInternalForbiddenError) Error() string { - return "field '" + e.field + "' is internal and can't be updated" -} - -func (e AppConfigInternalForbiddenError) Is(target error) bool { - // Ignore the field property when checking if an error is of the type AppConfigInternalForbiddenError - x := AppConfigInternalForbiddenError{} - return errors.As(target, &x) -} diff --git a/backend/internal/model/app_config_test.go b/backend/internal/model/app_config_test.go deleted file mode 100644 index 4f28eb99..00000000 --- a/backend/internal/model/app_config_test.go +++ /dev/null @@ -1,126 +0,0 @@ -// We use model_test here to avoid an import cycle -package model_test - -import ( - "reflect" - "strings" - "testing" - "time" - - "github.com/stretchr/testify/assert" - - "github.com/pocket-id/pocket-id/backend/internal/dto" - "github.com/pocket-id/pocket-id/backend/internal/model" -) - -func TestAppConfigVariable_AsMinutesDuration(t *testing.T) { - tests := []struct { - name string - value string - expected time.Duration - expectedSeconds int - }{ - { - name: "valid positive integer", - value: "60", - expected: 60 * time.Minute, - expectedSeconds: 3600, - }, - { - name: "valid zero integer", - value: "0", - expected: 0, - expectedSeconds: 0, - }, - { - name: "negative integer", - value: "-30", - expected: -30 * time.Minute, - expectedSeconds: -1800, - }, - { - name: "invalid non-integer", - value: "not-a-number", - expected: 0, - expectedSeconds: 0, - }, - { - name: "empty string", - value: "", - expected: 0, - expectedSeconds: 0, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - configVar := model.AppConfigVariable{ - Value: tt.value, - } - - result := configVar.AsDurationMinutes() - assert.Equal(t, tt.expected, result) - assert.Equal(t, tt.expectedSeconds, int(result.Seconds())) - }) - } -} - -// This test ensures that the model.AppConfig and dto.AppConfigUpdateDto structs match: -// - They should have the same properties, where the "json" tag of dto.AppConfigUpdateDto should match the "key" tag in model.AppConfig -// - dto.AppConfigDto should not include "internal" fields from model.AppConfig -// This test is primarily meant to catch discrepancies between the two structs as fields are added or removed over time -func TestAppConfigStructMatchesUpdateDto(t *testing.T) { - appConfigType := reflect.TypeFor[model.AppConfig]() - updateDtoType := reflect.TypeFor[dto.AppConfigUpdateDto]() - - // Process AppConfig fields - appConfigFields := make(map[string]string) - for field := range appConfigType.Fields() { - if field.Tag.Get("key") == "" { - // Skip internal fields - continue - } - - // Extract the key name from the tag (takes the part before any comma) - keyTag := field.Tag.Get("key") - keyName, _, _ := strings.Cut(keyTag, ",") - - appConfigFields[field.Name] = keyName - } - - // Process AppConfigUpdateDto fields - dtoFields := make(map[string]string) - for field := range updateDtoType.Fields() { - // Extract the json name from the tag (takes the part before any binding constraints) - jsonTag := field.Tag.Get("json") - jsonName, _, _ := strings.Cut(jsonTag, ",") - - dtoFields[jsonName] = field.Name - } - - // Verify every AppConfig field has a matching DTO field with the same name - for fieldName, keyName := range appConfigFields { - if strings.HasSuffix(fieldName, "ImageType") { - // Skip internal fields that shouldn't be in the DTO - continue - } - - // Check if there's a DTO field with a matching JSON tag - _, exists := dtoFields[keyName] - assert.True(t, exists, "Field %s with key '%s' in AppConfig has no matching field in AppConfigUpdateDto", fieldName, keyName) - } - - // Verify every DTO field has a matching AppConfig field - for jsonName, fieldName := range dtoFields { - // Find a matching field in AppConfig by key tag - found := false - for _, keyName := range appConfigFields { - if keyName == jsonName { - found = true - break - } - } - - assert.True(t, found, "Field %s with json tag '%s' in AppConfigUpdateDto has no matching field in AppConfig", fieldName, jsonName) - } -} diff --git a/backend/internal/service/app_config_service.go b/backend/internal/service/app_config_service.go deleted file mode 100644 index d2670fa9..00000000 --- a/backend/internal/service/app_config_service.go +++ /dev/null @@ -1,416 +0,0 @@ -package service - -import ( - "context" - "errors" - "fmt" - "os" - "reflect" - "strings" - "sync/atomic" - "time" - - "gorm.io/gorm" - "gorm.io/gorm/clause" - - "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" -) - -type AppConfigService struct { - dbConfig atomic.Pointer[model.AppConfig] - db *gorm.DB -} - -func NewAppConfigService(ctx context.Context, db *gorm.DB) (service *AppConfigService, err error) { - service = &AppConfigService{ - db: db, - } - - ctx, span := tracing.Start(ctx, "pocketid.appconfig.init") - defer tracing.End(span, err) - - // We need to assign to the "err" variable, do not inline this into the "if" - err = service.LoadDbConfig(ctx) - if err != nil { - return nil, fmt.Errorf("failed to initialize app config service: %w", err) - } - - return service, nil -} - -// GetDbConfig returns the application configuration. -// Important: Treat the object as read-only: do not modify its properties directly! -func (s *AppConfigService) GetDbConfig() *model.AppConfig { - v := s.dbConfig.Load() - if v == nil { - // This indicates a development-time error - panic("called GetDbConfig before DbConfig is loaded") - } - - return v -} - -func (s *AppConfigService) getDefaultDbConfig() *model.AppConfig { - // Values are the default ones - return &model.AppConfig{ - // General - AppName: model.AppConfigVariable{Value: "Pocket ID"}, - SessionDuration: model.AppConfigVariable{Value: "60"}, - HomePageURL: model.AppConfigVariable{Value: "/settings/account"}, - EmailsVerified: model.AppConfigVariable{Value: "false"}, - DisableAnimations: model.AppConfigVariable{Value: "false"}, - AllowOwnAccountEdit: model.AppConfigVariable{Value: "true"}, - AllowUserSignups: model.AppConfigVariable{Value: "disabled"}, - SignupDefaultUserGroupIDs: model.AppConfigVariable{Value: "[]"}, - SignupDefaultCustomClaims: model.AppConfigVariable{Value: "[]"}, - AccentColor: model.AppConfigVariable{Value: "default"}, - // Email - RequireUserEmail: model.AppConfigVariable{Value: "true"}, - SmtpHost: model.AppConfigVariable{}, - SmtpPort: model.AppConfigVariable{}, - SmtpFrom: model.AppConfigVariable{}, - SmtpUser: model.AppConfigVariable{}, - SmtpPassword: model.AppConfigVariable{}, - SmtpTls: model.AppConfigVariable{Value: "none"}, - SmtpSkipCertVerify: model.AppConfigVariable{Value: "false"}, - EmailLoginNotificationEnabled: model.AppConfigVariable{Value: "false"}, - EmailOneTimeAccessAsUnauthenticatedEnabled: model.AppConfigVariable{Value: "false"}, - EmailOneTimeAccessAsAdminEnabled: model.AppConfigVariable{Value: "false"}, - EmailApiKeyExpirationEnabled: model.AppConfigVariable{Value: "false"}, - EmailVerificationEnabled: model.AppConfigVariable{Value: "false"}, - // LDAP - LdapEnabled: model.AppConfigVariable{Value: "false"}, - LdapUrl: model.AppConfigVariable{}, - LdapBindDn: model.AppConfigVariable{}, - LdapBindPassword: model.AppConfigVariable{}, - LdapBase: model.AppConfigVariable{}, - LdapUserSearchFilter: model.AppConfigVariable{Value: "(objectClass=person)"}, - LdapUserGroupSearchFilter: model.AppConfigVariable{Value: "(objectClass=groupOfNames)"}, - LdapSkipCertVerify: model.AppConfigVariable{Value: "false"}, - LdapAttributeUserUniqueIdentifier: model.AppConfigVariable{}, - LdapAttributeUserUsername: model.AppConfigVariable{}, - LdapAttributeUserEmail: model.AppConfigVariable{}, - LdapAttributeUserFirstName: model.AppConfigVariable{}, - LdapAttributeUserLastName: model.AppConfigVariable{}, - LdapAttributeUserDisplayName: model.AppConfigVariable{Value: "cn"}, - LdapAttributeUserProfilePicture: model.AppConfigVariable{}, - LdapAttributeGroupMember: model.AppConfigVariable{Value: "member"}, - LdapAttributeGroupUniqueIdentifier: model.AppConfigVariable{}, - LdapAttributeGroupName: model.AppConfigVariable{}, - LdapAdminGroupName: model.AppConfigVariable{}, - LdapSoftDeleteUsers: model.AppConfigVariable{Value: "true"}, - } -} - -func (s *AppConfigService) updateAppConfigStartTransaction(ctx context.Context) (tx *gorm.DB, err error) { - // We start a transaction before doing any work, to ensure that we are the only ones updating the data in the database - // This works across multiple processes too - tx = s.db.Begin() - err = tx.Error - if err != nil { - return nil, fmt.Errorf("failed to begin database transaction: %w", err) - } - - // With SQLite there's nothing else we need to do, because a transaction blocks the entire database - // However, with Postgres we need to manually lock the table to prevent others from doing the same - switch s.db.Name() { - case "postgres": - // We do not use "NOWAIT" so this blocks until the database is available, or the context is canceled - // Here we use a context with a 10s timeout in case the database is blocked for longer - lockCtx, lockCancel := context.WithTimeout(ctx, 10*time.Second) - defer lockCancel() - err = tx. - WithContext(lockCtx). - Exec("LOCK TABLE app_config_variables IN ACCESS EXCLUSIVE MODE"). - Error - if err != nil { - tx.Rollback() - return nil, fmt.Errorf("failed to acquire lock on app_config_variables table: %w", err) - } - default: - // Nothing to do here - } - - return tx, nil -} - -func (s *AppConfigService) updateAppConfigUpdateDatabase(ctx context.Context, tx *gorm.DB, dbUpdate *[]model.AppConfigVariable) error { - err := tx. - WithContext(ctx). - Clauses(clause.OnConflict{ - // Perform an "upsert" if the key already exists, replacing the value - Columns: []clause.Column{{Name: "key"}}, - DoUpdates: clause.AssignmentColumns([]string{"value"}), - }). - Create(&dbUpdate). - Error - if err != nil { - return fmt.Errorf("failed to update config in database: %w", err) - } - - return nil -} - -func (s *AppConfigService) UpdateAppConfig(ctx context.Context, input dto.AppConfigUpdateDto) ([]model.AppConfigVariable, error) { - if common.EnvConfig.UiConfigDisabled { - return nil, &common.UiConfigDisabledError{} - } - - // Start the transaction - tx, err := s.updateAppConfigStartTransaction(ctx) - if err != nil { - return nil, 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 nil, fmt.Errorf("failed to reload config from database: %w", err) - } - - defaultCfg := s.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 - 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 - // Skip values that are internal only and can't be updated - if value == "" { - // Ignore errors here as we know the key exists - defaultValue, _, _ := defaultCfg.FieldByKey(key) - err = cfg.UpdateField(key, defaultValue, true) - } else { - err = cfg.UpdateField(key, value, true) - } - - // If we tried to update an internal field, ignore the error (and do not update in the DB) - if errors.Is(err, model.AppConfigInternalForbiddenError{}) { - continue - } else 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) - 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 -} - -// UpdateAppConfigValues updates the application configuration values in the database. -func (s *AppConfigService) UpdateAppConfigValues(ctx context.Context, keysAndValues ...string) error { - // Count of keysAndValues must be even - if len(keysAndValues)%2 != 0 { - return errors.New("invalid number of arguments received") - } - - // 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 := s.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 - // (Note the += 2, as we are iterating through key-value pairs) - dbUpdate := make([]model.AppConfigVariable, 0, 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, isInternal, err := defaultCfg.FieldByKey(key) - if err != nil { - return fmt.Errorf("invalid configuration key '%s': %w", key, err) - } - if !isInternal && common.EnvConfig.UiConfigDisabled { - return &common.UiConfigDisabledError{} - } - - // 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 - // Skip values that are internal only and can't be updated - if value == "" { - err = cfg.UpdateField(key, defaultValue, false) - } else { - err = cfg.UpdateField(key, value, false) - } - 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, - }) - } - - // Update the values in the database - err = s.updateAppConfigUpdateDatabase(ctx, tx, &dbUpdate) - if err != nil { - return err - } - - // Commit the changes to the DB, then finally save the updated config in the object - err = tx.Commit().Error - if err != nil { - return fmt.Errorf("failed to commit transaction: %w", err) - } - - s.dbConfig.Store(cfg) - - return nil -} - -func (s *AppConfigService) ListAppConfig(showAll bool) []model.AppConfigVariable { - return s.GetDbConfig().ToAppConfigVariableSlice(showAll, true) -} - -// LoadDbConfig loads the configuration values from the database into the DbConfig struct. -func (s *AppConfigService) LoadDbConfig(ctx context.Context) (err error) { - dest, err := s.loadDbConfigInternal(ctx, s.db) - if err != nil { - return err - } - - s.dbConfig.Store(dest) - - return nil -} - -func (s *AppConfigService) loadDbConfigInternal(ctx context.Context, tx *gorm.DB) (*model.AppConfig, error) { - // If the UI config is disabled, only load from the env - if common.EnvConfig.UiConfigDisabled { - dest, err := s.loadDbConfigFromEnv(ctx, tx) - return dest, err - } - - // First, start from the default configuration - dest := s.getDefaultDbConfig() - - // Load all configuration values from the database - // This loads all values in a single shot - var loaded []model.AppConfigVariable - queryCtx, queryCancel := context.WithTimeout(ctx, 10*time.Second) - defer queryCancel() - err := tx. - WithContext(queryCtx). - Find(&loaded).Error - if err != nil { - return nil, fmt.Errorf("failed to load configuration from the database: %w", err) - } - - // Iterate through all values loaded from the database - for _, v := range loaded { - // Find the field in the struct whose "key" tag matches, then update that - err = dest.UpdateField(v.Key, v.Value, false) - - // We ignore the case of fields that don't exist, as there may be leftover data in the database - if err != nil && !errors.Is(err, model.AppConfigKeyNotFoundError{}) { - return nil, fmt.Errorf("failed to process config for key '%s': %w", v.Key, err) - } - } - - return dest, nil -} - -func (s *AppConfigService) loadDbConfigFromEnv(ctx context.Context, tx *gorm.DB) (*model.AppConfig, error) { - // First, start from the default configuration - dest := s.getDefaultDbConfig() - - // Iterate through each field - rt := reflect.ValueOf(dest).Elem().Type() - rv := reflect.ValueOf(dest).Elem() - for i := range rt.NumField() { - field := rt.Field(i) - - // Get the key and internal tag values - key, attrs, _ := strings.Cut(field.Tag.Get("key"), ",") - - // Internal fields are loaded from the database as they can't be set from the environment - if attrs == "internal" { - var value string - err := tx.WithContext(ctx). - Model(&model.AppConfigVariable{}). - Where("key = ?", key). - Select("value"). - First(&value).Error - if err == nil { - rv.Field(i).FieldByName("Value").SetString(value) - } - continue - } - - envVarName := utils.CamelCaseToScreamingSnakeCase(key) - - // Set the value if it's set - value, ok := os.LookupEnv(envVarName) - if ok { - rv.Field(i).FieldByName("Value").SetString(value) - continue - } - - // If it's sensitive, we also allow reading from file - if attrs == "sensitive" { - fileName := os.Getenv(envVarName + "_FILE") - if fileName != "" { - // #nosec G703 - Value is provided by admin - b, err := os.ReadFile(fileName) - if err != nil { - return nil, fmt.Errorf("failed to read secret '%s' from file '%s': %w", envVarName, fileName, err) - } - rv.Field(i).FieldByName("Value").SetString(string(b)) - continue - } - } - } - - return dest, nil -} diff --git a/backend/internal/service/app_config_service_test.go b/backend/internal/service/app_config_service_test.go deleted file mode 100644 index f22684fc..00000000 --- a/backend/internal/service/app_config_service_test.go +++ /dev/null @@ -1,473 +0,0 @@ -package service - -import ( - "sync/atomic" - "testing" - - "github.com/stretchr/testify/require" - - "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" -) - -// NewTestAppConfigService is a function used by tests to create AppConfigService objects with pre-defined configuration values -func NewTestAppConfigService(config *model.AppConfig) *AppConfigService { - service := &AppConfigService{ - dbConfig: atomic.Pointer[model.AppConfig]{}, - } - service.dbConfig.Store(config) - - return service -} - -func TestLoadDbConfig(t *testing.T) { - t.Run("empty config table", func(t *testing.T) { - db := testutils.NewDatabaseForTest(t) - service := &AppConfigService{ - db: db, - } - - // Load the config - err := service.LoadDbConfig(t.Context()) - require.NoError(t, err) - - // Config should be equal to default config - require.Equal(t, service.GetDbConfig(), service.getDefaultDbConfig()) - }) - - t.Run("loads value from config table", func(t *testing.T) { - db := testutils.NewDatabaseForTest(t) - - // Populate the config table with some initial values - err := db. - Create([]model.AppConfigVariable{ - // Overrides default value - {Key: "appName", Value: "Test App"}, - {Key: "sessionDuration", Value: "5"}, - // Does not have a default value - {Key: "smtpHost", Value: "example"}, - }). - Error - require.NoError(t, err) - - // Load the config - service := &AppConfigService{ - db: db, - } - err = service.LoadDbConfig(t.Context()) - require.NoError(t, err) - - // Values should match expected ones - expect := service.getDefaultDbConfig() - expect.AppName.Value = "Test App" - expect.SessionDuration.Value = "5" - expect.SmtpHost.Value = "example" - require.Equal(t, service.GetDbConfig(), expect) - }) - - t.Run("ignores unknown config keys", func(t *testing.T) { - db := testutils.NewDatabaseForTest(t) - - // Add an entry with a key that doesn't exist in the config struct - err := db.Create([]model.AppConfigVariable{ - {Key: "__nonExistentKey", Value: "some value"}, - {Key: "appName", Value: "TestApp"}, // This one should still be loaded - }).Error - require.NoError(t, err) - - service := &AppConfigService{ - db: db, - } - // This should not fail, just ignore the unknown key - err = service.LoadDbConfig(t.Context()) - require.NoError(t, err) - - config := service.GetDbConfig() - require.Equal(t, "TestApp", config.AppName.Value) - }) - - t.Run("loading config multiple times", func(t *testing.T) { - db := testutils.NewDatabaseForTest(t) - - // Initial state - err := db.Create([]model.AppConfigVariable{ - {Key: "appName", Value: "InitialApp"}, - }).Error - require.NoError(t, err) - - service := &AppConfigService{ - db: db, - } - err = service.LoadDbConfig(t.Context()) - require.NoError(t, err) - require.Equal(t, "InitialApp", service.GetDbConfig().AppName.Value) - - // Update the database value - err = db.Model(&model.AppConfigVariable{}). - Where("key = ?", "appName"). - Update("value", "UpdatedApp").Error - require.NoError(t, err) - - // Load the config again, it should reflect the updated value - err = service.LoadDbConfig(t.Context()) - require.NoError(t, err) - require.Equal(t, "UpdatedApp", service.GetDbConfig().AppName.Value) - }) - - t.Run("loads config from env when UiConfigDisabled is true", func(t *testing.T) { - // Save the original state and restore it after the test - originalUiConfigDisabled := common.EnvConfig.UiConfigDisabled - defer func() { - common.EnvConfig.UiConfigDisabled = originalUiConfigDisabled - }() - - // Set environment variables for testing - t.Setenv("APP_NAME", "EnvTest App") - t.Setenv("SESSION_DURATION", "45") - - // Enable UiConfigDisabled to load from env - common.EnvConfig.UiConfigDisabled = true - - // Create database with config that should be ignored - db := testutils.NewDatabaseForTest(t) - err := db.Create([]model.AppConfigVariable{ - {Key: "appName", Value: "DB App"}, - {Key: "sessionDuration", Value: "120"}, - }).Error - require.NoError(t, err) - - service := &AppConfigService{ - db: db, - } - - // Load the config - err = service.LoadDbConfig(t.Context()) - require.NoError(t, err) - - // Config should be loaded from env, not DB - config := service.GetDbConfig() - require.Equal(t, "EnvTest App", config.AppName.Value, "Should load appName from env") - require.Equal(t, "45", config.SessionDuration.Value, "Should load sessionDuration from env") - }) - - t.Run("ignores env vars when UiConfigDisabled is false", func(t *testing.T) { - // Save the original state and restore it after the test - originalUiConfigDisabled := common.EnvConfig.UiConfigDisabled - defer func() { - common.EnvConfig.UiConfigDisabled = originalUiConfigDisabled - }() - - // Set environment variables that should be ignored - t.Setenv("APP_NAME", "EnvTest App") - t.Setenv("SESSION_DURATION", "45") - - // Make sure UiConfigDisabled is false to load from DB - common.EnvConfig.UiConfigDisabled = false - - // Create database with config values that should take precedence - db := testutils.NewDatabaseForTest(t) - err := db.Create([]model.AppConfigVariable{ - {Key: "appName", Value: "DB App"}, - {Key: "sessionDuration", Value: "120"}, - }).Error - require.NoError(t, err) - - service := &AppConfigService{ - db: db, - } - - // Load the config - err = service.LoadDbConfig(t.Context()) - require.NoError(t, err) - - // Config should be loaded from DB, not env - config := service.GetDbConfig() - require.Equal(t, "DB App", config.AppName.Value, "Should load appName from DB, not env") - require.Equal(t, "120", config.SessionDuration.Value, "Should load sessionDuration from DB, not env") - }) -} - -func TestUpdateAppConfigValues(t *testing.T) { - t.Run("update single value", func(t *testing.T) { - db := testutils.NewDatabaseForTest(t) - - // Create a service with default config - service := &AppConfigService{ - db: db, - } - err := service.LoadDbConfig(t.Context()) - require.NoError(t, err) - - // Update a single config value - err = service.UpdateAppConfigValues(t.Context(), "appName", "Test App") - require.NoError(t, err) - - // Verify in-memory config was updated - config := service.GetDbConfig() - require.Equal(t, "Test App", config.AppName.Value) - - // Verify database was updated - var dbValue model.AppConfigVariable - err = db.Where("key = ?", "appName").First(&dbValue).Error - require.NoError(t, err) - require.Equal(t, "Test App", dbValue.Value) - }) - - t.Run("update multiple values", func(t *testing.T) { - db := testutils.NewDatabaseForTest(t) - - // Create a service with default config - service := &AppConfigService{ - db: db, - } - err := service.LoadDbConfig(t.Context()) - require.NoError(t, err) - - // Update multiple config values - err = service.UpdateAppConfigValues( - t.Context(), - "appName", "Test App", - "sessionDuration", "30", - "smtpHost", "mail.example.com", - ) - require.NoError(t, err) - - // Verify in-memory config was updated - config := service.GetDbConfig() - require.Equal(t, "Test App", config.AppName.Value) - require.Equal(t, "30", config.SessionDuration.Value) - require.Equal(t, "mail.example.com", config.SmtpHost.Value) - - // Verify database was updated - var count int64 - db.Model(&model.AppConfigVariable{}).Count(&count) - require.Equal(t, int64(3), count) - - var appName, sessionDuration, smtpHost model.AppConfigVariable - err = db.Where("key = ?", "appName").First(&appName).Error - require.NoError(t, err) - require.Equal(t, "Test App", appName.Value) - - err = db.Where("key = ?", "sessionDuration").First(&sessionDuration).Error - require.NoError(t, err) - require.Equal(t, "30", sessionDuration.Value) - - err = db.Where("key = ?", "smtpHost").First(&smtpHost).Error - require.NoError(t, err) - require.Equal(t, "mail.example.com", smtpHost.Value) - }) - - t.Run("empty value resets to default", func(t *testing.T) { - db := testutils.NewDatabaseForTest(t) - - // Create a service with default config - service := &AppConfigService{ - db: db, - } - err := service.LoadDbConfig(t.Context()) - require.NoError(t, err) - - // First change the value - err = service.UpdateAppConfigValues(t.Context(), "sessionDuration", "30") - require.NoError(t, err) - require.Equal(t, "30", service.GetDbConfig().SessionDuration.Value) - - // Now set it to empty which should use default value - err = service.UpdateAppConfigValues(t.Context(), "sessionDuration", "") - require.NoError(t, err) - require.Equal(t, "60", service.GetDbConfig().SessionDuration.Value) // Default value from getDefaultDbConfig - }) - - t.Run("error with odd number of arguments", func(t *testing.T) { - db := testutils.NewDatabaseForTest(t) - - // Create a service with default config - service := &AppConfigService{ - db: db, - } - err := service.LoadDbConfig(t.Context()) - require.NoError(t, err) - - // Try to update with odd number of arguments - err = service.UpdateAppConfigValues(t.Context(), "appName", "Test App", "sessionDuration") - require.Error(t, err) - require.Contains(t, err.Error(), "invalid number of arguments") - }) - - t.Run("error with invalid key", func(t *testing.T) { - db := testutils.NewDatabaseForTest(t) - - // Create a service with default config - service := &AppConfigService{ - db: db, - } - err := service.LoadDbConfig(t.Context()) - require.NoError(t, err) - - // Try to update with invalid key - err = service.UpdateAppConfigValues(t.Context(), "nonExistentKey", "some value") - require.Error(t, err) - require.Contains(t, err.Error(), "invalid configuration key") - }) -} - -func TestUpdateAppConfig(t *testing.T) { - t.Run("updates configuration values from DTO", func(t *testing.T) { - db := testutils.NewDatabaseForTest(t) - - // Create a service with default config - service := &AppConfigService{ - db: db, - } - err := service.LoadDbConfig(t.Context()) - require.NoError(t, err) - - // Create update DTO - input := dto.AppConfigUpdateDto{ - AppName: "Updated App Name", - SessionDuration: "120", - SmtpHost: "smtp.example.com", - SmtpPort: "587", - } - - // Update config - updatedVars, err := service.UpdateAppConfig(t.Context(), input) - require.NoError(t, err) - - // Verify returned updated variables - require.NotEmpty(t, updatedVars) - - var foundAppName, foundSessionDuration, foundSmtpHost, foundSmtpPort bool - for _, v := range updatedVars { - switch v.Key { - case "appName": - require.Equal(t, "Updated App Name", v.Value) - foundAppName = true - case "sessionDuration": - require.Equal(t, "120", v.Value) - foundSessionDuration = true - case "smtpHost": - require.Equal(t, "smtp.example.com", v.Value) - foundSmtpHost = true - case "smtpPort": - require.Equal(t, "587", v.Value) - foundSmtpPort = true - } - } - require.True(t, foundAppName) - require.True(t, foundSessionDuration) - require.True(t, foundSmtpHost) - require.True(t, foundSmtpPort) - - // Verify in-memory config was updated - config := service.GetDbConfig() - require.Equal(t, "Updated App Name", config.AppName.Value) - require.Equal(t, "120", config.SessionDuration.Value) - require.Equal(t, "smtp.example.com", config.SmtpHost.Value) - require.Equal(t, "587", config.SmtpPort.Value) - - // Verify database was updated - var appName, sessionDuration, smtpHost, smtpPort model.AppConfigVariable - err = db.Where("key = ?", "appName").First(&appName).Error - require.NoError(t, err) - require.Equal(t, "Updated App Name", appName.Value) - - err = db.Where("key = ?", "sessionDuration").First(&sessionDuration).Error - require.NoError(t, err) - require.Equal(t, "120", sessionDuration.Value) - - err = db.Where("key = ?", "smtpHost").First(&smtpHost).Error - require.NoError(t, err) - require.Equal(t, "smtp.example.com", smtpHost.Value) - - err = db.Where("key = ?", "smtpPort").First(&smtpPort).Error - require.NoError(t, err) - require.Equal(t, "587", smtpPort.Value) - }) - - t.Run("empty values reset to defaults", func(t *testing.T) { - db := testutils.NewDatabaseForTest(t) - - // Create a service with default config and modify some values - service := &AppConfigService{ - db: db, - } - err := service.LoadDbConfig(t.Context()) - require.NoError(t, err) - - // First set some non-default values - err = service.UpdateAppConfigValues(t.Context(), - "appName", "Custom App", - "sessionDuration", "120", - ) - require.NoError(t, err) - - // Create update DTO with empty values to reset to defaults - input := dto.AppConfigUpdateDto{ - AppName: "", // Should reset to default "Pocket ID" - SessionDuration: "", // Should reset to default "60" - } - - // Update config - updatedVars, err := service.UpdateAppConfig(t.Context(), input) - require.NoError(t, err) - - // Verify returned updated variables (they should be empty strings in DB) - var foundAppName, foundSessionDuration bool - for _, v := range updatedVars { - switch v.Key { - case "appName": - require.Equal(t, "Pocket ID", v.Value) // Returns the default value - foundAppName = true - case "sessionDuration": - require.Equal(t, "60", v.Value) // Returns the default value - foundSessionDuration = true - } - } - require.True(t, foundAppName) - require.True(t, foundSessionDuration) - - // Verify in-memory config was reset to defaults - config := service.GetDbConfig() - require.Equal(t, "Pocket ID", config.AppName.Value) // Default value - require.Equal(t, "60", config.SessionDuration.Value) // Default value - - // Verify database was updated with empty values - for _, key := range []string{"appName", "sessionDuration"} { - var loaded model.AppConfigVariable - err = db.Where("key = ?", key).First(&loaded).Error - require.NoErrorf(t, err, "Failed to load DB value for key '%s'", key) - require.Emptyf(t, loaded.Value, "Loaded value for key '%s' is not empty", key) - } - }) - - t.Run("cannot update when UiConfigDisabled is true", func(t *testing.T) { - // Save the original state and restore it after the test - originalUiConfigDisabled := common.EnvConfig.UiConfigDisabled - defer func() { - common.EnvConfig.UiConfigDisabled = originalUiConfigDisabled - }() - - // Disable UI config - common.EnvConfig.UiConfigDisabled = true - - db := testutils.NewDatabaseForTest(t) - service := &AppConfigService{ - db: db, - } - err := service.LoadDbConfig(t.Context()) - require.NoError(t, err) - - // Try to update config - _, err = service.UpdateAppConfig(t.Context(), dto.AppConfigUpdateDto{ - AppName: "Should Not Update", - }) - - // Should get a UiConfigDisabledError - require.Error(t, err) - var uiConfigDisabledErr *common.UiConfigDisabledError - require.ErrorAs(t, err, &uiConfigDisabledErr) - }) -} diff --git a/backend/internal/service/audit_log_service.go b/backend/internal/service/audit_log_service.go index 7a2290e4..f45cac89 100644 --- a/backend/internal/service/audit_log_service.go +++ b/backend/internal/service/audit_log_service.go @@ -6,6 +6,7 @@ import ( "log/slog" userAgentParser "github.com/mileusna/useragent" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/model" "github.com/pocket-id/pocket-id/backend/internal/utils" "github.com/pocket-id/pocket-id/backend/internal/utils/email" @@ -14,17 +15,17 @@ import ( type AuditLogService struct { db *gorm.DB - appConfigService *AppConfigService emailService *EmailService geoliteService *GeoLiteService + appConfigService *appconfig.AppConfigService } -func NewAuditLogService(db *gorm.DB, appConfigService *AppConfigService, emailService *EmailService, geoliteService *GeoLiteService) *AuditLogService { +func NewAuditLogService(db *gorm.DB, emailService *EmailService, geoliteService *GeoLiteService, appConfigService *appconfig.AppConfigService) *AuditLogService { return &AuditLogService{ db: db, - appConfigService: appConfigService, emailService: emailService, geoliteService: geoliteService, + appConfigService: appConfigService, } } @@ -64,7 +65,8 @@ func (s *AuditLogService) Create(ctx context.Context, event model.AuditLogEvent, } // CreateNewSignInWithEmail creates a new audit log entry in the database and sends an email if the device hasn't been used before -func (s *AuditLogService) CreateNewSignInWithEmail(ctx context.Context, ipAddress, userAgent, userID string, tx *gorm.DB) model.AuditLog { +// emailLoginNotificationEnabled gates whether the notification email is sent, so the caller decides using the config it already loaded +func (s *AuditLogService) CreateNewSignInWithEmail(ctx context.Context, ipAddress, userAgent, userID string, tx *gorm.DB, emailLoginNotificationEnabled bool) model.AuditLog { createdAuditLog, ok := s.Create(ctx, model.AuditLogEventSignIn, ipAddress, userAgent, userID, model.AuditLogData{}, tx) if !ok { // At this point the transaction has been canceled already, and error has been logged @@ -90,15 +92,22 @@ func (s *AuditLogService) CreateNewSignInWithEmail(ctx context.Context, ipAddres } // If the user hasn't logged in from the same device before and email notifications are enabled, send an email - if s.appConfigService.GetDbConfig().EmailLoginNotificationEnabled.IsTrue() && count <= 1 { + if emailLoginNotificationEnabled && count <= 1 { go func() { // This runs in background, so use a context without cancellation (or it would be stopped when the request ends) // We still want to have a context derived from the request's to carry over tracing info innerCtx := context.WithoutCancel(ctx) + // This runs after the request has completed, so we resolve the current config rather than threading the request's snapshot into the goroutine + dbConfig, innerErr := s.appConfigService.GetConfig(innerCtx) + if innerErr != nil { + slog.ErrorContext(innerCtx, "Failed to load app configuration to send notification email", slog.Any("error", innerErr)) + return + } + // Note we don't use the transaction here because this is running in background var user model.User - innerErr := s.db. + innerErr = s.db. WithContext(innerCtx). Where("id = ?", userID). First(&user). @@ -112,7 +121,7 @@ func (s *AuditLogService) CreateNewSignInWithEmail(ctx context.Context, ipAddres return } - innerErr = SendEmail(innerCtx, s.emailService, email.Address{ + innerErr = SendEmail(innerCtx, s.emailService, dbConfig, email.Address{ Name: user.FullName(), Email: *user.Email, }, NewLoginTemplate, &NewLoginTemplateData{ diff --git a/backend/internal/service/e2etest_service.go b/backend/internal/service/e2etest_service.go index bd32bc38..21572adc 100644 --- a/backend/internal/service/e2etest_service.go +++ b/backend/internal/service/e2etest_service.go @@ -22,10 +22,12 @@ import ( "github.com/ory/fosite/compose" fositejwt "github.com/ory/fosite/token/jwt" "github.com/pocket-id/pocket-id/backend/internal/apikey" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "gorm.io/gorm" "github.com/pocket-id/pocket-id/backend/internal/api" "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" datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" "github.com/pocket-id/pocket-id/backend/internal/oidc" @@ -40,7 +42,7 @@ import ( type TestService struct { db *gorm.DB jwtService *JwtService - appConfigService *AppConfigService + appConfigService *appconfig.AppConfigService ldapService *LdapService fileStorage storage.FileStorage appLockService *AppLockService @@ -54,7 +56,7 @@ const ( e2eRefreshTokenExpiredFixtureToken = "X4vqwtRyCUaq51UafHea4Fsg8Km6CAns6vp3tuX4" ) -func NewTestService(db *gorm.DB, appConfigService *AppConfigService, jwtService *JwtService, ldapService *LdapService, appLockService *AppLockService, fileStorage storage.FileStorage) (*TestService, error) { +func NewTestService(db *gorm.DB, appConfigService *appconfig.AppConfigService, jwtService *JwtService, ldapService *LdapService, appLockService *AppLockService, fileStorage storage.FileStorage) (*TestService, error) { s := &TestService{ db: db, appConfigService: appConfigService, @@ -624,8 +626,8 @@ func (s *TestService) ResetApplicationImages(ctx context.Context) error { } func (s *TestService) ResetAppConfig(ctx context.Context) error { - // Reset all app config variables to their default values in the database - err := s.db.Session(&gorm.Session{AllowGlobalUpdate: true}).Model(&model.AppConfigVariable{}).Update("value", "").Error + // Reset all application configuration values through the singleton actor + _, err := s.appConfigService.UpdateAppConfig(ctx, dto.AppConfigUpdateDto{}) if err != nil { return err } @@ -646,12 +648,6 @@ func (s *TestService) ResetAppConfig(ctx context.Context) error { // The instance ID is loaded once at startup, so we also set it directly on the JWT service so it takes effect immediately s.jwtService.instanceID = testInstanceID - // Reload the app config from the database after resetting the values - err = s.appConfigService.LoadDbConfig(ctx) - if err != nil { - return err - } - // Reload the JWK if err := s.jwtService.LoadOrGenerateKey(ctx); err != nil { return err @@ -667,50 +663,39 @@ func (s *TestService) ResetLock(ctx context.Context) error { // SyncLdap triggers an LDAP synchronization func (s *TestService) SyncLdap(ctx context.Context) error { - return s.ldapService.SyncAll(ctx) + dbConfig, err := s.appConfigService.GetConfig(ctx) + if err != nil { + return fmt.Errorf("error loading app configuration: %w", err) + } + return s.ldapService.SyncAll(ctx, dbConfig) } -// SetLdapTestConfig writes the test LDAP config variables directly to the database. +// SetLdapTestConfig updates the LDAP configuration used by the end-to-end test server func (s *TestService) SetLdapTestConfig(ctx context.Context) error { - err := s.db.Transaction(func(tx *gorm.DB) error { - ldapConfigs := map[string]string{ - "ldapUrl": "ldap://lldap:3890", - "ldapBindDn": "uid=admin,ou=people,dc=pocket-id,dc=org", - "ldapBindPassword": "admin_password", - "ldapBase": "dc=pocket-id,dc=org", - "ldapUserSearchFilter": "(objectClass=person)", - "ldapUserGroupSearchFilter": "(objectClass=groupOfNames)", - "ldapSkipCertVerify": "true", - "ldapAttributeUserUniqueIdentifier": "uuid", - "ldapAttributeUserUsername": "uid", - "ldapAttributeUserEmail": "mail", - "ldapAttributeUserFirstName": "givenName", - "ldapAttributeUserLastName": "sn", - "ldapAttributeGroupUniqueIdentifier": "uuid", - "ldapAttributeGroupName": "uid", - "ldapAttributeGroupMember": "member", - "ldapAdminGroupName": "admin_group", - "ldapSoftDeleteUsers": "true", - "ldapEnabled": "true", - } - - for key, value := range ldapConfigs { - configVar := model.AppConfigVariable{Key: key, Value: value} - if err := tx.Create(&configVar).Error; err != nil { - return fmt.Errorf("failed to create config variable '%s': %w", key, err) - } - } - return nil - }) - + err := s.appConfigService.UpdateAppConfigValues(ctx, + "ldapUrl", "ldap://lldap:3890", + "ldapBindDn", "uid=admin,ou=people,dc=pocket-id,dc=org", + "ldapBindPassword", "admin_password", + "ldapBase", "dc=pocket-id,dc=org", + "ldapUserSearchFilter", "(objectClass=person)", + "ldapUserGroupSearchFilter", "(objectClass=groupOfNames)", + "ldapSkipCertVerify", "true", + "ldapAttributeUserUniqueIdentifier", "uuid", + "ldapAttributeUserUsername", "uid", + "ldapAttributeUserEmail", "mail", + "ldapAttributeUserFirstName", "givenName", + "ldapAttributeUserLastName", "sn", + "ldapAttributeGroupUniqueIdentifier", "uuid", + "ldapAttributeGroupName", "uid", + "ldapAttributeGroupMember", "member", + "ldapAdminGroupName", "admin_group", + "ldapSoftDeleteUsers", "true", + "ldapEnabled", "true", + ) if err != nil { return fmt.Errorf("failed to set LDAP test config: %w", err) } - if err := s.appConfigService.LoadDbConfig(ctx); err != nil { - return fmt.Errorf("failed to load app config: %w", err) - } - return nil } diff --git a/backend/internal/service/email_service.go b/backend/internal/service/email_service.go index 19e671f9..98ac360a 100644 --- a/backend/internal/service/email_service.go +++ b/backend/internal/service/email_service.go @@ -13,19 +13,19 @@ import ( "github.com/italypaleale/go-kit/emailer" "gorm.io/gorm" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/common" "github.com/pocket-id/pocket-id/backend/internal/model" "github.com/pocket-id/pocket-id/backend/internal/utils/email" ) type EmailService struct { - appConfigService *AppConfigService - db *gorm.DB - htmlTemplates map[string]*htemplate.Template - textTemplates map[string]*ttemplate.Template + db *gorm.DB + htmlTemplates map[string]*htemplate.Template + textTemplates map[string]*ttemplate.Template } -func NewEmailService(db *gorm.DB, appConfigService *AppConfigService) (*EmailService, error) { +func NewEmailService(db *gorm.DB) (*EmailService, error) { htmlTemplates, err := email.PrepareHTMLTemplates(emailTemplatesPaths) if err != nil { return nil, fmt.Errorf("prepare html templates: %w", err) @@ -37,14 +37,13 @@ func NewEmailService(db *gorm.DB, appConfigService *AppConfigService) (*EmailSer } return &EmailService{ - appConfigService: appConfigService, - db: db, - htmlTemplates: htmlTemplates, - textTemplates: textTemplates, + db: db, + htmlTemplates: htmlTemplates, + textTemplates: textTemplates, }, nil } -func (srv *EmailService) SendTestEmail(ctx context.Context, recipientUserId string) error { +func (srv *EmailService) SendTestEmail(ctx context.Context, dbConfig *appconfig.AppConfigModel, recipientUserId string) error { var user model.User err := srv.db. WithContext(ctx). @@ -58,18 +57,18 @@ func (srv *EmailService) SendTestEmail(ctx context.Context, recipientUserId stri return &common.UserEmailNotSetError{} } - return SendEmail(ctx, srv, + return SendEmail(ctx, srv, dbConfig, email.Address{ Email: *user.Email, Name: user.FullName(), }, TestTemplate, nil) } -func SendEmail[V any](ctx context.Context, srv *EmailService, toEmail email.Address, template email.Template[V], tData *V) error { - dbConfig := srv.appConfigService.GetDbConfig() +// SendEmail sends an email using the provided application configuration +func SendEmail[V any](ctx context.Context, srv *EmailService, dbConfig *appconfig.AppConfigModel, toEmail email.Address, template email.Template[V], tData *V) error { data := &email.TemplateData[V]{ - AppName: dbConfig.AppName.Value, + AppName: dbConfig.AppName.String(), LogoURL: common.EnvConfig.AppURL + "/api/application-images/email", Data: tData, } @@ -102,7 +101,7 @@ func SendEmail[V any](ctx context.Context, srv *EmailService, toEmail email.Addr } // getEmailer builds an emailer.Emailer from the current app config. -func (srv *EmailService) getEmailer(ctx context.Context, dbConfig *model.AppConfig) (emailer.Emailer, error) { +func (srv *EmailService) getEmailer(ctx context.Context, dbConfig *appconfig.AppConfigModel) (emailer.Emailer, error) { // We support SMTP only (for now) connString, err := smtpConnString(dbConfig) if err != nil { @@ -116,8 +115,8 @@ func (srv *EmailService) getEmailer(ctx context.Context, dbConfig *model.AppConf // smtpConnString builds the SMTP connection string that go-kit's emailer expects: // smtp://:@:?fromAddress=
&fromName=&tls=&insecureSkipVerify= -func smtpConnString(dbConfig *model.AppConfig) (string, error) { - host := dbConfig.SmtpHost.Value +func smtpConnString(dbConfig *appconfig.AppConfigModel) (string, error) { + host := dbConfig.SmtpHost.String() if host == "" { return "", errors.New("SMTP host is not configured") } @@ -126,28 +125,28 @@ func smtpConnString(dbConfig *model.AppConfig) (string, error) { Scheme: "smtp", Host: host, } - port := dbConfig.SmtpPort.Value + port := dbConfig.SmtpPort.String() if port != "" { u.Host = net.JoinHostPort(host, port) } // Include credentials when set - smtpUser := dbConfig.SmtpUser.Value - smtpPassword := dbConfig.SmtpPassword.Value + smtpUser := dbConfig.SmtpUser.String() + smtpPassword := dbConfig.SmtpPassword.String() if smtpUser != "" || smtpPassword != "" { u.User = url.UserPassword(smtpUser, smtpPassword) } // TLS values from config: none, starttls, tls - tlsMode := dbConfig.SmtpTls.Value + tlsMode := dbConfig.SmtpTls.String() if tlsMode == "" { tlsMode = "none" } // Build the query string args q := url.Values{} - q.Set("fromAddress", dbConfig.SmtpFrom.Value) - q.Set("fromName", dbConfig.AppName.Value) + q.Set("fromAddress", dbConfig.SmtpFrom.String()) + q.Set("fromName", dbConfig.AppName.String()) q.Set("tls", tlsMode) if dbConfig.SmtpSkipCertVerify.IsTrue() { q.Set("insecureSkipVerify", "true") diff --git a/backend/internal/service/import_service_test.go b/backend/internal/service/import_service_test.go index a53fabf0..b793bafc 100644 --- a/backend/internal/service/import_service_test.go +++ b/backend/internal/service/import_service_test.go @@ -54,12 +54,14 @@ func seedActorHostSchema(t *testing.T, db *gorm.DB) { func requireActorHostSchemaPreserved(t *testing.T, db *gorm.DB) { t.Helper() var tableRows int64 - require.NoError(t, db.Raw(`SELECT count(*) FROM francis_active_actors`).Scan(&tableRows).Error) + err := db.Raw(`SELECT count(*) FROM francis_active_actors`).Scan(&tableRows).Error + require.NoError(t, err) require.Equal(t, int64(1), tableRows, "francis_ tables and their rows must be preserved by an import") // The view is only valid if its backing table was preserved as well var viewCount int64 - require.NoError(t, db.Raw(`SELECT n FROM francis_host_active_actor_count`).Scan(&viewCount).Error) + err = db.Raw(`SELECT n FROM francis_host_active_actor_count`).Scan(&viewCount).Error + require.NoError(t, err) require.Equal(t, int64(1), viewCount, "francis_ views must be preserved by an import") } diff --git a/backend/internal/service/jwt_service.go b/backend/internal/service/jwt_service.go index 07ce8b21..fcc926da 100644 --- a/backend/internal/service/jwt_service.go +++ b/backend/internal/service/jwt_service.go @@ -43,19 +43,18 @@ const ( ) type JwtService struct { - db *gorm.DB - envConfig *common.EnvConfigSchema - privateKey jwk.Key - keyId string - appConfigService *AppConfigService - instanceID string - jwksEncoded []byte + db *gorm.DB + envConfig *common.EnvConfigSchema + privateKey jwk.Key + keyId string + instanceID string + jwksEncoded []byte } -func NewJwtService(ctx context.Context, db *gorm.DB, instanceID string, appConfigService *AppConfigService) (*JwtService, error) { +func NewJwtService(ctx context.Context, db *gorm.DB, instanceID string) (*JwtService, error) { service := &JwtService{} - err := service.init(ctx, db, instanceID, appConfigService, &common.EnvConfig) + err := service.init(ctx, db, instanceID, &common.EnvConfig) if err != nil { return nil, err } @@ -63,8 +62,7 @@ func NewJwtService(ctx context.Context, db *gorm.DB, instanceID string, appConfi return service, nil } -func (s *JwtService) init(ctx context.Context, db *gorm.DB, instanceID string, appConfigService *AppConfigService, envConfig *common.EnvConfigSchema) (err error) { - s.appConfigService = appConfigService +func (s *JwtService) init(ctx context.Context, db *gorm.DB, instanceID string, envConfig *common.EnvConfigSchema) (err error) { s.envConfig = envConfig s.db = db s.instanceID = instanceID @@ -183,12 +181,11 @@ func (s *JwtService) SetKey(privateKey jwk.Key) error { return nil } -func (s *JwtService) GenerateAccessToken(user model.User, authenticationMethod string) (string, error) { - +func (s *JwtService) GenerateAccessToken(user model.User, authenticationMethod string, sessionDuration time.Duration) (string, error) { now := time.Now() token, err := jwt.NewBuilder(). Subject(user.ID). - Expiration(now.Add(s.appConfigService.GetDbConfig().SessionDuration.AsDurationMinutes())). + Expiration(now.Add(sessionDuration)). IssuedAt(now). Issuer(s.envConfig.AppURL). JwtID(uuid.New().String()). diff --git a/backend/internal/service/jwt_service_test.go b/backend/internal/service/jwt_service_test.go index b0eae143..ca69836e 100644 --- a/backend/internal/service/jwt_service_test.go +++ b/backend/internal/service/jwt_service_test.go @@ -17,6 +17,7 @@ import ( "github.com/stretchr/testify/require" "gorm.io/gorm" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/common" "github.com/pocket-id/pocket-id/backend/internal/instanceid" "github.com/pocket-id/pocket-id/backend/internal/model" @@ -35,17 +36,17 @@ func newTestEnvConfig() *common.EnvConfigSchema { } } -func initJwtService(t *testing.T, db *gorm.DB, instanceID string, appConfig *AppConfigService, envConfig *common.EnvConfigSchema) *JwtService { +func initJwtService(t *testing.T, db *gorm.DB, instanceID string, _ *appconfig.AppConfigService, envConfig *common.EnvConfigSchema) *JwtService { t.Helper() service := &JwtService{} - err := service.init(t.Context(), db, instanceID, appConfig, envConfig) + err := service.init(t.Context(), db, instanceID, envConfig) require.NoError(t, err, "Failed to initialize JWT service") return service } -func setupJwtService(t *testing.T, instanceID string, appConfig *AppConfigService) (*JwtService, *gorm.DB, *common.EnvConfigSchema) { +func setupJwtService(t *testing.T, instanceID string, appConfig *appconfig.AppConfigService) (*JwtService, *gorm.DB, *common.EnvConfigSchema) { t.Helper() db := testutils.NewDatabaseForTest(t) @@ -70,7 +71,7 @@ func newTestDbAndEnv(t *testing.T) (*gorm.DB, *common.EnvConfigSchema) { return testutils.NewDatabaseForTest(t), newTestEnvConfig() } -func saveKeyToDatabase(t *testing.T, db *gorm.DB, instanceID string, envConfig *common.EnvConfigSchema, appConfig *AppConfigService, key jwk.Key) string { +func saveKeyToDatabase(t *testing.T, db *gorm.DB, instanceID string, envConfig *common.EnvConfigSchema, appConfig *appconfig.AppConfigService, key jwk.Key) string { t.Helper() keyProvider, err := jwkutils.GetKeyProvider(db, envConfig, instanceID) @@ -87,9 +88,7 @@ func saveKeyToDatabase(t *testing.T, db *gorm.DB, instanceID string, envConfig * } func TestJwtService_Init(t *testing.T) { - mockConfig := NewTestAppConfigService(&model.AppConfig{ - SessionDuration: model.AppConfigVariable{Value: "60"}, // 60 minutes - }) + mockConfig := appconfig.NewTestAppConfigService(nil) t.Run("should generate new key when none exists", func(t *testing.T) { db := testutils.NewDatabaseForTest(t) @@ -192,9 +191,7 @@ func TestJwtService_Init(t *testing.T) { } func TestJwtService_GetPublicJWK(t *testing.T) { - mockConfig := NewTestAppConfigService(&model.AppConfig{ - SessionDuration: model.AppConfigVariable{Value: "60"}, // 60 minutes - }) + mockConfig := appconfig.NewTestAppConfigService(nil) db := testutils.NewDatabaseForTest(t) mockEnvConfig := newTestEnvConfig() instanceID := newInstanceID(t, db) @@ -310,9 +307,8 @@ func TestJwtService_GetPublicJWK(t *testing.T) { } func TestGenerateVerifyAccessToken(t *testing.T) { - mockConfig := NewTestAppConfigService(&model.AppConfig{ - SessionDuration: model.AppConfigVariable{Value: "60"}, // 60 minutes - }) + const sessionDuration = time.Hour + mockConfig := appconfig.NewTestAppConfigService(nil) db, envConfig := newTestDbAndEnv(t) instanceID := newInstanceID(t, db) @@ -325,7 +321,7 @@ func TestGenerateVerifyAccessToken(t *testing.T) { IsAdmin: false, } - tokenString, err := service.GenerateAccessToken(user, "") + tokenString, err := service.GenerateAccessToken(user, "", sessionDuration) require.NoError(t, err, "Failed to generate access token") assert.NotEmpty(t, tokenString, "Token should not be empty") @@ -366,7 +362,7 @@ func TestGenerateVerifyAccessToken(t *testing.T) { IsAdmin: true, } - tokenString, err := service.GenerateAccessToken(adminUser, "") + tokenString, err := service.GenerateAccessToken(adminUser, "", sessionDuration) require.NoError(t, err, "Failed to generate access token") claims, err := service.VerifyAccessToken(tokenString) @@ -389,7 +385,7 @@ func TestGenerateVerifyAccessToken(t *testing.T) { Base: model.Base{ID: "user-with-auth-method"}, } - tokenString, err := service.GenerateAccessToken(user, AuthenticationMethodPhishingResistant) + tokenString, err := service.GenerateAccessToken(user, AuthenticationMethodPhishingResistant, sessionDuration) require.NoError(t, err, "Failed to generate access token") claims, err := service.VerifyAccessToken(tokenString) @@ -400,29 +396,6 @@ func TestGenerateVerifyAccessToken(t *testing.T) { assert.Equal(t, AuthenticationMethodPhishingResistant, authenticationMethod, "amr should match") }) - t.Run("uses session duration from config", func(t *testing.T) { - customMockConfig := NewTestAppConfigService(&model.AppConfig{ - SessionDuration: model.AppConfigVariable{Value: "30"}, // 30 minutes - }) - service, _, _ := setupJwtService(t, instanceID, customMockConfig) - - user := model.User{ - Base: model.Base{ID: "user456"}, - } - - tokenString, err := service.GenerateAccessToken(user, "") - require.NoError(t, err, "Failed to generate access token") - - claims, err := service.VerifyAccessToken(tokenString) - require.NoError(t, err, "Failed to verify generated token") - - expectedExp := time.Now().Add(30 * time.Minute) - expiration, ok := claims.Expiration() - assert.True(t, ok, "Expiration not found in token") - timeDiff := expectedExp.Sub(expiration).Minutes() - assert.InDelta(t, 0, timeDiff, 1.0, "Token should expire in approximately 30 minutes") - }) - t.Run("works with Ed25519 keys", func(t *testing.T) { origKeyID := createEdDSAKeyJWK(t, db, instanceID, envConfig, mockConfig) service := initJwtService(t, db, instanceID, mockConfig, envConfig) @@ -437,7 +410,7 @@ func TestGenerateVerifyAccessToken(t *testing.T) { IsAdmin: true, } - tokenString, err := service.GenerateAccessToken(user, "") + tokenString, err := service.GenerateAccessToken(user, "", sessionDuration) require.NoError(t, err, "Failed to generate access token with Ed25519 key") assert.NotEmpty(t, tokenString, "Token should not be empty") @@ -475,7 +448,7 @@ func TestGenerateVerifyAccessToken(t *testing.T) { IsAdmin: true, } - tokenString, err := service.GenerateAccessToken(user, "") + tokenString, err := service.GenerateAccessToken(user, "", sessionDuration) require.NoError(t, err, "Failed to generate access token with ECDSA key") assert.NotEmpty(t, tokenString, "Token should not be empty") @@ -513,7 +486,7 @@ func TestGenerateVerifyAccessToken(t *testing.T) { IsAdmin: true, } - tokenString, err := service.GenerateAccessToken(user, "") + tokenString, err := service.GenerateAccessToken(user, "", sessionDuration) require.NoError(t, err, "Failed to generate access token with RSA key") assert.NotEmpty(t, tokenString, "Token should not be empty") @@ -582,7 +555,7 @@ func TestTokenTypeValidator(t *testing.T) { }) } -func importKey(t *testing.T, db *gorm.DB, instanceID string, envConfig *common.EnvConfigSchema, appConfig *AppConfigService, privateKeyRaw any) string { +func importKey(t *testing.T, db *gorm.DB, instanceID string, envConfig *common.EnvConfigSchema, appConfig *appconfig.AppConfigService, privateKeyRaw any) string { t.Helper() privateKey, err := jwkutils.ImportRawKey(privateKeyRaw, "", "") @@ -597,7 +570,7 @@ var ( rsaKeyPrecomputeOnce sync.Once ) -func createRSA4096KeyJWK(t *testing.T, db *gorm.DB, instanceID string, envConfig *common.EnvConfigSchema, appConfig *AppConfigService) string { +func createRSA4096KeyJWK(t *testing.T, db *gorm.DB, instanceID string, envConfig *common.EnvConfigSchema, appConfig *appconfig.AppConfigService) string { t.Helper() rsaKeyPrecomputeOnce.Do(func() { @@ -612,7 +585,7 @@ func createRSA4096KeyJWK(t *testing.T, db *gorm.DB, instanceID string, envConfig return importKey(t, db, instanceID, envConfig, appConfig, rsaKeyPrecomputed) } -func createECDSAKeyJWK(t *testing.T, db *gorm.DB, instanceID string, envConfig *common.EnvConfigSchema, appConfig *AppConfigService) string { +func createECDSAKeyJWK(t *testing.T, db *gorm.DB, instanceID string, envConfig *common.EnvConfigSchema, appConfig *appconfig.AppConfigService) string { t.Helper() // Generate a new P-256 ECDSA key @@ -624,7 +597,7 @@ func createECDSAKeyJWK(t *testing.T, db *gorm.DB, instanceID string, envConfig * } // Helper function to create an Ed25519 key and save it as JWK -func createEdDSAKeyJWK(t *testing.T, db *gorm.DB, instanceID string, envConfig *common.EnvConfigSchema, appConfig *AppConfigService) string { +func createEdDSAKeyJWK(t *testing.T, db *gorm.DB, instanceID string, envConfig *common.EnvConfigSchema, appConfig *appconfig.AppConfigService) string { t.Helper() // Generate a new Ed25519 key pair diff --git a/backend/internal/service/ldap_service.go b/backend/internal/service/ldap_service.go index 73392c61..34f6a3f4 100644 --- a/backend/internal/service/ldap_service.go +++ b/backend/internal/service/ldap_service.go @@ -18,6 +18,7 @@ import ( "github.com/go-ldap/ldap/v3" "github.com/google/uuid" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/storage" "github.com/pocket-id/pocket-id/backend/internal/utils" "golang.org/x/text/unicode/norm" @@ -29,13 +30,12 @@ import ( ) type LdapService struct { - db *gorm.DB - httpClient *http.Client - appConfigService *AppConfigService - userService *UserService - groupService *UserGroupService - fileStorage storage.FileStorage - clientFactory func() (ldapClient, error) + db *gorm.DB + httpClient *http.Client + userService *UserService + groupService *UserGroupService + fileStorage storage.FileStorage + clientFactory func(dbConfig *appconfig.AppConfigModel) (ldapClient, error) } type savePicture struct { @@ -69,29 +69,26 @@ type ldapClient interface { Close() error } -func NewLdapService(db *gorm.DB, httpClient *http.Client, appConfigService *AppConfigService, userService *UserService, groupService *UserGroupService, fileStorage storage.FileStorage) *LdapService { +func NewLdapService(db *gorm.DB, httpClient *http.Client, userService *UserService, groupService *UserGroupService, fileStorage storage.FileStorage) *LdapService { service := &LdapService{ - db: db, - httpClient: httpClient, - appConfigService: appConfigService, - userService: userService, - groupService: groupService, - fileStorage: fileStorage, + db: db, + httpClient: httpClient, + userService: userService, + groupService: groupService, + fileStorage: fileStorage, } service.clientFactory = service.createClient return service } -func (s *LdapService) createClient() (ldapClient, error) { - dbConfig := s.appConfigService.GetDbConfig() - +func (s *LdapService) createClient(dbConfig *appconfig.AppConfigModel) (ldapClient, error) { if !dbConfig.LdapEnabled.IsTrue() { return nil, fmt.Errorf("LDAP is not enabled") } // Setup LDAP connection - client, err := ldap.DialURL(dbConfig.LdapUrl.Value, ldap.DialWithTLSConfig(&tls.Config{ + client, err := ldap.DialURL(dbConfig.LdapUrl.String(), ldap.DialWithTLSConfig(&tls.Config{ InsecureSkipVerify: dbConfig.LdapSkipCertVerify.IsTrue(), //nolint:gosec })) if err != nil { @@ -99,23 +96,24 @@ func (s *LdapService) createClient() (ldapClient, error) { } // Bind as service account - err = client.Bind(dbConfig.LdapBindDn.Value, dbConfig.LdapBindPassword.Value) + err = client.Bind(dbConfig.LdapBindDn.String(), dbConfig.LdapBindPassword.String()) if err != nil { return nil, fmt.Errorf("failed to bind to LDAP: %w", err) } return client, nil } -func (s *LdapService) SyncAll(ctx context.Context) error { +// SyncAll synchronizes LDAP using the provided application configuration +func (s *LdapService) SyncAll(ctx context.Context, dbConfig *appconfig.AppConfigModel) error { // Setup LDAP connection - client, err := s.clientFactory() + client, err := s.clientFactory(dbConfig) if err != nil { return fmt.Errorf("failed to create LDAP client: %w", err) } defer client.Close() // First, we fetch all users and group from LDAP, which is our "desired state" - desiredState, err := s.fetchDesiredState(ctx, client) + desiredState, err := s.fetchDesiredState(ctx, client, dbConfig) if err != nil { return fmt.Errorf("failed to fetch LDAP state: %w", err) } @@ -128,13 +126,13 @@ func (s *LdapService) SyncAll(ctx context.Context) error { defer tx.Rollback() // Reconcile users - savePictures, deleteFiles, err := s.reconcileUsers(ctx, tx, desiredState.users, desiredState.userIDs) + savePictures, deleteFiles, err := s.reconcileUsers(ctx, tx, desiredState.users, desiredState.userIDs, dbConfig) if err != nil { return fmt.Errorf("failed to sync users: %w", err) } // Reconcile groups - err = s.reconcileGroups(ctx, tx, desiredState.groups, desiredState.groupIDs) + err = s.reconcileGroups(ctx, tx, desiredState.groups, desiredState.groupIDs, dbConfig) if err != nil { return fmt.Errorf("failed to sync groups: %w", err) } @@ -167,15 +165,15 @@ func (s *LdapService) SyncAll(ctx context.Context) error { return nil } -func (s *LdapService) fetchDesiredState(ctx context.Context, client ldapClient) (ldapDesiredState, error) { +func (s *LdapService) fetchDesiredState(ctx context.Context, client ldapClient, dbConfig *appconfig.AppConfigModel) (ldapDesiredState, error) { // Fetch users first so we can use their DNs when resolving group members - users, userIDs, usernamesByDN, err := s.fetchUsersFromLDAP(ctx, client) + users, userIDs, usernamesByDN, err := s.fetchUsersFromLDAP(ctx, client, dbConfig) if err != nil { return ldapDesiredState{}, err } // Then fetch groups to complete the desired LDAP state snapshot - groups, groupIDs, err := s.fetchGroupsFromLDAP(ctx, client, usernamesByDN) + groups, groupIDs, err := s.fetchGroupsFromLDAP(ctx, client, usernamesByDN, dbConfig) if err != nil { return ldapDesiredState{}, err } @@ -183,7 +181,7 @@ func (s *LdapService) fetchDesiredState(ctx context.Context, client ldapClient) // Apply user admin flags from the desired group membership snapshot. // This intentionally uses the configured group member attribute rather than // relying on a user-side reverse-membership attribute such as memberOf. - s.applyAdminGroupMembership(users, groups) + s.applyAdminGroupMembership(users, groups, dbConfig) return ldapDesiredState{ users: users, @@ -193,15 +191,14 @@ func (s *LdapService) fetchDesiredState(ctx context.Context, client ldapClient) }, nil } -func (s *LdapService) applyAdminGroupMembership(desiredUsers []ldapDesiredUser, desiredGroups []ldapDesiredGroup) { - dbConfig := s.appConfigService.GetDbConfig() - if dbConfig.LdapAdminGroupName.Value == "" { +func (s *LdapService) applyAdminGroupMembership(desiredUsers []ldapDesiredUser, desiredGroups []ldapDesiredGroup, dbConfig *appconfig.AppConfigModel) { + if dbConfig.LdapAdminGroupName == "" { return } adminUsernames := make(map[string]struct{}) for _, group := range desiredGroups { - if group.input.Name != dbConfig.LdapAdminGroupName.Value { + if group.input.Name != string(dbConfig.LdapAdminGroupName) { continue } @@ -216,21 +213,19 @@ func (s *LdapService) applyAdminGroupMembership(desiredUsers []ldapDesiredUser, } } -func (s *LdapService) fetchGroupsFromLDAP(ctx context.Context, client ldapClient, usernamesByDN map[string]string) (desiredGroups []ldapDesiredGroup, ldapGroupIDs map[string]struct{}, err error) { - dbConfig := s.appConfigService.GetDbConfig() - +func (s *LdapService) fetchGroupsFromLDAP(ctx context.Context, client ldapClient, usernamesByDN map[string]string, dbConfig *appconfig.AppConfigModel) (desiredGroups []ldapDesiredGroup, ldapGroupIDs map[string]struct{}, err error) { // Query LDAP for all groups we want to manage searchAttrs := []string{ - dbConfig.LdapAttributeGroupName.Value, - dbConfig.LdapAttributeGroupUniqueIdentifier.Value, - dbConfig.LdapAttributeGroupMember.Value, + dbConfig.LdapAttributeGroupName.String(), + dbConfig.LdapAttributeGroupUniqueIdentifier.String(), + dbConfig.LdapAttributeGroupMember.String(), } searchReq := ldap.NewSearchRequest( - dbConfig.LdapBase.Value, + dbConfig.LdapBase.String(), ldap.ScopeWholeSubtree, 0, 0, 0, false, - dbConfig.LdapUserGroupSearchFilter.Value, + dbConfig.LdapUserGroupSearchFilter.String(), searchAttrs, []ldap.Control{}, ) @@ -244,21 +239,21 @@ func (s *LdapService) fetchGroupsFromLDAP(ctx context.Context, client ldapClient desiredGroups = make([]ldapDesiredGroup, 0, len(result.Entries)) for _, value := range result.Entries { - ldapID := convertLdapIdToString(value.GetAttributeValue(dbConfig.LdapAttributeGroupUniqueIdentifier.Value)) + ldapID := convertLdapIdToString(value.GetAttributeValue(dbConfig.LdapAttributeGroupUniqueIdentifier.String())) // Skip groups without a valid LDAP ID if ldapID == "" { - slog.Warn("Skipping LDAP group without a valid unique identifier", slog.String("attribute", dbConfig.LdapAttributeGroupUniqueIdentifier.Value)) + slog.Warn("Skipping LDAP group without a valid unique identifier", slog.String("attribute", dbConfig.LdapAttributeGroupUniqueIdentifier.String())) continue } ldapGroupIDs[ldapID] = struct{}{} // Get group members and add to the correct Group - groupMembers := value.GetAttributeValues(dbConfig.LdapAttributeGroupMember.Value) + groupMembers := value.GetAttributeValues(dbConfig.LdapAttributeGroupMember.String()) memberUsernames := make([]string, 0, len(groupMembers)) for _, member := range groupMembers { - username := s.resolveGroupMemberUsername(ctx, client, member, usernamesByDN) + username := s.resolveGroupMemberUsername(ctx, client, member, usernamesByDN, dbConfig.LdapAttributeUserUsername.String()) if username == "" { continue } @@ -267,8 +262,8 @@ func (s *LdapService) fetchGroupsFromLDAP(ctx context.Context, client ldapClient } syncGroup := dto.UserGroupCreateDto{ - Name: value.GetAttributeValue(dbConfig.LdapAttributeGroupName.Value), - FriendlyName: value.GetAttributeValue(dbConfig.LdapAttributeGroupName.Value), + Name: value.GetAttributeValue(dbConfig.LdapAttributeGroupName.String()), + FriendlyName: value.GetAttributeValue(dbConfig.LdapAttributeGroupName.String()), LdapID: ldapID, } dto.Normalize(&syncGroup) @@ -289,28 +284,26 @@ func (s *LdapService) fetchGroupsFromLDAP(ctx context.Context, client ldapClient return desiredGroups, ldapGroupIDs, nil } -func (s *LdapService) fetchUsersFromLDAP(ctx context.Context, client ldapClient) (desiredUsers []ldapDesiredUser, ldapUserIDs map[string]struct{}, usernamesByDN map[string]string, err error) { - dbConfig := s.appConfigService.GetDbConfig() - +func (s *LdapService) fetchUsersFromLDAP(ctx context.Context, client ldapClient, dbConfig *appconfig.AppConfigModel) (desiredUsers []ldapDesiredUser, ldapUserIDs map[string]struct{}, usernamesByDN map[string]string, err error) { // Query LDAP for all users we want to manage searchAttrs := []string{ "sn", "cn", - dbConfig.LdapAttributeUserUniqueIdentifier.Value, - dbConfig.LdapAttributeUserUsername.Value, - dbConfig.LdapAttributeUserEmail.Value, - dbConfig.LdapAttributeUserFirstName.Value, - dbConfig.LdapAttributeUserLastName.Value, - dbConfig.LdapAttributeUserProfilePicture.Value, - dbConfig.LdapAttributeUserDisplayName.Value, + dbConfig.LdapAttributeUserUniqueIdentifier.String(), + dbConfig.LdapAttributeUserUsername.String(), + dbConfig.LdapAttributeUserEmail.String(), + dbConfig.LdapAttributeUserFirstName.String(), + dbConfig.LdapAttributeUserLastName.String(), + dbConfig.LdapAttributeUserProfilePicture.String(), + dbConfig.LdapAttributeUserDisplayName.String(), } // Filters must start and finish with ()! searchReq := ldap.NewSearchRequest( - dbConfig.LdapBase.Value, + dbConfig.LdapBase.String(), ldap.ScopeWholeSubtree, 0, 0, 0, false, - dbConfig.LdapUserSearchFilter.Value, + dbConfig.LdapUserSearchFilter.String(), searchAttrs, []ldap.Control{}, ) @@ -326,28 +319,28 @@ func (s *LdapService) fetchUsersFromLDAP(ctx context.Context, client ldapClient) desiredUsers = make([]ldapDesiredUser, 0, len(result.Entries)) for _, value := range result.Entries { - username := norm.NFC.String(value.GetAttributeValue(dbConfig.LdapAttributeUserUsername.Value)) + username := norm.NFC.String(value.GetAttributeValue(dbConfig.LdapAttributeUserUsername.String())) if normalizedDN := normalizeLDAPDN(value.DN); normalizedDN != "" && username != "" { usernamesByDN[normalizedDN] = username } - ldapID := convertLdapIdToString(value.GetAttributeValue(dbConfig.LdapAttributeUserUniqueIdentifier.Value)) + ldapID := convertLdapIdToString(value.GetAttributeValue(dbConfig.LdapAttributeUserUniqueIdentifier.String())) // Skip users without a valid LDAP ID if ldapID == "" { - slog.Warn("Skipping LDAP user without a valid unique identifier", slog.String("attribute", dbConfig.LdapAttributeUserUniqueIdentifier.Value)) + slog.Warn("Skipping LDAP user without a valid unique identifier", slog.String("attribute", dbConfig.LdapAttributeUserUniqueIdentifier.String())) continue } ldapUserIDs[ldapID] = struct{}{} newUser := dto.UserCreateDto{ - Username: value.GetAttributeValue(dbConfig.LdapAttributeUserUsername.Value), - Email: utils.PtrOrNil(value.GetAttributeValue(dbConfig.LdapAttributeUserEmail.Value)), + Username: value.GetAttributeValue(dbConfig.LdapAttributeUserUsername.String()), + Email: utils.PtrOrNil(value.GetAttributeValue(dbConfig.LdapAttributeUserEmail.String())), EmailVerified: true, - FirstName: value.GetAttributeValue(dbConfig.LdapAttributeUserFirstName.Value), - LastName: value.GetAttributeValue(dbConfig.LdapAttributeUserLastName.Value), - DisplayName: value.GetAttributeValue(dbConfig.LdapAttributeUserDisplayName.Value), + FirstName: value.GetAttributeValue(dbConfig.LdapAttributeUserFirstName.String()), + LastName: value.GetAttributeValue(dbConfig.LdapAttributeUserLastName.String()), + DisplayName: value.GetAttributeValue(dbConfig.LdapAttributeUserDisplayName.String()), // Admin status is computed after groups are loaded so it can use the // configured group member attribute instead of a hard-coded memberOf. IsAdmin: false, @@ -369,16 +362,14 @@ func (s *LdapService) fetchUsersFromLDAP(ctx context.Context, client ldapClient) desiredUsers = append(desiredUsers, ldapDesiredUser{ ldapID: ldapID, input: newUser, - picture: value.GetAttributeValue(dbConfig.LdapAttributeUserProfilePicture.Value), + picture: value.GetAttributeValue(dbConfig.LdapAttributeUserProfilePicture.String()), }) } return desiredUsers, ldapUserIDs, usernamesByDN, nil } -func (s *LdapService) resolveGroupMemberUsername(ctx context.Context, client ldapClient, member string, usernamesByDN map[string]string) string { - dbConfig := s.appConfigService.GetDbConfig() - +func (s *LdapService) resolveGroupMemberUsername(ctx context.Context, client ldapClient, member string, usernamesByDN map[string]string, usernameAttr string) string { // First try the DN cache we built while loading users username, exists := usernamesByDN[normalizeLDAPDN(member)] if exists && username != "" { @@ -386,14 +377,15 @@ func (s *LdapService) resolveGroupMemberUsername(ctx context.Context, client lda } // Then try to extract the username directly from the DN - username = getDNProperty(dbConfig.LdapAttributeUserUsername.Value, member) + username = getDNProperty(usernameAttr, member) if username != "" { return norm.NFC.String(username) } // posixGroup (and similar) stores bare usernames in memberUid, not DNs. Treat any value // that is not a valid DN as the username directly — see https://github.com/pocket-id/pocket-id/issues/1408 - if _, err := ldap.ParseDN(member); err != nil { + _, err := ldap.ParseDN(member) + if err != nil { return norm.NFC.String(member) } @@ -403,7 +395,7 @@ func (s *LdapService) resolveGroupMemberUsername(ctx context.Context, client lda ldap.ScopeBaseObject, 0, 0, 0, false, "(objectClass=*)", - []string{dbConfig.LdapAttributeUserUsername.Value}, + []string{usernameAttr}, []ldap.Control{}, ) @@ -413,7 +405,7 @@ func (s *LdapService) resolveGroupMemberUsername(ctx context.Context, client lda return "" } - username = userResult.Entries[0].GetAttributeValue(dbConfig.LdapAttributeUserUsername.Value) + username = userResult.Entries[0].GetAttributeValue(usernameAttr) if username == "" { slog.WarnContext(ctx, "Could not extract username from group member DN", slog.String("member", member)) return "" @@ -422,7 +414,7 @@ func (s *LdapService) resolveGroupMemberUsername(ctx context.Context, client lda return norm.NFC.String(username) } -func (s *LdapService) reconcileGroups(ctx context.Context, tx *gorm.DB, desiredGroups []ldapDesiredGroup, ldapGroupIDs map[string]struct{}) error { +func (s *LdapService) reconcileGroups(ctx context.Context, tx *gorm.DB, desiredGroups []ldapDesiredGroup, ldapGroupIDs map[string]struct{}, dbConfig *appconfig.AppConfigModel) error { // Load the current LDAP-managed state from the database ldapGroupsInDB, ldapGroupsByID, err := s.loadLDAPGroupsInDB(ctx, tx) if err != nil { @@ -462,7 +454,7 @@ func (s *LdapService) reconcileGroups(ctx context.Context, tx *gorm.DB, desiredG continue } - _, err = s.groupService.updateInternal(ctx, databaseGroup.ID, desiredGroup.input, true, tx) + _, err = s.groupService.updateInternal(ctx, databaseGroup.ID, desiredGroup.input, true, tx, dbConfig) if err != nil { return fmt.Errorf("failed to update group '%s': %w", desiredGroup.input.Name, err) } @@ -498,9 +490,7 @@ func (s *LdapService) reconcileGroups(ctx context.Context, tx *gorm.DB, desiredG } //nolint:gocognit -func (s *LdapService) reconcileUsers(ctx context.Context, tx *gorm.DB, desiredUsers []ldapDesiredUser, ldapUserIDs map[string]struct{}) (savePictures []savePicture, deleteFiles []string, err error) { - dbConfig := s.appConfigService.GetDbConfig() - +func (s *LdapService) reconcileUsers(ctx context.Context, tx *gorm.DB, desiredUsers []ldapDesiredUser, ldapUserIDs map[string]struct{}, dbConfig *appconfig.AppConfigModel) (savePictures []savePicture, deleteFiles []string, err error) { // Load the current LDAP-managed state from the database ldapUsersInDB, ldapUsersByID, _, err := s.loadLDAPUsersInDB(ctx, tx) if err != nil { @@ -531,7 +521,7 @@ func (s *LdapService) reconcileUsers(ctx context.Context, tx *gorm.DB, desiredUs userID := databaseUser.ID if databaseUser.ID == "" { - createdUser, err := s.userService.CreateUserInternal(ctx, desiredUser.input, true, tx) + createdUser, err := s.userService.createUserInternal(ctx, desiredUser.input, true, tx, dbConfig) if errors.Is(err, &common.AlreadyInUseError{}) { slog.Warn("Skipping creating LDAP user", slog.String("username", desiredUser.input.Username), slog.Any("error", err)) continue @@ -542,7 +532,7 @@ func (s *LdapService) reconcileUsers(ctx context.Context, tx *gorm.DB, desiredUs userID = createdUser.ID ldapUsersByID[desiredUser.ldapID] = createdUser } else { - _, err = s.userService.updateUserInternal(ctx, databaseUser.ID, desiredUser.input, false, true, tx) + _, err = s.userService.updateUserInternal(ctx, databaseUser.ID, desiredUser.input, false, true, tx, dbConfig) if errors.Is(err, &common.AlreadyInUseError{}) { slog.Warn("Skipping updating LDAP user", slog.String("username", desiredUser.input.Username), slog.Any("error", err)) continue @@ -581,7 +571,7 @@ func (s *LdapService) reconcileUsers(ctx context.Context, tx *gorm.DB, desiredUs continue } - err = s.userService.deleteUserInternal(ctx, tx, user.ID, true) + err = s.userService.deleteUserInternal(ctx, tx, user.ID, true, dbConfig) if err != nil { if _, ok := errors.AsType[*common.LdapUserUpdateError](err); ok { return nil, nil, fmt.Errorf("failed to delete user %s: LDAP user must be disabled before deletion", user.Username) diff --git a/backend/internal/service/ldap_service_test.go b/backend/internal/service/ldap_service_test.go index d340cc74..6f935fc9 100644 --- a/backend/internal/service/ldap_service_test.go +++ b/backend/internal/service/ldap_service_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" "gorm.io/gorm" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/model" "github.com/pocket-id/pocket-id/backend/internal/storage" testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing" @@ -109,7 +110,8 @@ func TestLdapServiceSyncAllReconcilesUsersAndGroups(t *testing.T) { LdapID: &oldGroupLdapID, }).Error) - require.NoError(t, service.SyncAll(t.Context())) + err := service.SyncAll(t.Context(), defaultTestLDAPAppConfig()) + require.NoError(t, err) var alice model.User require.NoError(t, db.First(&alice, "ldap_id = ?", aliceLdapID).Error) @@ -144,8 +146,8 @@ func TestLdapServiceSyncAllReconcilesUsersAndGroups(t *testing.T) { // Regression: posixGroup uses memberUid (bare uid values), not member DNs — issue #1408. func TestLdapServiceSyncAllMapsPosixGroupMemberUid(t *testing.T) { appCfg := defaultTestLDAPAppConfig() - appCfg.LdapUserGroupSearchFilter = model.AppConfigVariable{Value: "(objectClass=posixGroup)"} - appCfg.LdapAttributeGroupMember = model.AppConfigVariable{Value: "memberUid"} + appCfg.LdapUserGroupSearchFilter = "(objectClass=posixGroup)" + appCfg.LdapAttributeGroupMember = "memberUid" service, db := newTestLdapServiceWithAppConfig(t, appCfg, newFakeLDAPClient( ldapSearchResult( @@ -175,7 +177,8 @@ func TestLdapServiceSyncAllMapsPosixGroupMemberUid(t *testing.T) { ), )) - require.NoError(t, service.SyncAll(t.Context())) + err := service.SyncAll(t.Context(), appCfg) + require.NoError(t, err) var group model.UserGroup require.NoError(t, db.Preload("Users").First(&group, "ldap_id = ?", "g-users").Error) @@ -217,7 +220,8 @@ func TestLdapServiceSyncAllHandlesDuplicateLDAPIDsInSingleRun(t *testing.T) { ), )) - require.NoError(t, service.SyncAll(t.Context())) + err := service.SyncAll(t.Context(), defaultTestLDAPAppConfig()) + require.NoError(t, err) var users []model.User require.NoError(t, db.Find(&users, "ldap_id = ?", "u-dup").Error) @@ -237,7 +241,7 @@ func TestLdapServiceSyncAllHandlesDuplicateLDAPIDsInSingleRun(t *testing.T) { func TestLdapServiceSyncAllSetsAdminFromGroupMembership(t *testing.T) { tests := []struct { name string - appConfig *model.AppConfig + appConfig *appconfig.AppConfigModel groupEntry *ldap.Entry groupName string groupLookup string @@ -255,10 +259,10 @@ func TestLdapServiceSyncAllSetsAdminFromGroupMembership(t *testing.T) { }, { name: "configured group name attribute differs from DN RDN", - appConfig: func() *model.AppConfig { + appConfig: func() *appconfig.AppConfigModel { cfg := defaultTestLDAPAppConfig() - cfg.LdapAttributeGroupName = model.AppConfigVariable{Value: "displayName"} - cfg.LdapAdminGroupName = model.AppConfigVariable{Value: "pocketid.admin"} + cfg.LdapAttributeGroupName = "displayName" + cfg.LdapAdminGroupName = "pocketid.admin" return cfg }(), groupEntry: ldapEntry("cn=admins,ou=groups,dc=example,dc=com", map[string][]string{ @@ -288,7 +292,8 @@ func TestLdapServiceSyncAllSetsAdminFromGroupMembership(t *testing.T) { ldapSearchResult(tt.groupEntry), )) - require.NoError(t, service.SyncAll(t.Context())) + err := service.SyncAll(t.Context(), tt.appConfig) + require.NoError(t, err) var user model.User require.NoError(t, db.First(&user, "ldap_id = ?", "u-testadmin").Error) @@ -308,7 +313,7 @@ func newTestLdapService(t *testing.T, client ldapClient) (*LdapService, *gorm.DB return newTestLdapServiceWithAppConfig(t, defaultTestLDAPAppConfig(), client) } -func newTestLdapServiceWithAppConfig(t *testing.T, appConfigModel *model.AppConfig, client ldapClient) (*LdapService, *gorm.DB) { +func newTestLdapServiceWithAppConfig(t *testing.T, appConfigModel *appconfig.AppConfigModel, client ldapClient) (*LdapService, *gorm.DB) { t.Helper() db := testutils.NewDatabaseForTest(t) @@ -316,48 +321,45 @@ func newTestLdapServiceWithAppConfig(t *testing.T, appConfigModel *model.AppConf fileStorage, err := storage.NewDatabaseStorage(db) require.NoError(t, err) - appConfig := NewTestAppConfigService(appConfigModel) - - groupService := NewUserGroupService(db, appConfig, nil) + groupService := NewUserGroupService(db, nil) userService := NewUserService( db, nil, nil, nil, - appConfig, NewCustomClaimService(db), NewAppImagesService(map[string]string{}, fileStorage), nil, fileStorage, ) - service := NewLdapService(db, &http.Client{}, appConfig, userService, groupService, fileStorage) - service.clientFactory = func() (ldapClient, error) { + service := NewLdapService(db, &http.Client{}, userService, groupService, fileStorage) + service.clientFactory = func(dbConfig *appconfig.AppConfigModel) (ldapClient, error) { return client, nil } return service, db } -func defaultTestLDAPAppConfig() *model.AppConfig { - return &model.AppConfig{ - RequireUserEmail: model.AppConfigVariable{Value: "false"}, - LdapEnabled: model.AppConfigVariable{Value: "true"}, - LdapBase: model.AppConfigVariable{Value: "dc=example,dc=com"}, - LdapUserSearchFilter: model.AppConfigVariable{Value: "(objectClass=person)"}, - LdapUserGroupSearchFilter: model.AppConfigVariable{Value: "(objectClass=groupOfNames)"}, - LdapAttributeUserUniqueIdentifier: model.AppConfigVariable{Value: "entryUUID"}, - LdapAttributeUserUsername: model.AppConfigVariable{Value: "uid"}, - LdapAttributeUserEmail: model.AppConfigVariable{Value: "mail"}, - LdapAttributeUserFirstName: model.AppConfigVariable{Value: "givenName"}, - LdapAttributeUserLastName: model.AppConfigVariable{Value: "sn"}, - LdapAttributeUserDisplayName: model.AppConfigVariable{Value: "displayName"}, - LdapAttributeUserProfilePicture: model.AppConfigVariable{Value: "jpegPhoto"}, - LdapAttributeGroupMember: model.AppConfigVariable{Value: "member"}, - LdapAttributeGroupUniqueIdentifier: model.AppConfigVariable{Value: "entryUUID"}, - LdapAttributeGroupName: model.AppConfigVariable{Value: "cn"}, - LdapAdminGroupName: model.AppConfigVariable{Value: "admins"}, - LdapSoftDeleteUsers: model.AppConfigVariable{Value: "true"}, +func defaultTestLDAPAppConfig() *appconfig.AppConfigModel { + return &appconfig.AppConfigModel{ + RequireUserEmail: "false", + LdapEnabled: "true", + LdapBase: "dc=example,dc=com", + LdapUserSearchFilter: "(objectClass=person)", + LdapUserGroupSearchFilter: "(objectClass=groupOfNames)", + LdapAttributeUserUniqueIdentifier: "entryUUID", + LdapAttributeUserUsername: "uid", + LdapAttributeUserEmail: "mail", + LdapAttributeUserFirstName: "givenName", + LdapAttributeUserLastName: "sn", + LdapAttributeUserDisplayName: "displayName", + LdapAttributeUserProfilePicture: "jpegPhoto", + LdapAttributeGroupMember: "member", + LdapAttributeGroupUniqueIdentifier: "entryUUID", + LdapAttributeGroupName: "cn", + LdapAdminGroupName: "admins", + LdapSoftDeleteUsers: "true", } } diff --git a/backend/internal/service/oidc_service.go b/backend/internal/service/oidc_service.go index d667e107..5f259a48 100644 --- a/backend/internal/service/oidc_service.go +++ b/backend/internal/service/oidc_service.go @@ -37,11 +37,10 @@ const ( ) type OidcService struct { - db *gorm.DB - jwtService *JwtService - appConfigService *AppConfigService - previewBuilder oidcClientPreviewBuilder - scimService *ScimService + db *gorm.DB + jwtService *JwtService + previewBuilder oidcClientPreviewBuilder + scimService *ScimService httpClient *http.Client fileStorage storage.FileStorage @@ -54,20 +53,18 @@ type oidcClientPreviewBuilder interface { func NewOidcService( db *gorm.DB, jwtService *JwtService, - appConfigService *AppConfigService, previewBuilder oidcClientPreviewBuilder, scimService *ScimService, httpClient *http.Client, fileStorage storage.FileStorage, ) (s *OidcService, err error) { s = &OidcService{ - db: db, - jwtService: jwtService, - appConfigService: appConfigService, - previewBuilder: previewBuilder, - scimService: scimService, - httpClient: httpClient, - fileStorage: fileStorage, + db: db, + jwtService: jwtService, + previewBuilder: previewBuilder, + scimService: scimService, + httpClient: httpClient, + fileStorage: fileStorage, } return s, nil diff --git a/backend/internal/service/oidc_service_test.go b/backend/internal/service/oidc_service_test.go index 5c9b4435..2d769e71 100644 --- a/backend/internal/service/oidc_service_test.go +++ b/backend/internal/service/oidc_service_test.go @@ -454,7 +454,7 @@ func TestOidcService_downloadAndSaveLogoFromURL(t *testing.T) { func TestOidcService_CreateClient_withDescription(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil) require.NoError(t, err) description := "A test client description" @@ -479,7 +479,7 @@ func TestOidcService_CreateClient_withDescription(t *testing.T) { func TestOidcService_CreateClient_withoutDescription(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil) require.NoError(t, err) input := dto.OidcClientCreateDto{ @@ -501,7 +501,7 @@ func TestOidcService_CreateClient_withoutDescription(t *testing.T) { func TestOidcService_UpdateClient_description(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil) require.NoError(t, err) // Create a client without a description diff --git a/backend/internal/service/one_time_access_service.go b/backend/internal/service/one_time_access_service.go index b22a5871..af561ee3 100644 --- a/backend/internal/service/one_time_access_service.go +++ b/backend/internal/service/one_time_access_service.go @@ -3,11 +3,13 @@ package service import ( "context" "errors" + "fmt" "log/slog" "net/url" "strings" "time" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/common" "github.com/pocket-id/pocket-id/backend/internal/model" datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" @@ -18,38 +20,34 @@ import ( ) type OneTimeAccessService struct { - db *gorm.DB - userService *UserService - appConfigService *AppConfigService - jwtService *JwtService - auditLogService *AuditLogService - emailService *EmailService + db *gorm.DB + userService *UserService + jwtService *JwtService + auditLogService *AuditLogService + emailService *EmailService } -func NewOneTimeAccessService(db *gorm.DB, userService *UserService, jwtService *JwtService, auditLogService *AuditLogService, emailService *EmailService, appConfigService *AppConfigService) *OneTimeAccessService { +func NewOneTimeAccessService(db *gorm.DB, userService *UserService, jwtService *JwtService, auditLogService *AuditLogService, emailService *EmailService) *OneTimeAccessService { return &OneTimeAccessService{ - db: db, - userService: userService, - appConfigService: appConfigService, - jwtService: jwtService, - auditLogService: auditLogService, - emailService: emailService, + db: db, + userService: userService, + jwtService: jwtService, + auditLogService: auditLogService, + emailService: emailService, } } -func (s *OneTimeAccessService) RequestOneTimeAccessEmailAsAdmin(ctx context.Context, userID string, ttl time.Duration) error { - isDisabled := !s.appConfigService.GetDbConfig().EmailOneTimeAccessAsAdminEnabled.IsTrue() - if isDisabled { +func (s *OneTimeAccessService) RequestOneTimeAccessEmailAsAdmin(ctx context.Context, dbConfig *appconfig.AppConfigModel, userID string, ttl time.Duration) error { + if !dbConfig.EmailOneTimeAccessAsAdminEnabled.IsTrue() { return &common.OneTimeAccessDisabledError{} } - _, err := s.requestOneTimeAccessEmailInternal(ctx, userID, "", ttl, false) + _, err := s.requestOneTimeAccessEmailInternal(ctx, userID, "", ttl, false, dbConfig) return err } -func (s *OneTimeAccessService) RequestOneTimeAccessEmailAsUnauthenticatedUser(ctx context.Context, userID, redirectPath string) (string, error) { - isDisabled := !s.appConfigService.GetDbConfig().EmailOneTimeAccessAsUnauthenticatedEnabled.IsTrue() - if isDisabled { +func (s *OneTimeAccessService) RequestOneTimeAccessEmailAsUnauthenticatedUser(ctx context.Context, dbConfig *appconfig.AppConfigModel, userID, redirectPath string) (string, error) { + if !dbConfig.EmailOneTimeAccessAsUnauthenticatedEnabled.IsTrue() { return "", &common.OneTimeAccessDisabledError{} } @@ -62,7 +60,7 @@ func (s *OneTimeAccessService) RequestOneTimeAccessEmailAsUnauthenticatedUser(ct return "", err } - deviceToken, err := s.requestOneTimeAccessEmailInternal(ctx, userId, redirectPath, 15*time.Minute, true) + deviceToken, err := s.requestOneTimeAccessEmailInternal(ctx, userId, redirectPath, 15*time.Minute, true, dbConfig) if err != nil { return "", err } else if deviceToken == nil { @@ -72,7 +70,7 @@ func (s *OneTimeAccessService) RequestOneTimeAccessEmailAsUnauthenticatedUser(ct return *deviceToken, nil } -func (s *OneTimeAccessService) requestOneTimeAccessEmailInternal(ctx context.Context, userID, redirectPath string, ttl time.Duration, withDeviceToken bool) (*string, error) { +func (s *OneTimeAccessService) requestOneTimeAccessEmailInternal(ctx context.Context, userID, redirectPath string, ttl time.Duration, withDeviceToken bool, dbConfig *appconfig.AppConfigModel) (*string, error) { tx := s.db.Begin() defer func() { tx.Rollback() @@ -110,7 +108,7 @@ func (s *OneTimeAccessService) requestOneTimeAccessEmailInternal(ctx context.Con linkWithCode = linkWithCode + "?redirect=" + encodedRedirectPath } - errInternal := SendEmail(innerCtx, s.emailService, email.Address{ + errInternal := SendEmail(innerCtx, s.emailService, dbConfig, email.Address{ Name: user.FullName(), Email: *user.Email, }, OneTimeAccessTemplate, &OneTimeAccessTemplateData{ @@ -151,7 +149,7 @@ func (s *OneTimeAccessService) CreateOneTimeAccessToken(ctx context.Context, use // Commit err = tx.Commit().Error if err != nil { - return "", err + return "", fmt.Errorf("error committing transaction: %w", err) } return token, nil @@ -171,7 +169,7 @@ func (s *OneTimeAccessService) createOneTimeAccessTokenInternal(ctx context.Cont return oneTimeAccessToken.Token, oneTimeAccessToken.DeviceToken, nil } -func (s *OneTimeAccessService) ExchangeOneTimeAccessToken(ctx context.Context, token, deviceToken, ipAddress, userAgent string) (model.User, string, error) { +func (s *OneTimeAccessService) ExchangeOneTimeAccessToken(ctx context.Context, dbConfig *appconfig.AppConfigModel, token, deviceToken, ipAddress, userAgent string) (model.User, string, error) { tx := s.db.Begin() defer func() { tx.Rollback() @@ -198,7 +196,11 @@ func (s *OneTimeAccessService) ExchangeOneTimeAccessToken(ctx context.Context, t return model.User{}, "", &common.UserDisabledError{} } - accessToken, err := s.jwtService.GenerateAccessToken(oneTimeAccessToken.User, AuthenticationMethodOneTimePassword) + accessToken, err := s.jwtService.GenerateAccessToken( + oneTimeAccessToken.User, + AuthenticationMethodOneTimePassword, + dbConfig.SessionDuration.AsDurationMinutes(), + ) if err != nil { return model.User{}, "", err } @@ -211,11 +213,16 @@ func (s *OneTimeAccessService) ExchangeOneTimeAccessToken(ctx context.Context, t return model.User{}, "", err } - s.auditLogService.Create(ctx, model.AuditLogEventOneTimeAccessTokenSignIn, ipAddress, userAgent, oneTimeAccessToken.User.ID, model.AuditLogData{}, tx) + s.auditLogService.Create( + ctx, model.AuditLogEventOneTimeAccessTokenSignIn, + ipAddress, userAgent, + oneTimeAccessToken.User.ID, model.AuditLogData{}, + tx, + ) err = tx.Commit().Error if err != nil { - return model.User{}, "", err + return model.User{}, "", fmt.Errorf("error committing transaction: %w", err) } return oneTimeAccessToken.User, accessToken, nil diff --git a/backend/internal/service/one_time_access_service_test.go b/backend/internal/service/one_time_access_service_test.go index 8d2964fb..ff946043 100644 --- a/backend/internal/service/one_time_access_service_test.go +++ b/backend/internal/service/one_time_access_service_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/require" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/common" "github.com/pocket-id/pocket-id/backend/internal/model" datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" @@ -14,11 +15,11 @@ import ( func TestExchangeOneTimeAccessTokenRejectsDisabledUser(t *testing.T) { db := testutils.NewDatabaseForTest(t) - appConfig := NewTestAppConfigService((&AppConfigService{}).getDefaultDbConfig()) + appConfig := appconfig.NewTestAppConfigService(nil) instanceID := newInstanceID(t, db) jwtService := initJwtService(t, db, instanceID, appConfig, newTestEnvConfig()) - auditLogService := NewAuditLogService(db, appConfig, nil, &GeoLiteService{}) - oneTimeAccessService := NewOneTimeAccessService(db, nil, jwtService, auditLogService, nil, appConfig) + auditLogService := NewAuditLogService(db, nil, &GeoLiteService{}, appConfig) + oneTimeAccessService := NewOneTimeAccessService(db, nil, jwtService, auditLogService, nil) user := model.User{ Base: model.Base{ID: "disabled-user"}, @@ -35,7 +36,8 @@ func TestExchangeOneTimeAccessTokenRejectsDisabledUser(t *testing.T) { } require.NoError(t, db.Create(&loginCode).Error) - exchangedUser, accessToken, err := oneTimeAccessService.ExchangeOneTimeAccessToken(t.Context(), loginCode.Token, "", "", "") + dbConfig := appconfig.NewTestConfig(nil) + exchangedUser, accessToken, err := oneTimeAccessService.ExchangeOneTimeAccessToken(t.Context(), dbConfig, loginCode.Token, "", "", "") var userDisabledErr *common.UserDisabledError require.ErrorAs(t, err, &userDisabledErr) diff --git a/backend/internal/service/user_group_service.go b/backend/internal/service/user_group_service.go index 0e37a5ac..fe942250 100644 --- a/backend/internal/service/user_group_service.go +++ b/backend/internal/service/user_group_service.go @@ -5,6 +5,7 @@ import ( "errors" "time" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" "gorm.io/gorm" @@ -15,13 +16,12 @@ import ( ) type UserGroupService struct { - db *gorm.DB - scimService *ScimService - appConfigService *AppConfigService + db *gorm.DB + scimService *ScimService } -func NewUserGroupService(db *gorm.DB, appConfigService *AppConfigService, scimService *ScimService) *UserGroupService { - return &UserGroupService{db: db, appConfigService: appConfigService, scimService: scimService} +func NewUserGroupService(db *gorm.DB, scimService *ScimService) *UserGroupService { + return &UserGroupService{db: db, scimService: scimService} } func (s *UserGroupService) List(ctx context.Context, name string, listRequestOptions utils.ListRequestOptions) (groups []model.UserGroup, response utils.PaginationResponse, err error) { @@ -62,7 +62,7 @@ func (s *UserGroupService) getInternal(ctx context.Context, id string, tx *gorm. return group, err } -func (s *UserGroupService) Delete(ctx context.Context, id string) error { +func (s *UserGroupService) Delete(ctx context.Context, cfg *appconfig.AppConfigModel, id string) error { tx := s.db.Begin() defer func() { tx.Rollback() @@ -79,7 +79,7 @@ func (s *UserGroupService) Delete(ctx context.Context, id string) error { } // Disallow deleting the group if it is an LDAP group and LDAP is enabled - if group.LdapID != nil && s.appConfigService.GetDbConfig().LdapEnabled.IsTrue() { + if group.LdapID != nil && cfg.LdapEnabled.IsTrue() { return &common.LdapUserGroupUpdateError{} } @@ -122,10 +122,9 @@ func (s *UserGroupService) createInternal(ctx context.Context, input dto.UserGro Preload("Users"). Create(&group). Error - if err != nil { - if errors.Is(err, gorm.ErrDuplicatedKey) { - return model.UserGroup{}, &common.AlreadyInUseError{Property: "name"} - } + if errors.Is(err, gorm.ErrDuplicatedKey) { + return model.UserGroup{}, &common.AlreadyInUseError{Property: "name"} + } else if err != nil { return model.UserGroup{}, err } @@ -136,13 +135,13 @@ func (s *UserGroupService) createInternal(ctx context.Context, input dto.UserGro return group, nil } -func (s *UserGroupService) Update(ctx context.Context, id string, input dto.UserGroupCreateDto) (group model.UserGroup, err error) { +func (s *UserGroupService) Update(ctx context.Context, cfg *appconfig.AppConfigModel, id string, input dto.UserGroupCreateDto) (group model.UserGroup, err error) { tx := s.db.Begin() defer func() { tx.Rollback() }() - group, err = s.updateInternal(ctx, id, input, false, tx) + group, err = s.updateInternal(ctx, id, input, false, tx, cfg) if err != nil { return model.UserGroup{}, err } @@ -155,15 +154,17 @@ func (s *UserGroupService) Update(ctx context.Context, id string, input dto.User return group, nil } -func (s *UserGroupService) updateInternal(ctx context.Context, id string, input dto.UserGroupCreateDto, isLdapSync bool, tx *gorm.DB) (group model.UserGroup, err error) { +func (s *UserGroupService) updateInternal(ctx context.Context, id string, input dto.UserGroupCreateDto, isLdapSync bool, tx *gorm.DB, cfg *appconfig.AppConfigModel) (group model.UserGroup, err error) { group, err = s.getInternal(ctx, id, tx) if err != nil { return model.UserGroup{}, err } // Disallow updating the group if it is an LDAP group and LDAP is enabled - if !isLdapSync && group.LdapID != nil && s.appConfigService.GetDbConfig().LdapEnabled.IsTrue() { - return model.UserGroup{}, &common.LdapUserGroupUpdateError{} + if !isLdapSync && group.LdapID != nil { + if cfg.LdapEnabled.IsTrue() { + return model.UserGroup{}, &common.LdapUserGroupUpdateError{} + } } group.Name = input.Name @@ -216,7 +217,7 @@ func (s *UserGroupService) updateUsersInternal(ctx context.Context, id string, u // Fetch the users based on the userIds var users []model.User if len(userIds) > 0 { - err := tx. + err = tx. WithContext(ctx). Where("id IN (?)", userIds). Find(&users). diff --git a/backend/internal/service/user_service.go b/backend/internal/service/user_service.go index 0f974e0d..34a1ee07 100644 --- a/backend/internal/service/user_service.go +++ b/backend/internal/service/user_service.go @@ -13,16 +13,17 @@ import ( "time" "github.com/google/uuid" - "github.com/pocket-id/pocket-id/backend/internal/utils/email" "gorm.io/gorm" "gorm.io/gorm/clause" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "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" datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" "github.com/pocket-id/pocket-id/backend/internal/storage" "github.com/pocket-id/pocket-id/backend/internal/utils" + "github.com/pocket-id/pocket-id/backend/internal/utils/email" profilepicture "github.com/pocket-id/pocket-id/backend/internal/utils/image" ) @@ -31,20 +32,18 @@ type UserService struct { jwtService *JwtService auditLogService *AuditLogService emailService *EmailService - appConfigService *AppConfigService customClaimService *CustomClaimService appImagesService *AppImagesService scimService *ScimService fileStorage storage.FileStorage } -func NewUserService(db *gorm.DB, jwtService *JwtService, auditLogService *AuditLogService, emailService *EmailService, appConfigService *AppConfigService, customClaimService *CustomClaimService, appImagesService *AppImagesService, scimService *ScimService, fileStorage storage.FileStorage) *UserService { +func NewUserService(db *gorm.DB, jwtService *JwtService, auditLogService *AuditLogService, emailService *EmailService, customClaimService *CustomClaimService, appImagesService *AppImagesService, scimService *ScimService, fileStorage storage.FileStorage) *UserService { return &UserService{ db: db, jwtService: jwtService, auditLogService: auditLogService, emailService: emailService, - appConfigService: appConfigService, customClaimService: customClaimService, appImagesService: appImagesService, scimService: scimService, @@ -185,9 +184,9 @@ func (s *UserService) UpdateProfilePicture(ctx context.Context, userID string, f return nil } -func (s *UserService) DeleteUser(ctx context.Context, userID string, allowLdapDelete bool) error { +func (s *UserService) DeleteUser(ctx context.Context, dbConfig *appconfig.AppConfigModel, userID string, allowLdapDelete bool) error { err := s.db.Transaction(func(tx *gorm.DB) error { - return s.deleteUserInternal(ctx, tx, userID, allowLdapDelete) + return s.deleteUserInternal(ctx, tx, userID, allowLdapDelete, dbConfig) }) if err != nil { return fmt.Errorf("failed to delete user '%s': %w", userID, err) @@ -203,9 +202,8 @@ func (s *UserService) DeleteUser(ctx context.Context, userID string, allowLdapDe return nil } -func (s *UserService) deleteUserInternal(ctx context.Context, tx *gorm.DB, userID string, allowLdapDelete bool) error { +func (s *UserService) deleteUserInternal(ctx context.Context, tx *gorm.DB, userID string, allowLdapDelete bool, cfg *appconfig.AppConfigModel) error { var user model.User - err := tx. WithContext(ctx). Where("id = ?", userID). @@ -217,8 +215,10 @@ func (s *UserService) deleteUserInternal(ctx context.Context, tx *gorm.DB, userI } // Disallow deleting the user if it is an LDAP user, LDAP is enabled, and the user is not disabled - if !allowLdapDelete && !user.Disabled && user.LdapID != nil && s.appConfigService.GetDbConfig().LdapEnabled.IsTrue() { - return &common.LdapUserUpdateError{} + if !allowLdapDelete && !user.Disabled && user.LdapID != nil { + if cfg.LdapEnabled.IsTrue() { + return &common.LdapUserUpdateError{} + } } err = tx.WithContext(ctx).Delete(&user).Error @@ -233,13 +233,13 @@ func (s *UserService) deleteUserInternal(ctx context.Context, tx *gorm.DB, userI return nil } -func (s *UserService) CreateUser(ctx context.Context, input dto.UserCreateDto) (model.User, error) { +func (s *UserService) CreateUser(ctx context.Context, dbConfig *appconfig.AppConfigModel, input dto.UserCreateDto) (model.User, error) { tx := s.db.Begin() defer func() { tx.Rollback() }() - user, err := s.CreateUserInternal(ctx, input, false, tx) + user, err := s.CreateUserInternal(ctx, dbConfig, input, false, tx) if err != nil { return model.User{}, err } @@ -252,8 +252,12 @@ func (s *UserService) CreateUser(ctx context.Context, input dto.UserCreateDto) ( return user, nil } -func (s *UserService) CreateUserInternal(ctx context.Context, input dto.UserCreateDto, isLdapSync bool, tx *gorm.DB) (model.User, error) { - if s.appConfigService.GetDbConfig().RequireUserEmail.IsTrue() && input.Email == nil { +func (s *UserService) CreateUserInternal(ctx context.Context, dbConfig *appconfig.AppConfigModel, input dto.UserCreateDto, isLdapSync bool, tx *gorm.DB) (model.User, error) { + return s.createUserInternal(ctx, input, isLdapSync, tx, dbConfig) +} + +func (s *UserService) createUserInternal(ctx context.Context, input dto.UserCreateDto, isLdapSync bool, tx *gorm.DB, cfg *appconfig.AppConfigModel) (model.User, error) { + if cfg.RequireUserEmail.IsTrue() && input.Email == nil { return model.User{}, &common.UserEmailNotSetError{} } @@ -313,13 +317,13 @@ func (s *UserService) CreateUserInternal(ctx context.Context, input dto.UserCrea // Apply default groups and claims for new non-LDAP users if !isLdapSync { if len(input.UserGroupIds) == 0 { - err = s.applyDefaultGroups(ctx, &user, tx) + err = s.applyDefaultGroups(ctx, &user, tx, cfg) if err != nil { return model.User{}, err } } - err = s.applyDefaultCustomClaims(ctx, &user, tx) + err = s.applyDefaultCustomClaims(ctx, &user, tx, cfg) if err != nil { return model.User{}, err } @@ -332,11 +336,9 @@ func (s *UserService) CreateUserInternal(ctx context.Context, input dto.UserCrea return user, nil } -func (s *UserService) applyDefaultGroups(ctx context.Context, user *model.User, tx *gorm.DB) error { - config := s.appConfigService.GetDbConfig() - +func (s *UserService) applyDefaultGroups(ctx context.Context, user *model.User, tx *gorm.DB, cfg *appconfig.AppConfigModel) error { var groupIDs []string - v := config.SignupDefaultUserGroupIDs.Value + v := cfg.SignupDefaultUserGroupIDs if v != "" && v != "[]" { err := json.Unmarshal([]byte(v), &groupIDs) if err != nil { @@ -396,11 +398,9 @@ func (s *UserService) touchUserGroups(ctx context.Context, tx *gorm.DB, ids []st Error } -func (s *UserService) applyDefaultCustomClaims(ctx context.Context, user *model.User, tx *gorm.DB) error { - config := s.appConfigService.GetDbConfig() - +func (s *UserService) applyDefaultCustomClaims(ctx context.Context, user *model.User, tx *gorm.DB, cfg *appconfig.AppConfigModel) error { var claims []dto.CustomClaimCreateDto - v := config.SignupDefaultCustomClaims.Value + v := cfg.SignupDefaultCustomClaims if v != "" && v != "[]" { err := json.Unmarshal([]byte(v), &claims) if err != nil { @@ -417,13 +417,13 @@ func (s *UserService) applyDefaultCustomClaims(ctx context.Context, user *model. return nil } -func (s *UserService) UpdateUser(ctx context.Context, userID string, updatedUser dto.UserCreateDto, updateOwnUser bool, isLdapSync bool) (model.User, error) { +func (s *UserService) UpdateUser(ctx context.Context, cfg *appconfig.AppConfigModel, userID string, updatedUser dto.UserCreateDto, updateOwnUser bool, isLdapSync bool) (model.User, error) { tx := s.db.Begin() defer func() { tx.Rollback() }() - user, err := s.updateUserInternal(ctx, userID, updatedUser, updateOwnUser, isLdapSync, tx) + user, err := s.updateUserInternal(ctx, userID, updatedUser, updateOwnUser, isLdapSync, tx, cfg) if err != nil { return model.User{}, err } @@ -436,8 +436,8 @@ func (s *UserService) UpdateUser(ctx context.Context, userID string, updatedUser return user, nil } -func (s *UserService) updateUserInternal(ctx context.Context, userID string, updatedUser dto.UserCreateDto, updateOwnUser bool, isLdapSync bool, tx *gorm.DB) (model.User, error) { - if s.appConfigService.GetDbConfig().RequireUserEmail.IsTrue() && updatedUser.Email == nil { +func (s *UserService) updateUserInternal(ctx context.Context, userID string, updatedUser dto.UserCreateDto, updateOwnUser bool, isLdapSync bool, tx *gorm.DB, cfg *appconfig.AppConfigModel) (model.User, error) { + if cfg.RequireUserEmail.IsTrue() && updatedUser.Email == nil { return model.User{}, &common.UserEmailNotSetError{} } @@ -453,8 +453,8 @@ func (s *UserService) updateUserInternal(ctx context.Context, userID string, upd } // Check if this is an LDAP user and LDAP is enabled - isLdapUser := user.LdapID != nil && s.appConfigService.GetDbConfig().LdapEnabled.IsTrue() - allowOwnAccountEdit := s.appConfigService.GetDbConfig().AllowOwnAccountEdit.IsTrue() + isLdapUser := user.LdapID != nil && cfg.LdapEnabled.IsTrue() + allowOwnAccountEdit := cfg.AllowOwnAccountEdit.IsTrue() if !isLdapSync && (isLdapUser || (!allowOwnAccountEdit && updateOwnUser)) { // Restricted update: Only locale can be changed when: @@ -472,7 +472,7 @@ func (s *UserService) updateUserInternal(ctx context.Context, userID string, upd if (user.Email == nil && updatedUser.Email != nil) || (user.Email != nil && updatedUser.Email != nil && *user.Email != *updatedUser.Email) { // Email has changed, reset email verification status - user.EmailVerified = s.appConfigService.GetDbConfig().EmailsVerified.IsTrue() + user.EmailVerified = cfg.EmailsVerified.IsTrue() } user.Email = updatedUser.Email @@ -639,7 +639,7 @@ func (s *UserService) disableUserInternal(ctx context.Context, tx *gorm.DB, user return nil } -func (s *UserService) SendEmailVerification(ctx context.Context, userID string) error { +func (s *UserService) SendEmailVerification(ctx context.Context, dbConfig *appconfig.AppConfigModel, userID string) error { user, err := s.GetUser(ctx, userID) if err != nil { return err @@ -666,7 +666,7 @@ func (s *UserService) SendEmailVerification(ctx context.Context, userID string) return err } - return SendEmail(ctx, s.emailService, email.Address{ + return SendEmail(ctx, s.emailService, dbConfig, email.Address{ Name: user.FullName(), Email: *user.Email, }, EmailVerificationTemplate, &EmailVerificationTemplateData{ diff --git a/backend/internal/service/user_service_test.go b/backend/internal/service/user_service_test.go index 3799b36f..3b885d69 100644 --- a/backend/internal/service/user_service_test.go +++ b/backend/internal/service/user_service_test.go @@ -6,13 +6,13 @@ import ( "github.com/stretchr/testify/require" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "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/storage" testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing" ) -func newTestUserService(t *testing.T, appConfig *AppConfigService) (*UserService, *UserGroupService) { +func newTestUserService(t *testing.T) (*UserService, *UserGroupService) { t.Helper() db := testutils.NewDatabaseForTest(t) @@ -25,22 +25,19 @@ func newTestUserService(t *testing.T, appConfig *AppConfigService) (*UserService nil, nil, nil, - appConfig, NewCustomClaimService(db), NewAppImagesService(map[string]string{}, fileStorage), nil, fileStorage, ) - groupService := NewUserGroupService(db, appConfig, nil) + groupService := NewUserGroupService(db, nil) return userService, groupService } func TestCreateUserBumpsGroupUpdatedAt(t *testing.T) { - appConfig := NewTestAppConfigService(&model.AppConfig{ - RequireUserEmail: model.AppConfigVariable{Value: "false"}, - }) - userService, groupService := newTestUserService(t, appConfig) + config := &appconfig.AppConfigModel{RequireUserEmail: "false"} + userService, groupService := newTestUserService(t) group, err := groupService.Create(t.Context(), dto.UserGroupCreateDto{ Name: "members", @@ -52,7 +49,7 @@ func TestCreateUserBumpsGroupUpdatedAt(t *testing.T) { // Create a user that is a member of the group // This mirrors signing up via an invite link that adds the user to a group email := "member@example.com" - _, err = userService.CreateUser(t.Context(), dto.UserCreateDto{ + _, err = userService.CreateUser(t.Context(), config, dto.UserCreateDto{ Username: "member", Email: &email, FirstName: "Group", @@ -70,10 +67,8 @@ func TestCreateUserBumpsGroupUpdatedAt(t *testing.T) { } func TestCreateUserBumpsDefaultGroupUpdatedAt(t *testing.T) { - appConfig := NewTestAppConfigService(&model.AppConfig{ - RequireUserEmail: model.AppConfigVariable{Value: "false"}, - }) - userService, groupService := newTestUserService(t, appConfig) + config := &appconfig.AppConfigModel{RequireUserEmail: "false"} + userService, groupService := newTestUserService(t) group, err := groupService.Create(t.Context(), dto.UserGroupCreateDto{ Name: "default", @@ -85,11 +80,11 @@ func TestCreateUserBumpsDefaultGroupUpdatedAt(t *testing.T) { // Configure the group as a default signup group defaultGroups, err := json.Marshal([]string{group.ID}) require.NoError(t, err) - appConfig.dbConfig.Load().SignupDefaultUserGroupIDs.Value = string(defaultGroups) + config.SignupDefaultUserGroupIDs = appconfig.AppConfigValue(defaultGroups) // Create a user without explicit group IDs, so the default groups apply email := "default@example.com" - _, err = userService.CreateUser(t.Context(), dto.UserCreateDto{ + _, err = userService.CreateUser(t.Context(), config, dto.UserCreateDto{ Username: "defaultmember", Email: &email, FirstName: "Default", diff --git a/backend/internal/usersignup/handler.go b/backend/internal/usersignup/handler.go index 5abdbcd7..c8d96ef2 100644 --- a/backend/internal/usersignup/handler.go +++ b/backend/internal/usersignup/handler.go @@ -1,6 +1,7 @@ package usersignup import ( + "fmt" "net/http" "time" @@ -16,10 +17,10 @@ const defaultSignupTokenDuration = time.Hour type handler struct { service *Service - appConfig AppConfigProvider + appConfig AppConfigResolver } -func newHandler(service *Service, appConfig AppConfigProvider) *handler { +func newHandler(service *Service, appConfig AppConfigResolver) *handler { return &handler{service: service, appConfig: appConfig} } @@ -48,13 +49,19 @@ func (h *handler) checkInitialAdminSetupAvailable(c *gin.Context) { // @Success 200 {object} dto.UserDto // @Router /api/signup/setup [post] func (h *handler) signUpInitialAdmin(c *gin.Context) { + config, err := h.appConfig.GetConfig(c.Request.Context()) + if err != nil { + _ = c.Error(fmt.Errorf("error loading app configuration: %w", err)) + return + } + var input signUpDto if err := dto.ShouldBindWithNormalizedJSON(c, &input); err != nil { _ = c.Error(err) return } - user, token, err := h.service.SignUpInitialAdmin(c.Request.Context(), input) + user, token, err := h.service.SignUpInitialAdmin(c.Request.Context(), config, input) if err != nil { _ = c.Error(err) return @@ -66,7 +73,7 @@ func (h *handler) signUpInitialAdmin(c *gin.Context) { return } - maxAge := int(h.appConfig.GetDbConfig().SessionDuration.AsDurationMinutes().Seconds()) + maxAge := int(config.SessionDuration.AsDurationMinutes().Seconds()) cookie.AddAccessTokenCookie(c, maxAge, token) c.JSON(http.StatusOK, userDto) @@ -169,6 +176,12 @@ func (h *handler) deleteSignupToken(c *gin.Context) { // @Success 201 {object} dto.UserDto // @Router /api/signup [post] func (h *handler) signup(c *gin.Context) { + config, err := h.appConfig.GetConfig(c.Request.Context()) + if err != nil { + _ = c.Error(fmt.Errorf("error loading app configuration: %w", err)) + return + } + var input signUpDto if err := dto.ShouldBindWithNormalizedJSON(c, &input); err != nil { _ = c.Error(err) @@ -178,13 +191,13 @@ func (h *handler) signup(c *gin.Context) { ipAddress := c.ClientIP() userAgent := c.GetHeader("User-Agent") - user, accessToken, err := h.service.SignUp(c.Request.Context(), input, ipAddress, userAgent) + user, accessToken, err := h.service.SignUp(c.Request.Context(), config, input, ipAddress, userAgent) if err != nil { _ = c.Error(err) return } - maxAge := int(h.appConfig.GetDbConfig().SessionDuration.AsDurationMinutes().Seconds()) + maxAge := int(config.SessionDuration.AsDurationMinutes().Seconds()) cookie.AddAccessTokenCookie(c, maxAge, accessToken) var userDto dto.UserDto diff --git a/backend/internal/usersignup/module.go b/backend/internal/usersignup/module.go index 54f4b0b8..8a7acacf 100644 --- a/backend/internal/usersignup/module.go +++ b/backend/internal/usersignup/module.go @@ -2,28 +2,31 @@ package usersignup import ( "context" + "time" "github.com/gin-gonic/gin" "gorm.io/gorm" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/dto" "github.com/pocket-id/pocket-id/backend/internal/model" ) type TokenService interface { - GenerateAccessToken(user model.User, authenticationMethod string) (string, error) + GenerateAccessToken(user model.User, authenticationMethod string, sessionDuration time.Duration) (string, error) } type AuditLogger interface { Create(ctx context.Context, event model.AuditLogEvent, ipAddress, userAgent, userID string, data model.AuditLogData, tx *gorm.DB) (model.AuditLog, bool) } -type AppConfigProvider interface { - GetDbConfig() *model.AppConfig +type UserCreator interface { + CreateUserInternal(ctx context.Context, dbConfig *appconfig.AppConfigModel, input dto.UserCreateDto, isLdapSync bool, tx *gorm.DB) (model.User, error) } -type UserCreator interface { - CreateUserInternal(ctx context.Context, input dto.UserCreateDto, isLdapSync bool, tx *gorm.DB) (model.User, error) +// AppConfigResolver loads the current application configuration, so handlers can pass it explicitly to the service methods that need it +type AppConfigResolver interface { + GetConfig(ctx context.Context) (*appconfig.AppConfigModel, error) } type Dependencies struct { @@ -31,8 +34,8 @@ type Dependencies struct { Signer TokenService AuditLog AuditLogger - AppConfig AppConfigProvider UserCreator UserCreator + AppConfig AppConfigResolver } type Module struct { diff --git a/backend/internal/usersignup/service.go b/backend/internal/usersignup/service.go index 0ec3a76d..26b23157 100644 --- a/backend/internal/usersignup/service.go +++ b/backend/internal/usersignup/service.go @@ -9,6 +9,7 @@ import ( "gorm.io/gorm" "gorm.io/gorm/clause" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "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" @@ -25,7 +26,6 @@ type Service struct { userCreator UserCreator signer TokenService auditLog AuditLogger - appConfig AppConfigProvider } func newService(deps Dependencies) *Service { @@ -34,11 +34,10 @@ func newService(deps Dependencies) *Service { userCreator: deps.UserCreator, signer: deps.Signer, auditLog: deps.AuditLog, - appConfig: deps.AppConfig, } } -func (s *Service) SignUp(ctx context.Context, signupData signUpDto, ipAddress, userAgent string) (model.User, string, error) { +func (s *Service) SignUp(ctx context.Context, config *appconfig.AppConfigModel, signupData signUpDto, ipAddress, userAgent string) (model.User, string, error) { tx := s.db.Begin() defer func() { tx.Rollback() @@ -46,8 +45,7 @@ func (s *Service) SignUp(ctx context.Context, signupData signUpDto, ipAddress, u tokenProvided := signupData.Token != "" - config := s.appConfig.GetDbConfig() - if config.AllowUserSignups.Value != "open" && !tokenProvided { + if config.AllowUserSignups.String() != "open" && !tokenProvided { return model.User{}, "", &common.OpenSignupDisabledError{} } @@ -84,15 +82,15 @@ func (s *Service) SignUp(ctx context.Context, signupData signUpDto, ipAddress, u LastName: signupData.LastName, DisplayName: strings.TrimSpace(signupData.FirstName + " " + signupData.LastName), UserGroupIds: userGroupIDs, - EmailVerified: s.appConfig.GetDbConfig().EmailsVerified.IsTrue(), + EmailVerified: config.EmailsVerified.IsTrue(), } - user, err := s.userCreator.CreateUserInternal(ctx, userToCreate, false, tx) + user, err := s.userCreator.CreateUserInternal(ctx, config, userToCreate, false, tx) if err != nil { return model.User{}, "", err } - accessToken, err := s.signer.GenerateAccessToken(user, "") + accessToken, err := s.signer.GenerateAccessToken(user, "", config.SessionDuration.AsDurationMinutes()) if err != nil { return model.User{}, "", err } @@ -122,7 +120,7 @@ func (s *Service) SignUp(ctx context.Context, signupData signUpDto, ipAddress, u return user, accessToken, nil } -func (s *Service) SignUpInitialAdmin(ctx context.Context, signUpData signUpDto) (model.User, string, error) { +func (s *Service) SignUpInitialAdmin(ctx context.Context, config *appconfig.AppConfigModel, signUpData signUpDto) (model.User, string, error) { tx := s.db.Begin() defer func() { tx.Rollback() @@ -145,12 +143,12 @@ func (s *Service) SignUpInitialAdmin(ctx context.Context, signUpData signUpDto) IsAdmin: true, } - user, err := s.userCreator.CreateUserInternal(ctx, userToCreate, false, tx) + user, err := s.userCreator.CreateUserInternal(ctx, config, userToCreate, false, tx) if err != nil { return model.User{}, "", err } - token, err := s.signer.GenerateAccessToken(user, authenticationMethodOneTimePassword) + token, err := s.signer.GenerateAccessToken(user, authenticationMethodOneTimePassword, config.SessionDuration.AsDurationMinutes()) if err != nil { return model.User{}, "", err } diff --git a/backend/internal/webauthn/handler.go b/backend/internal/webauthn/handler.go index 1cfeb447..77d9e9a9 100644 --- a/backend/internal/webauthn/handler.go +++ b/backend/internal/webauthn/handler.go @@ -1,6 +1,7 @@ package webauthn import ( + "fmt" "net/http" "github.com/gin-gonic/gin" @@ -13,16 +14,22 @@ import ( type handler struct { service *Service - appConfig AppConfigProvider + appConfig AppConfigResolver } -func newHandler(service *Service, appConfig AppConfigProvider) *handler { +func newHandler(service *Service, appConfig AppConfigResolver) *handler { return &handler{service: service, appConfig: appConfig} } func (h *handler) beginRegistration(c *gin.Context) { + dbConfig, err := h.appConfig.GetConfig(c.Request.Context()) + if err != nil { + _ = c.Error(fmt.Errorf("error loading app configuration: %w", err)) + return + } + userID := c.GetString("userID") - options, err := h.service.BeginRegistration(c.Request.Context(), userID) + options, err := h.service.BeginRegistration(c.Request.Context(), dbConfig, userID) if err != nil { _ = c.Error(err) return @@ -67,6 +74,12 @@ func (h *handler) beginLogin(c *gin.Context) { } func (h *handler) verifyLogin(c *gin.Context) { + dbConfig, err := h.appConfig.GetConfig(c.Request.Context()) + if err != nil { + _ = c.Error(fmt.Errorf("error loading app configuration: %w", err)) + return + } + sessionID, err := c.Cookie(cookie.SessionIdCookieName) if err != nil { _ = c.Error(&common.MissingSessionIdError{}) @@ -79,7 +92,7 @@ func (h *handler) verifyLogin(c *gin.Context) { return } - user, token, err := h.service.VerifyLogin(c.Request.Context(), sessionID, credentialAssertionData, c.ClientIP(), c.Request.UserAgent()) + user, token, err := h.service.VerifyLogin(c.Request.Context(), dbConfig, sessionID, credentialAssertionData, c.ClientIP(), c.Request.UserAgent()) if err != nil { _ = c.Error(err) return @@ -91,7 +104,7 @@ func (h *handler) verifyLogin(c *gin.Context) { return } - maxAge := int(h.appConfig.GetDbConfig().SessionDuration.AsDurationMinutes().Seconds()) + maxAge := int(dbConfig.SessionDuration.AsDurationMinutes().Seconds()) cookie.AddAccessTokenCookie(c, maxAge, token) c.JSON(http.StatusOK, userDto) diff --git a/backend/internal/webauthn/module.go b/backend/internal/webauthn/module.go index 41d945e8..47017145 100644 --- a/backend/internal/webauthn/module.go +++ b/backend/internal/webauthn/module.go @@ -8,22 +8,24 @@ import ( "github.com/lestrrat-go/jwx/v3/jwt" "gorm.io/gorm" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/model" ) type TokenService interface { - GenerateAccessToken(user model.User, authenticationMethod string) (string, error) + GenerateAccessToken(user model.User, authenticationMethod string, sessionDuration time.Duration) (string, error) VerifyAccessToken(tokenString string) (jwt.Token, error) GetAuthenticationMethod(token jwt.Token) (string, error) } type AuditLogger interface { Create(ctx context.Context, event model.AuditLogEvent, ipAddress, userAgent, userID string, data model.AuditLogData, tx *gorm.DB) (model.AuditLog, bool) - CreateNewSignInWithEmail(ctx context.Context, ipAddress, userAgent, userID string, tx *gorm.DB) model.AuditLog + CreateNewSignInWithEmail(ctx context.Context, ipAddress, userAgent, userID string, tx *gorm.DB, emailLoginNotificationEnabled bool) model.AuditLog } -type AppConfigProvider interface { - GetDbConfig() *model.AppConfig +// AppConfigResolver loads the current application configuration, so handlers can pass it explicitly to the service methods that need it +type AppConfigResolver interface { + GetConfig(ctx context.Context) (*appconfig.AppConfigModel, error) } type Dependencies struct { @@ -32,7 +34,7 @@ type Dependencies struct { Signer TokenService AuditLog AuditLogger - AppConfig AppConfigProvider + AppConfig AppConfigResolver } type Module struct { diff --git a/backend/internal/webauthn/service.go b/backend/internal/webauthn/service.go index 1fc01bcf..0443b9e9 100644 --- a/backend/internal/webauthn/service.go +++ b/backend/internal/webauthn/service.go @@ -13,6 +13,7 @@ import ( "gorm.io/gorm" "gorm.io/gorm/clause" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/common" "github.com/pocket-id/pocket-id/backend/internal/model" datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" @@ -23,17 +24,19 @@ import ( // It must match the value emitted by the JWT service in the access token's "amr" claim const authenticationMethodPhishingResistant = "phr" +const defaultRPDisplayName = "Pocket ID" + type Service struct { - db *gorm.DB - webAuthn *gowebauthn.WebAuthn - signer TokenService - auditLog AuditLogger - appConfig AppConfigProvider + db *gorm.DB + webAuthn *gowebauthn.WebAuthn + signer TokenService + auditLog AuditLogger } func newService(deps Dependencies) (*Service, error) { wa, err := gowebauthn.New(&gowebauthn.Config{ - RPDisplayName: deps.AppConfig.GetDbConfig().AppName.Value, + // Set a default value, it will be set again later + RPDisplayName: defaultRPDisplayName, RPID: utils.GetHostnameFromURL(deps.AppURL), RPOrigins: []string{deps.AppURL}, AuthenticatorSelection: protocol.AuthenticatorSelection{ @@ -57,22 +60,21 @@ func newService(deps Dependencies) (*Service, error) { } return &Service{ - db: deps.DB, - webAuthn: wa, - signer: deps.Signer, - auditLog: deps.AuditLog, - appConfig: deps.AppConfig, + db: deps.DB, + webAuthn: wa, + signer: deps.Signer, + auditLog: deps.AuditLog, }, nil } -func (s *Service) BeginRegistration(ctx context.Context, userID string) (*PublicKeyCredentialCreationOptions, error) { +func (s *Service) BeginRegistration(ctx context.Context, dbConfig *appconfig.AppConfigModel, userID string) (*PublicKeyCredentialCreationOptions, error) { + s.updateWebAuthnConfig(dbConfig) + tx := s.db.Begin() defer func() { tx.Rollback() }() - s.updateWebAuthnConfig() - var user model.User err := tx. WithContext(ctx). @@ -227,7 +229,7 @@ func (s *Service) BeginLogin(ctx context.Context) (*PublicKeyCredentialRequestOp }, nil } -func (s *Service) VerifyLogin(ctx context.Context, sessionID string, credentialAssertionData *protocol.ParsedCredentialAssertionData, ipAddress, userAgent string) (model.User, string, error) { +func (s *Service) VerifyLogin(ctx context.Context, dbConfig *appconfig.AppConfigModel, sessionID string, credentialAssertionData *protocol.ParsedCredentialAssertionData, ipAddress, userAgent string) (model.User, string, error) { tx := s.db.Begin() defer func() { tx.Rollback() @@ -270,12 +272,12 @@ func (s *Service) VerifyLogin(ctx context.Context, sessionID string, credentialA return model.User{}, "", &common.UserDisabledError{} } - token, err := s.signer.GenerateAccessToken(*user, authenticationMethodPhishingResistant) + token, err := s.signer.GenerateAccessToken(*user, authenticationMethodPhishingResistant, dbConfig.SessionDuration.AsDurationMinutes()) if err != nil { return model.User{}, "", err } - s.auditLog.CreateNewSignInWithEmail(ctx, ipAddress, userAgent, user.ID, tx) + s.auditLog.CreateNewSignInWithEmail(ctx, ipAddress, userAgent, user.ID, tx, dbConfig.EmailLoginNotificationEnabled.IsTrue()) err = tx.Commit().Error if err != nil { @@ -373,8 +375,8 @@ func (s *Service) UpdateCredential(ctx context.Context, userID, credentialID, na } // updateWebAuthnConfig updates the WebAuthn configuration with the app name as it can change during runtime -func (s *Service) updateWebAuthnConfig() { - s.webAuthn.Config.RPDisplayName = s.appConfig.GetDbConfig().AppName.Value +func (s *Service) updateWebAuthnConfig(dbConfig *appconfig.AppConfigModel) { + s.webAuthn.Config.RPDisplayName = dbConfig.AppName.String() } func (s *Service) CreateReauthenticationTokenWithAccessToken(ctx context.Context, accessToken string) (string, error) { diff --git a/backend/internal/webauthn/service_test.go b/backend/internal/webauthn/service_test.go index 0551de41..1ccfeece 100644 --- a/backend/internal/webauthn/service_test.go +++ b/backend/internal/webauthn/service_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/common" "github.com/pocket-id/pocket-id/backend/internal/model" datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" @@ -28,7 +29,7 @@ func newFakeSigner() *fakeSigner { return &fakeSigner{tokens: map[string]jwt.Token{}} } -func (s *fakeSigner) GenerateAccessToken(user model.User, authenticationMethod string) (string, error) { +func (s *fakeSigner) GenerateAccessToken(user model.User, authenticationMethod string, _ time.Duration) (string, error) { builder := jwt.NewBuilder(). Subject(user.ID). IssuedAt(time.Now()) @@ -85,7 +86,7 @@ func TestCreateReauthenticationTokenWithAccessToken(t *testing.T) { t.Run("accepts a fresh access token from WebAuthn login", func(t *testing.T) { service, signer, user := setupService(t) - accessToken, err := signer.GenerateAccessToken(user, authenticationMethodPhishingResistant) + accessToken, err := signer.GenerateAccessToken(user, authenticationMethodPhishingResistant, time.Hour) require.NoError(t, err) reauthenticationToken, err := service.CreateReauthenticationTokenWithAccessToken(t.Context(), accessToken) @@ -96,7 +97,7 @@ func TestCreateReauthenticationTokenWithAccessToken(t *testing.T) { t.Run("rejects a fresh access token from one-time access login", func(t *testing.T) { service, signer, user := setupService(t) - accessToken, err := signer.GenerateAccessToken(user, "otp") + accessToken, err := signer.GenerateAccessToken(user, "otp", time.Hour) require.NoError(t, err) reauthenticationToken, err := service.CreateReauthenticationTokenWithAccessToken(t.Context(), accessToken) @@ -108,7 +109,7 @@ func TestCreateReauthenticationTokenWithAccessToken(t *testing.T) { t.Run("rejects a fresh access token without an authentication method", func(t *testing.T) { service, signer, user := setupService(t) - accessToken, err := signer.GenerateAccessToken(user, "") + accessToken, err := signer.GenerateAccessToken(user, "", time.Hour) require.NoError(t, err) reauthenticationToken, err := service.CreateReauthenticationTokenWithAccessToken(t.Context(), accessToken) @@ -119,6 +120,18 @@ func TestCreateReauthenticationTokenWithAccessToken(t *testing.T) { }) } +func TestWebAuthnDisplayNameUsesRequestConfig(t *testing.T) { + service, err := newService(Dependencies{ + DB: testutils.NewDatabaseForTest(t), + AppURL: "https://example.com", + }) + require.NoError(t, err) + require.Equal(t, defaultRPDisplayName, service.webAuthn.Config.RPDisplayName) + + service.updateWebAuthnConfig(&appconfig.AppConfigModel{AppName: "Custom App"}) + require.Equal(t, "Custom App", service.webAuthn.Config.RPDisplayName) +} + func TestConsumeReauthenticationTokenReturnsTokenCreationTime(t *testing.T) { db := testutils.NewDatabaseForTest(t) service := &Service{db: db} diff --git a/backend/resources/migrations/postgres/20260718000000_freeze_config.down.sql b/backend/resources/migrations/postgres/20260718000000_freeze_config.down.sql new file mode 100644 index 00000000..5dbbaac9 --- /dev/null +++ b/backend/resources/migrations/postgres/20260718000000_freeze_config.down.sql @@ -0,0 +1,16 @@ +-- Recreate the standalone config table with the same schema it had before it was frozen +CREATE TABLE app_config_variables +( + key VARCHAR(100) NOT NULL PRIMARY KEY, + value TEXT NOT NULL +); + +-- Populate it from the frozen JSON document stored in the "kv" table +-- json_each expands the JSON object back into one row per key/value pair. +INSERT INTO app_config_variables (key, value) +SELECT je.key, je.value +FROM kv, json_each_text(kv."value"::json) AS je(key, value) +WHERE kv."key" = 'config_migrated'; + +-- Remove the frozen config from the "kv" table +DELETE FROM kv WHERE "key" = 'config_migrated'; diff --git a/backend/resources/migrations/postgres/20260718000000_freeze_config.up.sql b/backend/resources/migrations/postgres/20260718000000_freeze_config.up.sql new file mode 100644 index 00000000..cba856cd --- /dev/null +++ b/backend/resources/migrations/postgres/20260718000000_freeze_config.up.sql @@ -0,0 +1,12 @@ +-- Freeze the app configuration +-- Encode every row of the standalone config table as a single JSON object (mapping key -> value) and store it in the "kv" table under the "config_migrated" key +-- +-- json_object_agg aggregates all rows into a JSON object +-- The "HAVING count(*) > 0" clause ensures that nothing is written to the "kv" table when the config table is empty +INSERT INTO kv ("key", "value") +SELECT 'config_migrated', json_object_agg("key", "value")::text +FROM app_config_variables +HAVING count(*) > 0; + +-- Drop the now-frozen standalone config table +DROP TABLE app_config_variables; diff --git a/backend/resources/migrations/sqlite/20260718000000_freeze_config.down.sql b/backend/resources/migrations/sqlite/20260718000000_freeze_config.down.sql new file mode 100644 index 00000000..f807b25f --- /dev/null +++ b/backend/resources/migrations/sqlite/20260718000000_freeze_config.down.sql @@ -0,0 +1,22 @@ +PRAGMA foreign_keys=OFF; +BEGIN; + +-- Recreate the standalone config table with the same schema it had before it was frozen +CREATE TABLE app_config_variables +( + "key" TEXT NOT NULL PRIMARY KEY, + "value" TEXT NOT NULL +); + +-- Populate it from the frozen JSON document stored in the "kv" table +-- json_each expands the JSON object back into one row per key/value pair. +INSERT INTO app_config_variables ("key", "value") +SELECT je.key, je.value +FROM kv, json_each(kv."value") AS je +WHERE kv."key" = 'config_migrated'; + +-- Remove the frozen config from the "kv" table +DELETE FROM kv WHERE "key" = 'config_migrated'; + +COMMIT; +PRAGMA foreign_keys=ON; diff --git a/backend/resources/migrations/sqlite/20260718000000_freeze_config.up.sql b/backend/resources/migrations/sqlite/20260718000000_freeze_config.up.sql new file mode 100644 index 00000000..1b951fd2 --- /dev/null +++ b/backend/resources/migrations/sqlite/20260718000000_freeze_config.up.sql @@ -0,0 +1,18 @@ +PRAGMA foreign_keys=OFF; +BEGIN; + +-- Freeze the app configuration +-- Encode every row of the standalone config table as a single JSON object (mapping key -> value) and store it in the "kv" table under the "config_migrated" key +-- +-- json_group_object aggregates all rows into a JSON object +-- The "HAVING count(*) > 0" clause ensures that nothing is written to the "kv" table when the config table is empty +INSERT INTO kv ("key", "value") +SELECT 'config_migrated', json_group_object("key", "value") +FROM app_config_variables +HAVING count(*) > 0; + +-- Drop the now-frozen standalone config table +DROP TABLE app_config_variables; + +COMMIT; +PRAGMA foreign_keys=ON;