apokalipto / devise_saml_authenticatable

Devise SAML 2.0 authentication strategy
MIT License
297 stars 155 forks source link

newly created users aren't logged in? #227

Closed rakaur closed 2 years ago

rakaur commented 2 years ago

Hello. I am adding this gem to an existing app that currently uses database_authenticatable. I have added this, and I am supporting multiple IdPs with a settings adapter, etc. If the IdP user exists locally, everything works perfectly. However, when a user needs to be created, it seems as though they aren't signed in after creation. It creates the user, and then redirects to / which has before_action :authenticate_user! which immediately sends them back to the login screen. If they try to log in again, it will work the second time. I've hit a wall trying to figure this out.

Because I am also using database auth, I'm utilizing saml_update_resource_hook because I must add some things to new users to get them to validate:

config.saml_update_resource_hook = lambda do |user, saml_response, auth_value|
    unless user.persisted? # Creating a new local user
      user = User.create(email: auth_value,
                         password: Devise.friendly_token[0, 20],
                         role: Role.find(3))

      # Find the school that uses this IdP and add the new user to it
      group = InstitutionalGroup.find_by(idp_entity_name: saml_response.raw_response.issuers.first)
      return unless group.present?

      user.institutional_groups << group
    end

    saml_response.attributes.resource_keys.each do |key|
      user.send "#{key}=", saml_response.attribute_value_by_resource_key(key)
    end

    if (Devise.saml_use_subject)
      user.send "#{Devise.saml_default_user_key}=", auth_value
    end

    user.save!
  end

This seems to go okay, and the request to /users/saml/auth seems to go through. At the end of the request, user_signed_in? is true but current_user is nil, which seems strange. Then, the user is redirected to /. At the beginning of this request, before_action :authenticate_user! says there is no logged in user and redirects them to the database_authenticatable login page.

  User Create (0.8ms)  ...
  ↳ config/initializers/devise.rb:344:in `block (2 levels) in <main>'
  TRANSACTION (1.1ms)  COMMIT
  ↳ config/initializers/devise.rb:344:in `block (2 levels) in <main>'
Redirected to http://localhost:3000/
Completed 302 Found in 327ms (ActiveRecord: 27.3ms | Allocations: 103959)

Started GET "/" for ::1 at 2022-10-05 11:13:55 -0500
  User Load (0.4ms)  SELECT `users`.* FROM `users` WHERE `users`.`id` IS NULL ORDER BY `users`.`id` ASC LIMIT 1
Started GET "/sign_in" for ::1 at 2022-10-05 11:13:55 -0500
Processing by Devise::SessionsController#new as HTML

As you can see, when redirected to / Devise tries to run a query selecting a user by the id of NULL, which... seems wrong. However, if the user already existed, this doesn't happen. Instead, when they are redirected to /, authenticate_user! sees them as already logged in.

adamstegman commented 2 years ago

I think maybe the issue is the user = ... line in your saml_update_resource_hook. The model hooks create the User and pass it into the hook, then return that User back to warden. The User that your hook creates never makes it back to the model, so doesn't get handed back to warden. Instead of creating a new User, try assigning attributes:

config.saml_update_resource_hook = lambda do |user, saml_response, auth_value|
  unless user.persisted? # Creating a new local user
    user.assign_attributes(email: auth_value, password: Devise.friendly_token[0, 20], role: Role.find(3))
  ...
rakaur commented 2 years ago

I assumed this was the problem as well, but I've never heard of #assign_attributes and because of the way Devise's passwords are, I couldn't just modify the user that was passed in by using user.password = so I was unsure how else to do this. This seems to have worked though, thanks!