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.

Comments

richgerdes created an issue. See original summary.

richgerdes’s picture

Status: Active » Needs review
StatusFileSize
new984 bytes

The 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.

bkosborne’s picture

Status: Needs review » Needs work

Adding 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.

jedihe’s picture

StatusFileSize
new1.43 KB
new2.67 KB
new3.49 KB

Refactored 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.

jedihe’s picture

Status: Needs work » Needs review

Great! tests-only patch actually runs and shows the expected error.

Needs review.

ctrladel’s picture

Applied 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.

svendecabooter’s picture

Status: Needs review » Reviewed & tested by the community

  • svendecabooter committed 5cb16e5 on 8.x-1.x authored by jedihe
    Issue #3070335 by jedihe, richgerdes: Using multiple auth providers...
svendecabooter’s picture

Thanks 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.

svendecabooter’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.