diff --git a/app/controllers/api/users.php b/app/controllers/api/users.php index a4fac593e2..d295f337ac 100644 --- a/app/controllers/api/users.php +++ b/app/controllers/api/users.php @@ -1233,7 +1233,7 @@ Http::patch('/v1/users/:userId/impersonator') ] )) ->param('userId', '', fn (Database $dbForProject) => new UID($dbForProject->getAdapter()->getMaxUIDLength()), 'User ID.', false, ['dbForProject']) - ->param('impersonator', false, new Boolean(true), 'Whether the user can impersonate other users. When true, the user can browse project users to choose a target and can pass impersonation headers to act as that user.') + ->param('impersonator', false, new Boolean(true), 'Whether the user can impersonate other users. When true, the user can browse project users to choose a target and can pass impersonation headers to act as that user. Internal audit logs still attribute impersonated actions to the original impersonator and store the target user details only in internal audit payload data.') ->inject('response') ->inject('dbForProject') ->inject('queueForEvents') diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index c96c28aa14..2862e23524 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -382,8 +382,11 @@ Http::init() } if (!empty($user->getId())) { + $impersonatorUserId = $user->getAttribute('impersonatorUserId'); $accessedAt = $user->getAttribute('accessedAt', 0); - if (DateTime::formatTz(DateTime::addSeconds(new \DateTime(), -APP_USER_ACCESS)) > $accessedAt) { + + // Skip updating accessedAt for impersonated requests so we don't attribute activity to the target user. + if (!$impersonatorUserId && DateTime::formatTz(DateTime::addSeconds(new \DateTime(), -APP_USER_ACCESS)) > $accessedAt) { $user->setAttribute('accessedAt', DateTime::now()); if ($project->getId() !== 'console' && APP_MODE_ADMIN !== $mode) { diff --git a/app/init/resources.php b/app/init/resources.php index ff3a90ffd7..5499524363 100644 --- a/app/init/resources.php +++ b/app/init/resources.php @@ -480,9 +480,13 @@ Http::setResource('user', function (string $mode, Document $project, Document $c $targetUser = $userDb->getAuthorization()->skip(fn () => $userDb->findOne('users', [Query::equal('phone', [$impersonatePhone])])); } if ($targetUser !== null && !$targetUser->isEmpty()) { - $impersonatorUserId = $user->getId(); + $impersonator = clone $user; $user = clone $targetUser; - $user->setAttribute('impersonatorUserId', $impersonatorUserId); + $user->setAttribute('impersonatorUserId', $impersonator->getId()); + $user->setAttribute('impersonatorUserInternalId', $impersonator->getSequence()); + $user->setAttribute('impersonatorUserName', $impersonator->getAttribute('name', '')); + $user->setAttribute('impersonatorUserEmail', $impersonator->getAttribute('email', '')); + $user->setAttribute('impersonatorAccessedAt', $impersonator->getAttribute('accessedAt', 0)); } } diff --git a/docs/references/users/update-user-impersonator.md b/docs/references/users/update-user-impersonator.md new file mode 100644 index 0000000000..c20e9de29f --- /dev/null +++ b/docs/references/users/update-user-impersonator.md @@ -0,0 +1 @@ +Enable or disable whether a user can impersonate other users. When impersonation headers are used, the request runs as the target user for API behavior, while internal audit logs still attribute the action to the original impersonator and store the impersonated target details only in internal audit payload data. diff --git a/src/Appwrite/Platform/Tasks/Specs.php b/src/Appwrite/Platform/Tasks/Specs.php index e030b2eaaf..606c03bf10 100644 --- a/src/Appwrite/Platform/Tasks/Specs.php +++ b/src/Appwrite/Platform/Tasks/Specs.php @@ -163,19 +163,19 @@ class Specs extends Action 'ImpersonateUserId' => [ 'type' => 'apiKey', 'name' => 'X-Appwrite-Impersonate-User-Id', - 'description' => 'Impersonate a user by ID on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins.', + 'description' => 'Impersonate a user by ID on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data.', 'in' => 'header', ], 'ImpersonateUserEmail' => [ 'type' => 'apiKey', 'name' => 'X-Appwrite-Impersonate-User-Email', - 'description' => 'Impersonate a user by email on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins.', + 'description' => 'Impersonate a user by email on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data.', 'in' => 'header', ], 'ImpersonateUserPhone' => [ 'type' => 'apiKey', 'name' => 'X-Appwrite-Impersonate-User-Phone', - 'description' => 'Impersonate a user by phone on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins.', + 'description' => 'Impersonate a user by phone on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data.', 'in' => 'header', ], ], @@ -219,19 +219,19 @@ class Specs extends Action 'ImpersonateUserId' => [ 'type' => 'apiKey', 'name' => 'X-Appwrite-Impersonate-User-Id', - 'description' => 'Impersonate a user by ID on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins.', + 'description' => 'Impersonate a user by ID on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data.', 'in' => 'header', ], 'ImpersonateUserEmail' => [ 'type' => 'apiKey', 'name' => 'X-Appwrite-Impersonate-User-Email', - 'description' => 'Impersonate a user by email on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins.', + 'description' => 'Impersonate a user by email on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data.', 'in' => 'header', ], 'ImpersonateUserPhone' => [ 'type' => 'apiKey', 'name' => 'X-Appwrite-Impersonate-User-Phone', - 'description' => 'Impersonate a user by phone on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins.', + 'description' => 'Impersonate a user by phone on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data.', 'in' => 'header', ], ], @@ -275,19 +275,19 @@ class Specs extends Action 'ImpersonateUserId' => [ 'type' => 'apiKey', 'name' => 'X-Appwrite-Impersonate-User-Id', - 'description' => 'Impersonate a user by ID on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins.', + 'description' => 'Impersonate a user by ID on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data.', 'in' => 'header', ], 'ImpersonateUserEmail' => [ 'type' => 'apiKey', 'name' => 'X-Appwrite-Impersonate-User-Email', - 'description' => 'Impersonate a user by email on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins.', + 'description' => 'Impersonate a user by email on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data.', 'in' => 'header', ], 'ImpersonateUserPhone' => [ 'type' => 'apiKey', 'name' => 'X-Appwrite-Impersonate-User-Phone', - 'description' => 'Impersonate a user by phone on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins.', + 'description' => 'Impersonate a user by phone on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data.', 'in' => 'header', ], ], diff --git a/src/Appwrite/Platform/Workers/Audits.php b/src/Appwrite/Platform/Workers/Audits.php index d7c57c7fed..55ec39026b 100644 --- a/src/Appwrite/Platform/Workers/Audits.php +++ b/src/Appwrite/Platform/Workers/Audits.php @@ -82,22 +82,31 @@ class Audits extends Action $ip = $payload['ip'] ?? ''; $user = new Document($payload['user'] ?? []); - $userName = $user->getAttribute('name', ''); - $userEmail = $user->getAttribute('email', ''); + $impersonatorUserId = $user->getAttribute('impersonatorUserId'); + $actorUserId = $impersonatorUserId ?: $user->getId(); + $actorUserInternalId = $impersonatorUserId + ? $user->getAttribute('impersonatorUserInternalId') + : $user->getSequence(); + $actorUserName = $impersonatorUserId + ? $user->getAttribute('impersonatorUserName', '') + : $user->getAttribute('name', ''); + $actorUserEmail = $impersonatorUserId + ? $user->getAttribute('impersonatorUserEmail', '') + : $user->getAttribute('email', ''); $userType = $user->getAttribute('type', ACTIVITY_TYPE_USER); // Create event data $eventData = [ - 'userId' => $user->getSequence(), + 'userId' => $actorUserInternalId, 'event' => $event, 'resource' => $resource, 'userAgent' => $userAgent, 'ip' => $ip, 'location' => '', 'data' => [ - 'userId' => $user->getId(), - 'userName' => $userName, - 'userEmail' => $userEmail, + 'userId' => $actorUserId, + 'userName' => $actorUserName, + 'userEmail' => $actorUserEmail, 'userType' => $userType, 'mode' => $mode, 'data' => $auditPayload, @@ -105,6 +114,21 @@ class Audits extends Action 'time' => date("Y-m-d H:i:s", $message->getTimestamp()), ]; + if (!empty($impersonatorUserId)) { + $eventData['data']['data'] = \is_array($auditPayload) + ? \array_merge($auditPayload, [ + 'impersonatedUserId' => $user->getId(), + 'impersonatedUserName' => $user->getAttribute('name', ''), + 'impersonatedUserEmail' => $user->getAttribute('email', ''), + ]) + : [ + 'payload' => $auditPayload, + 'impersonatedUserId' => $user->getId(), + 'impersonatedUserName' => $user->getAttribute('name', ''), + 'impersonatedUserEmail' => $user->getAttribute('email', ''), + ]; + } + if (isset($this->logs[$project->getSequence()])) { $this->logs[$project->getSequence()]['logs'][] = $eventData; } else { diff --git a/src/Appwrite/Utopia/Response/Model/Account.php b/src/Appwrite/Utopia/Response/Model/Account.php index b2656b7f3f..aaaf501a88 100644 --- a/src/Appwrite/Utopia/Response/Model/Account.php +++ b/src/Appwrite/Utopia/Response/Model/Account.php @@ -118,7 +118,7 @@ class Account extends Model ]) ->addRule('impersonatorUserId', [ 'type' => self::TYPE_STRING, - 'description' => 'ID of the user performing the impersonation. Present only when the current request is impersonating another user.', + 'description' => 'ID of the original actor performing the impersonation. Present only when the current request is impersonating another user. Internal audit logs attribute the action to this user, while the impersonated target is recorded only in internal audit payload data.', 'required' => false, 'default' => '', 'example' => '5e5ea5c16897e', diff --git a/src/Appwrite/Utopia/Response/Model/Log.php b/src/Appwrite/Utopia/Response/Model/Log.php index bc2c923494..a8c00280d3 100644 --- a/src/Appwrite/Utopia/Response/Model/Log.php +++ b/src/Appwrite/Utopia/Response/Model/Log.php @@ -18,19 +18,19 @@ class Log extends Model ]) ->addRule('userId', [ 'type' => self::TYPE_STRING, - 'description' => 'User ID.', + 'description' => 'User ID of the actor recorded for this log. During impersonation, this is the original impersonator, not the impersonated target user.', 'default' => '', 'example' => '610fc2f985ee0', ]) ->addRule('userEmail', [ 'type' => self::TYPE_STRING, - 'description' => 'User Email.', + 'description' => 'User email of the actor recorded for this log. During impersonation, this is the original impersonator.', 'default' => '', 'example' => 'john@appwrite.io', ]) ->addRule('userName', [ 'type' => self::TYPE_STRING, - 'description' => 'User Name.', + 'description' => 'User name of the actor recorded for this log. During impersonation, this is the original impersonator.', 'default' => '', 'example' => 'John Doe', ]) diff --git a/src/Appwrite/Utopia/Response/Model/User.php b/src/Appwrite/Utopia/Response/Model/User.php index fb74452029..476778e68b 100644 --- a/src/Appwrite/Utopia/Response/Model/User.php +++ b/src/Appwrite/Utopia/Response/Model/User.php @@ -148,7 +148,7 @@ class User extends Model ]) ->addRule('impersonatorUserId', [ 'type' => self::TYPE_STRING, - 'description' => 'ID of the user performing the impersonation. Present only when the current request is impersonating another user.', + 'description' => 'ID of the original actor performing the impersonation. Present only when the current request is impersonating another user. Internal audit logs attribute the action to this user, while the impersonated target is recorded only in internal audit payload data.', 'required' => false, 'default' => '', 'example' => '5e5ea5c16897e',