Skip to content
This repository has been archived by the owner on Jul 24, 2023. It is now read-only.

Fix sign in with multiple fields #60

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

JuanitoFatas
Copy link
Contributor

@JuanitoFatas JuanitoFatas commented Mar 8, 2020

Hi Matt,

I was teaching a friend Rails and using Oath for authentication solution (of course). But I found the multiple fields auth does not work right now. There is already a solution in #55. I update his work so you could merge and release a new version of Oath.

can you just do a small write up about your solution. I assume the problem is that the params were being transformed twice. Was that the case?
-- halogenandtoast on #55 (comment)

Sign in with multiple fields:

# SessionsController
def create
  user = authenticate_session(session_params, email_or_username: [:email, :username])
end

In authenticate_session (L103) we already transformed the params

def authenticate_session session_params, field_map = nil
token_field = Oath.config.user_token_field
params_hash = Oath.transform_params(session_params).symbolize_keys
password = params_hash.fetch(token_field)
user = Oath.lookup(params_hash.except(token_field), field_map)
authenticate(user, password)
end

and in Oath.lookup's self.config.find_method

oath/lib/oath.rb

Lines 86 to 91 in f850ea5

def self.lookup(params, field_map)
if params.present?
fields = FieldMap.new(params, field_map).to_fields
self.config.find_method.call(fields)
end
end

which calls Oath::Configuration#default_method that tries to transform again

def default_find_method
->(params) do
updated_params = Oath.transform_params(params)
Oath.config.user_class.find_by(updated_params)
end
end

hence raises this error:

TypeError in SessionsController#create
wrong element type String at 0 (expected array)

The fix was not transformed twice.

cc @gracewashere Closes #55

c-lliope and others added 4 commits March 8, 2020 16:19
The recommended implementation for sign in
by email or username throws an error when looking up the User record.

Stack trace:
```
  1) User signs in with username or password
     Failure/Error: params.to_h

     TypeError:
       wrong element type String at 0 (expected array)
     # ./lib/monban/param_transformer.rb:25:in `to_h'
     # ./lib/monban/param_transformer.rb:25:in `sanitized_params'
     # ./lib/monban/param_transformer.rb:15:in `to_h'
     # ./lib/monban.rb:66:in `transform_params'
     # ./lib/monban/configuration.rb:57:in `block in default_find_method'
     # ./lib/monban.rb:90:in `lookup'
     # ./lib/monban/controller_helpers.rb:105:in `authenticate_session'
     # ./spec/rails_app/app/controllers/multiple_attributes_sessions_controller.rb:6:in `create'
     # ./lib/monban/back_door.rb:30:in `call'
     # ./spec/features/user/user_signs_in_spec.rb:25:in `block (2 levels) in <top (required)>'
```
This fix potentially skips over parameter sanitization.
If it does, the error seems to not be covered by any test cases.
The ActiveHash does not support pass in array
of query strings:

User.where(["email = ? OR username = ?", "example", "example"])

Mimic the real behavior in test instead.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants