diff --git a/.env.example b/.env.example index c2ee381d..98d099cd 100644 --- a/.env.example +++ b/.env.example @@ -22,6 +22,8 @@ GOCHAT_REDIS_POOL_SIZE=50 # ---- JWT ---- GOCHAT_JWT_SECRET=CHANGE_ME_TO_A_RANDOM_64_CHAR_STRING +# Optional during a bounded rotation window; comma-separated old 32+ byte secrets. +GOCHAT_JWT_PREVIOUS_SECRETS= GOCHAT_JWT_EXPIRY_HOURS=72 GOCHAT_JWT_ACCESS_EXPIRY_MINUTES=15 GOCHAT_JWT_REFRESH_EXPIRY_HOURS=168 diff --git a/backend/configs/config.dev.yaml b/backend/configs/config.dev.yaml index 3931912a..0e001d43 100644 --- a/backend/configs/config.dev.yaml +++ b/backend/configs/config.dev.yaml @@ -4,6 +4,9 @@ server: mode: "debug" +jwt: + allow_insecure_header_auth: true + database: name: "gochat_dev" log_level: "info" diff --git a/backend/configs/config.development.yaml b/backend/configs/config.development.yaml index f4ba888d..22c038fa 100644 --- a/backend/configs/config.development.yaml +++ b/backend/configs/config.development.yaml @@ -4,6 +4,9 @@ server: mode: "debug" +jwt: + allow_insecure_header_auth: true + database: dsn: "postgres://postgres:xiha02@localhost:5444/gochat_dev?sslmode=disable" log_level: "info" diff --git a/backend/configs/config.prod.yaml b/backend/configs/config.prod.yaml index 40d1d852..01e5c1ce 100644 --- a/backend/configs/config.prod.yaml +++ b/backend/configs/config.prod.yaml @@ -13,6 +13,9 @@ server: allow_credentials: true # needed for JWT cookie-based auth max_age: 86400 +jwt: + allow_insecure_header_auth: false + database: dsn: "postgres://gochat:CHANGE_ME@localhost:5432/gochat_production?sslmode=require" pool_max: 20 diff --git a/backend/configs/config.yaml b/backend/configs/config.yaml index 6cdda3a4..9d49d58b 100644 --- a/backend/configs/config.yaml +++ b/backend/configs/config.yaml @@ -27,6 +27,8 @@ redis: jwt: secret: "gochat_dev_secret_change_in_production" + previous_secrets: [] + allow_insecure_header_auth: false expiry_hours: 72 log: diff --git a/backend/internal/auth/coverage_test.go b/backend/internal/auth/coverage_test.go index b5a8cbe8..24fd7216 100644 --- a/backend/internal/auth/coverage_test.go +++ b/backend/internal/auth/coverage_test.go @@ -111,6 +111,21 @@ func TestJWTValidateAccessTokenInvalid(t *testing.T) { assert.Error(t, err) } +func TestJWTValidationAcceptsPreviousSecretDuringRotation(t *testing.T) { + oldConfig := &config.JWTConfig{Secret: "4kM9sT2vX7qP1dR8nC5hL3wF6bJ0zYgU", ExpiryHours: 1, RefreshExpiryHours: 24} + user := &model.User{Base: model.Base{ID: 1}, Provider: "email"} + pair, err := NewJWTService(oldConfig).GenerateTokenPair(user, 10, "agent") + require.NoError(t, err) + + rotated := NewJWTService(&config.JWTConfig{ + Secret: "9pN2xR7mV4kD8sQ1cF6hT3wL5bJ0zYgU", + PreviousSecrets: []string{oldConfig.Secret}, + }) + claims, err := rotated.ValidateAccessToken(pair.AccessToken) + require.NoError(t, err) + assert.Equal(t, user.ID, claims.UserID) +} + func TestJWTValidateAccessTokenRefreshTokenRejected(t *testing.T) { cfg := &config.JWTConfig{Secret: "testsecret", ExpiryHours: 1, RefreshExpiryHours: 24} svc := NewJWTService(cfg) diff --git a/backend/internal/auth/jwt.go b/backend/internal/auth/jwt.go index 5cadfe5d..48bc4b0a 100644 --- a/backend/internal/auth/jwt.go +++ b/backend/internal/auth/jwt.go @@ -44,6 +44,12 @@ func NewJWTService(cfg *config.JWTConfig) *JWTService { return &JWTService{cfg: cfg} } +// InsecureHeaderAuthAllowed reports whether trusted dev/test callers may use +// X-User-ID instead of a signed token. Release config validation rejects this. +func (s *JWTService) InsecureHeaderAuthAllowed() bool { + return s != nil && s.cfg != nil && s.cfg.AllowInsecureHeaderAuth +} + // GenerateTokenPair generates an access token + refresh token pair. // Access Token: 15min expiry with full Claims // Refresh Token: 7 days expiry, only UserID + Provider @@ -116,54 +122,40 @@ func (s *JWTService) GenerateTokenPairForClient(user *model.User, accountID uint // ValidateAccessToken validates an access JWT token and returns claims. func (s *JWTService) ValidateAccessToken(tokenString string) (*Claims, error) { - token, err := jwt.ParseWithClaims(tokenString, &Claims{}, func(token *jwt.Token) (interface{}, error) { - if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok { - return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"]) - } - return []byte(s.cfg.Secret), nil - }) - - if err != nil { - return nil, fmt.Errorf("failed to parse token: %w", err) - } - - claims, ok := token.Claims.(*Claims) - if !ok || !token.Valid { - return nil, errors.New("invalid token claims") - } - - // Verify it's an access token (not a refresh token) - if claims.Subject != fmt.Sprintf("user_%d", claims.UserID) { - return nil, errors.New("invalid token type: expected access token") - } - - return claims, nil + return s.validateToken(tokenString, "user", "access") } // ValidateRefreshToken validates a refresh JWT token and returns claims. func (s *JWTService) ValidateRefreshToken(tokenString string) (*Claims, error) { - token, err := jwt.ParseWithClaims(tokenString, &Claims{}, func(token *jwt.Token) (interface{}, error) { - if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok { - return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"]) + return s.validateToken(tokenString, "refresh", "refresh") +} + +func (s *JWTService) validateToken(tokenString, subjectPrefix, kind string) (*Claims, error) { + if s == nil || s.cfg == nil { + return nil, errors.New("JWT service is not configured") + } + + secrets := append([]string{s.cfg.Secret}, s.cfg.PreviousSecrets...) + var lastErr error + for _, secret := range secrets { + claims := &Claims{} + token, err := jwt.ParseWithClaims(tokenString, claims, func(token *jwt.Token) (interface{}, error) { + return []byte(secret), nil + }, jwt.WithValidMethods([]string{jwt.SigningMethodHS256.Alg()})) + if err != nil { + lastErr = err + continue } - return []byte(s.cfg.Secret), nil - }) - - if err != nil { - return nil, fmt.Errorf("failed to parse refresh token: %w", err) + if !token.Valid { + lastErr = errors.New("invalid token claims") + continue + } + if claims.Subject != fmt.Sprintf("%s_%d", subjectPrefix, claims.UserID) { + return nil, fmt.Errorf("invalid token type: expected %s token", kind) + } + return claims, nil } - - claims, ok := token.Claims.(*Claims) - if !ok || !token.Valid { - return nil, errors.New("invalid refresh token claims") - } - - // Verify it's a refresh token - if claims.Subject != fmt.Sprintf("refresh_%d", claims.UserID) { - return nil, errors.New("invalid token type: expected refresh token") - } - - return claims, nil + return nil, fmt.Errorf("failed to parse %s token: %w", kind, lastErr) } // RefreshAccessToken generates a new access token from a valid refresh token. diff --git a/backend/internal/config/config.go b/backend/internal/config/config.go index e249a565..8b62115c 100644 --- a/backend/internal/config/config.go +++ b/backend/internal/config/config.go @@ -138,12 +138,14 @@ type RedisConfig struct { } type JWTConfig struct { - Secret string `mapstructure:"secret"` - ExpiryHours int `mapstructure:"expiry_hours"` - RefreshExpiryHours int `mapstructure:"refresh_expiry_hours"` - AccessExpiryMinutes int `mapstructure:"access_expiry_minutes"` - Audience string `mapstructure:"audience"` - Issuer string `mapstructure:"issuer"` + Secret string `mapstructure:"secret"` + PreviousSecrets []string `mapstructure:"previous_secrets"` + AllowInsecureHeaderAuth bool `mapstructure:"allow_insecure_header_auth"` + ExpiryHours int `mapstructure:"expiry_hours"` + RefreshExpiryHours int `mapstructure:"refresh_expiry_hours"` + AccessExpiryMinutes int `mapstructure:"access_expiry_minutes"` + Audience string `mapstructure:"audience"` + Issuer string `mapstructure:"issuer"` } func (j JWTConfig) ExpiryDuration() time.Duration { @@ -481,6 +483,8 @@ func LoadWithEnv(env string) (*Config, error) { "GOCHAT_REDIS_POOL_SIZE": "redis.pool_size", "GOCHAT_JWT_SECRET": "jwt.secret", "JWT_SECRET": "jwt.secret", // Alias for compatibility (no prefix) + "GOCHAT_JWT_PREVIOUS_SECRETS": "jwt.previous_secrets", + "GOCHAT_JWT_ALLOW_INSECURE_HEADER_AUTH": "jwt.allow_insecure_header_auth", "GOCHAT_JWT_EXPIRY_HOURS": "jwt.expiry_hours", "GOCHAT_JWT_ACCESS_EXPIRY_MINUTES": "jwt.access_expiry_minutes", "GOCHAT_JWT_REFRESH_EXPIRY_HOURS": "jwt.refresh_expiry_hours", @@ -515,10 +519,16 @@ func LoadWithEnv(env string) (*Config, error) { "GOCHAT_OAUTH_GOOGLE_SCOPES": "oauth.google.scopes", } for envKey, configKey := range envBindings { + if envKey == "JWT_SECRET" { + continue + } if err := v.BindEnv(configKey, envKey); err != nil { return nil, fmt.Errorf("failed to bind env %s: %w", envKey, err) } } + if err := v.BindEnv("jwt.secret", "GOCHAT_JWT_SECRET", "JWT_SECRET"); err != nil { + return nil, fmt.Errorf("failed to bind JWT secret environment aliases: %w", err) + } // Set defaults setDefaults(v) @@ -537,7 +547,11 @@ func LoadWithEnv(env string) (*Config, error) { // Overlay environment-specific config: config.{env}.yaml if env != "" && env != "default" { - envFile := fmt.Sprintf("config.%s.yaml", env) + overlayEnv := env + if env == "production" { + overlayEnv = "prod" + } + envFile := fmt.Sprintf("config.%s.yaml", overlayEnv) // Search in the same directory as the base config baseConfigPath := v.ConfigFileUsed() if baseConfigPath != "" { @@ -564,10 +578,39 @@ func LoadWithEnv(env string) (*Config, error) { // Apply defaults for zero-valued fields applyZeroDefaults(&cfg) + if raw := strings.TrimSpace(os.Getenv("GOCHAT_JWT_PREVIOUS_SECRETS")); raw != "" { + cfg.JWT.PreviousSecrets = parseJWTSecretList(raw) + } + if err := validateRuntimeEnvironment(env, &cfg); err != nil { + return nil, err + } return &cfg, nil } +func parseJWTSecretList(raw string) []string { + var secrets []string + for _, secret := range strings.Split(raw, ",") { + if secret = strings.TrimSpace(secret); secret != "" { + secrets = append(secrets, secret) + } + } + return secrets +} + +func validateRuntimeEnvironment(env string, cfg *Config) error { + if (env == "prod" || env == "production") && cfg.Server.Mode != "release" { + return fmt.Errorf("production environment requires server.mode=release") + } + if cfg.Server.Mode != "release" { + return nil + } + if strings.TrimSpace(os.Getenv("GOCHAT_JWT_SECRET")) == "" && strings.TrimSpace(os.Getenv("JWT_SECRET")) == "" { + return fmt.Errorf("release mode requires GOCHAT_JWT_SECRET or JWT_SECRET from the environment") + } + return nil +} + // LoadDotEnvEnvironment loads workspace environment values into the process. // It is called before GOCHAT_ENV is read so the root .env can select the // environment-specific config overlay as well as configure direct os.Getenv diff --git a/backend/internal/config/config_test.go b/backend/internal/config/config_test.go index bd5aaef3..8d3229a3 100644 --- a/backend/internal/config/config_test.go +++ b/backend/internal/config/config_test.go @@ -120,7 +120,49 @@ func TestValidate_JWTSecretInProduction(t *testing.T) { err := Validate(cfg) assert.Error(t, err) - assert.Contains(t, err.Error(), "JWT secret must be changed in production") + assert.Contains(t, err.Error(), "JWT secret") +} + +func TestValidate_ReleaseJWTSecurity(t *testing.T) { + validSecret := "6vG3uP9qL2mR8xK5nD7sF4hJ1cB0wZyE" + base := func() *Config { + return &Config{ + Server: ServerConfig{Port: 8080, Mode: "release"}, + Database: DatabaseConfig{DSN: "postgres://user@localhost:5432/db"}, + Redis: RedisConfig{DSN: "redis://localhost:6379"}, + JWT: JWTConfig{Secret: validSecret}, + Log: LogConfig{Level: "info"}, + Worker: WorkerConfig{Concurrency: 1, BlockTimeoutS: 1, SweepIntervalS: 1}, + Search: SearchConfig{Engine: "meilisearch", Host: "http://localhost:7700"}, + } + } + + assert.NoError(t, Validate(base())) + short := base() + short.JWT.Secret = "too-short" + assert.ErrorContains(t, Validate(short), "at least 32 bytes") + placeholder := base() + placeholder.JWT.Secret = "gochat_dev_secret_change_in_production" + assert.ErrorContains(t, Validate(placeholder), "placeholder") + insecureHeaders := base() + insecureHeaders.JWT.AllowInsecureHeaderAuth = true + assert.ErrorContains(t, Validate(insecureHeaders), "header authentication") + duplicate := base() + duplicate.JWT.PreviousSecrets = []string{validSecret} + assert.ErrorContains(t, Validate(duplicate), "must be unique") +} + +func TestValidateRuntimeEnvironmentRequiresInjectedReleaseSecret(t *testing.T) { + t.Setenv("GOCHAT_JWT_SECRET", "") + t.Setenv("JWT_SECRET", "") + cfg := &Config{Server: ServerConfig{Mode: "release"}} + assert.ErrorContains(t, validateRuntimeEnvironment("prod", cfg), "requires GOCHAT_JWT_SECRET") + t.Setenv("GOCHAT_JWT_SECRET", "6vG3uP9qL2mR8xK5nD7sF4hJ1cB0wZyE") + assert.NoError(t, validateRuntimeEnvironment("production", cfg)) +} + +func TestParseJWTSecretList(t *testing.T) { + assert.Equal(t, []string{"old-one", "old-two"}, parseJWTSecretList(" old-one, ,old-two ")) } func TestValidate_InvalidLogLevel(t *testing.T) { @@ -128,7 +170,7 @@ func TestValidate_InvalidLogLevel(t *testing.T) { Server: ServerConfig{Host: "localhost", Port: 8080, Mode: "debug"}, Database: DatabaseConfig{DSN: "postgres://user@localhost:5432/db?sslmode=disable"}, Redis: RedisConfig{DSN: "redis://localhost:6379"}, - JWT: JWTConfig{Secret: "test-secret-key-min-32-chars!!"}, + JWT: JWTConfig{Secret: "6vG3uP9qL2mR8xK5nD7sF4hJ1cB0wZyE"}, Log: LogConfig{Level: "invalid"}, } @@ -188,7 +230,7 @@ func TestValidate_SearchDBFallbackRejectedInRelease(t *testing.T) { Server: ServerConfig{Host: "localhost", Port: 8080, Mode: "release"}, Database: DatabaseConfig{DSN: "postgres://user@localhost:5432/db?sslmode=disable"}, Redis: RedisConfig{DSN: "redis://localhost:6379"}, - JWT: JWTConfig{Secret: "test-secret-key-min-32-chars!!"}, + JWT: JWTConfig{Secret: "6vG3uP9qL2mR8xK5nD7sF4hJ1cB0wZyE"}, Log: LogConfig{Level: "info"}, Worker: WorkerConfig{Concurrency: 1, BlockTimeoutS: 5, SweepIntervalS: 30}, Search: SearchConfig{Engine: "db"}, diff --git a/backend/internal/config/validator.go b/backend/internal/config/validator.go index 0ffdd67d..9fc20f4e 100644 --- a/backend/internal/config/validator.go +++ b/backend/internal/config/validator.go @@ -50,9 +50,22 @@ func Validate(cfg *Config) error { } // JWT validation - if cfg.JWT.Secret == "" || cfg.JWT.Secret == "change-me-in-production" { - if cfg.Server.Mode == "release" { - return fmt.Errorf("JWT secret must be changed in production mode") + if cfg.Server.Mode == "release" { + if cfg.JWT.AllowInsecureHeaderAuth { + return fmt.Errorf("insecure header authentication is not allowed in release mode") + } + if err := validateProductionJWTSecret("JWT secret", cfg.JWT.Secret); err != nil { + return err + } + seen := map[string]bool{cfg.JWT.Secret: true} + for _, secret := range cfg.JWT.PreviousSecrets { + if err := validateProductionJWTSecret("previous JWT secret", secret); err != nil { + return err + } + if seen[secret] { + return fmt.Errorf("previous JWT secrets must be unique and differ from the active secret") + } + seen[secret] = true } } @@ -101,6 +114,20 @@ func Validate(cfg *Config) error { return nil } +func validateProductionJWTSecret(name, secret string) error { + secret = strings.TrimSpace(secret) + if len([]byte(secret)) < 32 { + return fmt.Errorf("%s must be at least 32 bytes (256 bits) in release mode", name) + } + normalized := strings.NewReplacer("-", "_", " ", "_").Replace(strings.ToLower(secret)) + for _, placeholder := range []string{"change_me", "changeme", "change_in_production", "replace_me", "your_jwt_secret", "default_secret", "example_secret", "test_secret", "quickstart"} { + if strings.Contains(normalized, placeholder) { + return fmt.Errorf("%s contains a common placeholder", name) + } + } + return nil +} + // ParseStatementTimeout converts the string timeout to time.Duration. func ParseStatementTimeout(timeout string) (time.Duration, error) { if timeout == "" { diff --git a/backend/internal/handler/api/v1/platform_account_handler.go b/backend/internal/handler/api/v1/platform_account_handler.go index b7379372..8c696bf4 100644 --- a/backend/internal/handler/api/v1/platform_account_handler.go +++ b/backend/internal/handler/api/v1/platform_account_handler.go @@ -80,7 +80,7 @@ func (h *PlatformAccountHandler) List(c *gin.Context) { // Reference: Chatwoot Platform::Api::V1::AccountsController#show // Requires: Permissible verification func (h *PlatformAccountHandler) Show(c *gin.Context) { - accountID, err := parseUintParam(c, "id") + accountID, err := parseUintAnyParam(c, "account_id", "id") if err != nil { response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrBadRequest, "invalid account ID") return @@ -154,7 +154,7 @@ func (h *PlatformAccountHandler) Create(c *gin.Context) { // Reference: Chatwoot Platform::Api::V1::AccountsController#update // Requires: Permissible verification func (h *PlatformAccountHandler) Update(c *gin.Context) { - accountID, err := parseUintParam(c, "id") + accountID, err := parseUintAnyParam(c, "account_id", "id") if err != nil { response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrBadRequest, "invalid account ID") return @@ -212,7 +212,7 @@ func (h *PlatformAccountHandler) Update(c *gin.Context) { // Reference: Chatwoot Platform::Api::V1::AccountsController#destroy // Requires: Permissible verification func (h *PlatformAccountHandler) Destroy(c *gin.Context) { - accountID, err := parseUintParam(c, "id") + accountID, err := parseUintAnyParam(c, "account_id", "id") if err != nil { response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrBadRequest, "invalid account ID") return diff --git a/backend/internal/handler/api/v1/platform_account_user_handler.go b/backend/internal/handler/api/v1/platform_account_user_handler.go index 88ffe02e..79e1c007 100644 --- a/backend/internal/handler/api/v1/platform_account_user_handler.go +++ b/backend/internal/handler/api/v1/platform_account_user_handler.go @@ -92,6 +92,10 @@ func (h *PlatformAccountUserHandler) Create(c *gin.Context) { response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrBadRequest, err.Error()) return } + if perm, err := h.permissibleRepo.FindByPlatformAppAndResource(c.Request.Context(), platformAppID, model.PermissibleTypeUser, req.UserID); err != nil || perm == nil { + response.AbortWithStatusError(c, http.StatusForbidden, response.ErrForbidden, "non permissible resource") + return + } acctUser, err := h.accountRepo.UpsertAccountUser(c.Request.Context(), accountID, req.UserID, req.Role) if err != nil { diff --git a/backend/internal/handler/api/v1/platform_e2e_test.go b/backend/internal/handler/api/v1/platform_e2e_test.go index 6ca797aa..7280f55f 100644 --- a/backend/internal/handler/api/v1/platform_e2e_test.go +++ b/backend/internal/handler/api/v1/platform_e2e_test.go @@ -93,10 +93,10 @@ func setupPlatformTokenTestE2E(t *testing.T) (*gin.Engine, *repository.Permissib platformGroup.DELETE("/users/:id", platformUser.Destroy) platformGroup.GET("/accounts", platformAccount.List) - platformGroup.GET("/accounts/:id", platformAccount.Show) + platformGroup.GET("/accounts/:account_id", platformAccount.Show) platformGroup.POST("/accounts", platformAccount.Create) - platformGroup.PATCH("/accounts/:id", platformAccount.Update) - platformGroup.DELETE("/accounts/:id", platformAccount.Destroy) + platformGroup.PATCH("/accounts/:account_id", platformAccount.Update) + platformGroup.DELETE("/accounts/:account_id", platformAccount.Destroy) platformGroup.GET("/agent_bots", platformAgentBot.List) platformGroup.GET("/agent_bots/:id", platformAgentBot.Show) @@ -105,11 +105,10 @@ func setupPlatformTokenTestE2E(t *testing.T) (*gin.Engine, *repository.Permissib platformGroup.DELETE("/agent_bots/:id", platformAgentBot.Destroy) platformGroup.POST("/agent_bots/:id/delete_avatar", platformAgentBot.DeleteAvatar) - // Gin wildcard constraint: nested routes under accounts/:id must reuse :id. - platformGroup.GET("/accounts/:id/account_users", platformAccountUser.Index) - platformGroup.POST("/accounts/:id/account_users", platformAccountUser.Create) - platformGroup.DELETE("/accounts/:id/account_users/destroy", platformAccountUser.Destroy) - platformGroup.DELETE("/accounts/:id/account_users/:user_id", platformAccountUser.Destroy) + platformGroup.GET("/accounts/:account_id/account_users", platformAccountUser.Index) + platformGroup.POST("/accounts/:account_id/account_users", platformAccountUser.Create) + platformGroup.DELETE("/accounts/:account_id/account_users/destroy", platformAccountUser.Destroy) + platformGroup.DELETE("/accounts/:account_id/account_users/:user_id", platformAccountUser.Destroy) return engine, permissibleRepo, userRepo, accountRepo } @@ -334,6 +333,25 @@ func TestPlatformAccountE2E_Show(t *testing.T) { assert.Equal(t, "Show Account", showData["name"]) } +func TestPlatformAccountE2E_Update(t *testing.T) { + engine, _, _, _ := setupPlatformTokenTestE2E(t) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/platform/api/v1/accounts", bytes.NewBufferString(`{"name":"Original Account"}`)) + req.Header.Set("Content-Type", "application/json") + engine.ServeHTTP(w, req) + require.Equal(t, http.StatusCreated, w.Code) + accountID := parseID(t, unpackData(t, w.Body.Bytes())) + + w = httptest.NewRecorder() + req = httptest.NewRequest(http.MethodPatch, "/platform/api/v1/accounts/"+accountID, bytes.NewBufferString(`{"name":"Updated Account"}`)) + req.Header.Set("Content-Type", "application/json") + engine.ServeHTTP(w, req) + + require.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "Updated Account", unpackData(t, w.Body.Bytes())["name"]) +} + func TestPlatformAccountE2E_Destroy(t *testing.T) { engine, _, _, _ := setupPlatformTokenTestE2E(t) @@ -465,6 +483,27 @@ func TestPlatformAccountUserE2E_Create(t *testing.T) { assert.Equal(t, "administrator", accountUser["role"]) } +func TestPlatformAccountUserE2E_CreateRejectsNonPermissibleUser(t *testing.T) { + engine, _, userRepo, _ := setupPlatformTokenTestE2E(t) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/platform/api/v1/accounts", bytes.NewBufferString(`{"name":"Permissible Account"}`)) + req.Header.Set("Content-Type", "application/json") + engine.ServeHTTP(w, req) + require.Equal(t, http.StatusCreated, w.Code) + accountID := parseID(t, unpackData(t, w.Body.Bytes())) + + user := &model.User{Name: "Non-permissible User", Email: "non-permissible@example.com", Provider: "email", Active: true} + require.NoError(t, userRepo.Create(t.Context(), user)) + + w = httptest.NewRecorder() + req = httptest.NewRequest(http.MethodPost, "/platform/api/v1/accounts/"+accountID+"/account_users", bytes.NewBufferString(fmt.Sprintf(`{"user_id":%d}`, user.ID))) + req.Header.Set("Content-Type", "application/json") + engine.ServeHTTP(w, req) + + assert.Equal(t, http.StatusForbidden, w.Code) +} + func TestPlatformAccountUserE2E_Index(t *testing.T) { engine, _, _, _ := setupPlatformTokenTestE2E(t) diff --git a/backend/internal/middleware/account_scope.go b/backend/internal/middleware/account_scope.go index be4aea4a..09912e6c 100644 --- a/backend/internal/middleware/account_scope.go +++ b/backend/internal/middleware/account_scope.go @@ -76,6 +76,9 @@ func AccountScope() gin.HandlerFunc { } } } + if isSuperAdminContext(c) { + roleStr = "super_admin" + } // Step 5: Get custom_role_id from JWT claims customRoleID := uint(0) @@ -150,6 +153,13 @@ func AccountScopeWithService(lookup RBACLookup) gin.HandlerFunc { } c.Set("account_id", accountID) + if isSuperAdminContext(c) { + c.Set("role", "super_admin") + c.Set("custom_role_id", uint(0)) + c.Set("policy_context", auth.NewPolicyContext(userID.(uint), accountID, "super_admin", 0, nil)) + c.Next() + return + } // Look up AccountUser to get role and CustomRoleID role, customRoleID, err := lookup.GetAccountUserRole(userID.(uint), accountID) @@ -201,6 +211,9 @@ func resolveScopedAccountID(c *gin.Context) (uint, bool) { if !hasRouteAccountID { return contextAccountID, true } + if isSuperAdminContext(c) { + return routeAccountID, true + } if contextAccountID == 0 { return routeAccountID, true } @@ -259,6 +272,11 @@ func getAccountID(c *gin.Context) uint { return 0 } +func isSuperAdminContext(c *gin.Context) bool { + userType, exists := c.Get("user_type") + return exists && isSuperAdminType(userType) +} + // --- RBACLookup Interface --- // Defines what the middleware needs from the service layer. // The service package implements this interface, avoiding circular imports. diff --git a/backend/internal/middleware/account_scope_test.go b/backend/internal/middleware/account_scope_test.go index 7be5e7e4..67664ef6 100644 --- a/backend/internal/middleware/account_scope_test.go +++ b/backend/internal/middleware/account_scope_test.go @@ -207,3 +207,21 @@ func TestAccountScopeWithService_AllowsVerifiedAccountSwitch(t *testing.T) { r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/api/v1/accounts/3", nil)) assert.Equal(t, http.StatusForbidden, w.Code, w.Body.String()) } + +func TestAccountScopeWithService_AllowsSuperAdminAcrossAccounts(t *testing.T) { + r := gin.New() + r.Use(func(c *gin.Context) { + c.Set("user_id", uint(1)) + c.Set("account_id", uint(1)) + c.Set("user_type", "super_admin") + c.Next() + }) + r.Use(AccountScopeWithService(accountScopeLookup{accountID: 999})) + r.PATCH("/api/v1/accounts/:account_id", SuperAdminOrAdministrator(), func(c *gin.Context) { + c.Status(http.StatusOK) + }) + + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodPatch, "/api/v1/accounts/2", nil)) + assert.Equal(t, http.StatusOK, w.Code, w.Body.String()) +} diff --git a/backend/internal/middleware/auth.go b/backend/internal/middleware/auth.go index 9854221c..d535277c 100644 --- a/backend/internal/middleware/auth.go +++ b/backend/internal/middleware/auth.go @@ -21,7 +21,7 @@ import ( // // Auth flow: // 1. Extract JWT from Authorization Bearer header → set user_id/account_id from claims -// 2. If no Authorization header, fall back to X-User-ID / X-Account-ID headers (dev/test mode) +// 2. If explicitly enabled, fall back to X-User-ID / X-Account-ID headers (dev/test only) // 3. Abort with 401 if neither source provides a valid identity func AuthMiddleware(cfg *config.JWTConfig) gin.HandlerFunc { jwtSvc := auth.NewJWTService(cfg) @@ -68,11 +68,12 @@ func AuthMiddlewareWithServiceAndDB(jwtSvc *auth.JWTService, db *gorm.DB) gin.Ha return } - // Fallback: X-User-ID / X-Account-ID headers for development and testing. + // Fallback: X-User-ID / X-Account-ID headers for explicitly enabled + // development and test environments only. // This allows integration tests and dev environments to bypass JWT while // still exercising the same middleware → handler pipeline. headerUserID := c.GetHeader("X-User-ID") - if headerUserID != "" { + if headerUserID != "" && jwtSvc.InsecureHeaderAuthAllowed() { userID, err := strconv.ParseUint(headerUserID, 10, 32) if err != nil { c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "invalid X-User-ID header"}) diff --git a/backend/internal/middleware/auth_test.go b/backend/internal/middleware/auth_test.go index a764bb19..6a0c3859 100644 --- a/backend/internal/middleware/auth_test.go +++ b/backend/internal/middleware/auth_test.go @@ -196,9 +196,10 @@ func TestAuthMiddleware_AllowsPlatformAdminThroughSuperAdminGuard(t *testing.T) assert.Equal(t, http.StatusOK, w.Code) } -func TestAuthMiddleware_FallbackHeaders(t *testing.T) { +func TestAuthMiddleware_FallbackHeadersRequireExplicitOptIn(t *testing.T) { gin.SetMode(gin.TestMode) cfg := makeJWTConfig() + cfg.AllowInsecureHeaderAuth = true r := gin.New() r.Use(AuthMiddleware(cfg)) r.GET("/test", func(c *gin.Context) { @@ -215,9 +216,23 @@ func TestAuthMiddleware_FallbackHeaders(t *testing.T) { assert.Equal(t, 200, w.Code) } +func TestAuthMiddleware_RejectsFallbackHeadersByDefault(t *testing.T) { + r := gin.New() + r.Use(AuthMiddleware(makeJWTConfig())) + r.GET("/test", func(c *gin.Context) { c.Status(http.StatusOK) }) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/test", nil) + req.Header.Set("X-User-ID", "5") + req.Header.Set("X-Account-ID", "10") + r.ServeHTTP(w, req) + assert.Equal(t, http.StatusUnauthorized, w.Code) +} + func TestAuthMiddleware_FallbackInvalidUserID(t *testing.T) { gin.SetMode(gin.TestMode) cfg := makeJWTConfig() + cfg.AllowInsecureHeaderAuth = true r := gin.New() r.Use(AuthMiddleware(cfg)) r.GET("/test", func(c *gin.Context) { diff --git a/backend/internal/middleware/coverage7_test.go b/backend/internal/middleware/coverage7_test.go index 5b895165..480df1dc 100644 --- a/backend/internal/middleware/coverage7_test.go +++ b/backend/internal/middleware/coverage7_test.go @@ -1343,7 +1343,7 @@ func TestAuthMiddleware_AccessTokenHeader_Cov7(t *testing.T) { } func TestAuthMiddleware_XUserID_Cov7(t *testing.T) { - jwtSvc := auth.NewJWTService(&config.JWTConfig{Secret: "test-secret", ExpiryHours: 1}) + jwtSvc := auth.NewJWTService(&config.JWTConfig{Secret: "test-secret", ExpiryHours: 1, AllowInsecureHeaderAuth: true}) w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) c.Request = httptest.NewRequest("GET", "/", nil) @@ -1355,7 +1355,7 @@ func TestAuthMiddleware_XUserID_Cov7(t *testing.T) { } func TestAuthMiddleware_InvalidXUserID_Cov7(t *testing.T) { - jwtSvc := auth.NewJWTService(&config.JWTConfig{Secret: "test-secret", ExpiryHours: 1}) + jwtSvc := auth.NewJWTService(&config.JWTConfig{Secret: "test-secret", ExpiryHours: 1, AllowInsecureHeaderAuth: true}) w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) c.Request = httptest.NewRequest("GET", "/", nil) @@ -1365,7 +1365,7 @@ func TestAuthMiddleware_InvalidXUserID_Cov7(t *testing.T) { } func TestAuthMiddleware_XUserIDWithAccount_Cov7(t *testing.T) { - jwtSvc := auth.NewJWTService(&config.JWTConfig{Secret: "test-secret", ExpiryHours: 1}) + jwtSvc := auth.NewJWTService(&config.JWTConfig{Secret: "test-secret", ExpiryHours: 1, AllowInsecureHeaderAuth: true}) w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) c.Request = httptest.NewRequest("GET", "/", nil) diff --git a/backend/internal/middleware/platform_app_auth.go b/backend/internal/middleware/platform_app_auth.go index 69057fdf..8bda4041 100644 --- a/backend/internal/middleware/platform_app_auth.go +++ b/backend/internal/middleware/platform_app_auth.go @@ -16,6 +16,7 @@ import ( "crypto/sha256" "encoding/hex" "net/http" + "time" "github.com/gin-gonic/gin" @@ -67,6 +68,11 @@ func PlatformAppAuth(db *gorm.DB) gin.HandlerFunc { "Failed to verify access token") return } + if accessToken.ExpiresAt != nil && !accessToken.ExpiresAt.After(time.Now()) { + response.AbortWithStatusError(c, http.StatusUnauthorized, response.ErrUnauthorized, + "Invalid access_token") + return + } // Step 4: Load the PlatformApp with its Permissibles var platformApp model.PlatformApp @@ -75,6 +81,11 @@ func PlatformAppAuth(db *gorm.DB) gin.HandlerFunc { "Invalid access_token") return } + if !platformApp.IsActive() { + response.AbortWithStatusError(c, http.StatusUnauthorized, response.ErrUnauthorized, + "Invalid access_token") + return + } // Step 5: Set platform_app in Gin context for downstream handlers c.Set("platform_app", platformApp) @@ -83,4 +94,4 @@ func PlatformAppAuth(db *gorm.DB) gin.HandlerFunc { c.Next() } -} \ No newline at end of file +} diff --git a/backend/internal/middleware/platform_app_auth_test.go b/backend/internal/middleware/platform_app_auth_test.go index 3853d58b..122de052 100644 --- a/backend/internal/middleware/platform_app_auth_test.go +++ b/backend/internal/middleware/platform_app_auth_test.go @@ -6,6 +6,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" @@ -96,6 +97,71 @@ func TestPlatformAppAuth_InvalidToken(t *testing.T) { assert.Equal(t, 401, w.Code) } +func TestPlatformAppAuth_InactiveOrDisabledAppRejected(t *testing.T) { + for _, tc := range []struct { + name string + active *bool + status string + }{ + {name: "inactive", active: ptrBool(false), status: "active"}, + {name: "disabled", active: ptrBool(true), status: "disabled"}, + } { + t.Run(tc.name, func(t *testing.T) { + db := newPlatformTestDB(t) + pa := model.PlatformApp{Name: tc.name, Active: tc.active, Status: tc.status} + require.NoError(t, db.Create(&pa).Error) + + rawToken := "inactive-platform-token" + hash := sha256.Sum256([]byte(rawToken)) + require.NoError(t, db.Create(&model.AccessToken{ + OwnerType: model.AccessTokenOwnerTypePlatformApp, + OwnerID: pa.ID, + Token: hex.EncodeToString(hash[:]), + TokenPrefix: rawToken[:8], + }).Error) + + router := gin.New() + router.Use(PlatformAppAuth(db)) + router.GET("/test", func(c *gin.Context) { c.Status(http.StatusOK) }) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/test", nil) + req.Header.Set("api_access_token", rawToken) + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusUnauthorized, w.Code) + }) + } +} + +func TestPlatformAppAuth_ExpiredTokenRejected(t *testing.T) { + db := newPlatformTestDB(t) + pa := model.PlatformApp{Name: "ExpiredTokenApp", Active: ptrBool(true), Status: "active"} + require.NoError(t, db.Create(&pa).Error) + + rawToken := "expired-platform-token" + hash := sha256.Sum256([]byte(rawToken)) + expiresAt := time.Now().Add(-time.Minute) + require.NoError(t, db.Create(&model.AccessToken{ + OwnerType: model.AccessTokenOwnerTypePlatformApp, + OwnerID: pa.ID, + Token: hex.EncodeToString(hash[:]), + TokenPrefix: rawToken[:8], + ExpiresAt: &expiresAt, + }).Error) + + router := gin.New() + router.Use(PlatformAppAuth(db)) + router.GET("/test", func(c *gin.Context) { c.Status(http.StatusOK) }) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/test", nil) + req.Header.Set("api_access_token", rawToken) + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusUnauthorized, w.Code) +} + func TestPlatformAppAuth_UserTokenRejected(t *testing.T) { db := newPlatformTestDB(t) router := gin.New() @@ -205,4 +271,4 @@ func TestPlatformAppAuth_ContextValues(t *testing.T) { assert.Equal(t, 200, w.Code) } -func ptrBool(b bool) *bool { return &b } \ No newline at end of file +func ptrBool(b bool) *bool { return &b } diff --git a/backend/internal/router/router.go b/backend/internal/router/router.go index 74218109..84c1da63 100644 --- a/backend/internal/router/router.go +++ b/backend/internal/router/router.go @@ -292,15 +292,14 @@ func RegisterRoutes( enterpriseV1.Use(middleware.AuthMiddlewareWithServiceAndDB(jwtService, db)) registerEnterpriseRoutes(enterpriseV1, handlers) - // Platform API routes — super admin only (ref: Chatwoot namespace :platform_app) - // Chatwoot uses a single /platform/api/v1 prefix with AccessTokenable concern in controller layer - // for auth differentiation. We merge SuperAdmin and AccessToken routes into one group. - platform := engine.Group("/platform/api/v1") - platform.Use(middleware.AuthMiddlewareWithServiceAndDB(jwtService, db)) - // SuperAdmin-only routes use additional middleware; AccessToken routes are open to platform app tokens - // Register both sets of routes in the same group (Gin does not allow duplicate prefix groups) - registerPlatformRoutes(platform, handlers) - registerPlatformTokenRoutes(platform, handlers) + // Platform administration uses signed super-admin identity; the Platform API + // uses a distinct PlatformApp access token, matching Chatwoot's controllers. + platformAdmin := engine.Group("/platform/api/v1") + platformAdmin.Use(middleware.AuthMiddlewareWithServiceAndDB(jwtService, db), middleware.SuperAdmin()) + registerPlatformRoutes(platformAdmin, handlers) + platformToken := engine.Group("/platform/api/v1") + platformToken.Use(middleware.PlatformAppAuth(db)) + registerPlatformTokenRoutes(platformToken, handlers) // Widget API routes — public + CORS for embed (ref: Chatwoot namespace :widget_api) widget := engine.Group("/widget") @@ -692,7 +691,7 @@ func registerV1Routes(g *gin.RouterGroup, h *Handlers) { // Account routes — scoped with AccountScope middleware (ref: Chatwoot namespace :accounts) // GetAll is outside AccountScope — platform admin level listing of all accounts - g.GET("/accounts/all", h.Account.GetAll) + g.GET("/accounts/all", middleware.SuperAdmin(), h.Account.GetAll) // Chatwoot account creation is user-scoped and the frontend posts to the // no-trailing-slash collection path before a new account id exists. g.POST("/accounts", h.Account.Create) @@ -703,15 +702,15 @@ func registerV1Routes(g *gin.RouterGroup, h *Handlers) { accounts.GET("/", h.Account.List) accounts.POST("/", h.Account.Create) accounts.GET("/:account_id", h.Account.Get) - accounts.PATCH("/:account_id", h.Account.Update) - accounts.PUT("/:account_id", h.Account.Update) - accounts.DELETE("/:account_id", h.Account.Delete) + accounts.PATCH("/:account_id", middleware.SuperAdminOrAdministrator(), h.Account.Update) + accounts.PUT("/:account_id", middleware.SuperAdminOrAdministrator(), h.Account.Update) + accounts.DELETE("/:account_id", middleware.SuperAdminOrAdministrator(), h.Account.Delete) // Account onboarding update (ref: Chatwoot resource :onboarding, only: [:update]) accounts.PATCH("/:account_id/onboarding", middleware.RoleCheck("administrator"), h.Account.UpdateOnboarding) accounts.GET("/:account_id/onboarding/help_center_generation", middleware.RoleCheck("administrator"), h.Account.HelpCenterGeneration) // Account settings (ref: Chatwoot accounts#update settings subset) - accounts.PUT("/:account_id/settings", h.Account.UpdateSettings) + accounts.PUT("/:account_id/settings", middleware.SuperAdminOrAdministrator(), h.Account.UpdateSettings) // Account file upload route (ref: Chatwoot api/v1/accounts/:account_id/upload) accounts.POST("/:account_id/upload", h.Upload.Upload) @@ -2076,7 +2075,6 @@ func accountScopeMiddleware(h *Handlers) gin.HandlerFunc { // Reference: Chatwoot namespace :platform_app (super_admin only) func registerPlatformRoutes(g *gin.RouterGroup, h *Handlers) { copilot := g.Group("/copilot") - copilot.Use(middleware.SuperAdmin()) { copilot.GET("/config", h.CopilotConfig.PlatformGet) copilot.PUT("/config", h.CopilotConfig.PlatformUpdate) diff --git a/backend/internal/router/router_test.go b/backend/internal/router/router_test.go index 73776203..0be4864c 100644 --- a/backend/internal/router/router_test.go +++ b/backend/internal/router/router_test.go @@ -121,6 +121,56 @@ func TestRegisterRoutesBootsWithChatwootParityConflictGroups(t *testing.T) { } } +func TestAccountManagementRoutesRejectUntrustedIdentityAndAgentAccess(t *testing.T) { + gin.SetMode(gin.TestMode) + jwtCfg := &config.JWTConfig{Secret: "route-security-test-secret", ExpiryHours: 1, RefreshExpiryHours: 24} + jwtSvc := auth.NewJWTService(jwtCfg) + tokens, err := jwtSvc.GenerateTokenPair(&model.User{ + Base: model.Base{ID: 7}, + Provider: "email", + }, 1, "agent") + if err != nil { + t.Fatalf("generate token: %v", err) + } + + engine := gin.New() + RegisterRoutes(engine, jwtSvc, nil, nil, &Handlers{}, nil, nil, jwtCfg, middleware.CORSConfig{}, nil) + + cases := []struct { + name string + method string + path string + token string + headers bool + wantStatus int + }{ + {name: "release headers only", method: http.MethodGet, path: "/api/v1/accounts/all", headers: true, wantStatus: http.StatusUnauthorized}, + {name: "agent lists all accounts", method: http.MethodGet, path: "/api/v1/accounts/all", token: tokens.AccessToken, wantStatus: http.StatusForbidden}, + {name: "agent uses platform administration", method: http.MethodGet, path: "/platform/api/v1/apps", token: tokens.AccessToken, wantStatus: http.StatusForbidden}, + {name: "agent updates account", method: http.MethodPatch, path: "/api/v1/accounts/1", token: tokens.AccessToken, wantStatus: http.StatusForbidden}, + {name: "agent updates settings", method: http.MethodPut, path: "/api/v1/accounts/1/settings", token: tokens.AccessToken, wantStatus: http.StatusForbidden}, + {name: "agent deletes account", method: http.MethodDelete, path: "/api/v1/accounts/1", token: tokens.AccessToken, wantStatus: http.StatusForbidden}, + {name: "cross tenant update", method: http.MethodPatch, path: "/api/v1/accounts/2", token: tokens.AccessToken, wantStatus: http.StatusForbidden}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + w := httptest.NewRecorder() + req := httptest.NewRequest(tc.method, tc.path, nil) + if tc.token != "" { + req.Header.Set("access-token", tc.token) + } + if tc.headers { + req.Header.Set("X-User-ID", "7") + req.Header.Set("X-Account-ID", "1") + } + engine.ServeHTTP(w, req) + if w.Code != tc.wantStatus { + t.Fatalf("expected %d, got %d: %s", tc.wantStatus, w.Code, w.Body.String()) + } + }) + } +} + func TestAPIV2LiveReportsRouterAuthAndAccountScope(t *testing.T) { gin.SetMode(gin.TestMode) db, err := gorm.Open(sqlite.Open("file::memory:?cache=shared"), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)})