Site-kit-wp: Analytics setup missing verification of user's access to existing tag found through front end request

Created on 3 Jul 2019  路  10Comments  路  Source: google/site-kit-wp

Bug Description


During analytics setup flow when existing tag is found the plugin only verifies the user's access to the property when found through wp_head hook and not through the front end request.

The user should be blocked from continuing Analytics configuration if hasn't got access to the property found.

Steps to reproduce

  1. Add Analytics tag in the theme header.php, make sure you don't have access to the property.
  2. Go to Settings > Connect More Services
  3. Click on Analytics setup link.
  4. User gets warned an existing tag was found but no warning about restricted access and not blocked from configuring Analytics.

Screenshots


Screen Shot 2019-07-03 at 14 56 53

Expected:

Screen Shot 2019-07-03 at 15 03 00

Additional Context

  • PHP Version: 7.1
  • OS: mac
  • Browser: chrome (all)
  • Plugin Version: Developer beta
  • Device: macbook

_Do not alter or remove anything below. The following sections will be managed by moderators only._

Acceptance criteria

  • Users who don't have access to an already existing property on the site should see a notification why that's the case and possible next steps.
  • Do not show drop-downs if the user can't in fact select anything.

Implementation Brief


Note: Work done previously on other issue and then split into new issue with opened PR https://github.com/google/site-kit-wp/pull/310

Add new datapoint tag-permission to make request in the case existing tag is found https://github.com/google/site-kit-wp/blob/develop/assets/js/modules/analytics/setup.js#L95
On the request to tag-permission check if user has access to the property and return response or
return WP_Error with expected message in the case the user has no permissions.

Changelog entry

  • Improve user access verification when encountering existing Analytics tags.
P0 Bug

All 10 comments

@ThierryA @marrrmarrr adding this to the definition board and noting that we already have a PR ready for this on.

@bjackson27 Which PR are you referring to?

Implementation brief looks good. Would generally be great to have more detail on how access is checked, but since there's already a PR that clarifies that, it's okay this time.

This is what I see when I output a tracking code in wp_head for a property that my user does not have access to:

image

@lucymtc This seems to be consistent with the acceptance criteria; has this been already fixed as a byproduct of another issue?

Using develop @ 5ba7ab9

@aaemnnosttv this issue is for when tag is hardcoded in the theme and not added through wp_head

@aaemnnosttv @swissspidy @lucymtc let's make sure Changelog entry is added on this issue.

Added a suggested changelog entry to the the issue:

Improved user access verification when encountering existing Analytics tags.

QA approved!

LGTM

Was this page helpful?
0 / 5 - 0 ratings

Related issues

tofumatt picture tofumatt  路  3Comments

felixarntz picture felixarntz  路  4Comments

jamesozzie picture jamesozzie  路  3Comments

Loganson picture Loganson  路  5Comments

quangbahoa picture quangbahoa  路  5Comments