swentel / indieweb

IndieWeb integration for Drupal
https://drupal.org/project/indieweb
GNU General Public License v2.0
37 stars 5 forks source link

Fix check for externalauth in login block #467

Closed eelkeblok closed 4 years ago

eelkeblok commented 4 years ago

This fixes #466. Was happy to find the config form basically did exactly what I wanted to add :)

It also adds some proper dependecy injection to the components involved and removes the logic from the block itself; it just replicated logic that was already in the form too.

swentel commented 4 years ago

This still crashes when:

I placed the block on any page, and as user 1, I get this exception: Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service "externalauth.authmap". in Drupal\Component\DependencyInjection\Container->get() (line 153 of /home/drupal/realize.be/core/lib/Drupal/Component/DependencyInjection/Container.php).

So I guess that still needs a fix :)

eelkeblok commented 4 years ago

Hmm. Interesting. Good catch. I guess I need to explicitly initialise the member variable as NULL, then.

eelkeblok commented 4 years ago

Which version of PHP are you running on? I can't reproduce this ATM, but my test environment is on PHP 7.0 because my personal site is too.

eelkeblok commented 4 years ago

Actually, scratch most of the above. That is just really strange, because before requesting the service at line 157 of IndieAuthLoginForm, I am checking if the container has it at all at line 156. Can you see where the service is being requested in your error case?

Edit: Could this have to do with the other use of the service in indieweb_indieauth_user_authenticated_with_domain_on_edit_form()? I suppose that should be changed in this PR too, if we mean to be more robust about not having externalauth available.

swentel commented 4 years ago

I'm running PHP 7.2.24 at the moment, but I don't think that would cause the exception, I'll have a look at it this evening normally.

eelkeblok commented 4 years ago

I've updated the other location where the service was being requested without checking for its existence. I hope that addresses the situation you found.

swentel commented 4 years ago

Hmm, I'm starting to wonder whether I cleanly applied the patch, sorry about that :/ (when I did git apply now again it simply didn't made the changes, crap)

Anyway, this does fix the problem, so great!