CUSTESC-36595: Retrieve the Wordpress domain only from the active zone #532
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -44,7 +44,15 @@ public function returnWordPressDomain() | |||
| // We tried to fetch a zone but it's possible we're using an API token, | |||
| // So try again with a zone name filtered API call | |||
| if (!$this->api->responseOk($response)) { | |||
| $zoneRequest = new Request('GET', 'zones/', array('name' => idn_to_ascii($this->wordpressAPI->getOriginalDomain(), IDNA_DEFAULT, INTL_IDNA_VARIANT_UTS46), array())); | |||
There was a problem hiding this comment.
we'll also want to update the call above on L42 in case the user is using API keys. iirc, you'll need to modify $this->request to include the params (or find the caller and update it there which is probably the more sustainable way forward).
There was a problem hiding this comment.
Following your suggestion, I've only changed the WordPressClientAPI.php file, which is the only other place I found performing a GET zones/ which may benefit from this.
There is a last GET zones/ call in https://github.com/aseure/Cloudflare-WordPress/blob/5c147468490835c94b5c8dd69952f80d23f30672/src/API/AbstractPluginActions.php#L99 but since it is only a call to check API key and email validity, I did not bother changing it to limit any unknown negative impact.
We recently noticed that a user got its zone incorrectly reported as inactive by the Wordpress plugin. After some investigations, it seems that two zones actually existed: an old purged one, and a new active zone. To guarantee that the plugin always retrieve the active one, if any, this commit changes the zone retrieval call to the Cloudflare API by adding a `status=active` query parameter to the request, [as documented here](https://developers.cloudflare.com/api/operations/zones-get).
aseure
force-pushed
the
aseure/CUSTESC-36595
branch
from
bec6e94 to
7db5b1e
Compare
jacobbednarz
approved these changes
Feb 27, 2024
aseure
added a commit
to aseure/Cloudflare-WordPress
that referenced
this pull request
Mar 4, 2024
**🔖 Summary** This release includes two minor fixes. First one is making sure that the only the active zone is fetched when retrieving the customer zone, preventing accidental fetching of a purged zone with the same name, which happened to at least one customer. Second change is about preventing some error banners from appearing on the user plugin page, which was due to failing calls to retrieve details about the Railgun settings of the user, which is now deprecated by Cloudflare, and whose endpoints have been decomissioned. **🚧 Changes** - [CUSTESC-36595: Retrieve the Wordpress domain only from the active zone](cloudflare#532) - [SPEED-777: Upgrade cloudflare-plugin-frontend to v3.8.0](cloudflare#535)
Merged
aseure
added a commit
to aseure/cloudflare-plugin-frontend
that referenced
this pull request
Mar 22, 2024
**Problem**
As part of an internal issue named CUSTESC-36595, one customer reported
us that his Cloudflare Wordpress plugin was reporting that his dashboard
was failing with the following error:
```json
"errors.noActiveZoneSelected": "It looks like your domain {domain} is not provisioned with Cloudflare. Please continue to {link} to secure and speed up your website.",
```
The problem was that the customer does have two Cloudflare zones with
the same name: one being active and the other got purged.
**Previous investigations**
We initially tried to fix this in the `Cloudflare-Wordpress` plugin
itself, via cloudflare/Cloudflare-WordPress#532,
by ensuring that the zone listing endpoint `/zones` was requested with
proper `status=active` query parameter to only retrieve active zone, if
any. However, the problem is not fixed on customers' side.
**Current investigation**
After looking once again at their HAR files, besides the endpoint
itself, we noticed that the initiator of the call was actually coming
from the compiled and minifed JavaScript file from this
`cloudflare-plugin-frontend` project.
This plugin also requests this zone listing endpoint `/zones` but it
also seems to use the zone list, so I did not want to change the logic
too much and retrieve only the active zones. Instead, I found out the
two places where that list of zones was used and from which a single
zone was being extracted.
**Fixing `src/actions/zoneProvision.js`**
For `src/actions/zoneProvision.js`, it seems we pass the list of zones
from the Cloudflare API to a dependency called `normalizr` which
normalize based on the name field here
https://github.com/cloudflare/cloudflare-plugin-frontend/blob/master/src/constants/Schemas.js#L13-L15,
which effectively removes duplicates, which was tested locally with:
```js
import { normalize, Schema, arrayOf } from 'normalizr';
const zones = [
{'id': 1, 'name': 'cloudflare.com', 'status': 'purged'},
{'id': 2, 'name': 'cloudflare.com', 'status': 'active'},
];
const zoneSchema = new Schema('zones', { idAttribute: 'name' });
const normalizedZones = normalize(zones, arrayOf(zoneSchema));
console.log(JSON.stringify(normalizedZones, '', null));
```
which outputs the following:
```
When merging two zones, found unequal data in their "id" values. Using the earlier value. 1 2
When merging two zones, found unequal data in their "status" values. Using the earlier value. purged active
{"entities":{"zones":{"cloudflare.com":{"id":1,"name":"cloudflare.com","status":"purged"}}},"result":["cloudflare.com","cloudflare.com"]}
```
This could indeed correlates with our problem considered that the purged
zone of the customer was indeed created before the active zone. Should
they discriminate based on their `id` field, the active zone would
indeed be discarded.
To prevent that issue, we are introducing a `deduplicateOnActiveZones()`
helper which deduplicates the list of zones whenever at least one active
zone is present. By doing so, the invocation to `normalizr` should now
only retain the active zone, and the following `asyncZoneSetActiveZone`
Redux dispatch receive that zone.
**Fixing `src/actions/zones.js`**
For `src/actions/zones.js`, it seems that only the first zone from the
zone list is being used, if defined. For that one, we simply added an
extra logic to attempt at extracting the first active zone first, if
any, and fallback to the original logic if no active zone is found.
aseure
added a commit
to aseure/cloudflare-plugin-frontend
that referenced
this pull request
Mar 22, 2024
**Problem**
As part of an internal issue named CUSTESC-36595, one customer reported
us that his Cloudflare Wordpress plugin was reporting that his dashboard
was failing with the following error:
```json
"errors.noActiveZoneSelected": "It looks like your domain {domain} is not provisioned with Cloudflare. Please continue to {link} to secure and speed up your website.",
```
The problem was that the customer does have two Cloudflare zones with
the same name: one being active and the other got purged.
**Previous investigations**
We initially tried to fix this in the `Cloudflare-Wordpress` plugin
itself, via cloudflare/Cloudflare-WordPress#532,
by ensuring that the zone listing endpoint `/zones` was requested with
proper `status=active` query parameter to only retrieve active zone, if
any. However, the problem is not fixed on customers' side.
**Current investigation**
After looking once again at their HAR files, besides the endpoint
itself, we noticed that the initiator of the call was actually coming
from the compiled and minifed JavaScript file from this
`cloudflare-plugin-frontend` project.
This plugin also requests this zone listing endpoint `/zones` but it
also seems to use the zone list, so I did not want to change the logic
too much and retrieve only the active zones. Instead, I found out the
two places where that list of zones was used and from which a single
zone was being extracted.
**Fixing `src/actions/zoneProvision.js`**
For `src/actions/zoneProvision.js`, it seems we pass the list of zones
from the Cloudflare API to a dependency called `normalizr` which
normalize based on the name field here
https://github.com/cloudflare/cloudflare-plugin-frontend/blob/master/src/constants/Schemas.js#L13-L15,
which effectively removes duplicates, which was tested locally with:
```js
import { normalize, Schema, arrayOf } from 'normalizr';
const zones = [
{'id': 1, 'name': 'cloudflare.com', 'status': 'purged'},
{'id': 2, 'name': 'cloudflare.com', 'status': 'active'},
];
const zoneSchema = new Schema('zones', { idAttribute: 'name' });
const normalizedZones = normalize(zones, arrayOf(zoneSchema));
console.log(JSON.stringify(normalizedZones, '', null));
```
which outputs the following:
```
When merging two zones, found unequal data in their "id" values. Using the earlier value. 1 2
When merging two zones, found unequal data in their "status" values. Using the earlier value. purged active
{"entities":{"zones":{"cloudflare.com":{"id":1,"name":"cloudflare.com","status":"purged"}}},"result":["cloudflare.com","cloudflare.com"]}
```
This could indeed correlates with our problem considered that the purged
zone of the customer was indeed created before the active zone. Should
they discriminate based on their `id` field, the active zone would
indeed be discarded.
To prevent that issue, we are introducing a `deduplicateOnActiveZones()`
helper which deduplicates the list of zones whenever at least one active
zone is present. By doing so, the invocation to `normalizr` should now
only retain the active zone, and the following `asyncZoneSetActiveZone`
Redux dispatch receive that zone.
**Fixing `src/actions/zones.js`**
For `src/actions/zones.js`, it seems that only the first zone from the
zone list is being used, if defined. For that one, we simply added an
extra logic to attempt at extracting the first active zone first, if
any, and fallback to the original logic if no active zone is found.
aseure
added a commit
to aseure/cloudflare-plugin-frontend
that referenced
this pull request
Mar 22, 2024
**Problem**
As part of an internal issue named CUSTESC-36595, one customer reported
us that his Cloudflare Wordpress plugin was reporting that his dashboard
was failing with the following error:
```json
"errors.noActiveZoneSelected": "It looks like your domain {domain} is not provisioned with Cloudflare. Please continue to {link} to secure and speed up your website.",
```
The problem was that the customer does have two Cloudflare zones with
the same name: one being active and the other got purged.
**Previous investigations**
We initially tried to fix this in the `Cloudflare-Wordpress` plugin
itself, via cloudflare/Cloudflare-WordPress#532,
by ensuring that the zone listing endpoint `/zones` was requested with
proper `status=active` query parameter to only retrieve active zone, if
any. However, the problem is not fixed on customers' side.
**Current investigation**
After looking once again at their HAR files, besides the endpoint
itself, we noticed that the initiator of the call was actually coming
from the compiled and minifed JavaScript file from this
`cloudflare-plugin-frontend` project.
This plugin also requests this zone listing endpoint `/zones` but it
also seems to use the zone list, so I did not want to change the logic
too much and retrieve only the active zones. Instead, I found out the
two places where that list of zones was used and from which a single
zone was being extracted.
**Fixing `src/actions/zoneProvision.js`**
For `src/actions/zoneProvision.js`, it seems we pass the list of zones
from the Cloudflare API to a dependency called `normalizr` which
normalize based on the name field here
https://github.com/cloudflare/cloudflare-plugin-frontend/blob/master/src/constants/Schemas.js#L13-L15,
which effectively removes duplicates, which was tested locally with:
```js
import { normalize, Schema, arrayOf } from 'normalizr';
const zones = [
{'id': 1, 'name': 'cloudflare.com', 'status': 'purged'},
{'id': 2, 'name': 'cloudflare.com', 'status': 'active'},
];
const zoneSchema = new Schema('zones', { idAttribute: 'name' });
const normalizedZones = normalize(zones, arrayOf(zoneSchema));
console.log(JSON.stringify(normalizedZones, '', null));
```
which outputs the following:
```
When merging two zones, found unequal data in their "id" values. Using the earlier value. 1 2
When merging two zones, found unequal data in their "status" values. Using the earlier value. purged active
{"entities":{"zones":{"cloudflare.com":{"id":1,"name":"cloudflare.com","status":"purged"}}},"result":["cloudflare.com","cloudflare.com"]}
```
This could indeed correlates with our problem considered that the purged
zone of the customer was indeed created before the active zone. Should
they discriminate based on their `id` field, the active zone would
indeed be discarded.
To prevent that issue, we are introducing a `deduplicateOnActiveZones()`
helper which deduplicates the list of zones whenever at least one active
zone is present. By doing so, the invocation to `normalizr` should now
only retain the active zone, and the following `asyncZoneSetActiveZone`
Redux dispatch receive that zone.
**Fixing `src/actions/zones.js`**
For `src/actions/zones.js`, it seems that only the first zone from the
zone list is being used, if defined. For that one, we simply added an
extra logic to attempt at extracting the first active zone first, if
any, and fallback to the original logic if no active zone is found.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We recently noticed that a user got its zone incorrectly reported as inactive by the Wordpress plugin. After some investigations, it seems that two zones actually existed: an old purged one, and a new active zone. To guarantee that the plugin always retrieve the active one, if any, this commit changes the zone retrieval call to the Cloudflare API by adding a
status=activequery parameter to the request, as documented here.