Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Double sign_out and undefined method 'email' error #219

Open
mahboob-hussain opened this issue Feb 21, 2022 · 4 comments
Open

Double sign_out and undefined method 'email' error #219

mahboob-hussain opened this issue Feb 21, 2022 · 4 comments

Comments

@mahboob-hussain
Copy link

mahboob-hussain commented Feb 21, 2022

I am using devise_saml_authenticatable to authenticate users with OneLogin as the idp. I am getting an error, undefined method 'email' for nil:Nil class after the sign_out request is repeated.

The sign_out sequence is:

GET "/users/sign_out" for 127.0.0.1
ActionController::Base Processing by Devise::SamlSessionsController#destroy as HTML
ActionController::Base Redirected to https://ordwaylabs.onelogin.com/trust/saml2/http-redirect/slo/1651823?SAMLRequest=fVHBTsMwDP2V3nJAbdKsgy1aKyZNSJMGB0AcuCC3ydZKaVziVMDfk3YcJiS4RIr9nt%2Bz34agt4M64AnH8GjeR0Mh2cWncxA6dCVrQxhIcY5ef8CXhZoydMbiqXNZgz0PfqTApzGST9jUG91508SaRZ5fL%2FOVXLBkvyvZmymOjRFLmeY6r9OiETIFWDapXlyvY0vf1HoVoUSj2TsK4ELJpJAyjUCZP4uVKtaqEK8seTGeZnsyEyz57K0jNXko2eidQqCOlIPekAqNetreH1QEKiAyflrrkjL8zxk8BmzQsmozodXszlfTqvEqFhuwLVJQCyHEfAbemwAaAmz4JeHMfojz97vkDn0P4W%2FhPMvnSqfT4wxVpofObrX2hohVPbQ1Yp21IxF07iq%2FvYgnpvIjfVarzr9fKVff
ActionController::Base Completed 302 Found in 15ms (ActiveRecord: 1.9ms)
Started GET "/users/sign_out" for 127.0.0.1
ActionController::Base Processing by Devise::SamlSessionsController#destroy as HTML
ActionController::Base Completed 500 Internal Server Error in 2ms (ActiveRecord: 0.0ms)
14:19:41 rails.1 | [2022-02-21T14:19:41.791] FATAL Rails DESKTOP-94MU5GO 310 70085015461320 2086da0a-94b7-41dc-82db-3b913d314334:
14:19:41 rails.1 | NoMethodError - undefined method `email' for nil:NilClass:

In my model User, the included devise modules are:

  devise :saml_authenticatable, :registerable, :confirmable, :recoverable, :rememberable, :trackable,
         :validatable, :session_limitable, :lockable, :timeoutable

The devise saml configuration I have is
==> Configuration for :saml_authenticatable

config.saml_attribute_map_resolver = "MyAttributeMapResolver"
config.saml_create_user = true
config.saml_update_user = true
config.saml_default_user_key = :email
config.saml_session_index_key = :session_index
config.saml_use_subject = true

config.saml_configure do |settings|
  settings.assertion_consumer_service_url     = "http://localhost:3000/users/idp/saml/auth"
  settings.assertion_consumer_service_binding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
  settings.name_identifier_format             = "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"
  settings.issuer                             = "http://localhost:3000/saml/metadata"
  settings.idp_slo_service_url                = "https://ordwaylabs.onelogin.com/trust/saml2/http-redirect/slo/1651823"
  settings.idp_sso_service_url                = "https://ordwaylabs.onelogin.com/trust/saml2/http-redirect/sso/35d9d080-9360-442a-b93c-12ad9d4bd098"
    settings.idp_cert = "-----BEGIN CERTIFICATE-----
<<certificate_blob>>
-----END CERTIFICATE-----"
  end

I do not have any logout route specified in routes.rb.

@adamstegman
Copy link
Collaborator

This does seem pretty weird. It's as if OneLogin redirected back to your logout path. How did you log out? Was there a button in your application?

@doconnor-clintel
Copy link
Contributor

We see similar:

NoMethodError: undefined method `username' for nil:NilClass
devise_saml_authenticatable (1.9.1) lib/devise_saml_authenticatable.rb:152:in `public_send'
devise_saml_authenticatable (1.9.1) lib/devise_saml_authenticatable.rb:152:in `block in <module:Devise>'
devise_saml_authenticatable (1.9.1) app/controllers/devise/saml_sessions_controller.rb:57:in `store_info_for_sp_initiated_logout'
activesupport (6.0.6.1) lib/active_support/callbacks.rb:428:in `block in make_lambda'

@doconnor-clintel
Copy link
Contributor

Simplest fix is likely just return if current_user.nil?

@sessionindex_for_sp_initiated_logout = current_user.public_send(Devise.saml_session_index_key)

@anna-dev
Copy link

I got the same issue and in our test cases i found out that store_info_for_sp_initiated_logout got called before verify_signed_out_user.
The problem is the following line in SamlSessionsController:
prepend_before_action :verify_signed_out_user, :store_info_for_sp_initiated_logout, only: :destroy
the second method gets called first.

I created a pull request to solve this issue: #260

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants