I run a site where I need to use multiple auth providers which are implemented using External Auth. The configuration and setup works, until one of the auth providers is removed from the user. Since on only one function exists to remove the auth data, delete($uid) this is called by the auth providers, which results in wiping out the auth data for ALL providers, instead of just for the provider the module/code is working with. As such, this module should have an api to only delete the a specific user and provider, not both.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 3070335-delete-with-provider-4--interdiff.patch | 3.49 KB | jedihe |
| #4 | 3070335-delete-with-provider-4.patch | 2.67 KB | jedihe |
| #4 | 3070335-delete-with-provider-4--tests-only.patch | 1.43 KB | jedihe |
Comments
Comment #2
richgerdesThe attached patch implements (
function remove($uid, $provider)) on the AuthMap service, which can be used by the providers to remove only their configuration for a user.Comment #3
bkosborneAdding a new method called remove is a bit confusing because you already have one called delete.
I suggest instead just adding the parameter to the existing delete method and make it optional.
But, either way, this modifies the external auth interface and could be considered a BC break if there are other implementations of the interface in the wild somewhere. But I really doubt there are, and it could just be included in the release notes.
Comment #4
jedihe commentedRefactored the patch according to #3. Also added a unit test and fixed an obvious wrong call in an existing unit test.
--tests-only patch may fail at invocation time, due to the change in method signature.
Comment #5
jedihe commentedGreat! tests-only patch actually runs and shows the expected error.
Needs review.
Comment #6
ctrladelApplied the patch from #4 along with the patch in #3123959: Using multiple auth providers results in all auth data being wiped for a user [simplesamlphp_auth] and everything is now working as expected for me, simplesamlphp_auth is no longer removing mappings from other auth providers.
Comment #7
svendecabooterComment #9
svendecabooterThanks for the patches and reviews!
I have now committed this fix.
Will add a warning to the release notes about the changed interface - but I do not think this will affect many installs.
Comment #10
svendecabooter