IdentityPython / djangosaml2

Django SAML2 Service Provider based on pySAML2
Apache License 2.0
254 stars 143 forks source link

Split authenticate into separate overridable function #374

Closed marcelkornblum closed 1 year ago

marcelkornblum commented 1 year ago

This PR splits the user authentication in the AssertionConsumerServiceView into a separate function that can be overridden separately. No functionality has been changed.

We've been using this library for a long time and have multiple IdPs that we connect to. Unfortunately that means we sometimes get duplicate user records created, where the same individual has accounts with more than one IdP. We're changing our flow to incorporate merging records together, but as part of that we need users to authenticate with their IdP for the second record / email address we will merge with. We want this to complete without them automatically then logging in to that account.

The cleanest way we can see to do this would be to override just the user authentication part of the post method of the AssertionConsumerServiceView meaning that we are confident that the user controls the account but we keep the user from their first auth available. This change means that we don't have to work around it with overrides to the backend and the handle_acs_failure methods, which would be possible but inelegant.

This small change would allow our flow to work tidily and be easy to understand. We appreciate any feedback you may have.

/cc @jafacakes2011 @lgarvey

peppelinux commented 1 year ago

Please can I ask you to increase the version number here as well? https://github.com/IdentityPython/djangosaml2/blob/master/setup.py#L30

I don't see this as an additional feature, so I'd go for 1.5.7, but feel free to give your suggestions also for this

marcelkornblum commented 1 year ago

Absolutely - and yes agree it's not a feature; I've bumped it to 1.5.7

Thanks!