Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix triggering a missing attribute violation #978

Merged
merged 1 commit into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions src/Sentry/Laravel/EventHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -274,14 +274,24 @@ private function configureUserScopeFromModel($authUser): void

// If the user is a Laravel Eloquent model we try to extract some common fields from it
if ($authUser instanceof Model) {
$username = $authUser->getAttribute('username');
$email = null;

if ($this->modelHasAttribute($authUser, 'email')) {
$email = $authUser->getAttribute('email');
} elseif ($this->modelHasAttribute($authUser, 'mail')) {
$email = $authUser->getAttribute('mail');
}

$username = $this->modelHasAttribute($authUser, 'username')
? (string)$authUser->getAttribute('username')
: null;

$userData = [
'id' => $authUser instanceof Authenticatable
? $authUser->getAuthIdentifier()
: $authUser->getKey(),
'email' => $authUser->getAttribute('email') ?? $authUser->getAttribute('mail'),
'username' => $username === null ? $username : (string)$username,
'email' => $email,
'username' => $username,
];
}

Expand All @@ -305,6 +315,15 @@ private function configureUserScopeFromModel($authUser): void
});
}

private function modelHasAttribute(Model $model, string $key): bool
{
// Taken from: https://github.com/laravel/framework/blob/v11.41.3/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php#L445
// Laravel 11 introduced the `hasAttribute` method we are (almost) mirroring here since it's not available in earlier Laravel versions we support
return array_key_exists($key, $model->getAttributes()) ||
$model->hasGetMutator($key) ||
(method_exists($model, 'hasAttributeMutator') && $model->hasAttributeMutator($key));
}

protected function octaneRequestReceivedHandler(Octane\RequestReceived $event): void
{
$this->prepareScopeForOctane();
Expand Down
26 changes: 15 additions & 11 deletions test/Sentry/EventHandler/AuthEventsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@ class AuthEventsTest extends TestCase

public function testAuthenticatedEventFillsUserOnScope(): void
{
$user = new AuthEventsTestUserModel();
$user = new AuthEventsTestUserModel;

$user->id = 123;
$user->username = 'username';
$user->email = '[email protected]';
$user->forceFill([
'id' => 123,
'username' => 'username',
'email' => '[email protected]',
]);

$scope = $this->getCurrentSentryScope();

Expand All @@ -29,17 +31,19 @@ public function testAuthenticatedEventFillsUserOnScope(): void

$this->assertNotNull($scope->getUser());

$this->assertEquals($scope->getUser()->getId(), 123);
$this->assertEquals($scope->getUser()->getUsername(), 'username');
$this->assertEquals($scope->getUser()->getEmail(), '[email protected]');
$this->assertEquals(123, $scope->getUser()->getId());
$this->assertEquals('username', $scope->getUser()->getUsername());
$this->assertEquals('[email protected]', $scope->getUser()->getEmail());
}

public function testAuthenticatedEventFillsUserOnScopeWhenUsernameIsNotAString(): void
{
$user = new AuthEventsTestUserModel();

$user->id = 123;
$user->username = 456;
$user->forceFill([
'id' => 123,
'username' => 456,
]);

$scope = $this->getCurrentSentryScope();

Expand All @@ -49,8 +53,8 @@ public function testAuthenticatedEventFillsUserOnScopeWhenUsernameIsNotAString()

$this->assertNotNull($scope->getUser());

$this->assertEquals($scope->getUser()->getId(), 123);
$this->assertEquals($scope->getUser()->getUsername(), '456');
$this->assertEquals(123, $scope->getUser()->getId());
$this->assertEquals('456', $scope->getUser()->getUsername());
}

public function testAuthenticatedEventDoesNotFillUserOnScopeWhenPIIShouldNotBeSent(): void
Expand Down