Issue #3426090: First stab at the scope granularity plugin system
Closes #3426090
Merge request reports
Activity
added 1 commit
- 571401b4 - Fix simple_oauth_static_scope tests and PHPCS/PHPStan
9 9 status: true 10 10 description: 'Test client_credentials description' 11 11 parent: 'static_scope:child' 12 granularity: 'permission' 12 granularity_id: 'permission' 13 granularity_configuration: - Comment on lines -12 to +13
The changed configuration structure is debatable in various aspects, here is the reasoning why I went with this:
- I personally find it confusing to have
granularitybe alongsidegranularity_configurationas they together make up the whole granularity, so to me it makes it more clear to explicitly distinguish the plugin ID asgranularity_id - I would have found it even better to just have a single
granularitytop-level property and nest the ID and the configuration below that. This is not possible, however, at least as far as I can tell, if we want to use a plugin collection for the dynamic scope, which I totally think makes sense.ConfigEntityBaseassumes that the configuration of the plugin collection is a top-level config entity property - Theoretically we could have a different structure for the static scopes and the dynamic ones, but to me consistency was more important here, that's why I changed it everywhere
- I personally find it confusing to have
109 104 '#title' => $this->t('Umbrella'), 110 105 '#default_value' => $scope->isUmbrella(), 111 106 '#description' => $this->t('An umbrella scope groups multiple scopes.'), 107 '#executes_submit_callback' => TRUE, 108 '#submit' => ['::ajaxRebuild'], 109 '#limit_validation_errors' => [], 110 '#ajax' => [ 111 'callback' => '::ajaxParentAndGranularity', 112 'wrapper' => 'oauth2-scope-parent-and-granularity-wrapper', 113 ], - Comment on lines +110 to +113
So visually the dynamic scope form looks pretty similar to before, but I did change it from
#statesto Ajax for one reason: I wanted to usePluginFormInterfacefor the plugins, but also allow plugin forms to contain required form elements. I.e. the permission is required if using the permission granularity. This cannot work if the plugin form declares elements as required via#requiredunless the base form, after asking the plugin to build its configuration form, goes through and switches all plugin form elements from#requiredto#states[reuqired]. That seemed a bit hacky, though, so I just went ahead and switched the form to using Ajax.
109 104 '#title' => $this->t('Umbrella'), 110 105 '#default_value' => $scope->isUmbrella(), 111 106 '#description' => $this->t('An umbrella scope groups multiple scopes.'), 107 '#executes_submit_callback' => TRUE, 108 '#submit' => ['::ajaxRebuild'], 109 '#limit_validation_errors' => [], 110 '#ajax' => [ 111 'callback' => '::ajaxParentAndGranularity', 112 'wrapper' => 'oauth2-scope-parent-and-granularity-wrapper', 113 ], 114 ]; 115 $form['parent_and_granularity'] = [ 116 '#type' => 'container', 117 '#prefix' => '<div id="oauth2-scope-parent-and-granularity-wrapper">', 118 '#suffix' => '</div>', 112 119 ]; - Comment on lines +115 to 119
This is the ugliest part of the switch to Ajax, because we have two separate things (parent and granularity) that depend on the state of the umbrella checkbox. I couldn't come up with a concise name for "parent and granularity" so just used that literally. Maybe there is some way to avoid this, but I couldn't find one. Note that since
#treeis not used this does not show up in the form values structure (and in the client) so the ugliness really is contained to the form structure here.
- src/Plugin/ScopeGranularity/Permission.php 0 → 100644
5 use Drupal\Component\Plugin\Exception\PluginException; 6 use Drupal\Core\Extension\ModuleExtensionList; 7 use Drupal\Core\Form\FormStateInterface; 8 use Drupal\Core\Plugin\ContainerFactoryPluginInterface; 9 use Drupal\Core\StringTranslation\TranslatableMarkup; 10 use Drupal\simple_oauth\Attribute\ScopeGranularity; 11 use Drupal\simple_oauth\Oauth2ScopeInterface; 12 use Drupal\simple_oauth\Plugin\ScopeGranularityBase; 13 use Drupal\user\PermissionHandlerInterface; 14 use Symfony\Component\DependencyInjection\ContainerInterface; 15 16 /** 17 * The permission scope granularity plugin. 18 */ 19 #[ScopeGranularity( 20 Oauth2ScopeInterface::GRANULARITY_PERMISSION, -
A nice side-effect of using attribute is that you can use constants for the IDs. That's why I kept around the constants. Note that I did not use them for the plugin configuration keys (even though they are the same string), as it feels like a different thing to me, but as with the other things, this is very much arguable.
102 96 * 103 * @return string|null 104 * Returns the permission. 105 */ 106 public function getPermission(): ?string; 107 108 /** 109 * Get the role. 110 * 111 * @return string|null 112 * Returns the role id. 97 * @return ?\Drupal\simple_oauth\Plugin\ScopeGranularityInterface 98 * Returns the granularity plugin, generally permission or role. 113 99 */ 114 public function getRole(): ?string; 100 public function getGranularity(): ?ScopeGranularityInterface; added 12 commits
-
e7223770...49486f2d - 6 commits from branch
project:6.0.x - ce2ac83a - First stab at the scope granularity plugin system
- ec541341 - Fix simple_oauth_static_scope tests and PHPCS/PHPStan
- 8503d86a - Fix PHPCS round 2
- 52771de9 - Add tests for the scope granularity
- e1c5519a - Switch to new-style constructors
- 32622617 - PHPCS and PHPStan fixes
Toggle commit list-
e7223770...49486f2d - 6 commits from branch