Extend Permissions for viewing the dashboard to include the shared dashboard

[aaemnnosttv]

Feature Description

Building on the new capabilities for dashboard sharing introduced in #4522, the existing capabilities for viewing the Site Kit dashboards should be extended to include users who can view the shared dashboard.

---

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

Acceptance criteria

• The logic behind the existing Permissions::VIEW_DASHBOARD capability for viewing the dashboard should be updated to include users who can view either the main/authenticated dashboard or the shared dashboard
  • The logic for the existing Permissions::VIEW_POSTS_INSIGHTS capability should also use this same logic
• A new Permissions::VIEW_AUTHENTICATED_DASHBOARD capability should be defined to be able to check if a user can visit the authenticated version of the dashboard specifically (essentially the same logic as viewing the dashboard has today)
• A new Permissions::VIEW_SPLASH capability should be defined similar to VIEW_DASHBOARD above but specifically for the splash page that allows for users who can authenticate OR users who can view the shared dashboard
  • The splash Screen registration should be updated to use this new permission instead of the current AUTHENTICATE which not all users of the shared dashboard can do
• The new capabilities should be added to Permissions::check_all_for_current_user
• Appropriate test coverage should be added for each new and updated capability

Implementation Brief

In Google\Site_Kit\Core\Permissions\Permissions class:

• Define new Custom base capabilities:
  • VIEW_AUTHENTICATED_DASHBOARD = googlesitekit_view_authenticated_dashboard
  • VIEW_SPLASH = googlesitekit_view_splash
• In the $this->base_to_core array, update the existing capabilities accordingly:

$this->base_to_core = array(
    // ...
    self::VIEW_DASHBOARD               => $editor_capability,
    // ...
);

• Also, conditionally add the newly added Base Capabilities when dashboardSharing feature flag is enabled:

    if ( Feature_Flags::enabled( 'dashboardSharing' ) ) {
        ...
        $this->base_to_core[ self::VIEW_SPLASH ]                  = $editor_capability;
        $this->base_to_core[ self::VIEW_AUTHENTICATED_DASHBOARD ] = 'manage_options';
        ---
    }

• Similarly, in the $this->network_base array, update the existing base capabilities accordingly and add the newly added Base Capabilities conditionally:

    $this->network_base = array(
        self::VIEW_DASHBOARD      => $admin_network_capability,
    );

    if ( Feature_Flags::enabled( 'dashboardSharing' ) ) {
        $this->network_base[ self::VIEW_SPLASH ]                  = $admin_network_capability;
        $this->network_base[ self::VIEW_AUTHENTICATED_DASHBOARD ] = 'manage_network_options';
    }

• In get_capabilities function, conditionally add the new capabilities when the dashboardSharing is enabled.

In Google\Site_Kit\Core\Admin\Screens class:

• The following changes should be conditionally done when dashboardSharing feature flag is enabled.
  • In the get_screens method, update the Screen entry for the splash screen and use the newly added Permissions::VIEW_SPLASH as it's capability.
  • Also change the Permissions::AUTHENTICATE condition to Permissions::VIEW_SPLASH on both
    no_access_redirect_module_to_dashboard and no_access_redirect_dashboard_to_splash methods.

Test Coverage

• Add test coverage for newly added capabilities and update existing ones in PermissionsTest.
• Update any other failing tests.

QA Brief

• With the dashboardSharing disabled and enabled:
  • Reset sitekit and go through the setup to make nothing is broken.
• Follow-up check that the Splash page shows up in the admin menu just as today (without dashboard sharing enabled) – i.e. unauthenticated admin

Changelog entry

• Extend and update custom capabilities for viewing dashboard and splash screens with logic for dashboard sharing.

[felixarntz]

@aaemnnosttv Hmm, I'm wondering about that. Is it overall more straightforward to include the "view shared dashboard" within "view dashboard", or would it be better to have them as two standalone capabilities that in the relevant areas we check both for?

I guess what I'm getting at is: Is there any googlesitekit_view_dashboard check at the moment where we would not want it to evaluate to true if the user can only view the shared dashboard?

[aaemnnosttv]

