Skip to content

Restrict editing module entity settings to users who have access #4825

Closed

Description

Feature Description

With the introduction of dashboard sharing, changing a module's connection (a.k.a. service entity) has substantial implications when shared – changing these settings requires taking on the responsibility of sharing (granting access via your Google account). Accordingly, changing these settings can only be done by admins who have access to the configured entity.


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

Acceptance criteria

  • Module settings edit views for AdSense, Analytics, and Search Console should be updated based on the current user's access to the module (see Implement new selector for checking module access #4802)
    • If the current user does not have access to the module, the inputs that control the module's connection (i.e. owned settings) should be disabled
      • The user should still be able to change "secondary" settings (i.e. non-owned settings, e.g. useSnippet) and submit those changes even if they don't have access to the entity
      • When disabled, raw values can be displayed as necessary in place of the usual "rich" display value where API entity access is needed (e.g. a property ID in place of the ID + name) but should still appear as the same type of input (e.g. a disabled Select with single selected value)
      • An ℹ️ notice should be displayed below the inputs (see the design in Figma)

        {module owner username} configured {module name} and you don’t have access to this {connection entity}. Contact them to share access or change the {connection entity}.

      • Note {connection entity} refers to a service specific term, such as "Analytics property", "Search Console property" or "AdSense account" and should use separate translation strings rather than a formatted string to populate
    • If the current user does have access to the module
      • The inputs for owned settings should remain editable as usual
      • If dashboardSharing is enabled and the user has changed any of the owned settings (see Expose owned module settings to client #5121), and is not the owner themselves, and the module is currently being shared with one or more roles:
        display a new ⚠️ notice at the bottom of the edit view with the following content:

        By clicking {submit button text}, you’re granting other users view-only access to data from {module name} via your Google account. You can always manage this later in the dashboard sharing settings.

  • Settings edit interfaces should remain in a loading state while the module access is being checked to avoid settings becoming disabled after being displayed

Implementation Brief

Implementing logic for when the user does not have access

  • In assets/js/modules/analytics/components/settings/SettingsEdit.js and assets/js/modules/search-console/components/settings/SettingsEdit.js::

    • Fetch the current user's module access by using the hasModuleAccess( slug ).
    • If it is undefined, return the <ProgressBar> component.
  • In assets/js/modules/search-console/components/common/PropertySelect.js, assets/js/modules/analytics/components/common/AccountSelect.js, assets/js/modules/analytics/components/common/PropertySelect.js, assets/js/modules/analytics/components/common/ProfileSelect.js and assets/js/modules/analytics-4/components/common/PropertySelect.js:

    • Fetch the current user's module access by using the hasModuleAccess( slug ) selector introduced in Implement new selector for checking module access #4802.
    • If there is a value, pass it to the disabled prop of the <Select> component so that the field is disabled if the value is true.
  • In assets/js/modules/search-console/components/settings/SettingsForm.js,

    • Fetch the current user's module access by using the hasModuleAccess( slug ) selector.
    • If the value is false, render the <SettingsNotice> component.
      • Use type=TYPE_INFO and icon=WarningIcon below the select field as described in the AC/Figma design.
      • Replace {connection entity} with "Search Console property" as mentioned in the AC.
      • To fetch the owner's username, use the getModule( slug ) selector from the core/modules store. Note that the owner object will only be set if the user has sufficient permissions, although all users who can edit settings (admins) should have it, it shouldn't be assumed that the object is there. As a fallback, "Another admin" can be replaced with the username in the message.
  • In assets/js/modules/analytics/components/settings/SettingsControls.js and assets/js/modules/analytics/components/settings/GA4SettingsControls.js:

    • Fetch the current user's module access by using the hasModuleAccess( slug ) selector.
    • If the value is false, render the <SettingsNotice> component as described above in search-console's <SettingsForm> component. Render it below the select fields in these components as per the Figma mock in the AC.

Implementing logic for when the user does have access

  • In assets/js/components/settings/, create a new component <EntityOwnershipChangeNotice> as follows:

    • The component should have a slug prop which accepts the current module-slug for which the notice is being rendered.
    • Check if the DashboardSharing feature flag is enabled, return an empty component if false.
    • Fetch the current user's module access by using the hasModuleAccess( slug ) selector introduced in Implement new selector for checking module access #4802, return an empty component if false.
    • Use the new haveOwnedSettingsChanged selector from the current module's store (Expose owned module settings to client #5121) to find out if any of the owned settings have changed. Return an empty component if false.
    • Use the getOwnerID() selector from the current module's store. Use the getID selector from the core/user store. Compare the IDs to find out if the current user is the owner of the module. If the current user is the owner, return an empty component.
    • If none of the above conditions are met, return the <SettingsNotice> component with type=TYPE_WARNING and text as per the AC. The {submit button text} should be replaced with Confirm Changes. The module name can be passed as a prop to this component.
  • In assets/js/modules/analytics/components/settings/SettingsForm.js and assets/js/modules/search-console/components/settings/SettingsForm.js,

    • Render the <EntityOwnershipChangeNotice> component created above at the end of the form.

Note: Owned settings for AdSense (accountID and clientID) are not editable in the edit settings view (SettingsEdit component). However, the Edit settings should be tested to ensure the settings still work when a user does not have access to the module.

StoryBook coverage

  • Ensure the above states are added to Storybook in module-search-console-settings.stories.js, module-analytics-settings.stories.js and in the new Storybook v6 assets/js/modules/analytics/components/settings/SettingsForm.stories.js.

Test Coverage

  • No new tests required.

QA Brief

  • This issue affects the Settings dropdown lists of "Owned Settings", fields such as "Account", "Property", "Profile", etc. Some of these components are used by multiple settings pages. So it would be beneficial to test that existing settings pages (mainly "add" and "edit") work normally without errors as before for all modules which use these dropdown fields (Search Console, Analytics, AdSense and Tag Manager).

  • Create an admin, say "Admin1" to setup SiteKit (this includes setting up a new Search Console property). Also setup Analytics and create a new property or use an existing one that is connected to Admin1's account.

  • To verify Search Console settings:

    • With Access: Setup SiteKit using another admin with a separate google account (say "Admin2"). This step verifies and gives access to the site's Search Console property to Admin2's Google Account, thus giving "access" to Admin2. Verify the ACs for when "current user has access". The SC Property field will only show a single value in the dropdown and so there is no value to change. For test purposes, use the Developer Settings plugin and add a new Custom Site URL. Come back to Search Console settings and now you will be able to "change" the Property dropdown as Admin2 which should display the "Yellow Warning" as in the AC.
    • Without Access: To remove Admin2's access, on the Search Console settings accordion, click on the 'See full details in Search Console'. In the Search Console dashboard that pops up, click on "Settings" in the left menu and click on the "REMOVE PROPERTY" button on the settings page. Go back to SiteKit, refresh the settings page and try to "edit" search console again. Verify the ACs for when "current user does NOT have access".
  • To verify Analytics Settings:

    • With Access: As Admin1, login to the "Analytics Dashboard" (by clicking on "See full details in Analytics" within the Analytics accordion in SiteKit's Settings). Click on the "Admin" settings cog in the left menu. Under the first "Account" column, click on "Account Access Management". In the popup, click on the + sign to add Admin2's Google Account to this account. Now login to SiteKit as Admin2, go to Analytics Settings and try editing them. Verify the ACs for when user has access.

    • Screenshot 2022-06-07 at 10 47 57
    • Without Access: In Admin1's "Analytics Dashboard" mentioned above, follow the steps to go to Account Settings. In the "Account Access Management" popup, click on the 3 dots next to Admin2's Google Account and click on remove access. Now login to SiteKit as Admin2, go to Analytics Settings and try editing them again. Verify the ACs for when user does not have access.

  • AdSense: There are no "owned settings" for AdSense and so no changes have been made specifically to the settings components here. They should just work normally as before.

  • Verify the new stories in Storybook: Search Console, Analytics old story format, Analytics new story format - no access and with access.

Changelog entry

  • Restrict editing module entity settings to users who have access, either by being the module owner or by having the module shared with them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    P0High priorityRolloverIssues which role over to the next sprintType: EnhancementImprovement of an existing feature

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions