Call to protected method Drupal\Core\Authentication\AuthenticationManager::getSortedProviders() from context '' in /Users/clemens/Sites/drupal/d8/www/modules/restui/restui.module on line 39

To make Rest UI work (on e13963221890fa3e8266633a845cc4c03d7a90ad) wait for #2456303: AuthenticationManager needs interface and ::getProviderKeys or edit core/lib/Drupal/Core/Authentication/AuthenticationManager.php aroudn line 259

  protected function getSortedProviders() {

into

  public function getSortedProviders() {

Comments

clemens.tolboom’s picture

Issue summary: View changes
Priority: Normal » Critical
Parent issue: » #2286971: Remove dependency of current_user on request and authentication manager

Changing this core method (killing kittens) back to public makes Rest UI work.

clemens.tolboom’s picture

clemens.tolboom’s picture

StatusFileSize
new1.25 KB
znerol’s picture

As pointed out by @alexpott in #2456303-16: AuthenticationManager needs interface and ::getProviderKeys a better way to collect the keys is to just collect them from the container using a service collector. Note that we currently are working on #2432585: Improve authentication manager service construction to support custom global service providers, please chime in over there.

clemens.tolboom’s picture

Status: Active » Needs work

@znerol thanks for the feedback.

@alexpott

How come the rest ui can't just get the authentication providers from the container?
I.e just add
- { name: service_collector, tag: authentication_provider, call: addProvider }

to your service and implement an add provider method.

Not sure what this means yet :-/ Feedback is welcome.

znerol’s picture

  1. Implement two new methods in an existing service (or add a new one):
    • addProvider($id)
    • getProviderKeys()
  2. Make sure to add the service_collector tag to the service with the new addProvider() method
  3. Use the getProviderKeys() from your own service wherever you wish.

Example of the service definition:

services:
  restui.providercollector:
    class: Drupal\restui\RestUIProviderCollector
    tags:
    - { name: service_collector, tag: authentication_provider, call: addProvider }

Example of the class:

class RestUIProviderCollector {
  protected $keys;
  public function addProvider($provider_id) {
    $keys[] = $provider_id;
  }
  public function getProviderKeys() {
    return $keys;
  }
}

Note that we are planning to introduce explicit provider keys instead of the magic applied to the service id in #2432585. This will render the convertServiceIdToProviderKey method superflous. Update: #2432585: Improve authentication manager service construction to support custom global service providers landed, provider_id argument now can be used.

clemens.tolboom’s picture

@znerol awesome :-)

clemens.tolboom’s picture

clemens.tolboom’s picture

Issue summary: View changes
StatusFileSize
new953 bytes

I cannot test against latest core as mysql is too old now :-(

Attached patch needs work

mikey_p’s picture

Status: Needs work » Needs review
StatusFileSize
new4.34 KB

Slightly renamed the collector and copied a few bits from the Authentication Manager over to make sure we get the same behavior.

clemens.tolboom’s picture

Status: Needs review » Fixed

Thanks to @znerol + @mikey_p

clemens.tolboom’s picture

I was too optimistic in stating fixed. We loosed the fatal which is good. See #2488496: UX + config quirks

clemens.tolboom’s picture

Category: Bug report » Task
Priority: Critical » Normal
Status: Fixed » Active

I reopen this as when #2490228: Add Authentication Collector lands we need to revert this.

clemens.tolboom’s picture

@znerol + @mikey_p please visit #2490228: Add Authentication Collector to make sure you get credits too.

andypost’s picture

Status: Active » Needs review
StatusFileSize
new12.87 KB

Here's a removal of custom collector in favour of core one that here since 8.0

PS: + additional code clean-up

clemens.tolboom’s picture

Status: Needs review » Fixed

Thanks.

Status: Fixed » Closed (fixed)

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

clemens.tolboom’s picture

@andypost please create a new issue for the cleanup. I agree we need to rinse this but it definitely needs a new issue. Thanks!

andypost’s picture