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

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

Open
valendesigns opened this issue Mar 14, 2019 · 5 comments
Open

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

valendesigns opened this issue Mar 14, 2019 · 5 comments

Comments

@valendesigns
Copy link
Contributor

valendesigns commented Mar 14, 2019

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member

jeffpaul commented Dec 3, 2019

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

@jeffpaul
Copy link
Member

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?

@jeffpaul jeffpaul assigned kkoppenhaver and unassigned derekherman Jan 21, 2020
@jeffpaul jeffpaul added this to To Do in PHP Compat on WordPress.org via automation Jun 23, 2020
@jeffpaul jeffpaul moved this from To Do to In progress in PHP Compat on WordPress.org Jun 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

4 participants