In services 3.x we are allowed to select multiple Authentication modules for a single endpoint but then every one of these runs. This means that if we want to allow users to use multiple auth methods but they can conflict. Each will be run and reset the `global $user;` variable one after the other (including setting it back to anon after another module set it to an admin user, etc).
Authentication is something that can really only happen once so if we allow multiple auth modules it seems like we should be trying each one expecting most to fail and then we should stop trying when we get a success. Currently we keep trying despite success and we *stop* trying modules on "failure". This seems broken.
What if we were to check to see whether the called function actually changed the global $user and if so, stop checking auth plugins?
This solves my use case but is a bit weird with the error handling for that particular hook. Authorization callbacks are supposed to return an error string on failure and if we want to allow multiple to run most will likely be failing, but the moment one fails the code currently dumps out and doesn't try the next one.
Initial patch coming in the comment...
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | services-multiple-auth-1394652-1.patch | 685 bytes | tizzo |
Comments
Comment #1
tizzo commentedInitial patch attached.
Comment #2
branana commentedI also wrote something that allows additional login methods to hook in. The difference is you do have to write a bit of extra code to fascilitate soemthing such as LDAP login
http://drupal.org/sandbox/branana/1300714
Comment #3
tizzo commentedIf I read your code properly it looks like you allow one "meta" resource for authenticating with multiple enabled backends? That's cool, but what I was actually trying to solve for was the allowance of more than one on *other* resources via the `alter_controllers` callback that can be used to give different authentication mechanisms for normal methods.
Comment #4
marcingy commentedThe patch in #1 makes sense to me but I would like Kyle to chime in before committing so rtbc it is.
Comment #5
kylebrowning commented1. I agree I don't like that it tries to authenticate and if it fails it exists, thats what really should be fixed.
What I think should really be done is if we pass an authentication method we move on, but if we don't we just log the error. So lets make this patch about that instead which will solve everyones problems, not sure yours.
2. I think what you're trying to solve can be solved a different way. It sounds like you want different authentication mechanisms per resource method, like login can be authenticated with session, and oauth, but create user can only be authenticated via oauth, if thats the case, this is already in the works via #1154420: Readd support for rich options for resources - needed by auth mechanism
Comment #6
kylebrowning commented