Problem/Motivation

I don't think separating roles by spaces is a robust enough approach.
We don't know what kind of role notation an API might return. It could be a machine readable name or a human readable name.
As you can imagine I've got the latter and am now in trouble :P
Both seem fair choices to me, but the latter tends to utilize spaces for readability which is kinda a problem now.

After digging through the code I found that the role mappings support quotes to handle role names with spaces.
This should be documented.

Steps to reproduce

Proposed resolution

Use a more robust e.g. a less likely used character as separator.
I'd propose a semicolon (;) I'd consider that to be quite unlikely to be used even in human readable role labels.
This certainly needs an update hook to convert the existing spaces separated values to semicolon separated ones.

Remaining tasks

  1. ✓ Write Code
  2. ✓ Review
  3. ✓ Merge

User interface changes

None

API changes

None

Data model changes

None

Issue fork openid_connect-3506034

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

das-peter created an issue. See original summary.

das-peter’s picture

das-peter commented
Title: Support for groups names with spaces » Document support for groups names with spaces by using quotes
Component: Code » Documentation
Issue summary: View changes

Only when digging through the code I found:

foreach ($roles as $role_id => $role) {
      $default = '';
      if (isset($role_mappings[$role_id]) && is_array($role_mappings[$role_id])) {
        // Surround any mappings with spaces with double quotes.
        foreach ($role_mappings[$role_id] as $key => $mapping) {
          if (strpos($mapping, ' ') !== FALSE) {
            $role_mappings[$role_id][$key] = '"' . $mapping . '"';
          }
        }
        $default = implode(' ', $role_mappings[$role_id]);
      }

So this is supported but just lacks documentation

das-peter opened merge request !140

das-peter’s picture

das-peter commented
Assigned: das-peter » Unassigned
Issue summary: View changes
Status: Needs work » Needs review

Added a hint to the role mapping help.
Am not sure how happy I am with the formatting / wording.
Happy for suggestions :)

pfrilling made their first commit to this issue’s fork.

pfrilling’s picture

pfrilling
Location Minster, OH
commented
Status: Needs review » Reviewed & tested by the community

Thanks for the contribution! I adjusted the language a bit to help clarify the functionality.

pfrilling’s picture

pfrilling
Location Minster, OH
commented
Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

  • pfrilling committed 83af2d82 on 3.x authored by das-peter 
    Issue #3506034 by das-peter, pfrilling: Document support for groups...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.