From 93bc2e5dbff25d34640cfbe94ee23402201e55d6 Mon Sep 17 00:00:00 2001 From: Tanvir Ahmed Date: Sat, 4 Jul 2026 18:41:45 +0000 Subject: [PATCH] fix: Admin impersonation - Fixed issue with admin not being able to stop impersonation due to their scopes not being persisted and passed through actor context. - Admin session recoverability implemented after stopping impersonation, allowing them to continue their session without needing to log in again. chore: JWT plugin now extract generic claims and have no reference to impersonation chore: Updated example env file fix: Issue with session state read permission blocking stop impersonation service path chore: Session & JWT flows fully working --- .agents/skills/handlers-and-http/SKILL.md | 2 +- .env.example | 6 +- internal/util/validation.go | 3 + .../access-control/constants/permissions.go | 1 + plugins/access-control/hooks.go | 2 + plugins/access-control/plugin.go | 1 + plugins/admin/api.go | 8 +- plugins/admin/constants/constants.go | 8 + plugins/admin/constants/errors.go | 14 +- plugins/admin/constants/permissions.go | 1 + .../admin/handlers/impersonation_handlers.go | 157 +++++++++++++++--- .../handlers/impersonation_handlers_test.go | 64 ++++--- plugins/admin/handlers/users_handlers.go | 26 ++- plugins/admin/handlers/users_handlers_test.go | 4 +- plugins/admin/hooks.go | 106 ++++++++++++ plugins/admin/migrations.go | 3 + plugins/admin/plugin.go | 15 +- plugins/admin/routes.go | 16 +- .../admin/services/impersonation_service.go | 42 ++++- .../services/impersonation_service_test.go | 52 +++++- plugins/admin/types/api.go | 131 ++++++++++++--- plugins/admin/types/models.go | 1 + plugins/admin/usecases/admin_usecases.go | 8 +- .../admin/usecases/impersonation_usecase.go | 44 ++++- .../usecases/impersonation_usecase_test.go | 60 +++++-- plugins/api-key/constants/permissions.go | 1 + plugins/api-key/plugin.go | 1 + plugins/bearer/hooks.go | 12 ++ plugins/jwt/hooks.go | 22 ++- plugins/jwt/hooks_test.go | 4 +- plugins/jwt/services/interfaces.go | 3 +- plugins/jwt/services/refresh_token_service.go | 29 +++- .../services/refresh_token_service_test.go | 6 +- plugins/jwt/services/token_service.go | 51 +++++- plugins/jwt/services/token_service_test.go | 2 +- plugins/jwt/tests/mocks.go | 12 +- plugins/organizations/constants/constants.go | 1 + plugins/organizations/plugin.go | 1 + 38 files changed, 762 insertions(+), 158 deletions(-) create mode 100644 plugins/admin/constants/constants.go diff --git a/.agents/skills/handlers-and-http/SKILL.md b/.agents/skills/handlers-and-http/SKILL.md index a86e6880..fd91a8d4 100644 --- a/.agents/skills/handlers-and-http/SKILL.md +++ b/.agents/skills/handlers-and-http/SKILL.md @@ -16,7 +16,7 @@ description: Implement HTTP handlers that parse requests, invoke use cases, and ## Key principles -1. **Thin handlers**: Business logic lives in services, not handlers. +1. **Thin handlers**: Business logic lives in services, not handlers. Always remember to only extract and validate request data, invoke the appropriate service or use case, and format the response. Avoid embedding business logic directly in the handler to maintain separation of concerns and improve testability. 2. **Service coordination**: Handlers invoke services which are interfaces 3. **HTTP boundaries**: Handle only HTTP serialization and status codes, as well as request parsing and validation. 4. **Error mapping**: Map domain errors returned from services to HTTP status codes via `constants/errors.go`. diff --git a/.env.example b/.env.example index 64b595de..0cae6dde 100644 --- a/.env.example +++ b/.env.example @@ -20,15 +20,13 @@ EVENT_BUS_CONSUMER_GROUP= # Authula ENV variables -# Path to your configuration file (e.g., ./config.toml) +# Path to your configuration file (e.g., ./config.toml) (optional) AUTHULA_CONFIG_PATH= -# The base URL of your API +# The base URL of your API (optional if configured in the config file) AUTHULA_BASE_URL= # Used for encryption. Generate using `openssl rand -hex 32` AUTHULA_SECRET= -# sqlite: "auth.db" # postgresql: "postgresql://username:password@localhost:5432/authula?sslmode=disable" -# mysql: "username:password@tcp(localhost:3306)/authula" AUTHULA_DATABASE_URL= GO_ENV=development diff --git a/internal/util/validation.go b/internal/util/validation.go index 881adc01..13129d09 100644 --- a/internal/util/validation.go +++ b/internal/util/validation.go @@ -18,6 +18,9 @@ func InitValidator() { } func ValidateStruct(s any) error { + if Validate == nil { + InitValidator() + } return Validate.Struct(s) } diff --git a/plugins/access-control/constants/permissions.go b/plugins/access-control/constants/permissions.go index 76460b15..8c87cc60 100644 --- a/plugins/access-control/constants/permissions.go +++ b/plugins/access-control/constants/permissions.go @@ -2,6 +2,7 @@ package constants // Access control permissions const ( + All = "access-control:*" RolesCreatePermission = "access-control:roles:create" RolesListPermission = "access-control:roles:list" RolesReadPermission = "access-control:roles:read" diff --git a/plugins/access-control/hooks.go b/plugins/access-control/hooks.go index 7672178b..9de5c5a7 100644 --- a/plugins/access-control/hooks.go +++ b/plugins/access-control/hooks.go @@ -72,8 +72,10 @@ func (p *AccessControlPlugin) hydrateActorScopes(reqCtx *models.RequestContext) } } } + case models.ActorMachine: } + return nil } diff --git a/plugins/access-control/plugin.go b/plugins/access-control/plugin.go index 9cdb9fd2..1902ab16 100644 --- a/plugins/access-control/plugin.go +++ b/plugins/access-control/plugin.go @@ -100,6 +100,7 @@ func (p *AccessControlPlugin) Close() error { func (p *AccessControlPlugin) ensurePermissions() error { if err := p.accessControlService.EnsurePermissions(context.Background(), []rootservices.PermissionDefinition{ + {Key: accesscontrolconstants.All, Description: "All access control permissions"}, {Key: accesscontrolconstants.RolesCreatePermission, Description: "Create roles in the access control system"}, {Key: accesscontrolconstants.RolesListPermission, Description: "List roles in the access control system"}, {Key: accesscontrolconstants.RolesReadPermission, Description: "Read a role in the access control system"}, diff --git a/plugins/admin/api.go b/plugins/admin/api.go index 22035822..5f205de4 100644 --- a/plugins/admin/api.go +++ b/plugins/admin/api.go @@ -96,12 +96,12 @@ func (a *API) GetImpersonationByID(ctx context.Context, actor *models.Actor, imp return a.useCases.GetImpersonationByID(ctx, actor, impersonationID) } -func (a *API) StartImpersonation(ctx context.Context, actor *models.Actor, actorUserID string, actorSessionID *string, ipAddress *string, userAgent *string, req types.StartImpersonationRequest) (*types.StartImpersonationResult, error) { - return a.useCases.StartImpersonation(ctx, actor, actorUserID, actorSessionID, ipAddress, userAgent, req) +func (a *API) StartImpersonation(ctx context.Context, actor *models.Actor, actorUserID string, actorSessionID *string, ipAddress *string, userAgent *string, req types.StartImpersonationRequest, impersonatorScopes []string, originalCookieValue string, originalCookieMaxAge int) (*types.StartImpersonationResult, error) { + return a.useCases.StartImpersonation(ctx, actor, actorUserID, actorSessionID, ipAddress, userAgent, req, impersonatorScopes, originalCookieValue, originalCookieMaxAge) } -func (a *API) StopImpersonation(ctx context.Context, actor *models.Actor, impersonatedUserID string, impersonatedSessionID string, req types.StopImpersonationRequest) error { - return a.useCases.StopImpersonation(ctx, actor, impersonatedUserID, impersonatedSessionID, req) +func (a *API) StopImpersonation(ctx context.Context, actor *models.Actor, impersonatedUserID string, impersonatedSessionID string, originalCookieValue string, req types.StopImpersonationRequest) (*types.StopImpersonationResult, error) { + return a.useCases.StopImpersonation(ctx, actor, impersonatedUserID, impersonatedSessionID, originalCookieValue, req) } // User state diff --git a/plugins/admin/constants/constants.go b/plugins/admin/constants/constants.go new file mode 100644 index 00000000..678cd8c4 --- /dev/null +++ b/plugins/admin/constants/constants.go @@ -0,0 +1,8 @@ +package constants + +const ( + ImpersonatorID = "impersonator_id" + ImpersonatorScopes = "impersonator_scopes" + ImpersonatorOriginalSessionToken = "impersonator_original_session_token" + OriginalSessionCookieSuffix = ".original" +) diff --git a/plugins/admin/constants/errors.go b/plugins/admin/constants/errors.go index bff2481d..d7c805d2 100644 --- a/plugins/admin/constants/errors.go +++ b/plugins/admin/constants/errors.go @@ -3,10 +3,12 @@ package constants import "errors" var ( - ErrUserIDRequired = errors.New("user ID is required") - ErrProviderIDRequired = errors.New("provider ID is required") - ErrAccountIDRequired = errors.New("account ID is required") - ErrAccessTokenExpiresAtBeforeNow = errors.New("access token expiration time must be in the future") - ErrRefreshTokenExpiresAtBeforeNow = errors.New("refresh token expiration time must be in the future") - ErrNoPropertiesProvided = errors.New("at least one property must be provided for update") + ErrUserIDRequired = errors.New("user ID is required") + ErrProviderIDRequired = errors.New("provider ID is required") + ErrAccountIDRequired = errors.New("account ID is required") + ErrAccessTokenExpiresAtBeforeNow = errors.New("access token expiration time must be in the future") + ErrRefreshTokenExpiresAtBeforeNow = errors.New("refresh token expiration time must be in the future") + ErrNoPropertiesProvided = errors.New("at least one property must be provided for update") + ErrBannedUntilBeforeNow = errors.New("banned until time must be in the future") + ErrImpersonationExpiresAtBeforeNow = errors.New("impersonation expiration time must be in the future") ) diff --git a/plugins/admin/constants/permissions.go b/plugins/admin/constants/permissions.go index 324fcb4e..9ca166c9 100644 --- a/plugins/admin/constants/permissions.go +++ b/plugins/admin/constants/permissions.go @@ -1,6 +1,7 @@ package constants const ( + All = "admin:*" UsersCreatePermission = "admin:users:create" UsersListPermission = "admin:users:list" UsersReadPermission = "admin:users:read" diff --git a/plugins/admin/handlers/impersonation_handlers.go b/plugins/admin/handlers/impersonation_handlers.go index df8a9f9b..21d70264 100644 --- a/plugins/admin/handlers/impersonation_handlers.go +++ b/plugins/admin/handlers/impersonation_handlers.go @@ -5,6 +5,7 @@ import ( "github.com/Authula/authula/internal/util" "github.com/Authula/authula/models" + "github.com/Authula/authula/plugins/admin/constants" "github.com/Authula/authula/plugins/admin/types" "github.com/Authula/authula/plugins/admin/usecases" ) @@ -20,9 +21,8 @@ func NewGetAllImpersonationsHandler(useCase usecases.ImpersonationUseCase) *GetA func (h *GetAllImpersonationsHandler) Handler() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { reqCtx, _ := models.GetRequestContext(r.Context()) - actor := reqCtx.Actor - rows, err := h.useCase.GetAllImpersonations(r.Context(), actor) + rows, err := h.useCase.GetAllImpersonations(r.Context(), reqCtx.Actor) if err != nil { respondImpersonationError(reqCtx, err) return @@ -43,10 +43,9 @@ func NewGetImpersonationByIDHandler(useCase usecases.ImpersonationUseCase) *GetI func (h *GetImpersonationByIDHandler) Handler() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { reqCtx, _ := models.GetRequestContext(r.Context()) - actor := reqCtx.Actor - impersonationID := r.PathValue("impersonation_id") - impersonation, err := h.useCase.GetImpersonationByID(r.Context(), actor, impersonationID) + impersonationID := r.PathValue("impersonation_id") + impersonation, err := h.useCase.GetImpersonationByID(r.Context(), reqCtx.Actor, impersonationID) if err != nil { respondImpersonationError(reqCtx, err) return @@ -57,35 +56,62 @@ func (h *GetImpersonationByIDHandler) Handler() http.HandlerFunc { } type StartImpersonationHandler struct { - useCase usecases.ImpersonationUseCase + useCase usecases.ImpersonationUseCase + globalConfig *models.Config } -func NewStartImpersonationHandler(useCase usecases.ImpersonationUseCase) *StartImpersonationHandler { - return &StartImpersonationHandler{useCase: useCase} +func NewStartImpersonationHandler( + useCase usecases.ImpersonationUseCase, + globalConfig *models.Config, +) *StartImpersonationHandler { + return &StartImpersonationHandler{ + useCase: useCase, + globalConfig: globalConfig, + } } func (h *StartImpersonationHandler) Handler() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() - reqCtx, _ := models.GetRequestContext(ctx) + reqCtx, _ := models.GetRequestContext(r.Context()) actor := reqCtx.Actor impersonatorUserID := getUserID(reqCtx) - if impersonatorUserID == nil { reqCtx.SetJSONResponse(http.StatusUnauthorized, map[string]any{"message": "Unauthorized"}) reqCtx.Handled = true return } - var payload types.StartImpersonationRequest - if err := util.ParseJSON(r, &payload); err != nil { - reqCtx.SetJSONResponse(http.StatusUnprocessableEntity, map[string]any{"message": "invalid request body"}) + impersonatorScopes := make([]string, len(reqCtx.Actor.Scopes)) + copy(impersonatorScopes, reqCtx.Actor.Scopes) + + var req types.StartImpersonationRequest + if err := util.ParseJSON(r, &req); err != nil { + reqCtx.SetJSONResponse(http.StatusUnprocessableEntity, map[string]any{"message": err.Error()}) + reqCtx.Handled = true + return + } + if err := req.Validate(); err != nil { + reqCtx.SetJSONResponse(http.StatusUnprocessableEntity, map[string]any{"message": err.Error()}) reqCtx.Handled = true return } + originalCookieValue, _ := r.Cookie(h.globalConfig.Session.CookieName) + originalCookieMaxAge := 0 + if originalCookieValue != nil { + originalCookieMaxAge = originalCookieValue.MaxAge + } + originalCookieVal := "" + if originalCookieValue != nil { + originalCookieVal = originalCookieValue.Value + } + userAgent := r.UserAgent() - result, err := h.useCase.StartImpersonation(r.Context(), actor, *impersonatorUserID, getSessionID(reqCtx), &reqCtx.ClientIP, &userAgent, payload) + result, err := h.useCase.StartImpersonation( + r.Context(), actor, *impersonatorUserID, getSessionID(reqCtx), + &reqCtx.ClientIP, &userAgent, req, + impersonatorScopes, originalCookieVal, originalCookieMaxAge, + ) if err != nil { respondImpersonationError(reqCtx, err) return @@ -97,9 +123,28 @@ func (h *StartImpersonationHandler) Handler() http.HandlerFunc { return } + sessionConfig := h.globalConfig.Session + sameSite := sameSiteFromSessionConfig(&sessionConfig) + + if result.OriginalCookieToken != "" { + http.SetCookie(w, &http.Cookie{ + Name: sessionConfig.CookieName + constants.OriginalSessionCookieSuffix, + Value: result.OriginalCookieToken, + Path: "/", + HttpOnly: sessionConfig.HttpOnly, + Secure: sessionConfig.Secure, + SameSite: sameSite, + MaxAge: result.OriginalCookieMaxAge, + }) + } + reqCtx.SetActorInContext(&models.Actor{ - ID: result.Impersonation.TargetUserID, + ID: result.TargetUserID, Type: models.ActorUser, + Claims: map[string]any{ + constants.ImpersonatorID: result.ImpersonatorUserID, + constants.ImpersonatorScopes: result.ImpersonatorScopes, + }, }) if result.SessionID != nil && *result.SessionID != "" { reqCtx.Values[models.ContextSessionID.String()] = *result.SessionID @@ -116,18 +161,23 @@ func (h *StartImpersonationHandler) Handler() http.HandlerFunc { } type StopImpersonationHandler struct { - useCase usecases.ImpersonationUseCase + useCase usecases.ImpersonationUseCase + globalConfig *models.Config } -func NewStopImpersonationHandler(useCase usecases.ImpersonationUseCase) *StopImpersonationHandler { - return &StopImpersonationHandler{useCase: useCase} +func NewStopImpersonationHandler( + useCase usecases.ImpersonationUseCase, + globalConfig *models.Config, +) *StopImpersonationHandler { + return &StopImpersonationHandler{ + useCase: useCase, + globalConfig: globalConfig, + } } func (h *StopImpersonationHandler) Handler() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() - reqCtx, _ := models.GetRequestContext(ctx) - actor := reqCtx.Actor + reqCtx, _ := models.GetRequestContext(r.Context()) impersonatedUserID := getUserID(reqCtx) impersonatedSessionID := getSessionID(reqCtx) @@ -137,13 +187,57 @@ func (h *StopImpersonationHandler) Handler() http.HandlerFunc { return } + originalCookieValue := "" + originalCookie, err := r.Cookie(h.globalConfig.Session.CookieName + constants.OriginalSessionCookieSuffix) + if err == nil { + originalCookieValue = originalCookie.Value + } + + if originalCookieValue == "" { + if reqCtx.Actor == nil || reqCtx.Actor.Claims == nil { + reqCtx.SetJSONResponse(http.StatusUnauthorized, map[string]any{"message": "no original session found"}) + reqCtx.Handled = true + return + } + if _, ok := reqCtx.Actor.Claims[constants.ImpersonatorID]; !ok { + reqCtx.SetJSONResponse(http.StatusUnauthorized, map[string]any{"message": "no original session found"}) + reqCtx.Handled = true + return + } + } + impersonationID := r.PathValue("impersonation_id") - if err := h.useCase.StopImpersonation(r.Context(), actor, *impersonatedUserID, *impersonatedSessionID, types.StopImpersonationRequest{ImpersonationID: &impersonationID}); err != nil { + result, err := h.useCase.StopImpersonation( + r.Context(), reqCtx.Actor, *impersonatedUserID, *impersonatedSessionID, + originalCookieValue, + types.StopImpersonationRequest{ImpersonationID: &impersonationID}, + ) + if err != nil { respondImpersonationError(reqCtx, err) return } - reqCtx.Values[models.ContextAuthSignOut.String()] = true + sessionConfig := h.globalConfig.Session + sameSite := sameSiteFromSessionConfig(&sessionConfig) + + http.SetCookie(w, &http.Cookie{ + Name: sessionConfig.CookieName, + Value: result.OriginalSessionToken, + Path: "/", + HttpOnly: sessionConfig.HttpOnly, + Secure: sessionConfig.Secure, + SameSite: sameSite, + }) + + http.SetCookie(w, &http.Cookie{ + Name: sessionConfig.CookieName + constants.OriginalSessionCookieSuffix, + Value: "", + Path: "/", + HttpOnly: sessionConfig.HttpOnly, + Secure: sessionConfig.Secure, + SameSite: sameSite, + MaxAge: -1, + }) reqCtx.SetJSONResponse(http.StatusOK, &types.StopImpersonationResponse{Message: "Impersonation stopped"}) } @@ -161,12 +255,10 @@ func getSessionID(reqCtx *models.RequestContext) *string { if !ok || value == nil { return nil } - sessionID, ok := value.(string) if !ok || sessionID == "" { return nil } - return &sessionID } @@ -178,3 +270,16 @@ func respondImpersonationError(reqCtx *models.RequestContext, err error) { func mapImpersonationErrorStatus(err error) int { return mapAdminHttpErrorStatus(err) } + +func sameSiteFromSessionConfig(sessionConfig *models.SessionConfig) http.SameSite { + switch sessionConfig.SameSite { + case "strict": + return http.SameSiteStrictMode + case "none": + return http.SameSiteNoneMode + case "lax": + return http.SameSiteLaxMode + default: + return http.SameSiteLaxMode + } +} diff --git a/plugins/admin/handlers/impersonation_handlers_test.go b/plugins/admin/handlers/impersonation_handlers_test.go index f9c9999e..8b6b1c4e 100644 --- a/plugins/admin/handlers/impersonation_handlers_test.go +++ b/plugins/admin/handlers/impersonation_handlers_test.go @@ -11,11 +11,23 @@ import ( internalerrors "github.com/Authula/authula/internal/errors" internaltests "github.com/Authula/authula/internal/tests" "github.com/Authula/authula/models" + adminconstants "github.com/Authula/authula/plugins/admin/constants" adminhandlers "github.com/Authula/authula/plugins/admin/handlers" admintests "github.com/Authula/authula/plugins/admin/tests" "github.com/Authula/authula/plugins/admin/types" ) +func testGlobalConfig() *models.Config { + return &models.Config{ + Session: models.SessionConfig{ + CookieName: "authula.session_token", + HttpOnly: true, + Secure: false, + SameSite: "lax", + }, + } +} + func TestGetAllImpersonationsHandler(t *testing.T) { t.Parallel() @@ -100,7 +112,7 @@ func TestStartImpersonationHandler_Unauthorized(t *testing.T) { t.Parallel() useCase, _, _, _, _ := admintests.NewImpersonationUseCaseFixture(t) - handler := adminhandlers.NewStartImpersonationHandler(useCase) + handler := adminhandlers.NewStartImpersonationHandler(useCase, testGlobalConfig()) req, w, reqCtx := internaltests.NewHandlerRequest(t, http.MethodPost, "/admin/impersonations", internaltests.MarshalToJSON(t, types.StartImpersonationRequest{TargetUserID: "target-1", Reason: "support"}), nil) @@ -113,7 +125,7 @@ func TestStartImpersonationHandler_InvalidJSON(t *testing.T) { t.Parallel() useCase, _, _, _, _ := admintests.NewImpersonationUseCaseFixture(t) - handler := adminhandlers.NewStartImpersonationHandler(useCase) + handler := adminhandlers.NewStartImpersonationHandler(useCase, testGlobalConfig()) req, w, reqCtx := internaltests.NewHandlerRequest(t, http.MethodPost, "/admin/impersonations", []byte("{invalid"), nil) userID := "actor-1" @@ -121,7 +133,7 @@ func TestStartImpersonationHandler_InvalidJSON(t *testing.T) { handler.Handler()(w, req) - internaltests.AssertErrorMessage(t, reqCtx, http.StatusUnprocessableEntity, "invalid request body") + internaltests.AssertErrorMessage(t, reqCtx, http.StatusUnprocessableEntity, "invalid character 'i' looking for beginning of object key string") } func TestStartImpersonationHandler_UseCaseError(t *testing.T) { @@ -131,7 +143,7 @@ func TestStartImpersonationHandler_UseCaseError(t *testing.T) { impRepo.On("UserExists", mock.Anything, "actor-1").Return(true, nil).Once() impRepo.On("UserExists", mock.Anything, "target-1").Return(true, nil).Once() tokenSvc.On("Generate").Return("", internalerrors.ErrForbidden).Once() - handler := adminhandlers.NewStartImpersonationHandler(useCase) + handler := adminhandlers.NewStartImpersonationHandler(useCase, testGlobalConfig()) req, w, reqCtx := internaltests.NewHandlerRequest(t, http.MethodPost, "/admin/impersonations", internaltests.MarshalToJSON(t, types.StartImpersonationRequest{TargetUserID: "target-1", Reason: "support"}), nil) userID := "actor-1" @@ -167,7 +179,7 @@ func TestStartImpersonationHandler_SuccessSetsContextValues(t *testing.T) { ).Return(&models.Session{ID: sessionID, IPAddress: internaltests.PtrString(ipAddress), UserAgent: internaltests.PtrString(userAgent)}, nil).Once() impRepo.On("CreateImpersonation", mock.Anything, mock.AnythingOfType("*types.Impersonation")).Return(nil).Once() sessionStateRepo.On("Upsert", mock.Anything, mock.AnythingOfType("*types.AdminSessionState")).Return(nil).Once() - handler := adminhandlers.NewStartImpersonationHandler(useCase) + handler := adminhandlers.NewStartImpersonationHandler(useCase, testGlobalConfig()) req, w, reqCtx := internaltests.NewHandlerRequest(t, http.MethodPost, "/admin/impersonations", internaltests.MarshalToJSON(t, types.StartImpersonationRequest{TargetUserID: "target-1", Reason: "support"}), nil) req.Header.Set("User-Agent", userAgent) @@ -210,7 +222,7 @@ func TestStopImpersonationHandler(t *testing.T) { t.Parallel() useCase, _, _, _, _ := admintests.NewImpersonationUseCaseFixture(t) - handler := adminhandlers.NewStopImpersonationHandler(useCase) + handler := adminhandlers.NewStopImpersonationHandler(useCase, testGlobalConfig()) req, w, reqCtx := internaltests.NewHandlerRequest(t, http.MethodPost, "/admin/impersonations/imp-1/stop", nil, nil) req.SetPathValue("impersonation_id", "imp-1") @@ -223,7 +235,7 @@ func TestStopImpersonationHandler(t *testing.T) { t.Parallel() useCase, _, _, _, _ := admintests.NewImpersonationUseCaseFixture(t) - handler := adminhandlers.NewStopImpersonationHandler(useCase) + handler := adminhandlers.NewStopImpersonationHandler(useCase, testGlobalConfig()) req, w, reqCtx := internaltests.NewHandlerRequest(t, http.MethodPost, "/admin/impersonations/imp-1/stop", nil, nil) req.SetPathValue("impersonation_id", "imp-1") actorID := "actor-1" @@ -234,40 +246,46 @@ func TestStopImpersonationHandler(t *testing.T) { internaltests.AssertErrorMessage(t, reqCtx, http.StatusUnauthorized, "Unauthorized") }) - t.Run("error", func(t *testing.T) { + t.Run("no original cookie", func(t *testing.T) { t.Parallel() - useCase, impRepo, sessionStateRepo, _, _ := admintests.NewImpersonationUseCaseFixture(t) - actorID := "actor-1" - sessionStateRepo.On("GetBySessionID", mock.Anything, "session-1").Return(&types.AdminSessionState{SessionID: "session-1", ImpersonatorUserID: &actorID}, nil).Once() - impRepo.On("GetActiveImpersonationByID", mock.Anything, "imp-1").Return((*types.Impersonation)(nil), nil).Once() - handler := adminhandlers.NewStopImpersonationHandler(useCase) + useCase, _, _, _, _ := admintests.NewImpersonationUseCaseFixture(t) + handler := adminhandlers.NewStopImpersonationHandler(useCase, testGlobalConfig()) req, w, reqCtx := internaltests.NewHandlerRequest(t, http.MethodPost, "/admin/impersonations/imp-1/stop", nil, nil) req.SetPathValue("impersonation_id", "imp-1") - reqCtx.Actor = &models.Actor{ID: actorID, Type: models.ActorUser} + reqCtx.Actor = &models.Actor{ID: "actor-1", Type: models.ActorUser} reqCtx.Values[models.ContextSessionID.String()] = "session-1" handler.Handler()(w, req) - internaltests.AssertErrorMessage(t, reqCtx, http.StatusNotFound, "not found") - impRepo.AssertExpectations(t) - sessionStateRepo.AssertExpectations(t) + internaltests.AssertErrorMessage(t, reqCtx, http.StatusUnauthorized, "no original session found") }) t.Run("success", func(t *testing.T) { t.Parallel() actorID := "actor-1" - - useCase, impRepo, sessionStateRepo, _, _ := admintests.NewImpersonationUseCaseFixture(t) - sessionStateRepo.On("GetBySessionID", mock.Anything, "session-1").Return(&types.AdminSessionState{SessionID: "session-1", ImpersonatorUserID: &actorID}, nil).Once() - impRepo.On("GetActiveImpersonationByID", mock.Anything, "imp-1").Return(&types.Impersonation{ID: "imp-1", ActorUserID: "actor-1"}, nil).Once() + originalSessionToken := "orig-session-token" + + useCase, impRepo, sessionStateRepo, sessSvc, tokenSvc := admintests.NewImpersonationUseCaseFixture(t) + origSessionID := "orig-session" + sessionStateRepo.On("GetBySessionID", mock.Anything, "session-1").Return(&types.AdminSessionState{SessionID: "session-1", ImpersonatorUserID: &actorID, ImpersonatorSessionID: &origSessionID}, nil).Once() + impRepo.On("GetActiveImpersonationByID", mock.Anything, "imp-1").Return(&types.Impersonation{ID: "imp-1", ActorUserID: "actor-1", ImpersonationSessionID: internaltests.PtrString("session-1")}, nil).Once() + sessionStateRepo.On("Upsert", mock.Anything, mock.AnythingOfType("*types.AdminSessionState")).Return(nil).Once() + sessSvc.On("Delete", mock.Anything, "session-1").Return(nil).Once() impRepo.On("EndImpersonation", mock.Anything, "imp-1", mock.AnythingOfType("*string")).Return(nil).Once() - handler := adminhandlers.NewStopImpersonationHandler(useCase) + tokenSvc.On("Hash", originalSessionToken).Return("hashed-original").Once() + sessSvc.On("GetByToken", mock.Anything, "hashed-original").Return(&models.Session{ID: origSessionID, UserID: actorID}, nil).Once() + handler := adminhandlers.NewStopImpersonationHandler(useCase, testGlobalConfig()) + cn := testGlobalConfig().Session.CookieName req, w, reqCtx := internaltests.NewHandlerRequest(t, http.MethodPost, "/admin/impersonations/imp-1/stop", nil, nil) req.SetPathValue("impersonation_id", "imp-1") + req.AddCookie(&http.Cookie{ + Name: cn + adminconstants.OriginalSessionCookieSuffix, + Value: originalSessionToken, + }) reqCtx.Actor = &models.Actor{ID: actorID, Type: models.ActorUser} reqCtx.Values[models.ContextSessionID.String()] = "session-1" @@ -282,5 +300,7 @@ func TestStopImpersonationHandler(t *testing.T) { } impRepo.AssertExpectations(t) sessionStateRepo.AssertExpectations(t) + sessSvc.AssertExpectations(t) + tokenSvc.AssertExpectations(t) }) } diff --git a/plugins/admin/handlers/users_handlers.go b/plugins/admin/handlers/users_handlers.go index e4d742c8..8893aeea 100644 --- a/plugins/admin/handlers/users_handlers.go +++ b/plugins/admin/handlers/users_handlers.go @@ -25,14 +25,19 @@ func (h *CreateUserHandler) Handler() http.HandlerFunc { reqCtx, _ := models.GetRequestContext(ctx) actor := reqCtx.Actor - var payload types.CreateUserRequest - if err := util.ParseJSON(r, &payload); err != nil { - reqCtx.SetJSONResponse(http.StatusUnprocessableEntity, map[string]any{"message": "invalid request body"}) + var request types.CreateUserRequest + if err := util.ParseJSON(r, &request); err != nil { + reqCtx.SetJSONResponse(http.StatusUnprocessableEntity, map[string]any{"message": err.Error()}) + reqCtx.Handled = true + return + } + if err := request.Validate(); err != nil { + reqCtx.SetJSONResponse(http.StatusUnprocessableEntity, map[string]any{"message": err.Error()}) reqCtx.Handled = true return } - user, err := h.useCase.Create(ctx, actor, payload) + user, err := h.useCase.Create(ctx, actor, request) if err != nil { respondUsersError(reqCtx, err) return @@ -133,14 +138,19 @@ func (h *UpdateUserHandler) Handler() http.HandlerFunc { actor := reqCtx.Actor userID := r.PathValue("user_id") - var payload types.UpdateUserRequest - if err := util.ParseJSON(r, &payload); err != nil { - reqCtx.SetJSONResponse(http.StatusUnprocessableEntity, map[string]any{"message": "invalid request body"}) + var request types.UpdateUserRequest + if err := util.ParseJSON(r, &request); err != nil { + reqCtx.SetJSONResponse(http.StatusUnprocessableEntity, map[string]any{"message": err.Error()}) + reqCtx.Handled = true + return + } + if err := request.Validate(); err != nil { + reqCtx.SetJSONResponse(http.StatusUnprocessableEntity, map[string]any{"message": err.Error()}) reqCtx.Handled = true return } - user, err := h.useCase.Update(r.Context(), actor, userID, payload) + user, err := h.useCase.Update(r.Context(), actor, userID, request) if err != nil { respondUsersError(reqCtx, err) return diff --git a/plugins/admin/handlers/users_handlers_test.go b/plugins/admin/handlers/users_handlers_test.go index 5c1ae3cf..e9e3feca 100644 --- a/plugins/admin/handlers/users_handlers_test.go +++ b/plugins/admin/handlers/users_handlers_test.go @@ -29,7 +29,7 @@ func TestCreateUserHandler(t *testing.T) { name: "invalid request body", body: []byte("{invalid"), expectedStatus: http.StatusUnprocessableEntity, - expectedMessage: "invalid request body", + expectedMessage: "invalid character 'i' looking for beginning of object key string", }, { name: "use case error", @@ -260,7 +260,7 @@ func TestUpdateUserHandler(t *testing.T) { body: []byte("{invalid"), pathParams: map[string]string{"user_id": "user-1"}, expectedStatus: http.StatusUnprocessableEntity, - expectedMessage: "invalid request body", + expectedMessage: "invalid character 'i' looking for beginning of object key string", }, { name: "use case error", diff --git a/plugins/admin/hooks.go b/plugins/admin/hooks.go index bc676fa1..e02a3948 100644 --- a/plugins/admin/hooks.go +++ b/plugins/admin/hooks.go @@ -5,6 +5,7 @@ import ( "time" "github.com/Authula/authula/models" + adminconstants "github.com/Authula/authula/plugins/admin/constants" ) func (p *AdminPlugin) Hooks() []models.Hook { @@ -14,6 +15,16 @@ func (p *AdminPlugin) Hooks() []models.Hook { Handler: p.enforceState, Order: 15, }, + { + Stage: models.HookBefore, + Handler: p.resolveImpersonationOriginalSession, + Order: 17, + }, + { + Stage: models.HookBefore, + Handler: p.addImpersonationWhitelistScopes, + Order: 25, + }, } } @@ -64,3 +75,98 @@ func (p *AdminPlugin) enforceState(reqCtx *models.RequestContext) error { return nil } + +func (p *AdminPlugin) resolveImpersonationOriginalSession(reqCtx *models.RequestContext) error { + if reqCtx.Actor == nil || reqCtx.Actor.Type != models.ActorUser { + return nil + } + + // Check if impersonation claims are already present (JWT path) + if _, hasID := reqCtx.Actor.Claims[adminconstants.ImpersonatorID]; hasID { + return nil + } + + cookieName := p.pluginCtx.GetConfig().Session.CookieName + originalCookieName := cookieName + adminconstants.OriginalSessionCookieSuffix + + cookie, err := reqCtx.Request.Cookie(originalCookieName) + if err != nil { + return nil + } + + rawSessionID, hasSessionID := reqCtx.Values[models.ContextSessionID.String()] + if !hasSessionID || rawSessionID == nil { + return nil + } + currentSessionID, ok := rawSessionID.(string) + if !ok || currentSessionID == "" { + return nil + } + + // Verify the current session is an impersonation session via AdminSessionState + sessionState, err := p.Api.SessionStateRepository().GetBySessionID(reqCtx.Request.Context(), currentSessionID) + if err != nil || sessionState == nil || sessionState.ImpersonatorUserID == nil { + return nil + } + + hashedOriginal := p.tokenService.Hash(cookie.Value) + originalSession, err := p.sessionService.GetByToken(reqCtx.Request.Context(), hashedOriginal) + if err != nil || originalSession == nil { + p.clearOriginalCookie(reqCtx.ResponseWriter, cookieName) + return nil + } + + // Verify the original session matches the impersonator's recorded session ID + if sessionState.ImpersonatorSessionID == nil || *sessionState.ImpersonatorSessionID != originalSession.ID { + p.clearOriginalCookie(reqCtx.ResponseWriter, cookieName) + return nil + } + + if originalSession.ExpiresAt.Before(time.Now().UTC()) { + p.clearOriginalCookie(reqCtx.ResponseWriter, cookieName) + return nil + } + + if reqCtx.Actor.Claims == nil { + reqCtx.Actor.Claims = make(map[string]any) + } + reqCtx.Actor.Claims[adminconstants.ImpersonatorID] = originalSession.UserID + reqCtx.Actor.Claims[adminconstants.ImpersonatorOriginalSessionToken] = cookie.Value + + return nil +} + +func (p *AdminPlugin) addImpersonationWhitelistScopes(reqCtx *models.RequestContext) error { + if reqCtx.Actor == nil || reqCtx.Actor.Type != models.ActorUser { + return nil + } + + impersonatorID, hasID := reqCtx.Actor.GetClaimString(adminconstants.ImpersonatorID) + if !hasID || impersonatorID == "" { + return nil + } + + whitelist := []string{adminconstants.SessionStateReadPermission, adminconstants.ImpersonationsStopPermission} + + seen := make(map[string]struct{}, len(reqCtx.Actor.Scopes)+len(whitelist)) + for _, s := range reqCtx.Actor.Scopes { + seen[s] = struct{}{} + } + for _, s := range whitelist { + if _, exists := seen[s]; !exists { + reqCtx.Actor.Scopes = append(reqCtx.Actor.Scopes, s) + seen[s] = struct{}{} + } + } + + return nil +} + +func (p *AdminPlugin) clearOriginalCookie(w http.ResponseWriter, cookieName string) { + http.SetCookie(w, &http.Cookie{ + Name: cookieName + adminconstants.OriginalSessionCookieSuffix, + Value: "", + Path: "/", + MaxAge: -1, + }) +} diff --git a/plugins/admin/migrations.go b/plugins/admin/migrations.go index ab63c384..114a0aa9 100644 --- a/plugins/admin/migrations.go +++ b/plugins/admin/migrations.go @@ -66,6 +66,7 @@ func adminSQLiteInitial() migrations.Migration { revoked_reason TEXT, revoked_by_user_id TEXT, impersonator_user_id TEXT, + impersonator_session_id TEXT, impersonation_reason TEXT, impersonation_expires_at TIMESTAMP, created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, @@ -155,6 +156,7 @@ func adminPostgresInitial() migrations.Migration { revoked_reason TEXT, revoked_by_user_id UUID, impersonator_user_id UUID, + impersonator_session_id UUID, impersonation_reason TEXT, impersonation_expires_at TIMESTAMP WITH TIME ZONE, created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT NOW(), @@ -237,6 +239,7 @@ func adminMySQLInitial() migrations.Migration { revoked_reason TEXT NULL, revoked_by_user_id BINARY(16) NULL, impersonator_user_id BINARY(16) NULL, + impersonator_session_id BINARY(16) NULL, impersonation_reason TEXT NULL, impersonation_expires_at TIMESTAMP NULL, created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, diff --git a/plugins/admin/plugin.go b/plugins/admin/plugin.go index bd2fbcf3..f6e24492 100644 --- a/plugins/admin/plugin.go +++ b/plugins/admin/plugin.go @@ -16,10 +16,13 @@ import ( ) type AdminPlugin struct { + globalConfig *models.Config config types.AdminPluginConfig - ctx *models.PluginContext + pluginCtx *models.PluginContext logger models.Logger Api *API + sessionService rootservices.SessionService + tokenService rootservices.TokenService accessControlService rootservices.AccessControlService } @@ -41,7 +44,8 @@ func (p *AdminPlugin) Config() any { } func (p *AdminPlugin) Init(ctx *models.PluginContext) error { - p.ctx = ctx + p.globalConfig = ctx.GetConfig() + p.pluginCtx = ctx p.logger = ctx.Logger if err := util.LoadPluginConfig(ctx.GetConfig(), p.Metadata().ID, &p.config); err != nil { @@ -59,11 +63,13 @@ func (p *AdminPlugin) Init(ctx *models.PluginContext) error { if !ok { return fmt.Errorf("required service %s is not registered", models.ServiceSession.String()) } + p.sessionService = sessionService tokenService, ok := ctx.ServiceRegistry.Get(models.ServiceToken.String()).(rootservices.TokenService) if !ok { return fmt.Errorf("required service %s is not registered", models.ServiceToken.String()) } + p.tokenService = tokenService passwordService, ok := ctx.ServiceRegistry.Get(models.ServicePassword.String()).(rootservices.PasswordService) if !ok { @@ -91,7 +97,7 @@ func (p *AdminPlugin) Init(ctx *models.PluginContext) error { userStateRepo, sessionStateRepo, impersonationRepo, - ctx.GetConfig().Session.ExpiresIn, + p.globalConfig.Session.ExpiresIn, authorizer, ) p.Api = NewAPI( @@ -114,7 +120,7 @@ func (p *AdminPlugin) DependsOn() []string { } func (p *AdminPlugin) Routes() []models.Route { - return Routes(p.Api) + return p.buildRoutes(p.Api) } func (p *AdminPlugin) Close() error { @@ -123,6 +129,7 @@ func (p *AdminPlugin) Close() error { func (p *AdminPlugin) ensurePermissions() error { if err := p.accessControlService.EnsurePermissions(context.Background(), []rootservices.PermissionDefinition{ + {Key: adminconstants.All, Description: "All admin permissions"}, {Key: adminconstants.UsersCreatePermission, Description: "Create users"}, {Key: adminconstants.UsersListPermission, Description: "List users"}, {Key: adminconstants.UsersReadPermission, Description: "Read user details"}, diff --git a/plugins/admin/routes.go b/plugins/admin/routes.go index 40f3da3f..e57679ad 100644 --- a/plugins/admin/routes.go +++ b/plugins/admin/routes.go @@ -25,7 +25,7 @@ func newRouteUseCases(api *API) routeUseCases { } } -func Routes(api *API) []models.Route { +func (p *AdminPlugin) buildRoutes(api *API) []models.Route { usecases := newRouteUseCases(api) return []models.Route{ @@ -250,17 +250,23 @@ func Routes(api *API) []models.Route { Method: http.MethodPost, Path: "/admin/impersonations", Middleware: []func(http.Handler) http.Handler{ - middleware.RequireAuthenticated(), + middleware.RequireActor(models.ActorUser), }, - Handler: adminhandlers.NewStartImpersonationHandler(usecases.impersonation).Handler(), + Handler: adminhandlers.NewStartImpersonationHandler( + usecases.impersonation, + p.globalConfig, + ).Handler(), }, { Method: http.MethodPost, Path: "/admin/impersonations/{impersonation_id}/stop", Middleware: []func(http.Handler) http.Handler{ - middleware.RequireAuthenticated(), + middleware.RequireActor(models.ActorUser), }, - Handler: adminhandlers.NewStopImpersonationHandler(usecases.impersonation).Handler(), + Handler: adminhandlers.NewStopImpersonationHandler( + usecases.impersonation, + p.globalConfig, + ).Handler(), }, } } diff --git a/plugins/admin/services/impersonation_service.go b/plugins/admin/services/impersonation_service.go index 06e7b5c1..5f0af363 100644 --- a/plugins/admin/services/impersonation_service.go +++ b/plugins/admin/services/impersonation_service.go @@ -86,6 +86,9 @@ func (s *ImpersonationService) StartImpersonation( ipAddress *string, userAgent *string, req types.StartImpersonationRequest, + impersonatorScopes []string, + originalCookieValue string, + originalCookieMaxAge int, ) (*types.StartImpersonationResult, error) { if err := s.authorizer.AuthorizeScope(ctx, actor, adminconstants.ImpersonationsStartPermission); err != nil { return nil, err @@ -140,7 +143,7 @@ func (s *ImpersonationService) StartImpersonation( var impersonationSessionID *string var rawSessionToken *string - if s.sessionService != nil && s.tokenService != nil { + if s.tokenService != nil && s.sessionService != nil { rawToken, err := s.tokenService.Generate() if err != nil { return nil, err @@ -179,10 +182,11 @@ func (s *ImpersonationService) StartImpersonation( return nil, err } - if impersonationSessionID != nil && s.sessionStateRepo != nil { + if impersonationSessionID != nil { state := &types.AdminSessionState{ SessionID: *impersonationSessionID, ImpersonatorUserID: &actorUserID, + ImpersonatorSessionID: actorSessionID, ImpersonationReason: &reason, ImpersonationExpiresAt: &expiresAt, } @@ -191,13 +195,41 @@ func (s *ImpersonationService) StartImpersonation( } } + // Compute .original cookie value and max age + var originalCookieToken string + impersonationMaxAge := 0 + if originalCookieValue != "" { + originalCookieToken = originalCookieValue + impersonationMaxAge = int(time.Until(expiresAt).Seconds()) + if originalCookieMaxAge > 0 && originalCookieMaxAge < impersonationMaxAge { + impersonationMaxAge = originalCookieMaxAge + } + if impersonationMaxAge < 0 { + impersonationMaxAge = 0 + } + } + return &types.StartImpersonationResult{ - Impersonation: impersonation, - SessionID: impersonationSessionID, - SessionToken: rawSessionToken, + Impersonation: impersonation, + SessionID: impersonationSessionID, + SessionToken: rawSessionToken, + ImpersonatorUserID: actorUserID, + ImpersonatorScopes: impersonatorScopes, + OriginalCookieToken: originalCookieToken, + OriginalCookieMaxAge: impersonationMaxAge, + TargetUserID: targetUserID, }, nil } +func (s *ImpersonationService) ValidateImpersonationCookie(ctx context.Context, originalCookieValue string) (*models.Session, error) { + hashedOriginal := s.tokenService.Hash(originalCookieValue) + originalSession, err := s.sessionService.GetByToken(ctx, hashedOriginal) + if err != nil || originalSession == nil { + return nil, internalerrors.ErrForbidden + } + return originalSession, nil +} + func (s *ImpersonationService) StopImpersonation(ctx context.Context, actor *models.Actor, actorUserID string, request types.StopImpersonationRequest) error { if err := s.authorizer.AuthorizeScope(ctx, actor, adminconstants.ImpersonationsStopPermission); err != nil { return err diff --git a/plugins/admin/services/impersonation_service_test.go b/plugins/admin/services/impersonation_service_test.go index 5c16aaaa..b5df4d8e 100644 --- a/plugins/admin/services/impersonation_service_test.go +++ b/plugins/admin/services/impersonation_service_test.go @@ -64,9 +64,9 @@ func TestImpersonationService_StartImpersonation_validation(t *testing.T) { tc.setup(impRepo) } - ipAddress := internaltests.PtrString("127.0.0.1") - userAgent := internaltests.PtrString("user-agent") - _, err := svc.StartImpersonation(ctx, internaltests.TestActor(), tc.actor, nil, ipAddress, userAgent, tc.req) + ipAddress := new("127.0.0.1") + userAgent := new("user-agent") + _, err := svc.StartImpersonation(ctx, internaltests.TestActor(), tc.actor, nil, ipAddress, userAgent, tc.req, nil, "", 0) if tc.want != nil { require.ErrorIs(t, err, tc.want) } else { @@ -97,15 +97,19 @@ func TestImpersonationService_StartImpersonation_success(t *testing.T) { sessRepo.On("Upsert", mock.Anything, mock.Anything).Return(nil).Once() req := admintypes.StartImpersonationRequest{TargetUserID: "target", Reason: "reason", ExpiresInSeconds: func(i int) *int { return &i }(60)} - ipAddress := internaltests.PtrString("127.0.0.1") - userAgent := internaltests.PtrString("user-agent") - res, err := svc.StartImpersonation(ctx, internaltests.TestActor(), "actor", nil, ipAddress, userAgent, req) + ipAddress := new("127.0.0.1") + userAgent := new("user-agent") + res, err := svc.StartImpersonation(ctx, internaltests.TestActor(), "actor", nil, ipAddress, userAgent, req, []string{"scope1"}, "orig-token", 100) require.NoError(t, err) require.NotNil(t, res) require.NotNil(t, res.SessionID) require.Equal(t, "sess1", *res.SessionID) require.NotNil(t, res.SessionToken) require.Equal(t, rawToken, *res.SessionToken) + require.Equal(t, "actor", res.ImpersonatorUserID) + require.Equal(t, []string{"scope1"}, res.ImpersonatorScopes) + require.Equal(t, "target", res.TargetUserID) + require.Equal(t, "orig-token", res.OriginalCookieToken) impRepo.AssertExpectations(t) sessRepo.AssertExpectations(t) @@ -127,9 +131,9 @@ func TestImpersonationService_StartImpersonation_noSessionServices(t *testing.T) impRepo.On("CreateImpersonation", mock.Anything, mock.Anything).Return(nil).Once() req := admintypes.StartImpersonationRequest{TargetUserID: "target", Reason: "reason"} - ipAddress := internaltests.PtrString("127.0.0.1") - userAgent := internaltests.PtrString("user-agent") - res, err := svc.StartImpersonation(ctx, internaltests.TestActor(), "actor", nil, ipAddress, userAgent, req) + ipAddress := new("127.0.0.1") + userAgent := new("user-agent") + res, err := svc.StartImpersonation(ctx, internaltests.TestActor(), "actor", nil, ipAddress, userAgent, req, nil, "", 0) require.NoError(t, err) require.NotNil(t, res) require.Nil(t, res.SessionID) @@ -139,6 +143,36 @@ func TestImpersonationService_StartImpersonation_noSessionServices(t *testing.T) sessRepo.AssertExpectations(t) } +func TestImpersonationService_ValidateImpersonationCookie(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + t.Run("session not found returns forbidden", func(t *testing.T) { + t.Parallel() + svc, _, _, sessSvc, tokSvc := newImpersonationServiceFixture() + tokSvc.On("Hash", "orig-token").Return("hashed-orig").Once() + sessSvc.On("GetByToken", mock.Anything, "hashed-orig").Return((*models.Session)(nil), nil).Once() + _, err := svc.ValidateImpersonationCookie(ctx, "orig-token") + require.ErrorIs(t, err, internalerrors.ErrForbidden) + tokSvc.AssertExpectations(t) + sessSvc.AssertExpectations(t) + }) + + t.Run("success returns session", func(t *testing.T) { + t.Parallel() + svc, _, _, sessSvc, tokSvc := newImpersonationServiceFixture() + tokSvc.On("Hash", "orig-token").Return("hashed-orig").Once() + sessSvc.On("GetByToken", mock.Anything, "hashed-orig").Return(&models.Session{ID: "orig-sess", UserID: "actor-1"}, nil).Once() + sess, err := svc.ValidateImpersonationCookie(ctx, "orig-token") + require.NoError(t, err) + require.NotNil(t, sess) + require.Equal(t, "orig-sess", sess.ID) + tokSvc.AssertExpectations(t) + sessSvc.AssertExpectations(t) + }) +} + func TestImpersonationService_StopImpersonation(t *testing.T) { t.Parallel() diff --git a/plugins/admin/types/api.go b/plugins/admin/types/api.go index 48ade237..4dad30a3 100644 --- a/plugins/admin/types/api.go +++ b/plugins/admin/types/api.go @@ -3,18 +3,23 @@ package types import ( "time" + "github.com/Authula/authula/internal/util" "github.com/Authula/authula/models" "github.com/Authula/authula/plugins/admin/constants" ) type CreateUserRequest struct { - Name string `json:"name"` - Email string `json:"email"` + Name string `json:"name" validate:"required"` + Email string `json:"email" validate:"required,email"` EmailVerified *bool `json:"email_verified,omitempty"` Image *string `json:"image,omitempty"` Metadata map[string]any `json:"metadata,omitempty"` } +func (req *CreateUserRequest) Validate() error { + return util.ValidateStruct(req) +} + type CreateUserResponse struct { User *models.User `json:"user"` } @@ -24,13 +29,27 @@ type GetUserByIDResponse struct { } type UpdateUserRequest struct { - Name *string `json:"name,omitempty"` - Email *string `json:"email,omitempty"` + Name *string `json:"name,omitempty" validate:"omitempty"` + Email *string `json:"email,omitempty" validate:"omitempty,email"` EmailVerified *bool `json:"email_verified,omitempty"` Image *string `json:"image,omitempty"` Metadata map[string]any `json:"metadata,omitempty"` } +func (req *UpdateUserRequest) Validate() error { + if err := util.ValidateStruct(req); err != nil { + return err + } + + if req.Name == nil && req.Email == nil && + req.EmailVerified == nil && req.Image == nil && + req.Metadata == nil { + return constants.ErrNoPropertiesProvided + } + + return nil +} + type UpdateUserResponse struct { User *models.User `json:"user"` } @@ -45,8 +64,8 @@ type UsersPage struct { } type CreateAccountRequest struct { - ProviderID string `json:"provider_id"` - AccountID string `json:"account_id"` + ProviderID string `json:"provider_id" validate:"required"` + AccountID string `json:"account_id" validate:"required"` AccessToken *string `json:"access_token,omitempty"` RefreshToken *string `json:"refresh_token,omitempty"` IDToken *string `json:"id_token,omitempty"` @@ -57,11 +76,8 @@ type CreateAccountRequest struct { } func (req *CreateAccountRequest) Validate() error { - if req.ProviderID == "" { - return constants.ErrProviderIDRequired - } - if req.AccountID == "" { - return constants.ErrAccountIDRequired + if err := util.ValidateStruct(req); err != nil { + return err } if req.AccessTokenExpiresAt != nil && req.AccessTokenExpiresAt.Before(time.Now()) { return constants.ErrAccessTokenExpiresAtBeforeNow @@ -69,6 +85,7 @@ func (req *CreateAccountRequest) Validate() error { if req.RefreshTokenExpiresAt != nil && req.RefreshTokenExpiresAt.Before(time.Now()) { return constants.ErrRefreshTokenExpiresAtBeforeNow } + return nil } @@ -104,6 +121,7 @@ func (req *UpdateAccountRequest) Validate() error { req.Password == nil { return constants.ErrNoPropertiesProvided } + return nil } @@ -128,17 +146,41 @@ type UpsertUserStateResponse struct { } type CreateUserStateRequest struct { - Banned bool `json:"banned"` + Banned bool `json:"banned" validate:"required"` BannedUntil *time.Time `json:"banned_until,omitempty"` BannedReason *string `json:"banned_reason,omitempty"` } +func (req *CreateUserStateRequest) Validate() error { + if err := util.ValidateStruct(req); err != nil { + return err + } + + if req.Banned && req.BannedUntil != nil && req.BannedUntil.Before(time.Now()) { + return constants.ErrBannedUntilBeforeNow + } + + return nil +} + type UpsertUserStateRequest struct { - Banned bool `json:"banned"` + Banned bool `json:"banned" validate:"required"` BannedUntil *time.Time `json:"banned_until,omitempty"` BannedReason *string `json:"banned_reason,omitempty"` } +func (req *UpsertUserStateRequest) Validate() error { + if err := util.ValidateStruct(req); err != nil { + return err + } + + if req.Banned && req.BannedUntil != nil && req.BannedUntil.Before(time.Now()) { + return constants.ErrBannedUntilBeforeNow + } + + return nil +} + type DeleteUserStateResponse struct { Message string `json:"message"` } @@ -148,6 +190,18 @@ type BanUserRequest struct { Reason *string `json:"reason,omitempty"` } +func (req *BanUserRequest) Validate() error { + if err := util.ValidateStruct(req); err != nil { + return err + } + + if req.BannedUntil != nil && req.BannedUntil.Before(time.Now()) { + return constants.ErrBannedUntilBeforeNow + } + + return nil +} + type BanUserResponse struct { State *AdminUserState `json:"state"` } @@ -161,21 +215,45 @@ type GetSessionStateResponse struct { } type CreateSessionStateRequest struct { - Revoke bool `json:"revoke"` + Revoke bool `json:"revoke" validate:"required"` RevokedReason *string `json:"revoked_reason,omitempty"` ImpersonatorUserID *string `json:"impersonator_user_id,omitempty"` ImpersonationReason *string `json:"impersonation_reason,omitempty"` ImpersonationExpiresAt *time.Time `json:"impersonation_expires_at,omitempty"` } +func (req *CreateSessionStateRequest) Validate() error { + if err := util.ValidateStruct(req); err != nil { + return err + } + + if req.ImpersonationExpiresAt != nil && req.ImpersonationExpiresAt.Before(time.Now()) { + return constants.ErrImpersonationExpiresAtBeforeNow + } + + return nil +} + type UpsertSessionStateRequest struct { - Revoke bool `json:"revoke"` + Revoke bool `json:"revoke" validate:"required"` RevokedReason *string `json:"revoked_reason,omitempty"` ImpersonatorUserID *string `json:"impersonator_user_id,omitempty"` ImpersonationReason *string `json:"impersonation_reason,omitempty"` ImpersonationExpiresAt *time.Time `json:"impersonation_expires_at,omitempty"` } +func (req *UpsertSessionStateRequest) Validate() error { + if err := util.ValidateStruct(req); err != nil { + return err + } + + if req.ImpersonationExpiresAt != nil && req.ImpersonationExpiresAt.Before(time.Now()) { + return constants.ErrImpersonationExpiresAtBeforeNow + } + + return nil +} + type UpsertSessionStateResponse struct { State *AdminSessionState `json:"state"` } @@ -197,15 +275,24 @@ type GetImpersonationByIDResponse struct { } type StartImpersonationRequest struct { - TargetUserID string `json:"target_user_id"` - Reason string `json:"reason"` + TargetUserID string `json:"target_user_id" validate:"required"` + Reason string `json:"reason" validate:"required"` ExpiresInSeconds *int `json:"expires_in_seconds,omitempty"` } +func (req *StartImpersonationRequest) Validate() error { + return util.ValidateStruct(req) +} + type StartImpersonationResult struct { - Impersonation *Impersonation `json:"impersonation"` - SessionID *string `json:"session_id,omitempty"` - SessionToken *string `json:"session_token,omitempty"` + Impersonation *Impersonation `json:"impersonation"` + SessionID *string `json:"session_id,omitempty"` + SessionToken *string `json:"session_token,omitempty"` + ImpersonatorUserID string `json:"-"` + ImpersonatorScopes []string `json:"-"` + OriginalCookieToken string `json:"-"` + OriginalCookieMaxAge int `json:"-"` + TargetUserID string `json:"-"` } type StartImpersonationResponse struct { @@ -216,6 +303,10 @@ type StopImpersonationRequest struct { ImpersonationID *string `json:"impersonation_id,omitempty"` } +type StopImpersonationResult struct { + OriginalSessionToken string `json:"-"` +} + type StopImpersonationResponse struct { Message string `json:"message"` } diff --git a/plugins/admin/types/models.go b/plugins/admin/types/models.go index 41187aa8..5c16d1aa 100644 --- a/plugins/admin/types/models.go +++ b/plugins/admin/types/models.go @@ -46,6 +46,7 @@ type AdminSessionState struct { RevokedReason *string `json:"revoked_reason" bun:"column:revoked_reason"` RevokedByUserID *string `json:"revoked_by_user_id" bun:"column:revoked_by_user_id"` ImpersonatorUserID *string `json:"impersonator_user_id" bun:"column:impersonator_user_id"` + ImpersonatorSessionID *string `json:"impersonator_session_id" bun:"column:impersonator_session_id"` ImpersonationReason *string `json:"impersonation_reason" bun:"column:impersonation_reason"` ImpersonationExpiresAt *time.Time `json:"impersonation_expires_at" bun:"column:impersonation_expires_at"` CreatedAt time.Time `json:"created_at" bun:"column:created_at,default:current_timestamp"` diff --git a/plugins/admin/usecases/admin_usecases.go b/plugins/admin/usecases/admin_usecases.go index 43fad9ba..e0a7897d 100644 --- a/plugins/admin/usecases/admin_usecases.go +++ b/plugins/admin/usecases/admin_usecases.go @@ -117,12 +117,12 @@ func (u *AdminUseCases) GetImpersonationByID(ctx context.Context, actor *models. return u.impersonation.GetImpersonationByID(ctx, actor, impersonationID) } -func (u *AdminUseCases) StartImpersonation(ctx context.Context, actor *models.Actor, actorUserID string, actorSessionID *string, ipAddress *string, userAgent *string, req types.StartImpersonationRequest) (*types.StartImpersonationResult, error) { - return u.impersonation.StartImpersonation(ctx, actor, actorUserID, actorSessionID, ipAddress, userAgent, req) +func (u *AdminUseCases) StartImpersonation(ctx context.Context, actor *models.Actor, actorUserID string, actorSessionID *string, ipAddress *string, userAgent *string, req types.StartImpersonationRequest, impersonatorScopes []string, originalCookieValue string, originalCookieMaxAge int) (*types.StartImpersonationResult, error) { + return u.impersonation.StartImpersonation(ctx, actor, actorUserID, actorSessionID, ipAddress, userAgent, req, impersonatorScopes, originalCookieValue, originalCookieMaxAge) } -func (u *AdminUseCases) StopImpersonation(ctx context.Context, actor *models.Actor, impersonatedUserID string, impersonatedSessionID string, request types.StopImpersonationRequest) error { - return u.impersonation.StopImpersonation(ctx, actor, impersonatedUserID, impersonatedSessionID, request) +func (u *AdminUseCases) StopImpersonation(ctx context.Context, actor *models.Actor, impersonatedUserID string, impersonatedSessionID string, originalCookieValue string, request types.StopImpersonationRequest) (*types.StopImpersonationResult, error) { + return u.impersonation.StopImpersonation(ctx, actor, impersonatedUserID, impersonatedSessionID, originalCookieValue, request) } func (u *AdminUseCases) GetUserState(ctx context.Context, actor *models.Actor, userID string) (*types.AdminUserState, error) { diff --git a/plugins/admin/usecases/impersonation_usecase.go b/plugins/admin/usecases/impersonation_usecase.go index 68bff8f4..e39c2454 100644 --- a/plugins/admin/usecases/impersonation_usecase.go +++ b/plugins/admin/usecases/impersonation_usecase.go @@ -5,6 +5,7 @@ import ( internalerrors "github.com/Authula/authula/internal/errors" "github.com/Authula/authula/models" + adminconstants "github.com/Authula/authula/plugins/admin/constants" "github.com/Authula/authula/plugins/admin/services" "github.com/Authula/authula/plugins/admin/types" ) @@ -32,24 +33,53 @@ func (u ImpersonationUseCase) GetImpersonationByID(ctx context.Context, actor *m return u.impersonationService.GetImpersonationByID(ctx, actor, impersonationID) } -func (u ImpersonationUseCase) StartImpersonation(ctx context.Context, actor *models.Actor, actorUserID string, actorSessionID *string, ipAddress *string, userAgent *string, req types.StartImpersonationRequest) (*types.StartImpersonationResult, error) { - return u.impersonationService.StartImpersonation(ctx, actor, actorUserID, actorSessionID, ipAddress, userAgent, req) +func (u ImpersonationUseCase) StartImpersonation(ctx context.Context, actor *models.Actor, actorUserID string, actorSessionID *string, ipAddress *string, userAgent *string, req types.StartImpersonationRequest, impersonatorScopes []string, originalCookieValue string, originalCookieMaxAge int) (*types.StartImpersonationResult, error) { + return u.impersonationService.StartImpersonation(ctx, actor, actorUserID, actorSessionID, ipAddress, userAgent, req, impersonatorScopes, originalCookieValue, originalCookieMaxAge) } -func (u ImpersonationUseCase) StopImpersonation(ctx context.Context, actor *models.Actor, impersonatedUserID string, impersonatedSessionID string, request types.StopImpersonationRequest) error { +func (u ImpersonationUseCase) StopImpersonation(ctx context.Context, actor *models.Actor, impersonatedUserID string, impersonatedSessionID string, originalCookieValue string, request types.StopImpersonationRequest) (*types.StopImpersonationResult, error) { sessionState, err := u.stateService.GetSessionState(ctx, actor, impersonatedSessionID) if err != nil { - return err + return nil, err } if sessionState == nil || sessionState.ImpersonatorUserID == nil { - return internalerrors.ErrUnauthorized + return nil, internalerrors.ErrUnauthorized } actorUserID := *sessionState.ImpersonatorUserID if actorUserID == "" { - return internalerrors.ErrUnauthorized + return nil, internalerrors.ErrUnauthorized } - return u.impersonationService.StopImpersonation(ctx, actor, actorUserID, request) + if originalCookieValue != "" { + originalSession, err := u.impersonationService.ValidateImpersonationCookie(ctx, originalCookieValue) + if err != nil { + return nil, err + } + + if sessionState.ImpersonatorSessionID == nil || *sessionState.ImpersonatorSessionID != originalSession.ID { + return nil, internalerrors.ErrUnauthorized + } + } else { + if actor == nil || actor.Claims == nil { + return nil, internalerrors.ErrUnauthorized + } + impersonatorID, ok := actor.Claims[adminconstants.ImpersonatorID] + if !ok { + return nil, internalerrors.ErrUnauthorized + } + impersonatorIDStr, ok := impersonatorID.(string) + if !ok || impersonatorIDStr == "" || impersonatorIDStr != actorUserID { + return nil, internalerrors.ErrUnauthorized + } + } + + if err := u.impersonationService.StopImpersonation(ctx, actor, actorUserID, request); err != nil { + return nil, err + } + + return &types.StopImpersonationResult{ + OriginalSessionToken: originalCookieValue, + }, nil } diff --git a/plugins/admin/usecases/impersonation_usecase_test.go b/plugins/admin/usecases/impersonation_usecase_test.go index 56573e4d..c4e5bd7a 100644 --- a/plugins/admin/usecases/impersonation_usecase_test.go +++ b/plugins/admin/usecases/impersonation_usecase_test.go @@ -70,7 +70,7 @@ func TestImpersonationUseCase_StartImpersonation(t *testing.T) { t.Parallel() useCase, _, _, _, _ := admintests.NewImpersonationUseCaseFixture(t) - _, err := useCase.StartImpersonation(context.Background(), internaltests.TestActor(), "", nil, internaltests.PtrString("127.0.0.1"), internaltests.PtrString("user-agent"), admintypes.StartImpersonationRequest{TargetUserID: "t", Reason: "r"}) + _, err := useCase.StartImpersonation(context.Background(), internaltests.TestActor(), "", nil, internaltests.PtrString("127.0.0.1"), internaltests.PtrString("user-agent"), admintypes.StartImpersonationRequest{TargetUserID: "t", Reason: "r"}, nil, "", 0) assert.ErrorIs(t, err, internalerrors.ErrBadRequest) }) @@ -89,11 +89,15 @@ func TestImpersonationUseCase_StartImpersonation(t *testing.T) { impRepo.On("CreateImpersonation", mock.Anything, mock.AnythingOfType("*types.Impersonation")).Return(nil).Once() sessionStateRepo.On("Upsert", mock.Anything, mock.AnythingOfType("*types.AdminSessionState")).Return(nil).Once() - res, err := useCase.StartImpersonation(context.Background(), internaltests.TestActor(), "actor-1", internaltests.PtrString("sess-actor"), ipAddress, userAgent, admintypes.StartImpersonationRequest{TargetUserID: "target-1", Reason: "reason"}) + res, err := useCase.StartImpersonation(context.Background(), internaltests.TestActor(), "actor-1", internaltests.PtrString("sess-actor"), ipAddress, userAgent, admintypes.StartImpersonationRequest{TargetUserID: "target-1", Reason: "reason"}, []string{"scope1", "scope2"}, "orig-token", 100) assert.NoError(t, err) assert.NotNil(t, res) assert.Equal(t, "sess", *res.SessionID) assert.Equal(t, "tok", *res.SessionToken) + assert.Equal(t, "actor-1", res.ImpersonatorUserID) + assert.Equal(t, []string{"scope1", "scope2"}, res.ImpersonatorScopes) + assert.Equal(t, "target-1", res.TargetUserID) + assert.Equal(t, "orig-token", res.OriginalCookieToken) impRepo.AssertExpectations(t) sessionStateRepo.AssertExpectations(t) @@ -111,7 +115,7 @@ func TestImpersonationUseCase_StartImpersonation(t *testing.T) { impRepo.On("UserExists", mock.Anything, "target-1").Return(true, nil).Once() tokenSvc.On("Generate").Return("", errors.New("fail")).Once() - _, err := useCase.StartImpersonation(context.Background(), internaltests.TestActor(), "actor-1", nil, ipAddress, userAgent, admintypes.StartImpersonationRequest{TargetUserID: "target-1", Reason: "reason"}) + _, err := useCase.StartImpersonation(context.Background(), internaltests.TestActor(), "actor-1", nil, ipAddress, userAgent, admintypes.StartImpersonationRequest{TargetUserID: "target-1", Reason: "reason"}, nil, "", 0) assert.Error(t, err) impRepo.AssertExpectations(t) tokenSvc.AssertExpectations(t) @@ -121,12 +125,14 @@ func TestImpersonationUseCase_StartImpersonation(t *testing.T) { func TestImpersonationUseCase_StopImpersonation(t *testing.T) { t.Parallel() + origSessionID := "orig-sess" + t.Run("error when session state lookup fails", func(t *testing.T) { t.Parallel() useCase, _, sessionStateRepo, _, _ := admintests.NewImpersonationUseCaseFixture(t) sessionStateRepo.On("GetBySessionID", mock.Anything, "impersonated-session-1").Return((*admintypes.AdminSessionState)(nil), internalerrors.ErrNotFound).Once() - err := useCase.StopImpersonation(context.Background(), internaltests.TestActor(), "target-1", "impersonated-session-1", admintypes.StopImpersonationRequest{}) + _, err := useCase.StopImpersonation(context.Background(), internaltests.TestActor(), "target-1", "impersonated-session-1", "orig-token", admintypes.StopImpersonationRequest{}) assert.ErrorIs(t, err, internalerrors.ErrNotFound) sessionStateRepo.AssertExpectations(t) }) @@ -137,7 +143,7 @@ func TestImpersonationUseCase_StopImpersonation(t *testing.T) { useCase, _, sessionStateRepo, _, _ := admintests.NewImpersonationUseCaseFixture(t) sessionStateRepo.On("GetBySessionID", mock.Anything, "impersonated-session-1").Return(&admintypes.AdminSessionState{SessionID: "impersonated-session-1"}, nil).Once() - err := useCase.StopImpersonation(context.Background(), internaltests.TestActor(), "target-1", "impersonated-session-1", admintypes.StopImpersonationRequest{}) + _, err := useCase.StopImpersonation(context.Background(), internaltests.TestActor(), "target-1", "impersonated-session-1", "orig-token", admintypes.StopImpersonationRequest{}) assert.ErrorIs(t, err, internalerrors.ErrUnauthorized) sessionStateRepo.AssertExpectations(t) }) @@ -145,36 +151,64 @@ func TestImpersonationUseCase_StopImpersonation(t *testing.T) { t.Run("not found when no active impersonation for resolved actor", func(t *testing.T) { t.Parallel() - useCase, impRepo, sessionStateRepo, _, _ := admintests.NewImpersonationUseCaseFixture(t) + useCase, impRepo, sessionStateRepo, sessionSvc, tokenSvc := admintests.NewImpersonationUseCaseFixture(t) actorID := "actor-1" - sessionStateRepo.On("GetBySessionID", mock.Anything, "impersonated-session-1").Return(&admintypes.AdminSessionState{SessionID: "impersonated-session-1", ImpersonatorUserID: &actorID}, nil).Once() + sessionID := "impersonated-session-1" + sessionStateRepo.On("GetBySessionID", mock.Anything, sessionID).Return(&admintypes.AdminSessionState{SessionID: sessionID, ImpersonatorUserID: &actorID, ImpersonatorSessionID: &origSessionID}, nil).Once() + tokenSvc.On("Hash", "orig-token").Return("hashed-orig").Once() + sessionSvc.On("GetByToken", mock.Anything, "hashed-orig").Return(&models.Session{ID: origSessionID, UserID: actorID}, nil).Once() impRepo.On("GetLatestActiveImpersonationByActor", mock.Anything, "actor-1").Return((*admintypes.Impersonation)(nil), nil).Once() - err := useCase.StopImpersonation(context.Background(), internaltests.TestActor(), "target-1", "impersonated-session-1", admintypes.StopImpersonationRequest{}) + _, err := useCase.StopImpersonation(context.Background(), internaltests.TestActor(), "target-1", sessionID, "orig-token", admintypes.StopImpersonationRequest{}) assert.ErrorIs(t, err, internalerrors.ErrNotFound) sessionStateRepo.AssertExpectations(t) + tokenSvc.AssertExpectations(t) + sessionSvc.AssertExpectations(t) impRepo.AssertExpectations(t) }) + t.Run("session binding mismatch", func(t *testing.T) { + t.Parallel() + + useCase, _, sessionStateRepo, sessionSvc, tokenSvc := admintests.NewImpersonationUseCaseFixture(t) + actorID := "actor-1" + otherSessionID := "other-sess" + sessionStateRepo.On("GetBySessionID", mock.Anything, "impersonated-session-1").Return(&admintypes.AdminSessionState{SessionID: "impersonated-session-1", ImpersonatorUserID: &actorID, ImpersonatorSessionID: &otherSessionID}, nil).Once() + tokenSvc.On("Hash", "orig-token").Return("hashed-orig").Once() + sessionSvc.On("GetByToken", mock.Anything, "hashed-orig").Return(&models.Session{ID: origSessionID, UserID: actorID}, nil).Once() + + _, err := useCase.StopImpersonation(context.Background(), internaltests.TestActor(), "target-1", "impersonated-session-1", "orig-token", admintypes.StopImpersonationRequest{}) + assert.ErrorIs(t, err, internalerrors.ErrUnauthorized) + sessionStateRepo.AssertExpectations(t) + tokenSvc.AssertExpectations(t) + sessionSvc.AssertExpectations(t) + }) + t.Run("successful stop uses impersonator from session state", func(t *testing.T) { t.Parallel() - useCase, impRepo, sessionStateRepo, sessionSvc, _ := admintests.NewImpersonationUseCaseFixture(t) + useCase, impRepo, sessionStateRepo, sessionSvc, tokenSvc := admintests.NewImpersonationUseCaseFixture(t) actorID := "actor-1" targetID := "target-1" - imp := &admintypes.Impersonation{ID: "imp-1", ActorUserID: "actor-1", ImpersonationSessionID: internaltests.PtrString("sess")} - sessionStateRepo.On("GetBySessionID", mock.Anything, "impersonated-session-1").Return(&admintypes.AdminSessionState{SessionID: "impersonated-session-1", ImpersonatorUserID: &actorID}, nil).Once() + sessionID := "impersonated-session-1" + imp := &admintypes.Impersonation{ID: "imp-1", ActorUserID: "actor-1", ImpersonationSessionID: &sessionID} + sessionStateRepo.On("GetBySessionID", mock.Anything, sessionID).Return(&admintypes.AdminSessionState{SessionID: sessionID, ImpersonatorUserID: &actorID, ImpersonatorSessionID: &origSessionID}, nil).Once() impRepo.On("GetLatestActiveImpersonationByActor", mock.Anything, "actor-1").Return(imp, nil).Once() sessionStateRepo.On("Upsert", mock.Anything, mock.Anything).Return(nil).Once() - sessionSvc.On("Delete", mock.Anything, "sess").Return(nil).Once() + sessionSvc.On("Delete", mock.Anything, sessionID).Return(nil).Once() impRepo.On("EndImpersonation", mock.Anything, "imp-1", mock.AnythingOfType("*string")).Return(nil).Once() + tokenSvc.On("Hash", "orig-token").Return("hashed-orig").Once() + sessionSvc.On("GetByToken", mock.Anything, "hashed-orig").Return(&models.Session{ID: origSessionID, UserID: actorID}, nil).Once() - err := useCase.StopImpersonation(context.Background(), internaltests.TestActor(), targetID, "impersonated-session-1", admintypes.StopImpersonationRequest{}) + res, err := useCase.StopImpersonation(context.Background(), internaltests.TestActor(), targetID, sessionID, "orig-token", admintypes.StopImpersonationRequest{}) assert.NoError(t, err) + assert.NotNil(t, res) + assert.Equal(t, "orig-token", res.OriginalSessionToken) impRepo.AssertExpectations(t) sessionStateRepo.AssertExpectations(t) sessionSvc.AssertExpectations(t) + tokenSvc.AssertExpectations(t) }) } diff --git a/plugins/api-key/constants/permissions.go b/plugins/api-key/constants/permissions.go index 728376f3..2d83b74e 100644 --- a/plugins/api-key/constants/permissions.go +++ b/plugins/api-key/constants/permissions.go @@ -1,6 +1,7 @@ package constants const ( + All = "org:api-key:*" OrgApiKeyCreate = "org:api-key:create" OrgApiKeyList = "org:api-key:list" OrgApiKeyRead = "org:api-key:read" diff --git a/plugins/api-key/plugin.go b/plugins/api-key/plugin.go index 88f38427..147d79ba 100644 --- a/plugins/api-key/plugin.go +++ b/plugins/api-key/plugin.go @@ -145,6 +145,7 @@ func (p *ApiKeyPlugin) runCleanupLoop(interval time.Duration) { func (p *ApiKeyPlugin) ensurePermissions() error { if err := p.accessControlService.EnsurePermissions(context.Background(), []rootservices.PermissionDefinition{ + {Key: apiconstants.All, Description: "All organization API key permissions"}, {Key: apiconstants.OrgApiKeyCreate, Description: "Create API keys for the organization"}, {Key: apiconstants.OrgApiKeyList, Description: "List API keys for the organization"}, {Key: apiconstants.OrgApiKeyRead, Description: "Read an API key for the organization"}, diff --git a/plugins/bearer/hooks.go b/plugins/bearer/hooks.go index e72f673b..4fd1e6fc 100644 --- a/plugins/bearer/hooks.go +++ b/plugins/bearer/hooks.go @@ -60,6 +60,12 @@ func (p *BearerPlugin) validateBearerToken(reqCtx *models.RequestContext) error reqCtx.SetActorInContext(actor) + if sessionID, ok := actor.Claims[models.ContextSessionID.String()]; ok { + if sid, ok := sessionID.(string); ok && sid != "" { + reqCtx.Values[models.ContextSessionID.String()] = sid + } + } + return nil } @@ -82,5 +88,11 @@ func (p *BearerPlugin) validateBearerTokenOptional(reqCtx *models.RequestContext reqCtx.SetActorInContext(actor) + if sessionID, ok := actor.Claims[models.ContextSessionID.String()]; ok { + if sid, ok := sessionID.(string); ok && sid != "" { + reqCtx.Values[models.ContextSessionID.String()] = sid + } + } + return nil } diff --git a/plugins/jwt/hooks.go b/plugins/jwt/hooks.go index 57851aab..e20b2298 100644 --- a/plugins/jwt/hooks.go +++ b/plugins/jwt/hooks.go @@ -1,7 +1,9 @@ package jwt import ( + "encoding/json" "fmt" + "maps" "net/http" "time" @@ -61,7 +63,8 @@ func (p *JWTPlugin) issueTokensHook(reqCtx *models.RequestContext) error { return nil } - tokenPair, err := p.jwtService.(jwtservices.TokenService).GenerateUserToken(ctx, reqCtx.Actor.ID, sessionID) + extraClaims := extraClaimsFromActor(reqCtx.Actor) + tokenPair, err := p.jwtService.(jwtservices.TokenService).GenerateUserToken(ctx, reqCtx.Actor.ID, sessionID, extraClaims) if err != nil { p.Logger.Error("failed to generate user JWT tokens", "user_id", reqCtx.Actor.ID, "session_id", sessionID, "error", err) return fmt.Errorf("failed to generate authentication tokens: %w", err) @@ -96,6 +99,13 @@ func (p *JWTPlugin) issueTokensHook(reqCtx *models.RequestContext) error { return nil } +func extraClaimsFromActor(actor *models.Actor) map[string]any { + if actor == nil || actor.Claims == nil { + return nil + } + return actor.Claims +} + func (p *JWTPlugin) respondHook(reqCtx *models.RequestContext) error { if reqCtx.Actor == nil { return nil @@ -115,6 +125,16 @@ func (p *JWTPlugin) respondHook(reqCtx *models.RequestContext) error { payload["refresh_token"] = refresh } + if reqCtx.ResponseReady && len(reqCtx.ResponseBody) > 0 { + var existing map[string]any + if err := json.Unmarshal(reqCtx.ResponseBody, &existing); err == nil { + maps.Copy(existing, payload) + reqCtx.SetJSONResponse(reqCtx.ResponseStatus, existing) + reqCtx.Handled = true + return nil + } + } + reqCtx.SetJSONResponse(http.StatusOK, payload) reqCtx.Handled = true return nil diff --git a/plugins/jwt/hooks_test.go b/plugins/jwt/hooks_test.go index fbdf0ebf..a72e502a 100644 --- a/plugins/jwt/hooks_test.go +++ b/plugins/jwt/hooks_test.go @@ -70,7 +70,7 @@ func TestIssueTokensHook(t *testing.T) { AccessToken: "access-token-1", RefreshToken: "refresh-token-1", } - tokenSvc.On("GenerateUserToken", mock.Anything, "user-1", "sess-1").Return(pair, nil) + tokenSvc.On("GenerateUserToken", mock.Anything, "user-1", "sess-1", mock.Anything).Return(pair, nil) refreshSvc.On("StoreInitialRefreshToken", mock.Anything, "refresh-token-1", "sess-1", mock.Anything).Return(nil) }, check: func(t *testing.T, reqCtx *models.RequestContext) { @@ -93,7 +93,7 @@ func TestIssueTokensHook(t *testing.T) { reqCtx.Values[models.ContextSessionID.String()] = "sess-1" }, mock: func(tokenSvc *jwttests.MockTokenService, refreshSvc *jwttests.MockRefreshTokenService) { - tokenSvc.On("GenerateUserToken", mock.Anything, "user-1", "sess-1").Return(nil, errHookTest) + tokenSvc.On("GenerateUserToken", mock.Anything, "user-1", "sess-1", mock.Anything).Return(nil, errHookTest) }, wantErr: "failed to generate authentication tokens", }, diff --git a/plugins/jwt/services/interfaces.go b/plugins/jwt/services/interfaces.go index 934a5635..794b43f4 100644 --- a/plugins/jwt/services/interfaces.go +++ b/plugins/jwt/services/interfaces.go @@ -10,8 +10,9 @@ import ( ) type TokenService interface { - GenerateUserToken(ctx context.Context, userID string, sessionID string) (*types.TokenPair, error) + GenerateUserToken(ctx context.Context, userID string, sessionID string, extraClaims map[string]any) (*types.TokenPair, error) GenerateMachineToken(ctx context.Context, clientID string, organizationID string, scopes []string) (*types.TokenPair, error) + ExtractClaims(ctx context.Context, token string) (map[string]any, error) } type KeyService interface { diff --git a/plugins/jwt/services/refresh_token_service.go b/plugins/jwt/services/refresh_token_service.go index c9e7df15..54e00a06 100644 --- a/plugins/jwt/services/refresh_token_service.go +++ b/plugins/jwt/services/refresh_token_service.go @@ -98,7 +98,7 @@ func (s *refreshTokenService) RefreshTokensWithMetadata(ctx context.Context, ref s.emitTokenReuseRecoveredEvent(record.SessionID, tokenHash, deltaMs, gracePeriodMs, auditMeta) // Continue with normal token rotation - user stays logged in - return s.completeTokenRotation(ctx, tokenHash, record) + return s.completeTokenRotation(ctx, tokenHash, record, refreshToken) } } @@ -132,11 +132,11 @@ func (s *refreshTokenService) RefreshTokensWithMetadata(ctx context.Context, ref } // Token not revoked - normal rotation flow - return s.completeTokenRotation(ctx, tokenHash, record) + return s.completeTokenRotation(ctx, tokenHash, record, refreshToken) } // completeTokenRotation handles the token rotation after validation passes -func (s *refreshTokenService) completeTokenRotation(ctx context.Context, tokenHash string, record *types.RefreshToken) (*types.RefreshTokenResponse, error) { +func (s *refreshTokenService) completeTokenRotation(ctx context.Context, tokenHash string, record *types.RefreshToken, rawRefreshToken string) (*types.RefreshTokenResponse, error) { // Check if token is expired if time.Now().After(record.ExpiresAt) { return nil, fmt.Errorf("refresh token expired") @@ -160,8 +160,27 @@ func (s *refreshTokenService) completeTokenRotation(ctx context.Context, tokenHa return nil, fmt.Errorf("failed to rotate token") } - // STEP 2: Generate new token pair - tokenPair, err := s.jwtService.GenerateUserToken(ctx, session.UserID, record.SessionID) + // STEP 2: Extract non-standard claims from old refresh token to carry forward + var extraClaims map[string]any + if s.jwtService != nil { + if claims, err := s.jwtService.ExtractClaims(ctx, rawRefreshToken); err == nil { + extraClaims = make(map[string]any) + for k, v := range claims { + switch k { + case "sub", "user_id", "session_id", "iat", "exp", "jti", + "token_type", "actor_type", "organization_id", "scopes": + continue + default: + extraClaims[k] = v + } + } + if len(extraClaims) == 0 { + extraClaims = nil + } + } + } + + tokenPair, err := s.jwtService.GenerateUserToken(ctx, session.UserID, record.SessionID, extraClaims) if err != nil { s.logger.Error("failed to generate new tokens", "user_id", session.UserID, "session_id", record.SessionID, "error", err) return nil, fmt.Errorf("failed to generate tokens") diff --git a/plugins/jwt/services/refresh_token_service_test.go b/plugins/jwt/services/refresh_token_service_test.go index 687bcca8..402b4856 100644 --- a/plugins/jwt/services/refresh_token_service_test.go +++ b/plugins/jwt/services/refresh_token_service_test.go @@ -81,7 +81,8 @@ func TestRefreshTokenService_RefreshTokens(t *testing.T) { f.repo.On("GetRefreshToken", ctx, mock.Anything).Return(record, nil) f.sessionSvc.On("GetByID", ctx, "sess-1").Return(session, nil) f.repo.On("RevokeRefreshToken", ctx, mock.Anything).Return(nil) - f.tokenSvc.On("GenerateUserToken", ctx, "user-1", "sess-1").Return(tokenPair, nil) + f.tokenSvc.On("ExtractClaims", mock.Anything, mock.Anything).Return(map[string]any{}, nil).Maybe() + f.tokenSvc.On("GenerateUserToken", ctx, "user-1", "sess-1", mock.Anything).Return(tokenPair, nil) f.repo.On("StoreRefreshToken", ctx, mock.Anything).Return(nil) }, }, @@ -150,7 +151,8 @@ func TestRefreshTokenService_RefreshTokens(t *testing.T) { f.repo.On("SetLastReuseAttempt", ctx, mock.Anything).Return(nil) f.sessionSvc.On("GetByID", ctx, "sess-1").Return(session, nil) f.repo.On("RevokeRefreshToken", ctx, mock.Anything).Return(nil) - f.tokenSvc.On("GenerateUserToken", ctx, "user-1", "sess-1").Return(tokenPair, nil) + f.tokenSvc.On("ExtractClaims", mock.Anything, mock.Anything).Return(map[string]any{}, nil).Maybe() + f.tokenSvc.On("GenerateUserToken", ctx, "user-1", "sess-1", mock.Anything).Return(tokenPair, nil) f.repo.On("StoreRefreshToken", ctx, mock.Anything).Return(nil) }, }, diff --git a/plugins/jwt/services/token_service.go b/plugins/jwt/services/token_service.go index e6ee8c4a..6e33aed0 100644 --- a/plugins/jwt/services/token_service.go +++ b/plugins/jwt/services/token_service.go @@ -49,11 +49,11 @@ func NewJWTService( } } -func (s *tokenService) detectAlgorithmFromKey(k jwk.Key) jwa.SignatureAlgorithm { +func (s *tokenService) detectAlgorithmFromKey() jwa.SignatureAlgorithm { return jwa.EdDSA() } -func (s *tokenService) GenerateUserToken(ctx context.Context, userID string, sessionID string) (*types.TokenPair, error) { +func (s *tokenService) GenerateUserToken(ctx context.Context, userID string, sessionID string, extraClaims map[string]any) (*types.TokenPair, error) { if sessionID == "" { return nil, errors.New("session id is required to generate tokens") } @@ -77,7 +77,7 @@ func (s *tokenService) GenerateUserToken(ctx context.Context, userID string, ses return nil, fmt.Errorf("failed to set key ID: %w", err) } - keyAlgorithm := s.detectAlgorithmFromKey(privKey) + keyAlgorithm := s.detectAlgorithmFromKey() now := time.Now() jti := uuid.New().String() @@ -107,6 +107,12 @@ func (s *tokenService) GenerateUserToken(ctx context.Context, userID string, ses return nil, fmt.Errorf("failed to set actor_type: %w", err) } + for k, v := range extraClaims { + if err := accessClaims.Set(k, v); err != nil { + return nil, fmt.Errorf("failed to set extra claim %s: %w", k, err) + } + } + accessTokenBytes, err := jwt.Sign(accessClaims, jwt.WithKey(keyAlgorithm, privKey)) if err != nil { return nil, fmt.Errorf("failed to sign access token: %w", err) @@ -138,6 +144,12 @@ func (s *tokenService) GenerateUserToken(ctx context.Context, userID string, ses return nil, fmt.Errorf("failed to set actor_type in refresh token: %w", err) } + for k, v := range extraClaims { + if err := refreshClaims.Set(k, v); err != nil { + return nil, fmt.Errorf("failed to set extra claim %s in refresh token: %w", k, err) + } + } + refreshTokenBytes, err := jwt.Sign(refreshClaims, jwt.WithKey(keyAlgorithm, privKey)) if err != nil { return nil, fmt.Errorf("failed to sign refresh token: %w", err) @@ -171,7 +183,7 @@ func (s *tokenService) GenerateMachineToken(ctx context.Context, clientID string return nil, fmt.Errorf("failed to set key ID: %w", err) } - keyAlgorithm := s.detectAlgorithmFromKey(privKey) + keyAlgorithm := s.detectAlgorithmFromKey() now := time.Now() jti := uuid.New().String() @@ -320,5 +332,36 @@ func (s *tokenService) ValidateToken(ctx context.Context, token string) (*models actor.ID = userID actor.Type = models.ActorUser + // Passthrough any extra claims from the token into Actor.Claims + for _, key := range parsedToken.Keys() { + switch key { + case jwt.SubjectKey, jwt.IssuedAtKey, jwt.ExpirationKey, jwt.JwtIDKey, + "user_id", "session_id", "token_type", "actor_type", + "organization_id", "scopes": + continue + default: + var val any + if err := parsedToken.Get(key, &val); err == nil { + actor.Claims[key] = val + } + } + } + return actor, nil } + +func (s *tokenService) ExtractClaims(ctx context.Context, token string) (map[string]any, error) { + parsedToken, err := jwt.Parse([]byte(token), jwt.WithValidate(false)) + if err != nil { + return nil, fmt.Errorf("failed to parse token: %w", err) + } + + claims := make(map[string]any) + for _, key := range parsedToken.Keys() { + var val any + if err := parsedToken.Get(key, &val); err == nil { + claims[key] = val + } + } + return claims, nil +} diff --git a/plugins/jwt/services/token_service_test.go b/plugins/jwt/services/token_service_test.go index 6188f24a..ff61ea28 100644 --- a/plugins/jwt/services/token_service_test.go +++ b/plugins/jwt/services/token_service_test.go @@ -320,7 +320,7 @@ func TestTokenService_GenerateUserToken(t *testing.T) { tt.setupMocks(f) ctx := context.Background() - pair, err := svc.GenerateUserToken(ctx, tt.userID, tt.sessionID) + pair, err := svc.GenerateUserToken(ctx, tt.userID, tt.sessionID, nil) if tt.wantErr != "" { require.Error(t, err) diff --git a/plugins/jwt/tests/mocks.go b/plugins/jwt/tests/mocks.go index b85ca98d..d0f7541a 100644 --- a/plugins/jwt/tests/mocks.go +++ b/plugins/jwt/tests/mocks.go @@ -82,14 +82,22 @@ var _ repositories.JWKSRepository = (*MockJWKSRepository)(nil) type MockTokenService struct{ mock.Mock } -func (m *MockTokenService) GenerateUserToken(ctx context.Context, userID string, sessionID string) (*jwtservicesTypes.TokenPair, error) { - args := m.Called(ctx, userID, sessionID) +func (m *MockTokenService) GenerateUserToken(ctx context.Context, userID string, sessionID string, extraClaims map[string]any) (*jwtservicesTypes.TokenPair, error) { + args := m.Called(ctx, userID, sessionID, extraClaims) if args.Get(0) == nil { return nil, args.Error(1) } return args.Get(0).(*jwtservicesTypes.TokenPair), args.Error(1) } +func (m *MockTokenService) ExtractClaims(ctx context.Context, token string) (map[string]any, error) { + args := m.Called(ctx, token) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(map[string]any), args.Error(1) +} + func (m *MockTokenService) GenerateMachineToken(ctx context.Context, clientID string, organizationID string, scopes []string) (*jwtservicesTypes.TokenPair, error) { args := m.Called(ctx, clientID, organizationID, scopes) if args.Get(0) == nil { diff --git a/plugins/organizations/constants/constants.go b/plugins/organizations/constants/constants.go index a38a9075..0234903c 100644 --- a/plugins/organizations/constants/constants.go +++ b/plugins/organizations/constants/constants.go @@ -3,6 +3,7 @@ package constants // Permissions const ( + All = "organizations:*" OrganizationsCreatePermission = "organizations:create" OrganizationsListPermission = "organizations:list" OrganizationsReadPermission = "organizations:read" diff --git a/plugins/organizations/plugin.go b/plugins/organizations/plugin.go index a1a26970..3d4aeb0e 100644 --- a/plugins/organizations/plugin.go +++ b/plugins/organizations/plugin.go @@ -121,6 +121,7 @@ func (p *OrganizationsPlugin) Close() error { func (p *OrganizationsPlugin) ensurePermissions() error { if err := p.accessControlService.EnsurePermissions(context.Background(), []rootservices.PermissionDefinition{ + {Key: orgconstants.All, Description: "All organizations permissions"}, {Key: orgconstants.OrganizationsCreatePermission, Description: "Create organizations"}, {Key: orgconstants.OrganizationsListPermission, Description: "List organizations"}, {Key: orgconstants.OrganizationsReadPermission, Description: "Read organization details"},