Sentry-laravel: Setting the user IP (with trusted proxies)

[stayallive]

As reported in getsentry/sentry-php#585, when using trusted proxies (since Laravel 5.5) the Sentry SDK possibly doesn't get the right user IP address (not tested in Laravel but is the case in Symfony). We should retrieve this info from the framework.

[georgeboot]

As a workaround, I created a SentryServiceProvider.php in which I added the following code:

```php
use Sentry\Laravel\Facade as Sentry;
use Sentry\State\Scope;

Sentry::configureScope(function (Scope $scope): void {
$scope->setUser([
'id' => auth()->id(),
'email' => auth()->user()->email ?? null
'ip_address' => request()->ip(),
]);
});

[shiny]

When the latest Laravel Framework running on php-fpm which in a docker container, the ip address info is noneffective like 172.23.0.8.

Docker getting widely used today, suggest a better document for this situation.

[shiny]

My solution:

app/Exceptions/Handler.php

    /**
     * Report or log an exception.
     *
     * @param  \Exception  $exception
     * @return void
     */
    public function report(Exception $exception)
    {
        if (app()->bound('sentry') && $this->shouldReport($exception)) {
            \Sentry\configureScope(function (\Sentry\State\Scope $scope) {
                $scope->setUser([
                    'ip_address' => request()->ip()
                ]);
            });
            app('sentry')
                ->captureException($exception);
        }

        parent::report($exception);
    }

[markkimsal]

None of the solutions worked for me. I had to call app('sentry')->getClient() to ensure the top Layer had a client before calling configScope. Calling configureScope too early silently fails.

//app/Providers/SentryUserProvider.php
namespace App\Providers;
use Illuminate\Support\ServiceProvider;

class SentryUserProvider extends ServiceProvider
{
    public function boot()
    {
        if (app()->bound('sentry') && config('app.debug') == false) {
            //this is required to get a client and a layer in the sentry hub...
            //if there's no client, the configure scope just silently fails
            app('sentry')->getClient();
            \Sentry\Laravel\Integration::configureScope(function (\Sentry\State\Scope $scope): void {
                $u = \Auth()->user()
                $scope->setUser([
                    'email' => optional($u)->email,
                    'id' => optional($u)->id,
                    'name' => optional($u)->first_name. ' ' .optional($u)->last_name,
                    'ip_address' => request()->ip(),
                ]);
            });
        }
    }
}

//app/Http/Middleware/TrustProxies.php
namespace App\Http\Middleware;

use Illuminate\Http\Request
use Fideloper\Proxy\TrustProxies as Middleware;

class TrustProxies extends Middleware
{
    protected $proxies = [
        '10.0.0.0/8',   //for vpns, etc
        '172.0.0.0/8',  //for docker
    ]; 
    protected $headers = Request::HEADER_X_FORWARDED_ALL; 
}

//config/app.php
    'providers' => [
        // ...
        App\Providers\SentryUserProvider::class,
    ],

[adam1010]

Is this on the roadmap? I use the configureScope technique and it works most of the time, but many requests still show my load balancer's IP address on Sentry. (So the user context has not been set before Sentry triggers the event)

In the base sentry-php repo they're defaulting to the REMOTE_ADDR if the user context hasn't been set:
https://github.com/getsentry/sentry-php/blob/master/src/Integration/RequestIntegration.php#L103

Since the sentry-php repo is responsible for passing in that request object, we could easily overwrite REMOTE_ADDR using Request::ip() so that it's correct when the user context isn't set.

[stayallive]

I have a draft PR in the works trying to solve this: #419.

First iteration I added the IP address like you all have been doing, after the user authenticated, but it needs to happen much sooner and I'm still trying to see what the best event would be to do that where the request is available to us. Open to suggestions!

[markkimsal]

There doesn't seem to be any internal laravel events earlier than authorized user.

The solutions presented here all use the ServiceProvider feature of Laravel. Why not just do it in LaravelSentryServiceProvider::configure() where the user ID is set?

It seems like Sentry considers the IP to be a property of the user and not of the event/log. But, I can't envision a log or crash where I would not want to see the IP reported. So I would suggest uncoupling the IP from any sort of user events.

            $client = SentryLaravel::getClient(array_merge(array(  // ....
            // ...
            /// ));
            if ($app->bound('request')) {
                 /** @var \Illuminate\Http\Request $request */
                 $request = $app->make('request');
                 $client->user_context([
                       'ip_address' => $request->ip(),
                 ]);
            }

[adam1010]

When is $app->bound('sentry') equal to false? The example code would cause the IP for the user to not be set when false which would then cause line 101 (https://github.com/getsentry/sentry-php/blob/master/src/Integration/RequestIntegration.php#L101) to default it to REMOTE_ADDR

Is there a guaranteed way to make sure that sentry is bound so that we don't just skip initializing the user? If not, it seems the solution is to pass in the Event with REMOTE_ADDR set to Request::ip() on line 76 in that same file.

[adam1010]

Has there been any movement here? I'm getting wildly inaccurate "unique user" metrics from Sentry because the load balancer has a fixed IP address so most traffic shows as having that same IP

[adam1010]

This seems to fix the issue: In my AppServiceProvider.php

  public function register(){
    if(app()->bound('sentry')){
      app('sentry')->getClient()->getOptions()
        ->setBeforeSendCallback(function(Event $e){
          $e->setRequest(array_merge($e->getRequest(), ['url' => request()->fullUrl()]));
          $e->getUserContext()->setIpAddress(request->ip());
          return $e;
        });
    }

[stayallive]

So did anyone try the changes I proposed in #419 already to see if that solves the issue? 😄

I do like to uncouple it from the authenticated event because yes that would be nice, but I also am at a loss what other event to take for this because Laravel does not have a "I am handling an event now" event.

The reason why we check $app->bound('request') because we could be in that code on a console invocation which would not have a request available, it's just a safety, this will always be there on a normal HTTP request.

---

I have added a global middleware to #419 that should set the request IP and also should run after the trust proxies middleware, please test it out and let me know it that is a good solution.