From 3ecda60f3b026ce529d90196f7dab99de37aa8ad Mon Sep 17 00:00:00 2001 From: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com> Date: Sat, 18 Jul 2026 09:42:59 -0700 Subject: [PATCH] Completed code updates --- backend/internal/appconfig/model_test.go | 2 +- backend/internal/appconfig/service.go | 10 +-- backend/internal/appconfig/service_test.go | 3 +- backend/internal/appconfig/testing_unit.go | 17 +++++ .../internal/bootstrap/router_bootstrap.go | 8 ++- .../internal/bootstrap/services_bootstrap.go | 28 ++++---- .../controller/app_config_controller.go | 6 +- .../internal/controller/user_controller.go | 4 +- .../internal/instanceid/instanceid_test.go | 18 ++--- backend/internal/job/api_key_expiry_job.go | 2 +- backend/internal/job/ldap_job.go | 2 +- .../middleware/auth_middleware_test.go | 7 +- backend/internal/service/audit_log_service.go | 16 ++--- backend/internal/service/e2etest_service.go | 69 +++++++------------ backend/internal/service/email_service.go | 29 +++++--- backend/internal/service/jwt_service.go | 21 +++--- backend/internal/service/jwt_service_test.go | 4 +- backend/internal/service/ldap_service.go | 47 ++++++------- backend/internal/service/ldap_service_test.go | 19 ++--- backend/internal/service/oidc_service.go | 24 +++---- backend/internal/service/oidc_service_test.go | 6 +- .../service/one_time_access_service.go | 24 +++---- .../service/one_time_access_service_test.go | 7 +- .../internal/service/user_group_service.go | 24 +++---- backend/internal/service/user_service.go | 64 ++++++++--------- backend/internal/service/user_service_test.go | 27 ++++---- backend/internal/usersignup/handler.go | 25 +++++-- backend/internal/usersignup/module.go | 10 +-- backend/internal/usersignup/service.go | 23 +++++-- backend/internal/webauthn/handler.go | 13 +++- backend/internal/webauthn/module.go | 13 ++-- backend/internal/webauthn/service.go | 30 +++++--- backend/internal/webauthn/service_test.go | 8 +-- 33 files changed, 311 insertions(+), 299 deletions(-) diff --git a/backend/internal/appconfig/model_test.go b/backend/internal/appconfig/model_test.go index d5e68984..961760f3 100644 --- a/backend/internal/appconfig/model_test.go +++ b/backend/internal/appconfig/model_test.go @@ -216,7 +216,7 @@ func TestAppConfigModel_Update(t *testing.T) { err := m.Update(map[string]string{"thisKeyDoesNotExist": "value"}) require.Error(t, err) - assert.EqualError(t, err, "cannot find config key 'thisKeyDoesNotExist'") + require.EqualError(t, err, "cannot find config key 'thisKeyDoesNotExist'") notFound, ok := errors.AsType[AppConfigKeyNotFoundError](err) require.True(t, ok) diff --git a/backend/internal/appconfig/service.go b/backend/internal/appconfig/service.go index 29c9a255..3eb92f4a 100644 --- a/backend/internal/appconfig/service.go +++ b/backend/internal/appconfig/service.go @@ -179,19 +179,19 @@ func (s *AppConfigService) loadDbConfigFromEnv() (*AppConfigModel, error) { for i := range rt.NumField() { field := rt.Field(i) - // Get the key and internal tag values - key, attrs, _ := strings.Cut(field.Tag.Get("key"), ",") + // 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).Set(reflect.ValueOf(value)) + rv.Field(i).SetString(value) continue } // If it's sensitive, we also allow reading from file - if attrs == "sensitive" { + if field.Tag.Get("sensitive") == "true" { fileName := os.Getenv(envVarName + "_FILE") if fileName != "" { // #nosec G703 - Value is provided by admin @@ -200,7 +200,7 @@ func (s *AppConfigService) loadDbConfigFromEnv() (*AppConfigModel, error) { return nil, fmt.Errorf("failed to read secret '%s' from file '%s': %w", envVarName, fileName, err) } - rv.Field(i).Set(reflect.ValueOf(string(b))) + rv.Field(i).SetString(string(b)) continue } } diff --git a/backend/internal/appconfig/service_test.go b/backend/internal/appconfig/service_test.go index 4c5e2e2f..1e3b08ad 100644 --- a/backend/internal/appconfig/service_test.go +++ b/backend/internal/appconfig/service_test.go @@ -104,6 +104,7 @@ func TestService_NewService(t *testing.T) { 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) @@ -112,7 +113,7 @@ func TestService_NewService(t *testing.T) { cfg, err := svc.GetConfig(t.Context()) require.NoError(t, err) - assert.Equal(t, *getDefaultConfig(), *cfg) + assert.Equal(t, AppConfigValue("Environment App"), cfg.AppName) }) } diff --git a/backend/internal/appconfig/testing_unit.go b/backend/internal/appconfig/testing_unit.go index 9c3a833b..1e7718fb 100644 --- a/backend/internal/appconfig/testing_unit.go +++ b/backend/internal/appconfig/testing_unit.go @@ -3,6 +3,10 @@ // This file contains utils for unit tests and it's only built when the "unit" tag is set package appconfig +import ( + "context" +) + // NewTestAppConfigService is a function used by tests to create AppConfigService objects with pre-defined configuration values func NewTestAppConfigService(config *AppConfigModel) *AppConfigService { if config == nil { @@ -16,3 +20,16 @@ func NewTestAppConfigService(config *AppConfigModel) *AppConfigService { return service } + +// NewTestContext returns a context that resolves the provided application configuration +func NewTestContext(ctx context.Context, config *AppConfigModel) context.Context { + if config == nil { + config = getDefaultConfig() + } + + resolver := appConfigResolver(func(context.Context) (*AppConfigModel, error) { + return config, nil + }) + + return context.WithValue(ctx, appConfigCtxKey{}, resolver) +} diff --git a/backend/internal/bootstrap/router_bootstrap.go b/backend/internal/bootstrap/router_bootstrap.go index 9969045f..3c6d0f76 100644 --- a/backend/internal/bootstrap/router_bootstrap.go +++ b/backend/internal/bootstrap/router_bootstrap.go @@ -23,6 +23,7 @@ import ( "gorm.io/gorm" "github.com/pocket-id/pocket-id/backend/frontend" + "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/controller" "github.com/pocket-id/pocket-id/backend/internal/middleware" @@ -140,13 +141,14 @@ func registerRoutes(r *gin.Engine, db *gorm.DB, svc *services, rateLimitServices } // Initialize middleware for specific routes + appConfigMiddleware := appconfig.NewAppConfigMiddleware(svc.appConfigService) authMiddleware := middleware.NewAuthMiddleware(svc.apiKeyModule, svc.userService, svc.jwtService) fileSizeLimitMiddleware := middleware.NewFileSizeLimitMiddleware() rateLimitMiddleware := middleware.NewRateLimitMiddleware(rateLimitServices) apiRateLimitMiddleware := rateLimitMiddleware.Add(middleware.RateLimitAPI) - apiGroup := r.Group("/api", apiRateLimitMiddleware) - baseGroup := r.Group("/", apiRateLimitMiddleware) + apiGroup := r.Group("/api", appConfigMiddleware.Add(), apiRateLimitMiddleware) + baseGroup := r.Group("/", appConfigMiddleware.Add(), apiRateLimitMiddleware) svc.apiKeyModule.RegisterRoutes(apiGroup, authMiddleware.WithAdminNotRequired().Add(), @@ -158,7 +160,7 @@ 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.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) diff --git a/backend/internal/bootstrap/services_bootstrap.go b/backend/internal/bootstrap/services_bootstrap.go index 57b6738d..20baa984 100644 --- a/backend/internal/bootstrap/services_bootstrap.go +++ b/backend/internal/bootstrap/services_bootstrap.go @@ -68,24 +68,25 @@ func initServices( 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.jwtService, err = service.NewJwtService(ctx, db, instanceID) if err != nil { return nil, fmt.Errorf("failed to create JWT service: %w", err) } svc.customClaimService = service.NewCustomClaimService(db) - svc.webauthnModule, err = webauthn.New(webauthn.Dependencies{ - DB: db, - AppURL: common.EnvConfig.AppURL, - Signer: svc.jwtService, - AuditLog: svc.auditLogService, + svc.webauthnModule, err = webauthn.New(ctx, webauthn.Dependencies{ + DB: db, + AppURL: common.EnvConfig.AppURL, + Signer: svc.jwtService, + AuditLog: svc.auditLogService, + AppConfig: svc.appConfigService, }) if err != nil { return nil, fmt.Errorf("failed to create WebAuthn module: %w", err) @@ -114,14 +115,14 @@ func initServices( 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, @@ -135,10 +136,9 @@ func initServices( DB: db, Signer: svc.jwtService, AuditLog: svc.auditLogService, - AppConfig: svc.appConfigService, UserCreator: svc.userService, }) - 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/controller/app_config_controller.go b/backend/internal/controller/app_config_controller.go index d852d2a1..82e722c3 100644 --- a/backend/internal/controller/app_config_controller.go +++ b/backend/internal/controller/app_config_controller.go @@ -53,11 +53,12 @@ type AppConfigController struct { // @Success 200 {array} dto.PublicAppConfigVariableDto // @Router /api/application-configuration [get] func (acc *AppConfigController) listAppConfigHandler(c *gin.Context) { - configuration, err := acc.appConfigService.ListAppConfig(c.Request.Context(), false) + dbConfig, err := appconfig.FromCtx(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 { @@ -91,11 +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, err := acc.appConfigService.ListAppConfig(c.Request.Context(), true) + dbConfig, err := appconfig.FromCtx(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 { diff --git a/backend/internal/controller/user_controller.go b/backend/internal/controller/user_controller.go index bf603747..5b13cd7a 100644 --- a/backend/internal/controller/user_controller.go +++ b/backend/internal/controller/user_controller.go @@ -23,12 +23,11 @@ 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 *appconfig.AppConfigService) { +func NewUserController(group *gin.RouterGroup, authMiddleware *middleware.AuthMiddleware, rateLimitMiddleware *middleware.RateLimitMiddleware, userService *service.UserService, oneTimeAccessService *service.OneTimeAccessService, webAuthnService *webauthn.Module) { uc := UserController{ userService: userService, oneTimeAccessService: oneTimeAccessService, webAuthnService: webAuthnService, - appConfigService: appConfigService, } group.GET("/users", authMiddleware.Add(), uc.listUsersHandler) @@ -66,7 +65,6 @@ type UserController struct { userService *service.UserService oneTimeAccessService *service.OneTimeAccessService webAuthnService *webauthn.Module - appConfigService *appconfig.AppConfigService } // getUserGroupsHandler godoc 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 7209e2b5..74e63fb4 100644 --- a/backend/internal/job/api_key_expiry_job.go +++ b/backend/internal/job/api_key_expiry_job.go @@ -51,7 +51,7 @@ func (j *ApiKeyEmailJobs) checkAndNotifyExpiringApiKeys(ctx context.Context) err continue } - err = service.SendEmail(ctx, j.emailService, email.Address{ + err = service.SendEmailWithConfig(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 ca749a52..1f68d47a 100644 --- a/backend/internal/job/ldap_job.go +++ b/backend/internal/job/ldap_job.go @@ -31,5 +31,5 @@ func (j *LdapJobs) syncLdap(ctx context.Context) error { return nil } - return j.ldapService.SyncAll(ctx) + return j.ldapService.SyncAllWithConfig(ctx, dbConfig) } diff --git a/backend/internal/middleware/auth_middleware_test.go b/backend/internal/middleware/auth_middleware_test.go index 568b29c6..22c98b43 100644 --- a/backend/internal/middleware/auth_middleware_test.go +++ b/backend/internal/middleware/auth_middleware_test.go @@ -12,7 +12,6 @@ import ( "gorm.io/gorm" "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/instanceid" "github.com/pocket-id/pocket-id/backend/internal/model" @@ -34,15 +33,13 @@ func TestWithApiKeyAuthDisabled(t *testing.T) { db := testutils.NewDatabaseForTest(t) - appConfigService := appconfig.NewTestAppConfigService(nil) - 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) diff --git a/backend/internal/service/audit_log_service.go b/backend/internal/service/audit_log_service.go index 678c70da..290aedce 100644 --- a/backend/internal/service/audit_log_service.go +++ b/backend/internal/service/audit_log_service.go @@ -14,18 +14,16 @@ import ( ) type AuditLogService struct { - db *gorm.DB - appConfigService *appconfig.AppConfigService - emailService *EmailService - geoliteService *GeoLiteService + db *gorm.DB + emailService *EmailService + geoliteService *GeoLiteService } -func NewAuditLogService(db *gorm.DB, appConfigService *appconfig.AppConfigService, emailService *EmailService, geoliteService *GeoLiteService) *AuditLogService { +func NewAuditLogService(db *gorm.DB, emailService *EmailService, geoliteService *GeoLiteService) *AuditLogService { return &AuditLogService{ - db: db, - appConfigService: appConfigService, - emailService: emailService, - geoliteService: geoliteService, + db: db, + emailService: emailService, + geoliteService: geoliteService, } } diff --git a/backend/internal/service/e2etest_service.go b/backend/internal/service/e2etest_service.go index 091ec782..16b691c7 100644 --- a/backend/internal/service/e2etest_service.go +++ b/backend/internal/service/e2etest_service.go @@ -27,6 +27,7 @@ import ( "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" @@ -625,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 } @@ -647,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 @@ -671,48 +666,32 @@ func (s *TestService) SyncLdap(ctx context.Context) error { return s.ldapService.SyncAll(ctx) } -// 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 := appconfig.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) } - err = s.appConfigService.LoadDbConfig(ctx) - if 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 e68df9e3..b73b6531 100644 --- a/backend/internal/service/email_service.go +++ b/backend/internal/service/email_service.go @@ -20,13 +20,12 @@ import ( ) type EmailService struct { - appConfigService *appconfig.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 *appconfig.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) @@ -38,10 +37,9 @@ func NewEmailService(db *gorm.DB, appConfigService *appconfig.AppConfigService) } return &EmailService{ - appConfigService: appConfigService, - db: db, - htmlTemplates: htmlTemplates, - textTemplates: textTemplates, + db: db, + htmlTemplates: htmlTemplates, + textTemplates: textTemplates, }, nil } @@ -67,10 +65,19 @@ func (srv *EmailService) SendTestEmail(ctx context.Context, recipientUserId stri } func SendEmail[V any](ctx context.Context, srv *EmailService, toEmail email.Address, template email.Template[V], tData *V) error { - dbConfig := srv.appConfigService.GetDbConfig() + dbConfig, err := appconfig.FromCtx(ctx) + if err != nil { + return fmt.Errorf("error loading app configuration: %w", err) + } + + return SendEmailWithConfig(ctx, srv, dbConfig, toEmail, template, tData) +} + +// SendEmailWithConfig sends an email with an explicitly loaded configuration for call chains that do not originate from an HTTP request +func SendEmailWithConfig[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, } diff --git a/backend/internal/service/jwt_service.go b/backend/internal/service/jwt_service.go index 6c4659d0..fcc926da 100644 --- a/backend/internal/service/jwt_service.go +++ b/backend/internal/service/jwt_service.go @@ -13,7 +13,6 @@ 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/common" "github.com/pocket-id/pocket-id/backend/internal/model" jwkutils "github.com/pocket-id/pocket-id/backend/internal/utils/jwk" @@ -44,19 +43,18 @@ const ( ) type JwtService struct { - db *gorm.DB - envConfig *common.EnvConfigSchema - privateKey jwk.Key - keyId string - appConfigService *appconfig.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 *appconfig.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 } @@ -64,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 *appconfig.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 diff --git a/backend/internal/service/jwt_service_test.go b/backend/internal/service/jwt_service_test.go index 646b4fab..ca69836e 100644 --- a/backend/internal/service/jwt_service_test.go +++ b/backend/internal/service/jwt_service_test.go @@ -36,11 +36,11 @@ func newTestEnvConfig() *common.EnvConfigSchema { } } -func initJwtService(t *testing.T, db *gorm.DB, instanceID string, appConfig *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 diff --git a/backend/internal/service/ldap_service.go b/backend/internal/service/ldap_service.go index 40ca74eb..2bf917ec 100644 --- a/backend/internal/service/ldap_service.go +++ b/backend/internal/service/ldap_service.go @@ -30,13 +30,12 @@ import ( ) type LdapService struct { - db *gorm.DB - httpClient *http.Client - appConfigService *appconfig.AppConfigService - userService *UserService - groupService *UserGroupService - fileStorage storage.FileStorage - clientFactory func(dbConfig *appconfig.AppConfigModel) (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 { @@ -70,14 +69,13 @@ type ldapClient interface { Close() error } -func NewLdapService(db *gorm.DB, httpClient *http.Client, appConfigService *appconfig.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 @@ -110,7 +108,15 @@ func (s *LdapService) SyncAll(ctx context.Context) error { if err != nil { return fmt.Errorf("error loading app configuration: %w", err) } + return s.syncAll(ctx, dbConfig) +} +// SyncAllWithConfig synchronizes LDAP with an explicitly loaded configuration for call chains that do not originate from an HTTP request +func (s *LdapService) SyncAllWithConfig(ctx context.Context, dbConfig *appconfig.AppConfigModel) error { + return s.syncAll(ctx, dbConfig) +} + +func (s *LdapService) syncAll(ctx context.Context, dbConfig *appconfig.AppConfigModel) error { // Setup LDAP connection client, err := s.clientFactory(dbConfig) if err != nil { @@ -173,7 +179,7 @@ func (s *LdapService) SyncAll(ctx context.Context) 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 } @@ -290,12 +296,7 @@ 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, err := appconfig.FromCtx(ctx) - if err != nil { - return nil, nil, nil, fmt.Errorf("error loading app configuration: %w", err) - } - +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", @@ -532,7 +533,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 @@ -543,7 +544,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 diff --git a/backend/internal/service/ldap_service_test.go b/backend/internal/service/ldap_service_test.go index 3e8fb5ca..540f25ab 100644 --- a/backend/internal/service/ldap_service_test.go +++ b/backend/internal/service/ldap_service_test.go @@ -110,7 +110,8 @@ func TestLdapServiceSyncAllReconcilesUsersAndGroups(t *testing.T) { LdapID: &oldGroupLdapID, }).Error) - require.NoError(t, service.SyncAll(t.Context())) + err := service.SyncAllWithConfig(t.Context(), defaultTestLDAPAppConfig()) + require.NoError(t, err) var alice model.User require.NoError(t, db.First(&alice, "ldap_id = ?", aliceLdapID).Error) @@ -176,7 +177,8 @@ func TestLdapServiceSyncAllMapsPosixGroupMemberUid(t *testing.T) { ), )) - require.NoError(t, service.SyncAll(t.Context())) + err := service.SyncAllWithConfig(t.Context(), appCfg) + require.NoError(t, err) var group model.UserGroup require.NoError(t, db.Preload("Users").First(&group, "ldap_id = ?", "g-users").Error) @@ -218,7 +220,8 @@ func TestLdapServiceSyncAllHandlesDuplicateLDAPIDsInSingleRun(t *testing.T) { ), )) - require.NoError(t, service.SyncAll(t.Context())) + err := service.SyncAllWithConfig(t.Context(), defaultTestLDAPAppConfig()) + require.NoError(t, err) var users []model.User require.NoError(t, db.Find(&users, "ldap_id = ?", "u-dup").Error) @@ -289,7 +292,8 @@ func TestLdapServiceSyncAllSetsAdminFromGroupMembership(t *testing.T) { ldapSearchResult(tt.groupEntry), )) - require.NoError(t, service.SyncAll(t.Context())) + err := service.SyncAllWithConfig(t.Context(), tt.appConfig) + require.NoError(t, err) var user model.User require.NoError(t, db.First(&user, "ldap_id = ?", "u-testadmin").Error) @@ -317,22 +321,19 @@ func newTestLdapServiceWithAppConfig(t *testing.T, appConfigModel *appconfig.App fileStorage, err := storage.NewDatabaseStorage(db) require.NoError(t, err) - appConfig := 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 := NewLdapService(db, &http.Client{}, userService, groupService, fileStorage) service.clientFactory = func(dbConfig *appconfig.AppConfigModel) (ldapClient, error) { return client, nil } diff --git a/backend/internal/service/oidc_service.go b/backend/internal/service/oidc_service.go index b97cc304..5f259a48 100644 --- a/backend/internal/service/oidc_service.go +++ b/backend/internal/service/oidc_service.go @@ -16,7 +16,6 @@ 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" @@ -38,11 +37,10 @@ const ( ) type OidcService struct { - db *gorm.DB - jwtService *JwtService - appConfigService *appconfig.AppConfigService - previewBuilder oidcClientPreviewBuilder - scimService *ScimService + db *gorm.DB + jwtService *JwtService + previewBuilder oidcClientPreviewBuilder + scimService *ScimService httpClient *http.Client fileStorage storage.FileStorage @@ -55,20 +53,18 @@ type oidcClientPreviewBuilder interface { func NewOidcService( db *gorm.DB, jwtService *JwtService, - appConfigService *appconfig.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 4bf72c3c..c95f84a3 100644 --- a/backend/internal/service/one_time_access_service.go +++ b/backend/internal/service/one_time_access_service.go @@ -20,22 +20,20 @@ import ( ) type OneTimeAccessService struct { - db *gorm.DB - userService *UserService - appConfigService *appconfig.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 *appconfig.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, } } diff --git a/backend/internal/service/one_time_access_service_test.go b/backend/internal/service/one_time_access_service_test.go index 24977ec9..3f819c19 100644 --- a/backend/internal/service/one_time_access_service_test.go +++ b/backend/internal/service/one_time_access_service_test.go @@ -18,8 +18,8 @@ func TestExchangeOneTimeAccessTokenRejectsDisabledUser(t *testing.T) { 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{}) + oneTimeAccessService := NewOneTimeAccessService(db, nil, jwtService, auditLogService, nil) user := model.User{ Base: model.Base{ID: "disabled-user"}, @@ -36,7 +36,8 @@ func TestExchangeOneTimeAccessTokenRejectsDisabledUser(t *testing.T) { } require.NoError(t, db.Create(&loginCode).Error) - exchangedUser, accessToken, err := oneTimeAccessService.ExchangeOneTimeAccessToken(t.Context(), loginCode.Token, "", "", "") + ctx := appconfig.NewTestContext(t.Context(), nil) + exchangedUser, accessToken, err := oneTimeAccessService.ExchangeOneTimeAccessToken(ctx, 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 f0f46baa..de8a8876 100644 --- a/backend/internal/service/user_group_service.go +++ b/backend/internal/service/user_group_service.go @@ -17,13 +17,12 @@ import ( ) type UserGroupService struct { - db *gorm.DB - scimService *ScimService - appConfigService *appconfig.AppConfigService + db *gorm.DB + scimService *ScimService } -func NewUserGroupService(db *gorm.DB, appConfigService *appconfig.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) { @@ -167,14 +166,15 @@ func (s *UserGroupService) updateInternal(ctx context.Context, id string, input return model.UserGroup{}, err } - cfg, err := appconfig.FromCtx(ctx) - if err != nil { - return model.UserGroup{}, fmt.Errorf("error loading app configuration: %w", err) - } - // Disallow updating the group if it is an LDAP group and LDAP is enabled - if !isLdapSync && group.LdapID != nil && cfg.LdapEnabled.IsTrue() { - return model.UserGroup{}, &common.LdapUserGroupUpdateError{} + if !isLdapSync && group.LdapID != nil { + cfg, err := appconfig.FromCtx(ctx) + if err != nil { + return model.UserGroup{}, fmt.Errorf("error loading app configuration: %w", err) + } + if cfg.LdapEnabled.IsTrue() { + return model.UserGroup{}, &common.LdapUserGroupUpdateError{} + } } group.Name = input.Name diff --git a/backend/internal/service/user_service.go b/backend/internal/service/user_service.go index 35053c15..72c0b847 100644 --- a/backend/internal/service/user_service.go +++ b/backend/internal/service/user_service.go @@ -32,20 +32,18 @@ type UserService struct { jwtService *JwtService auditLogService *AuditLogService emailService *EmailService - appConfigService *appconfig.AppConfigService customClaimService *CustomClaimService appImagesService *AppImagesService scimService *ScimService fileStorage storage.FileStorage } -func NewUserService(db *gorm.DB, jwtService *JwtService, auditLogService *AuditLogService, emailService *EmailService, appConfigService *appconfig.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, @@ -205,13 +203,8 @@ func (s *UserService) DeleteUser(ctx context.Context, userID string, allowLdapDe } func (s *UserService) deleteUserInternal(ctx context.Context, tx *gorm.DB, userID string, allowLdapDelete bool) error { - cfg, err := appconfig.FromCtx(ctx) - if err != nil { - return fmt.Errorf("error loading app configuration: %w", err) - } - var user model.User - err = tx. + err := tx. WithContext(ctx). Where("id = ?", userID). Clauses(clause.Locking{Strength: "UPDATE"}). @@ -222,8 +215,14 @@ 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 && cfg.LdapEnabled.IsTrue() { - return &common.LdapUserUpdateError{} + if !allowLdapDelete && !user.Disabled && user.LdapID != nil { + cfg, err := appconfig.FromCtx(ctx) + if err != nil { + return fmt.Errorf("error loading app configuration: %w", err) + } + if cfg.LdapEnabled.IsTrue() { + return &common.LdapUserUpdateError{} + } } err = tx.WithContext(ctx).Delete(&user).Error @@ -262,7 +261,10 @@ func (s *UserService) CreateUserInternal(ctx context.Context, input dto.UserCrea if err != nil { return model.User{}, fmt.Errorf("error loading app configuration: %w", err) } + return s.createUserInternal(ctx, input, isLdapSync, tx, cfg) +} +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{} } @@ -295,7 +297,7 @@ func (s *UserService) CreateUserInternal(ctx context.Context, input dto.UserCrea user.LdapID = &input.LdapID } - err = tx.WithContext(ctx).Create(&user).Error + err := tx.WithContext(ctx).Create(&user).Error if errors.Is(err, gorm.ErrDuplicatedKey) { // Do not follow this path if we're using LDAP, as we don't want to roll-back the transaction here if !isLdapSync { @@ -323,13 +325,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 } @@ -342,16 +344,11 @@ 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 { - cfg, err := appconfig.FromCtx(ctx) - if err != nil { - return fmt.Errorf("error loading app configuration: %w", err) - } - +func (s *UserService) applyDefaultGroups(ctx context.Context, user *model.User, tx *gorm.DB, cfg *appconfig.AppConfigModel) error { var groupIDs []string v := cfg.SignupDefaultUserGroupIDs if v != "" && v != "[]" { - err = json.Unmarshal([]byte(v), &groupIDs) + err := json.Unmarshal([]byte(v), &groupIDs) if err != nil { return fmt.Errorf("invalid SignupDefaultUserGroupIDs JSON: %w", err) } @@ -409,16 +406,11 @@ 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 { - cfg, err := appconfig.FromCtx(ctx) - if err != nil { - return fmt.Errorf("error loading app configuration: %w", err) - } - +func (s *UserService) applyDefaultCustomClaims(ctx context.Context, user *model.User, tx *gorm.DB, cfg *appconfig.AppConfigModel) error { var claims []dto.CustomClaimCreateDto v := cfg.SignupDefaultCustomClaims if v != "" && v != "[]" { - err = json.Unmarshal([]byte(v), &claims) + err := json.Unmarshal([]byte(v), &claims) if err != nil { return fmt.Errorf("invalid SignupDefaultCustomClaims JSON: %w", err) } @@ -434,12 +426,17 @@ func (s *UserService) applyDefaultCustomClaims(ctx context.Context, user *model. } func (s *UserService) UpdateUser(ctx context.Context, userID string, updatedUser dto.UserCreateDto, updateOwnUser bool, isLdapSync bool) (model.User, error) { + cfg, err := appconfig.FromCtx(ctx) + if err != nil { + return model.User{}, fmt.Errorf("error loading app configuration: %w", err) + } + 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 } @@ -452,18 +449,13 @@ 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) { - cfg, err := appconfig.FromCtx(ctx) - if err != nil { - return model.User{}, fmt.Errorf("error loading app configuration: %w", err) - } - +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{} } var user model.User - err = tx. + err := tx. WithContext(ctx). Where("id = ?", userID). Clauses(clause.Locking{Strength: "UPDATE"}). diff --git a/backend/internal/service/user_service_test.go b/backend/internal/service/user_service_test.go index 3799b36f..a7d584a7 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,20 @@ 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"} + ctx := appconfig.NewTestContext(t.Context(), config) + userService, groupService := newTestUserService(t) group, err := groupService.Create(t.Context(), dto.UserGroupCreateDto{ Name: "members", @@ -52,7 +50,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(ctx, dto.UserCreateDto{ Username: "member", Email: &email, FirstName: "Group", @@ -70,10 +68,9 @@ 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"} + ctx := appconfig.NewTestContext(t.Context(), config) + userService, groupService := newTestUserService(t) group, err := groupService.Create(t.Context(), dto.UserGroupCreateDto{ Name: "default", @@ -85,11 +82,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(ctx, dto.UserCreateDto{ Username: "defaultmember", Email: &email, FirstName: "Default", diff --git a/backend/internal/usersignup/handler.go b/backend/internal/usersignup/handler.go index 5abdbcd7..1dda51c0 100644 --- a/backend/internal/usersignup/handler.go +++ b/backend/internal/usersignup/handler.go @@ -1,11 +1,13 @@ package usersignup import ( + "fmt" "net/http" "time" "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/utils" @@ -15,12 +17,11 @@ import ( const defaultSignupTokenDuration = time.Hour type handler struct { - service *Service - appConfig AppConfigProvider + service *Service } -func newHandler(service *Service, appConfig AppConfigProvider) *handler { - return &handler{service: service, appConfig: appConfig} +func newHandler(service *Service) *handler { + return &handler{service: service} } func (h *handler) checkInitialAdminSetupAvailable(c *gin.Context) { @@ -48,6 +49,12 @@ 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 := appconfig.FromCtx(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) @@ -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 := appconfig.FromCtx(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) @@ -184,7 +197,7 @@ func (h *handler) signup(c *gin.Context) { 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..a4fc57bd 100644 --- a/backend/internal/usersignup/module.go +++ b/backend/internal/usersignup/module.go @@ -2,6 +2,7 @@ package usersignup import ( "context" + "time" "github.com/gin-gonic/gin" "gorm.io/gorm" @@ -11,17 +12,13 @@ import ( ) 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, input dto.UserCreateDto, isLdapSync bool, tx *gorm.DB) (model.User, error) } @@ -31,7 +28,6 @@ type Dependencies struct { Signer TokenService AuditLog AuditLogger - AppConfig AppConfigProvider UserCreator UserCreator } @@ -44,7 +40,7 @@ func New(deps Dependencies) *Module { service := newService(deps) return &Module{ service: service, - handler: newHandler(service, deps.AppConfig), + handler: newHandler(service), } } diff --git a/backend/internal/usersignup/service.go b/backend/internal/usersignup/service.go index 0ec3a76d..8b3307e0 100644 --- a/backend/internal/usersignup/service.go +++ b/backend/internal/usersignup/service.go @@ -3,12 +3,14 @@ package usersignup import ( "context" "errors" + "fmt" "strings" "time" "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 +27,6 @@ type Service struct { userCreator UserCreator signer TokenService auditLog AuditLogger - appConfig AppConfigProvider } func newService(deps Dependencies) *Service { @@ -34,11 +35,15 @@ 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) { + config, err := appconfig.FromCtx(ctx) + if err != nil { + return model.User{}, "", fmt.Errorf("error loading app configuration: %w", err) + } + tx := s.db.Begin() defer func() { tx.Rollback() @@ -46,8 +51,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,7 +88,7 @@ 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) @@ -92,7 +96,7 @@ func (s *Service) SignUp(ctx context.Context, signupData signUpDto, ipAddress, u 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 } @@ -123,6 +127,11 @@ func (s *Service) SignUp(ctx context.Context, signupData signUpDto, ipAddress, u } func (s *Service) SignUpInitialAdmin(ctx context.Context, signUpData signUpDto) (model.User, string, error) { + config, err := appconfig.FromCtx(ctx) + if err != nil { + return model.User{}, "", fmt.Errorf("error loading app configuration: %w", err) + } + tx := s.db.Begin() defer func() { tx.Rollback() @@ -150,7 +159,7 @@ func (s *Service) SignUpInitialAdmin(ctx context.Context, signUpData signUpDto) 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 4799f7c9..7d616a5f 100644 --- a/backend/internal/webauthn/handler.go +++ b/backend/internal/webauthn/handler.go @@ -1,19 +1,20 @@ package webauthn import ( + "fmt" "net/http" "github.com/gin-gonic/gin" "github.com/go-webauthn/webauthn/protocol" + "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/utils/cookie" ) type handler struct { - service *Service - appConfig AppConfigProvider + service *Service } func newHandler(service *Service) *handler { @@ -67,6 +68,12 @@ func (h *handler) beginLogin(c *gin.Context) { } func (h *handler) verifyLogin(c *gin.Context) { + dbConfig, err := appconfig.FromCtx(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{}) @@ -91,7 +98,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 d7976d95..41a45dce 100644 --- a/backend/internal/webauthn/module.go +++ b/backend/internal/webauthn/module.go @@ -23,16 +23,13 @@ type AuditLogger interface { CreateNewSignInWithEmail(ctx context.Context, ipAddress, userAgent, userID string, tx *gorm.DB, dbConfig *appconfig.AppConfigModel) model.AuditLog } -type AppConfigProvider interface { - GetDbConfig() *model.AppConfig -} - type Dependencies struct { DB *gorm.DB AppURL string - Signer TokenService - AuditLog AuditLogger + Signer TokenService + AuditLog AuditLogger + AppConfig *appconfig.AppConfigService } type Module struct { @@ -40,8 +37,8 @@ type Module struct { handler *handler } -func New(deps Dependencies) (*Module, error) { - service, err := newService(deps) +func New(ctx context.Context, deps Dependencies) (*Module, error) { + service, err := newService(ctx, deps) if err != nil { return nil, err } diff --git a/backend/internal/webauthn/service.go b/backend/internal/webauthn/service.go index ebae1552..16afb611 100644 --- a/backend/internal/webauthn/service.go +++ b/backend/internal/webauthn/service.go @@ -31,9 +31,14 @@ type Service struct { auditLog AuditLogger } -func newService(deps Dependencies) (*Service, error) { +func newService(ctx context.Context, deps Dependencies) (*Service, error) { + dbConfig, err := deps.AppConfig.GetConfig(ctx) + if err != nil { + return nil, fmt.Errorf("error loading app configuration: %w", err) + } + wa, err := gowebauthn.New(&gowebauthn.Config{ - RPDisplayName: deps.AppConfig.GetDbConfig().AppName.Value, + RPDisplayName: dbConfig.AppName.String(), RPID: utils.GetHostnameFromURL(deps.AppURL), RPOrigins: []string{deps.AppURL}, AuthenticatorSelection: protocol.AuthenticatorSelection{ @@ -65,15 +70,18 @@ func newService(deps Dependencies) (*Service, error) { } func (s *Service) BeginRegistration(ctx context.Context, userID string) (*PublicKeyCredentialCreationOptions, error) { + err := s.updateWebAuthnConfig(ctx) + if err != nil { + return nil, err + } + tx := s.db.Begin() defer func() { tx.Rollback() }() - s.updateWebAuthnConfig() - var user model.User - err := tx. + err = tx. WithContext(ctx). Preload("Credentials"). Find(&user, "id = ?", userID). @@ -274,7 +282,7 @@ 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 } @@ -377,8 +385,14 @@ 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(ctx context.Context) error { + dbConfig, err := appconfig.FromCtx(ctx) + if err != nil { + return fmt.Errorf("error loading app configuration: %w", err) + } + + s.webAuthn.Config.RPDisplayName = dbConfig.AppName.String() + return nil } 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..7cda9fa9 100644 --- a/backend/internal/webauthn/service_test.go +++ b/backend/internal/webauthn/service_test.go @@ -28,7 +28,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 +85,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 +96,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 +108,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)