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

Question: Trying my hand at designing a captcha extension, would like some input. #499

Closed
joshchernoff opened this issue Apr 21, 2020 · 3 comments

Comments

@joshchernoff
Copy link
Contributor

joshchernoff commented Apr 21, 2020

Herse my very ruff wip.
https://github.com/MorphicPro/morphic.pro/commit/62cb8fae9fc232e5cf63090d16d3d24d1ffac77c

Currently, using elixir-captcha I am generating an image and text via the user's changeset.
https://github.com/MorphicPro/morphic.pro/commit/62cb8fae9fc232e5cf63090d16d3d24d1ffac77c#diff-6a620c43912e03175251aa7fbb6f3aa5R42-R51

I then render the captcha image in my registration's new action via a helper
https://github.com/MorphicPro/morphic.pro/commit/62cb8fae9fc232e5cf63090d16d3d24d1ffac77c#diff-f94b95d43a63baff3d691072e1841b85R5-R10

But before I do that I take the generated captcha's text from the changeset and save it to the conn's session via a controller callback.
https://github.com/MorphicPro/morphic.pro/commit/62cb8fae9fc232e5cf63090d16d3d24d1ffac77c#diff-93175fac10f8bcced302881ac47ca435R18-R27

So that all I need to set up my registrations new action where I have a captcha rending in the view and its text saved in a session for later verification.

Screen Shot 2020-04-21 at 2 12 49 PM

But then I'm not sure as to the correct way of dealing with the form submission part.

I have a virtual field for the user to supply a return value.
https://github.com/MorphicPro/morphic.pro/commit/62cb8fae9fc232e5cf63090d16d3d24d1ffac77c#diff-6a620c43912e03175251aa7fbb6f3aa5R16

My first thought was I could amend the Conn's params via the before_process for the registrations create to inject the valid captcha text from the conn's pow_captcha_store session then I could do the captcha validation from inside the changeset pipeline.

Though I have a feeling that's not a good idea. Not sure if I can even update a conn's params once Plug.Parsers has ran anyways. This is where I'm a little bit at a loss for where to take this.

Maybe the issue here is trying to use callback vs just handling each request explicitly calling into my own context?
So I thought to come here to get some opinions.

Thanks.

  • Josh
@danschultzer
Copy link
Collaborator

Sorry for the delay Josh. Been super busy with work the last few weeks.

You may find this extension and production implementation interesting (from issue #324 (comment)): https://github.com/derpibooru/philomena/tree/master/lib/pow_captcha

You'll have to dig a bit to find each working piece, but they insert the captcha with JS in the template, and then verify in controller callbacks. I feel this is the right approach. CAPTCHA is something that belongs in the plug conn rather than something the ecto app should know about. Same as a rate limiter is best handled exclusively at the proxy level.

So in your case I would remove all the logic from the MorphicPro, and just focus on MorphicPro Web.

I also have an issue open for a reCAPTCHA guide on powauth.com: pow-auth/pow_site#4

@danschultzer
Copy link
Collaborator

danschultzer commented Apr 23, 2020

Oh, sorry I think I missed the point and didn't really answer your question.

CAPTCHA is usually done with JS, so forms can first be submitted once the JS enables it. I realize that what you wish is to add it as a changeset error. I think it's still best handled exclusively at the controller level, but you should be able to pass it along with the params as you describe:

My first thought was I could amend the Conn's params via the before_process for the registrations create to inject the valid captcha text from the conn's pow_captcha_store session then I could do the captcha validation from inside the changeset pipeline.

So I guess it would look something like this:

  def before_process(
    Pow.Phoenix.RegistrationController,
    :create,
    conn,
    _config
  ) do
    captcha_text = Conn.get_session(conn, :pow_captcha_store)
    user_params = Map.put(conn.params["user"], "captcha_text", captcha_text)

    %{conn | params: Map.put(conn.params, "user", user_params)}
  end

But yeah this doesn't feel right. I think it's more appropriate to generate and verify it at controller level, and just add the changeset error there. Though that would be easiest with custom registration controller, but you could probably also handle it with a plug where you render the form and halt the conn, e.g.:

def call(conn, opts) do
  case validate_captcha(conn) do
    :ok ->
      conn

    :error ->
      changeset = Pow.Plug.change_user(conn, conn.params["user"])
      changeset = Ecto.Changeset.add_error(conn, :captcha, "does not match, try again")
      
      conn
      |> assign(:changeset, %{changeset | action: :insert})
      |> render(Pow.Phoenix.RegistrationView, "new.html")
      |> halt()
  end
end

@joshchernoff
Copy link
Contributor Author

thanks

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

2 participants