- Merge request !11Draft: 2.x add config for cookie/jwt auth and check for CSRF token β (Closed) created by ptmkenny
- Status changed to Postponed
8 months ago 4:53am 27 April 2024 - π―π΅Japan ptmkenny
I'm changing this from a support request to a feature request, since cookie auth is not supported.
The tricky thing about adding cookie auth support is that you also have to handle checking for the CSRF token.
I'm adding a draft MR as a proof of concept that uses cookie auth exclusively.
I imagine that most decoupled applications use either JWT OR cookies, not both, so if cookie auth support were to be added to the module, it should probably be a toggle (module defaults to JWT/Oauth, and you have the option of switching to cookie auth).
- πΊπΈUnited States frob US
I don't understand why it would be OR. I am planning on using both.
The DX has been that alternative auth was required to be added through a route alter.
if ($route = $collection->get('jsonrpc.handler')) { $route->setOption('_auth', ['jwt_auth']); }
Are you saying that you're adding this as an option in the UI or through the plugin attributes? I would be all for adding this as a configurable option in the plugin.
- π―π΅Japan ptmkenny
I was thinking about adding this as a UI option.
The challenge of making it support both is that cookies require CSRF but the others do not.
- Status changed to Needs review
4 months ago 2:02pm 2 September 2024 - π―π΅Japan ptmkenny
I've added an MR that adds a configuration page that allows the user to select from among basic auth, cookie auth, and oauth2.
The defaults will be basic auth and oauth2 to maintain backwards compatibility.
Please try out the MR and let me know what you think. I am only using cookie auth, so I need to hear from other people who are using different auth systems before I commit this.
- πΊπΈUnited States frob US
I have not reviewed the MR but I don't quite get the expected DX here. So far everything this module does is in code. I think it is odd to add configuration to this. You're saying now, after a developer writes a plugin, they will need to go through and set the config for their new method in the UI somewhere?
I think something like that might be a good sub-module if it allows for overwriting the auth options for existing plugins but I would expect to be able to set the allowed auth types in the plugin definition.
Update: I have now reviewed the MR and I see something I had not considered. The Auth type is set for the jsonrpc route so it doesn't make the most sense to have it set on a per method basis, though I do think, maybe, an extra level of method filtering for auth types wouldn't be a bad idea. That is likely out of the scope of this issue.
How would one go about adding more auth types if this MR goes live. I use jwt β when I am using this module and that doesn't seem to be supported out of the box nor does a way present itself to add this as an option.
- π―π΅Japan ptmkenny
@frob Thank you for your comments.
Because this is a module for developers, basically everything is done in code. However, I think it would be nice to have a UI option to allow the auth method(s) to be set for the JSON-RPC route. For people newly installing the module, they can choose their auth method(s) and then be done, no need to write a RouteSubscriber if they are not using the module defaults (basic_auth and oauth2). This should also make it easier for people who don't specialize in Drupal to get a decoupled site up and running faster.
For people who already have a working setup, this MR should not break anything; everything should continue to work as it did before.
As for adding more auth types, they will need to be added manually. I just added JWT per your comment. I'm happy to add other auth methods as people request them.
- Merge request !303.x add config for cookie/jwt auth and check for CSRF token β (Merged) created by ptmkenny
- πΊπΈUnited States frob US
Can we add a plugin type or hook for adding authorization options? That should make it easier to manage in the long run.
- π―π΅Japan ptmkenny
@frob You can already add a RouteSubscriber to change the authorization options. What would a plugin type/hook allow you to do that you can't already do in a RouteSubscriber?
- π―π΅Japan ptmkenny
In addition to fixing the basic_auth tests, we need to test a POST cookie request to make sure that it only works when a CSRF token is required.
- π―π΅Japan ptmkenny
Ok, I think the 3.x version of this is ready to go (MR30). (I will need to recreate the 2.x branch because I accidentally rebased to 3.x during the course of development.)
It is backwards compatible and includes duplicate tests for cookies everywhere basic_auth is used. Anyone interested in using an authentication method other than oauth, please let me know if this approach works for you.
- π―π΅Japan ptmkenny
ptmkenny β changed the visibility of the branch cookie to hidden.
- Merge request !402.x add config for cookie/jwt auth and check for CSRF token β (Closed) created by ptmkenny
- π―π΅Japan ptmkenny
ptmkenny β changed the visibility of the branch cookie_auth_3 to hidden.
- π―π΅Japan ptmkenny
This will be a 3.x feature and not backported to 2.x.
-
ptmkenny β
committed 295c7444 on 3.x
Issue #3284838 by ptmkenny, frob, cspitzlay: User-configurable...
-
ptmkenny β
committed 295c7444 on 3.x
Automatically closed - issue fixed for 2 weeks with no activity.