apokalipto / devise_saml_authenticatable

Devise SAML 2.0 authentication strategy
MIT License
293 stars 152 forks source link

Unable to override create method to handle user creation validation error. #144

Open lethunder opened 5 years ago

lethunder commented 5 years ago

Hello, It there a way to override create method to handle validation error during user creattiln process?

Thanks in advance

adamstegman commented 5 years ago

Yes, you can set devise.saml_update_resource_hook to a proc. Use the default as a starting point and you can customize from there!

lethunder commented 5 years ago

Thanks. Let me check this

Le mar. 16 juil. 2019 à 18:25, Adam ⚛ Stegman notifications@github.com a écrit :

Yes, you can set devise.saml_default_update_resource_hook to a proc. Use the default https://github.com/apokalipto/devise_saml_authenticatable/blob/3934b53af92a4ea91b3d3c266535c020a4b807c9/lib/devise_saml_authenticatable.rb#L96 as a starting point and you can customize from there!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/apokalipto/devise_saml_authenticatable/issues/144?email_source=notifications&email_token=ABPOAVGIUXMWHSZD2GVGK6DP7XY6VA5CNFSM4IECVFKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2BMZHQ#issuecomment-511888542, or mute the thread https://github.com/notifications/unsubscribe-auth/ABPOAVFHNWGTTXX5ZEPM5YDP7XY6VANCNFSM4IECVFKA .

lethunder commented 5 years ago

Hey, actually I'm not sure how this can be usefull for what I intend to do. I created a saml sessions controller which inherit from devise saml sessions controller. The reason why is because after getting user infos from idp, I'll need to do some verifications or at least if validation fails to catch it without rasing error to user and redirect him to homepage.

Le mar. 16 juil. 2019 à 18:36, Didier BANLOCK didier.banlock@gmail.com a écrit :

Thanks. Let me check this

Le mar. 16 juil. 2019 à 18:25, Adam ⚛ Stegman notifications@github.com a écrit :

Yes, you can set devise.saml_default_update_resource_hook to a proc. Use the default https://github.com/apokalipto/devise_saml_authenticatable/blob/3934b53af92a4ea91b3d3c266535c020a4b807c9/lib/devise_saml_authenticatable.rb#L96 as a starting point and you can customize from there!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/apokalipto/devise_saml_authenticatable/issues/144?email_source=notifications&email_token=ABPOAVGIUXMWHSZD2GVGK6DP7XY6VA5CNFSM4IECVFKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2BMZHQ#issuecomment-511888542, or mute the thread https://github.com/notifications/unsubscribe-auth/ABPOAVFHNWGTTXX5ZEPM5YDP7XY6VANCNFSM4IECVFKA .

adamstegman commented 5 years ago

Redirection can be tricky, depending on when you want to do it!

The basic flow is something like:

So that flow goes through this gem, devise, and warden at various points—if you raise an error, you might be able to catch it in the controller, in #create, but I'm not sure.

Another place you could catch it is in the FailureApp#redirect_url - this is called by warden when an unauthorized response is occurring. We use this in our app to display an error page when the wrong scope is in use.

lethunder commented 5 years ago

hey, thanks for your response. I did something on my side that is working.

i override self.authenticate_with_saml by updating this piece of code

if Devise.saml_update_user || (resource.new_record? &&

Devise.saml_create_user) Devise.saml_update_resource_hook.call(resource, decorated_response, auth_value) rescue nil end

by adding rescue nil i avoid app to raise brutal error to user

Then i my controller Auth::SamlSessionsController < Devise::SamlSessionsController i added

before_action :redirect_user_if_registration_fails, on: :create

private def redirect_user_if_registration_fails if current_user.new_record? redirect_to new_user_session_path, :alert => current_user.errors.full_messages.first end end

It means that after create if user is saved then we redirect to root path otherwise something went wrong and me display error message

Let me know if it makes sense for you. I can under my work if not

Le jeu. 18 juil. 2019 à 04:38, Adam ⚛ Stegman notifications@github.com a écrit :

Redirection can be tricky, depending on when you want to do it!

The basic flow is something like:

So that flow goes through this gem, devise, and warden at various points—if you raise an error, you might be able to catch it in the controller, in #create, but I'm not sure.

Another place you could catch it is in the FailureApp#redirect_url https://github.com/plataformatec/devise/blob/9aa17eec07719a97385dd40fa05c4029983a1cd5/lib/devise/failure_app.rb#L116

  • this is called by warden when an unauthorized response is occurring. We use this in our app to display an error page when the wrong scope is in use.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/apokalipto/devise_saml_authenticatable/issues/144?email_source=notifications&email_token=ABPOAVCOVZ43GQAS35PBPUDP77JSDA5CNFSM4IECVFKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2HESHA#issuecomment-512641308, or mute the thread https://github.com/notifications/unsubscribe-auth/ABPOAVFWCK6ETJZ22VV54NLP77JSDANCNFSM4IECVFKA .

-- Cordialement, Didier Banlock

luke-zhou commented 3 years ago

I am currently having the same issue.

      if resource.nil?
        if Devise.saml_create_user
          logger.info("Creating user(#{auth_value}).")
          resource = new
        else
          logger.info("User(#{auth_value}) not found.  Not configured to create the user.")
          return nil
        end
      end
      if Devise.saml_update_user || (resource.new_record? && Devise.saml_create_user)
        Devise.saml_update_resource_hook.call(resource, decorated_response, auth_value)
      end
      resource

` I think in the model.rb, if creating fails should return nil in the resource instead of an empty resource. In that case, the strategy can fail the authentication

adamstegman commented 3 years ago

Great catch! That does sound like the right response.