The OpenID Connect module already offers various hooks to alter userdata provided from an external IdP, extend the user property mapping provided by the module, and react on pre and post authorize events. Yet, modules providing individualized client plugins have to implement a custom client plugin AND hook into the various events of the OpenID Connect module.
In order to making the module really pluggable in terms of allowing other modules to implement a custom catch-all client plugin, that can map non-standard claims provided by the related IdP to user properties and to react on pre and post authorize, we should provide according plugin methods that will be called by the OpenID Connect module at the according events.
Add methods as onPreAuthorize(UserInterface $account, array $context), onPostAuthorize(UserInterface $account, array $context) and onUserinfoSave(UserInterface $account, array $context) to the OpenIDConnectClientInterface and OpenIDConnectClientBase. Let client plugins override these methods. Invoke the methods, if the user is (about to be) authorized using the related client.
Community review.
None.
Additional methods will be available for client plugins. They can optionally be implemented.
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | openid_connect-2962602-13.patch | 12 KB | rp7 |
| #10 | openid_connect-2962602-10.patch | 11.96 KB | gugalamaciek |
| #9 | openid_connect-2962602-9.patch | 11.34 KB | erik frèrejean |
Show commandsStart 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:
changes
Comments
Comment #2
mario steinitzThe attached patch for community review. Any thoughts/features to add?
Comment #3
mario steinitzComment #4
sunI like the idea and where this is going. The overall direction makes sense to me; it would simplify custom implementations. I agree that a custom client plugin should ideally be self-contained.
Of course, if this was Drupal Core, then we would define one or more interfaces for those event methods and every client implementation would then signify that it needs/supports them by implementing those interfaces.
The calling framework code would then simply check whether the client implements the interface, and invoke it upfront – or automatically register it as a first callback for the event.
Comment #5
mario steinitz@sun: That's an even better approach. Attached a version that introduces a general
OpenIDConnectClientEventsInterfaceproviding all three events.We could probably find a better name or split it up into several interfaces, so the plugins don't have to implement three methods when only one of them is required.
Just wondering, which other events would make sense to be available in plugins.
Comment #6
mario steinitzAnd a version with dedicated interfaces for each of the events. Adds a number of additional interfaces, but I agree that this is even more flexible.
Comment #7
erik frèrejeanRerolled the patch on the latest HEAD because it didn't apply cleanly anymore.
Comment #8
liesm commentedThe test fails in #7 do not seem to be related to this patch, we've been running this patch for some time now without any issues.
Is there anything to do on this to get it rolling again? Otherwise I feel this can be RTBC'ed.
Comment #9
erik frèrejeanThis needed a re-roll.
Comment #10
gugalamaciek commentedHere it's the patch for 8.x-2.x version. But in my opinion it's bad way to solve things like that. I think that in D8/9 it should be done via events. See my comment here: https://www.drupal.org/project/openid_connect/issues/3114319#comment-142...
FYI, if you are using v1, patch for 8.x-1.x is not complete.
onPostAuthorize()should be added tocompleteAuthorization().Comment #12
jcnventuraComment #13
rp7 commentedRe-rolled against 3.x-dev