@felixarntz I'm not sure we have a need for them to be separate, do we? The main reason I thought to add this is due to permission checks which rely on a single capability (e.g. add_menu_page) rather than a callback where we can have composable logic such as REST endpoints.

Alternatively, we could conditionally register the relevant screens using a different capability based on the current user? It's a little strange though to do this check at registration time though: if the user can do X then register this item which requires the user can do X 😄

site-kit-wp/includes/Core/Admin/Screens.php

Lines 278 to 282 in a27f086

	new Screen(
	self::PREFIX . 'dashboard',
	array(
	'title' => __( 'Dashboard', 'google-site-kit' ),
	'capability' => Permissions::VIEW_DASHBOARD,

This would apply to the Splash page as well

site-kit-wp/includes/Core/Admin/Screens.php

Lines 344 to 348 in a27f086

	$screens[] = new Screen(
	self::PREFIX . 'splash',
	array(
	'title' => __( 'Dashboard', 'google-site-kit' ),
	'capability' => Permissions::AUTHENTICATE,

Would you prefer to do some ternary condition here or should we establish a new capability that would work for both while keeping view_dashboard limited to the main dashboard as it is today?

[felixarntz]

@aaemnnosttv The ternary approach on registration indeed seems weird, but also WordPress core is doing that itself for some of its admin screens. But yeah, I like the idea of introducing two new capabilities so that we have one to access the "authenticated dashboard", one to access the "shared dashboard" and another one to access "either dashboard".

How about we make the existing googlesitekit_view_dashboard capability the one for "either dashboard" and then introduce two new capabilities including a more specific term for the two individual versions of the dashboard?

[aaemnnosttv]

How about we make the existing googlesitekit_view_dashboard capability the one for "either dashboard" and then introduce two new capabilities including a more specific term for the two individual versions of the dashboard?

That sounds good to me. It could be useful for logic in the splash page as well. I assume you're okay with those being defined in the same issue here so I'll update the ACs to include that.

Edit: Actually we shouldn't need an extra dedicated permission for being able to view the shared dashboard because that is already defined in #4522 – so I think we'd only need to add a new capability for being able to view the authenticated dashboard?

As for the splash page however, should we define a new capability for being able to view it other than the current AUTHENTICATE which wouldn't work for shared dashboard users?

[felixarntz]

Sounds good.

As for the splash page however, should we define a new capability for being able to view it other than the current AUTHENTICATE which wouldn't work for shared dashboard users?

Good question. I guess the cleanest approach there would be to introduce a capability googlesitekit_view_splash? That could then be based on either VIEW_SHARED_DASHBOARD or AUTHENTICATE.

[eugene-manuilov]

Thanks, @kuasha420. Mostly looks good to me. Have one comment for you:

In Google\Site_Kit\Core\Admin\Screens class:

• In the get_screens method, update the Screen entry for the splash screen and use the newly added Permissions::VIEW_SPLASH as it's capability.
• Also change the Permissions::AUTHENTICATE condition to Permissions::VIEW_SPLASH on both no_access_redirect_module_to_dashboard and no_access_redirect_dashboard_to_splash methods.

These changes should happen only if the dashboardSharing feature flag is enabled.

[wpdarren]

QA Update: ✅

With the dashboardSharing disabled and enabled:

• Reset Site Kit and went through the setup and no errors appear in the console or FE.
• I also created a new admin user and logged in and went through the set up successfully after it was initially setup.

Added QA:Eng label as per QAB.

[aaemnnosttv]

QA ⚠️

As it is now, VIEW_SPLASH does not work for a non-admin who can read shared module data but does not have the dismissal from viewing it and clicking through to the shared dashboard.

This has to due with the logic being tied to the capability for viewing the shared dashboard (which requires the dismissal).

I'll put together a follow-up PR with a few changes that should fix this.

[aaemnnosttv]

@wpdarren back to you for another pass 👍 QAB has been updated.

[wpdarren]

QA Update: ✅

With the dashboardSharing disabled and enabled:

• Reset Site Kit and went through the setup and no errors appear in the console or FE.
• I also created a new admin user and logged in and went through the set up successfully after it was initially setup.

With the dashboardSharing disabled:

• The Splash page shows up in the admin menu, i.e. unauthenticated admin