search

wptide / wptide

:ocean: Tide is a series of automated tests run against every WordPress.org theme and plugin
http://make.wordpress.org/tide
MIT License
82 stars 29 forks source link

Rate limit changes should be reflected on the API Client #178

Open valendesigns opened 5 years ago

valendesigns commented 5 years ago

Issue Overview

Currently the rate limit is incrementing on the Audit Client instead of the API Client for proxied requests. Meaning that the PHPCS & Lighthouse Servers use an Audit Client to proxy the request on behalf of the API Client. Those servers make POST requests to the API to write data on behalf of the wporg user and should be exempt of all rate limiting.

Steps to Reproduce

You would login to the WP admin and check the rate limit values on the user profile page, then run an audit and check that the used API requests goes up for the wporg user (or any other user for that matter) instead of the audit-server user.

Expected Behavior

The used requests for the wporg user go up when requesting an audit, and the audit-server is exempt from rate limiting.

Current Behavior

When requesting an audit the audit-server used requests go up and the rate limit for the wporg user stay the same, unless a POST request was used to request the audit and in that case both users get 1 request added to the totals instead of 2 to the wporg user and zero to the audit-server.

Possible Solutions

Inside the rate limit class check for the Audit Client role and if we're doing a proxied request for the API Client. The request would increment the user the request was proxied for. However, if the request_client was not set then it could only mean the request was made directly and we should increment the limit or completely remove rate limiting from the audit client role. Up for discussion on this.

kkoppenhaver commented 4 years ago

Started to look into this, but when I hit http://tide.local/api/tide/v1/audit/wporg/theme/twentyseventeen/2.1/ after running all the setup steps, I just get the homepage of the WP install. Still trying to track down why that's happening.

kkoppenhaver commented 4 years ago

Activating the WP Tide API and WPOrg Tide API plugins seems to have helped as well as setting the permalink settings to Post name

kkoppenhaver commented 4 years ago

We've discovered that this is a configuration level issue rather than a code level problem.

Swapping the API_KEY and API_SECRET values that are in the .env file to those for the wporg user caused the counts to be incremented correctly, so this change will need to be made in the cloud as well.

jeffpaul commented 4 years ago

@derekherman sounds like we need you to change the config (.env) files on GCP to resolve this, is that correct?

jeffpaul commented 4 years ago

From today's Tidechat:

We need to figure out a fix, but essentially it doesn’t assign the request to the api user account but to the audit client We have to use the api key:secret for the audit client to make requests but as the request comes in it should know what user it’s making a request for but it’s not working as expected It should proxy the request then assign it to the api client user

@kkoppenhaver are you able to help take a look at what's needed here?