RoleAccessCheck should provide a reason when denying access

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

MR should be updated to 10.1
There are also a number of open threads on 1407 that should be addressed

This seems like a super helpful feature. Is there any security implication?

Needs new patch or maybe @amjad1233 can change target branch to 10.1.x

Great suggestion let me get my hands dirty in this one.

Around to review again if needed. Just wanted to double check that there’s no security issue announcing permissions?

re-roll and fix for #19

Open questions are

1. is secure exposing roles machine names
2. suggestion to add protected method to generate the reason

@andypost Thanks for the re-roll. 💪💪💪

I tried and patch applies cleanly and site also builds up fine.

patching file 'core/modules/user/src/Access/RoleAccessCheck.php'
patching file 'core/tests/Drupal/Tests/Core/Route/RoleAccessCheckTest.php'

Regarding Question 1 : I believe we are doing that already at certain places, but I agree with hiding as much as we can. With that said, I will defer question to @larowlan.

Regarding Question 2: I am not sure if this class will only be extended or there will be places where we create new instance of the class... In the later case Protected would fail. I think we should crack this one in 📌 Deprecate empty AccessInterface and remove usages Needs work . May be we explicitly define it's signature in interface.

1) is secure exposing roles machine names

I think we'd need to load the role and call $role->access('view label') for each one

2) I still like adding a method to simplify the complexity here, nested if/else can be removed with early returns in a new method

+++ b/core/modules/user/src/Access/RoleAccessCheck.php
@@ -44,10 +45,37 @@ public function access(Route $route, AccountInterface $account) {
+    else {
+      return sprintf('One of the %s %s %s roles are required', implode(', ', $roles), $condition, $last);

No need for else here, as we return above, even more simplification from the new method 💪

I looked into the permissions question and roles require 'administer permissions' to view their labels, which we're not going to grant here, obviously.

I'll ask around

I have uploaded the patch
as per #30 comment.