devise_saml_authenticatable icon indicating copy to clipboard operation
devise_saml_authenticatable copied to clipboard

What about a saml_success_callback?

Open vinhboy opened this issue 5 years ago • 3 comments

Is there any particular reason why there is no saml_success_callback, similar to saml_failed_callback?

I know I can use Devise after_sign_in_path_for, but I think it would be more useful if I can specify it as part of the :saml_authenticatable strategy so that I can treat SAML authenticated users differently than a regular Devise login.

Are there any flaws in my thinking here? Thanks.

vinhboy avatar Sep 06 '19 06:09 vinhboy

You're right, generally I prefer that we delegate responsibility to Devise—this is a plugin, not a full authentication system.

it would be more useful if I can specify it as part of the :saml_authenticatable strategy so that I can treat SAML authenticated users differently than a regular Devise login.

That's an interesting perspective. Why do you need to treat them differently?

If you're already supporting multiple authentication strategies, then you can store some information regarding how the user logged in and use that in your application code to treat them differently.

adamstegman avatar Sep 06 '19 17:09 adamstegman

If you're already supporting multiple authentication strategies, then you can store some information regarding how the user logged in and use that in your application code to treat them differently.

Thank you for sharing that article, there are some good examples there. I will follow a similar pattern.

generally I prefer that we delegate responsibility to Devise—this is a plugin

I agree with that

Why do you need to treat them differently?

The application I am working on already has a working authentication system. I am adding SAML as an alternate auth strategy. I prefer to not have to go in and muck with the current auth system at all. Being able to configure everything in the saml context would just make things easier to implement and if one day I want to get rid of it, I can just delete it in one place.

For example, I need SAML authenticated users to be redirected to a different landing page after successful login so that I can authorize them properly. Following the example above I can achieve that, which is great. But doing that requires I modify existing code.

Up until this point, with this plugin, I have managed to isolate all SAML code to just the saml hooks. That is a huge a win. If I can get a saml_success_callback then I'd have 100% of what I need.

I could probably put in a PR if you think that is a valid use case. Thanks!

vinhboy avatar Sep 06 '19 19:09 vinhboy

I think you're going to have to modify existing code anyway—see the "logout" section of that wiki page. During logout, your app should redirect to the IdP so the user is logged out there as well.

Adding something like after_action :authorize_saml_user only: :create to the Devise::SamlSessionsController could help keep your code isolated from the rest of the application. Maybe not all in one file though. 🙂

adamstegman avatar Sep 06 '19 22:09 adamstegman