thamtech / yii2-ratelimiter-advanced

Advanced Rate Limiter is a Yii2 filter to enforce or monitor request rate limits.
Other
38 stars 1 forks source link

"Trying to get property 'id' of non-object" error #2

Open tsvetiligo opened 4 years ago

tsvetiligo commented 4 years ago

I try to use "Basic Limit Per User ID" code from recipes and get "Trying to get property 'id' of non-object" error. I use HttpBearerAuth and user don't login yet then this code runs.

I want to use limit per user if user is logged in, and if not - limit per IP. How can I achieve this behavior?

behaviors() code in my controller:

public function behaviors()
    {
        $behaviors = parent::behaviors();

        unset($behaviors['rateLimiter']);

        $behaviors['authenticator'] = [
            'class' => \yii\filters\auth\CompositeAuth::className(),
            'authMethods' => [
                \yii\filters\auth\HttpBearerAuth::className(),
            ],
        ];

        $behaviors['verbs'] = [
            'class' => \yii\filters\VerbFilter::className(),
            'actions' => $this->actions,
        ];

        // remove authentication filter
        $auth = $behaviors['authenticator'];
        unset($behaviors['authenticator']);

        // add CORS filter
        $behaviors['corsFilter'] = [
            'class' => \yii\filters\Cors::className(),
            'cors' => [
                'Origin' => ['*'],
                'Access-Control-Request-Method' => ['GET', 'POST', 'PUT', 'DELETE', 'OPTIONS'],
                'Access-Control-Request-Headers' => ['*'],
            ],
        ];

        // re-add authentication filter
        $behaviors['authenticator'] = $auth;
        // avoid authentication on CORS-pre-flight requests (HTTP OPTIONS method)
        $behaviors['authenticator']['except'] = array_merge($this->getExcept(), ['options']);
        $behaviors['authenticator']['optional'] = $this->getOptional();

        $behaviors['access'] = [
            'class' => \yii\filters\AccessControl::className(),
            'rules' => $this->rules,
        ];

        $behaviors['rateLimiter'] = [
            'class' => 'thamtech\ratelimiter\RateLimiter',
            'components' => [
                'rateLimit' => [
                    'definitions' => [
                        'basic' => [
                            // at most 100 requests within 600 seconds
                            'limit' => 100,
                            'window' => 600,
                            // this causes a separate rate limit to be tracked for
                            // each user ID
                            'identifier' => Yii::$app->user->getIdentity()->id,
                        ],
                    ],
                ],
                'allowanceStorage' => [
                    'cache' => 'cache', // use Yii::$app->cache component
                ],
                // add X-Rate-Limit-* HTTP headers to the response
                'as rateLimitHeaders' => 'thamtech\ratelimiter\handlers\RateLimitHeadersHandler',
                // throw TooManyRequestsHttpException when the limit is exceeded
                'as tooManyRequestsException' => 'thamtech\ratelimiter\handlers\TooManyRequestsHttpExceptionHandler',
            ]
        ];

        return $behaviors;
    }

Also I try to implement RateLimitInterface in my User model, but it don't work at all.

in User model:

public function getRateLimit($context)
    {
        $RateLimit = new RateLimit();
        $RateLimit->limit = 1;
        $RateLimit->window = 60;
        $RateLimit->identifier = $this->getId();

        return $RateLimit;
    }

in controller:

$behaviors['rateLimiter'] = [
            'class' => 'thamtech\ratelimiter\RateLimiter',
            'components' => [
                'rateLimit' => [
                    'definitions' => [
                        'user' => [
                            'class' => 'app\models\User', // implements RateLimitInterface
                        ],
                    ],
                ],
                'allowanceStorage' => [
                    'cache' => 'cache', // use Yii::$app->cache component
                ],
                // add X-Rate-Limit-* HTTP headers to the response
                'as rateLimitHeaders' => 'thamtech\ratelimiter\handlers\RateLimitHeadersHandler',
                // throw TooManyRequestsHttpException when the limit is exceeded
                'as tooManyRequestsException' => 'thamtech\ratelimiter\handlers\TooManyRequestsHttpExceptionHandler',
            ]
        ];
tyler-ham commented 4 years ago

There are a couple of ways you might go about this. If some of your actions are authenticated and some are not, you could establish two separate rate limiter behaviors and use only and except arrays to specify which actions each should apply two.

The rate limiter behavior applying to authenticated actions on which the user identity is established could reference Yii::$app->user->getIdentity()->id as the identifier. The rate limiter behavior applying to unauthenticated actions on which the user identity is not established could reference an inline method that returns the IP address, like function($context, $rateLimitId) {return $context->request->getUserIP();}.

Alternatively, you may prefer to just use your user identity's ID when available and fallback to IP address regardless of the action being called. In this case, you could combine this logic into your own identifier function like:

'identifier' => function($context, $rateLimitId) {
    if (!empty(Yii::$app->user->getIdentity()->id)) {
        return Yii::$app->user->getIdentity()->id;
    }
    return $context->request->getUserIP();
}
tsvetiligo commented 4 years ago

I apply rate limiter behavior to authenticated actions and get "Trying to get property 'id' of non-object" error. My behaviors() method in the code above. Standard Yii2 rate limiter works fine in this behaviors().

tyler-ham commented 4 years ago

There are a couple of problems with your approach on the User model implementing RateLimitInterface. One might be issue #3 (if so, see the workaround in that issue).

The other is that your User model probably doesn't have an ID yet, so the $this->getId() call is probably returning an empty/null value. It gets instantiated like this:

$rateLimit = Yii::createObject([
    'class' => 'app\models\User',
]);

You wouldn't expect to be able to get an ID from $rateLimit->getId() in this case unless your User model does some initialization to a default user record. A more typical way of accessing a User model would involve retrieving a record from the database first, like:

$id = 123;
$user = User::findOne($id);

Then you would expect 123 to be returned by $user->getId().

In any case, unless you are wanting to vary the limit and window values depending on the request context, you shouldn't need to implement your own instance of RateLimitInterface. When just the identifier is varying, you should have all the flexibility you need to set the identifier property of the default rate limit definition:

// logged-in identity's ID
'identifier' => Yii::$app->user->getIdentity()->id,

// IP address
'identifier' => function($context, $rateLimitId) {
    return $context->request->getUserIP();
},

// Any other value you want to come up with
'identifier' => function($context, $rateLimitId) {
    $identifier = ...; // any other static, dynamic value or algorithm you need
    return $identifier;
},
tyler-ham commented 4 years ago

Does the "Trying to get property 'id' of non-object" message refer to the Yii::$app->user->getIdentity()->id line?

Try wrapping the identity definition in a function:

'identifier' => function($context, $rateLimitId) {
    return Yii::$app->user->getIdentity()->id;
},

That would defer referencing the ID until the rate limit is actually being checked as opposed to when the behavior is being defined.

tsvetiligo commented 4 years ago

"Basic Limit Per User ID" from Recipes still don't work if I add wrapping (no error, no rate limit), but if I get example from README.md and add wrapping it works great! Thanx! Please update docs.

tyler-ham commented 4 years ago

I'm glad to hear you got it working. Feel free to submit a pull request on the documentation updates.