diff --git a/api/spec/requests/devise/authentication_spec.rb b/api/spec/requests/devise/authentication_spec.rb index 782c8f096..fdd53d72a 100644 --- a/api/spec/requests/devise/authentication_spec.rb +++ b/api/spec/requests/devise/authentication_spec.rb @@ -3,7 +3,7 @@ module MnoEnterprise RSpec.describe "Remote Authentication", type: :request do - let(:user) { build(:user) } + let(:user) { build(:user, password_valid: true) } before { api_stub_for(get: "/users/#{user.id}", response: from_api(user)) } before { api_stub_for(put: "/users/#{user.id}", response: from_api(user)) } diff --git a/core/app/models/mno_enterprise/user.rb b/core/app/models/mno_enterprise/user.rb index 09b89f812..9abc1178c 100644 --- a/core/app/models/mno_enterprise/user.rb +++ b/core/app/models/mno_enterprise/user.rb @@ -51,7 +51,7 @@ class User < BaseResource :last_sign_in_ip, :confirmation_token, :confirmed_at, :confirmation_sent_at, :unconfirmed_email, :failed_attempts, :unlock_token, :locked_at, :name, :surname, :company, :phone, :phone_country_code, :geo_country_code, :geo_state_code, :geo_city, :website, :orga_on_create, :sso_session, :current_password_required, :admin_role, - :api_key, :api_secret, :developer, :kpi_enabled, :external_id, :meta_data + :api_key, :api_secret, :developer, :kpi_enabled, :external_id, :meta_data, :password_valid define_model_callbacks :validation #required by Devise @@ -94,14 +94,14 @@ class User < BaseResource # The auth_hash includes an email and password # Return nil in case of failure def self.authenticate(auth_hash) - u = self.post("user_sessions", auth_hash) - + # retro compatibilty issue: previous version were returning nil if the user was not found or if the password was invalid + # see https://maestrano.atlassian.net/browse/MNOE-209 + # we now validate that the password is valid in /core/lib/devise/strategies/remote_authenticatable.rb + u = self.post("user_sessions", auth_hash.merge(validate_password: true)) if u && u.id u.clear_attribute_changes! - return u end - - nil + return u end diff --git a/core/lib/devise/strategies/remote_authenticatable.rb b/core/lib/devise/strategies/remote_authenticatable.rb index 279c6b5ef..7dc2229b5 100644 --- a/core/lib/devise/strategies/remote_authenticatable.rb +++ b/core/lib/devise/strategies/remote_authenticatable.rb @@ -1,19 +1,20 @@ module Devise module Strategies class RemoteAuthenticatable < Authenticatable - + # def valid? # true || params[scope] # end - + # For an example check : https://github.com/plataformatec/devise/blob/master/lib/devise/strategies/database_authenticatable.rb # Method called by warden to authenticate a resource. def authenticate! # authentication_hash doesn't include the password auth_params = params[scope] - + + # mapping.to is a wrapper over the resource model - resource = mapping.to.new + resource = mapping.to.new.remote_authentication(auth_params) return fail! unless resource @@ -24,7 +25,7 @@ def authenticate! # # If the block returns true the resource will be loged in # If the block returns false the authentication will fail! - if validate(resource){ resource = resource.remote_authentication(auth_params) } + if validate(resource) { resource.password_valid } success!(resource) end end @@ -35,10 +36,10 @@ def authenticate! Warden::Strategies.add :remote_authenticatable, Devise::Strategies::RemoteAuthenticatable Devise.add_module :remote_authenticatable, strategy: true, controller: :sessions, route: :session -Warden::Manager.after_authentication do |user,auth,opts| +Warden::Manager.after_authentication do |user, auth, opts| Rails.cache.delete(['user', user.to_key]) if user end -Warden::Manager.before_logout do |user,auth,opts| +Warden::Manager.before_logout do |user, auth, opts| Rails.cache.delete(['user', user.to_key]) if user end