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.

Sign up for GitHub

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

feat: add recaptcha functionality to API registration #574

Merged
merged 21 commits into from
Feb 24, 2023
Merged

feat: add recaptcha functionality to API registration #574

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
925b877
wip: add nlwstein/recaptcha fork temporarily to support recaptcha fea…
nlwstein Feb 2, 2023
fc3fc03
wip: basic recaptcha functionality
nlwstein Feb 2, 2023
7219602
wip: resolve credo complaints
nlwstein Feb 2, 2023
41dda13
wip: better error handling, passes dialyzer
nlwstein Feb 2, 2023
c0a5c54
wip: remove js disable
nlwstein Feb 2, 2023
0358fc1
wip: better formatting for recaptcha
nlwstein Feb 2, 2023
29cc03c
Update apps/api_web/lib/api_web/templates/client_portal/user/_new.htm…
nlwstein Feb 2, 2023
2d770e6
wip: revert default-src
nlwstein Feb 2, 2023
d9ff22b
wip: attempt to use specific commit on original repo
nlwstein Feb 6, 2023
5f44334
wip: use with statement for recaptcha logic
nlwstein Feb 6, 2023
ac6ce1a
Merge branch 'master' into nlws-registration-recaptcha
nlwstein Feb 21, 2023
0bfadd9
Merge branch 'master' into nlws-registration-recaptcha
nlwstein Feb 21, 2023
40beccb
wip: only show recaptcha when MIX_ENV is prod
nlwstein Feb 21, 2023
21b1e82
wip: added feature flag and moved env var loading
nlwstein Feb 22, 2023
e3153e8
chore: apply formatter
nlwstein Feb 22, 2023
f1b117b
wip: better dev mode behavior, added changeset preservation when fail…
nlwstein Feb 22, 2023
e193989
wip: disable submit until recaptcha complete
nlwstein Feb 22, 2023
66f5ebc
wip: move function definition so the higher specificity one comes first
nlwstein Feb 22, 2023
d3c2654
Update apps/api_web/lib/api_web/controllers/portal/user_controller.ex
nlwstein Feb 24, 2023
289944e
test: ensure recaptcha widget is displayed when feature is enabled
nlwstein Feb 24, 2023
bd56f9c
test: add testing credentials for recaptcha
nlwstein Feb 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions apps/api_web/config/config.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ config :mime, :types, %{

config :ja_serializer, key_format: :underscored

config :recaptcha,
verify_url: "https://www.google.com/recaptcha/api/siteverify",
public_key: {:system, "RECAPTCHA_PUBLIC_KEY"},
secret: {:system, "RECAPTCHA_PRIVATE_KEY"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: for dynamic configuration, we typically load it using System.get_env/2 in config/runtime.exs now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: for dynamic configuration, we typically load it using System.get_env/2 in config/runtime.exs now.

I added the env vars to that file, and added a feature flag to disable the recaptcha code outside of prod builds.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it out, and the first thing I noticed was that the form fields were cleared if I submitted without the captcha. Otherwise it seems to work well.

I added some frontend code to only enable the submit button once the user completes the recaptcha, so this won't happen outside of someone attempting to do something weird.

Deployed to dev-green 🙂


config :api_web, :phoenix_swagger,
swagger_files: %{
"priv/static/swagger.json" => [router: ApiWeb.Router, endpoint: ApiWeb.Endpoint]
Expand Down
30 changes: 19 additions & 11 deletions apps/api_web/lib/api_web/controllers/portal/user_controller.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,26 @@ defmodule ApiWeb.ClientPortal.UserController do
|> render("new.html", changeset: changeset)
end

def create(conn, %{"user" => user_params}) do
case ApiAccounts.register_user(user_params) do
{:ok, user} ->
conn
|> put_session(:user_id, user.id)
|> configure_session(renew: true)
|> redirect(to: portal_path(conn, :index))

{:error, %ApiAccounts.Changeset{} = changeset} ->
def create(conn, %{"user" => user_params, "g-recaptcha-response" => recaptcha}) do
case Recaptcha.verify(recaptcha) do
nlwstein marked this conversation as resolved.
Show resolved Hide resolved
{:ok, _response} ->
case ApiAccounts.register_user(user_params) do
{:ok, user} ->
conn
|> put_session(:user_id, user.id)
|> configure_session(renew: true)
|> redirect(to: portal_path(conn, :index))

{:error, %ApiAccounts.Changeset{} = changeset} ->
conn
|> assign(:pre_container_template, "_new.html")
|> render("new.html", changeset: changeset)
end

{:error, _errors} ->
conn
|> assign(:pre_container_template, "_new.html")
|> render("new.html", changeset: changeset)
|> put_flash(:error, "Registration failed due to incorrect or missing captcha.")
|> redirect(to: user_path(conn, :new))
end
end

Expand Down
7 changes: 4 additions & 3 deletions apps/api_web/lib/api_web/router.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,12 @@ defmodule ApiWeb.Router do

@content_security_policy Enum.join(
[
"default-src 'none'",
"default-src 'self'",
nlwstein marked this conversation as resolved.
Show resolved Hide resolved
"img-src 'self' cdn.mbta.com",
"style-src 'self' 'unsafe-inline' maxcdn.bootstrapcdn.com fonts.googleapis.com",
"script-src 'self' maxcdn.bootstrapcdn.com code.jquery.com",
"font-src fonts.gstatic.com maxcdn.bootstrapcdn.com"
"script-src 'self' maxcdn.bootstrapcdn.com code.jquery.com www.google.com www.gstatic.com",
"font-src fonts.gstatic.com maxcdn.bootstrapcdn.com",
"frame-src www.google.com"
],
"; "
)
Expand Down
5 changes: 4 additions & 1 deletion apps/api_web/lib/api_web/templates/client_portal/user/_new.html.heex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@
</.form_group>

<div class="form-group">
<%= submit "Register", class: "btn btn-primary" %>
<%= raw Recaptcha.Template.display %>
nlwstein marked this conversation as resolved.
Show resolved Hide resolved
</div>
<div class="form-group">
<%= submit "Register", id: "register-btn", class: "btn btn-primary" %>
</div>
<% end %>
</div>
Expand Down
7 changes: 4 additions & 3 deletions apps/api_web/mix.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ defmodule ApiWeb.Mixfile do
#
# Type "mix help compile.app" for more information
def application do
extra_applications = [:logger, :phoenix_swagger | env_applications(Mix.env())]
extra_applications = [:logger, :recaptcha, :phoenix_swagger | env_applications(Mix.env())]
[mod: {ApiWeb, []}, extra_applications: extra_applications]
end

Expand Down Expand Up @@ -77,10 +77,11 @@ defmodule ApiWeb.Mixfile do
{:phoenix_swagger, github: "mbta/phoenix_swagger", branch: "master"},
{:ex_json_schema, "~> 0.6.2"},
{:diskusage_logger, "~> 0.2.0", only: :prod},
{:jason, "~> 1.0"},
{:jason, "~> 1.2"},
{:stream_data, "~> 0.4", only: :test},
{:plug_cowboy, "~> 2.1"},
{:sobelow, "~> 0.11", only: :dev, runtime: false}
{:sobelow, "~> 0.11", only: :dev, runtime: false},
{:recaptcha, git: "https://github.com/nlwstein/recaptcha.git", tag: "github-actions-support"}
]
end
end
1 change: 1 addition & 0 deletions mix.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
"polyline": {:hex, :polyline, "1.3.0", "58b9c268b100486df504b8af9b0d341107f733227a8f8095ed457d14d721d614", [:mix], [{:vector, "~> 1.0", [hex: :vector, repo: "hexpm", optional: false]}], "hexpm", "ab37fc3ab073c32fcd3f7c5711c2c20328b750ee27a4f4fab73541afc7242f8b"},
"proper": {:hex, :proper, "1.4.0", "89a44b8c39d28bb9b4be8e4d715d534905b325470f2e0ec5e004d12484a79434", [:rebar3], [], "hexpm", "18285842185bd33efbda97d134a5cb5a0884384db36119fee0e3cfa488568cbb"},
"ranch": {:hex, :ranch, "1.8.0", "8c7a100a139fd57f17327b6413e4167ac559fbc04ca7448e9be9057311597a1d", [:make, :rebar3], [], "hexpm", "49fbcfd3682fab1f5d109351b61257676da1a2fdbe295904176d5e521a2ddfe5"},
"recaptcha": {:git, "https://github.com/nlwstein/recaptcha.git", "ee6e0a6a5e206a59355b11123f5f5391f27f831a", [tag: "github-actions-support"]},
"rstar": {:git, "https://github.com/armon/erl-rstar.git", "a406b2cce609029bf65b9ccfbe93a0416c0ee0cd", []},
"sobelow": {:hex, :sobelow, "0.11.1", "23438964486f8112b41e743bbfd402da3e5b296fdc9eacab29914b79c48916dd", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "9897363a7eff96f4809304a90aad819e2ad5e5d24db547af502885146746a53c"},
"ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.6", "cf344f5692c82d2cd7554f5ec8fd961548d4fd09e7d22f5b62482e5aeaebd4b0", [:make, :mix, :rebar3], [], "hexpm", "bdb0d2471f453c88ff3908e7686f86f9be327d065cc1ec16fa4540197ea04680"},
Expand Down