ueberauth / ueberauth_identity

A username/password Strategy for Überauth
MIT License
79 stars 21 forks source link

Support for form_for field in config.exs #5

Closed imranismail closed 8 years ago

imranismail commented 8 years ago

Issue

Params submitted from this form doesn't get captured into auth struct

# web/templates/auth/request

<%= form_for @changeset, @callback_url, fn f -> %>
  <%= if f.errors != [] do %>
    <div class="alert alert-danger">
      <p>Oops, something went wrong! Please check the errors below:</p>
      <ul>
        <%= for {attr, message} <- f.errors do %>
          <li><%= humanize(attr) %> <%= message %></li>
        <% end %>
      </ul>
    </div>
  <% end %>

  <div class="form-group">
    <label>Email</label>
    <%= text_input f, :email, class: "form-control" %>
  </div>

  <div class="form-group">
    <label>Password</label>
    <%= password_input f, :password, class: "form-control" %>
  </div>

  <div class="form-group">
    <%= submit "Login", class: "btn btn-primary" %>
  </div>
<% end %>

params

%{"_csrf_token" => "GXElfW8jABYYE15FAQoPdjcWKT4MNgAAw8W7ZQjLagluU267oTpRbQ==",
  "_utf8" => "✓", "provider" => "identity",
  "user" => %{"email" => "john.doe@gmail.com", "password" => "password"}}
# config/config.exs
config :ueberauth, Ueberauth,
  providers: [
    identity: {Ueberauth.Strategy.Identity, [
      callback_methods: ["POST"],
      uid_field: :email,
      password_field: :password
    ]}
  ]

Suggestion

allow specifying model to be authenticated against in config so that it can be made useful to collect the fields from a form_for submission

config :ueberauth, Ueberauth,
  providers: [
    identity: {Ueberauth.Strategy.Identity, [
      callback_methods: ["POST"],
      uid_field: :email,
      password_field: :password,
      model: MyApp.User
    ]}
  ]
hassox commented 8 years ago

The problem here is that the params are not "email" or "password". They're nested under the "user" key rather than being top level.

I'd suggest that rather than including a model which would add significant coupling and complexity, that we allow the params key that it is nested under to be specified.

Thoughts?

imranismail commented 8 years ago

Yep that's right, that could work too. I think it's more versatile that way and it would cover a lot more corner cases.