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

API Tokens from Model with HasApiTokens Trait Incorrectly Recorded as User ID #415

Open
Sairahcaz opened this issue Oct 23, 2024 · 4 comments

Comments

@Sairahcaz
Copy link

Sairahcaz commented Oct 23, 2024

Pulse Version

v1.2.5

Laravel Version

v11.29.0

PHP Version

8.2.12

Livewire Version

v3.5.4

Database Driver & Version

mysql Ver 8.0.39-0ubuntu0.22.04.1 for Linux on x86_64 ((Ubuntu))

Description

Let's say you have a model called Client and it has the HasApiTokens trait.
If you create api tokens for this model and use them to auth/interact with your API, the id of the associated Client is used as the user id for the Application Usage pulse card.

In the screenshot below you can see Syafiq (lets say user id 100) and Aaln (user id 101) are listed, but actually the traffic is made by the api requests with client tokens (client with id 100 and id 101).

Bildschirmfoto 2024-10-23 um 23 06 15

Steps To Reproduce

Create model xyz give it the HasApiToken trait, create a token, interact with your api...

@timacdonald
Copy link
Member

I suppose because it is a stateless request, it doesn't pull the actual user's details into the guard and only has the token to work with.

You would likely need to bind a custom instance of Laravel\Pulse\Contracts\ResolvesUsers to the container.

I wonder if we could have a built in implementation just for Sanctum / Passport?

@jessarcher
Copy link
Member

There are some notes about this in the docs, but essentially Pulse only captures the user's ID by default. This is a problem when there are multiple models that authenticate because there's no way to determine which model the ID belongs to when resolving it in the dashboard.

As @timacdonald mentioned, you'd need to bind a custom instance of Laravel\Pulse\Contracts\ResolvesUsers.

For this scenario, you'd probably want the key method to return a concatenated representation of the model and user ID. For example, user:100 or ["App\Models\User", 100].

The find method will receive a key to display in the dashboard and must return an object containing the users name and optionally an extra and avatar field. You could parse the key back into a model and ID before retrieving it, however, to avoid N+1 queries, the relevant keys will first be passed to the load method. In this method you could group them by model and then run a single query per model. You'd want to hold the values in a local property so that you can retrieve them from the find method without running an additional queries.

See the Laravel\Pulse\Users class for the default implementation.

@Sairahcaz
Copy link
Author

@jessarcher thanks for your help. It worked!

That bug really drove me nuts, as I really was thinking user xyz was doing all the traffic but he wasn't :D

Maybe it would be cool to have this solution built in to the package itself!?
If multiple auth/user model types are detected you could show tabs above the users listing. In my case for instance there would be a tab for "Users" and "Clients". It would not affect the current UI (just only if multiple types are detected).

Unfortunately the old data can't be used anymore and I needed to execute php artisan pulse:clear, cause it was no longer compatible with the code, this should be taken care of or also support the old structure...

@Sairahcaz
Copy link
Author

One thing I also noticed:
There is also an issue, if the User or in my case the Client model has a global scope.

I have a simple multi tenant approach, where I use a global scope to add the current auth users team_id to all queries like so:

public static function bootHasTeamScope(): void
{
    parent::addGlobalScope('team', function (Builder $query) {
        if (auth()->check()) {
            $query->where('team_id', auth()->user()->current_team_id);
        }
    });

    static::saving(function ($model) {
        if (auth()->check()) {
            $model->team_id = auth()->user()->current_team_id;
        }
    });
}

Cause of that, pulse would only show the Users from the current auth users team (let's say an admin who is viewing the pulse dashboard).

Thats why I also needed to add withoutGlobalScopes in the load function:

$this->resolvedUsers = $model::withoutGlobalScopes()->findMany($keys);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants