-
Notifications
You must be signed in to change notification settings - Fork 74
Allow the eWallet to work without integration #401
Conversation
3b0a7b6
to
77944b7
Compare
Remaining:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving with a few questions/comments :)
Crypto.fake_verify() | ||
handle_fail_auth(conn, :invalid_login_credentials) | ||
|
||
:user_not_admin -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any difference between this case and the one above? Can't we juse use:
_ ->
Crypto.fake_verify()
handle_fail_auth(conn, :invalid_login_credentials)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. Midway it was a bit different but not anymore. Updating.
|
||
error -> | ||
handle_error(conn, error) | ||
{:error, code} when is_atom(code) -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
@@ -257,7 +257,7 @@ defmodule AdminAPI.V1.AdminAuth.AccountMembershipControllerTest do | |||
assert response["data"]["code"] == "client:invalid_parameter" | |||
|
|||
assert response["data"]["description"] == | |||
"The `redirect_url` is not allowed to be used. Got: '#{redirect_url}'." | |||
"The given `redirect_url` is not allowed to be used. Got: '#{redirect_url}'." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we change this error to -> "The given redirect_url
is not allowed. Got: '#{redirect_url}'." ?
@@ -7,6 +7,7 @@ defmodule EWallet.Application do | |||
|
|||
def start(_type, _args) do | |||
import Supervisor.Spec | |||
DeferredConfig.populate(:ewallet) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
defp html(link) do | ||
""" | ||
<p> | ||
<strong>Click the link to complete the email verification: </strong> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to complete the email verification process
{:error, error} -> | ||
case Map.get(ErrorHandler.errors(), error) do | ||
%{description: description} -> | ||
text(conn, "Unable to verify your email address. " <> description) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We were unable to verify your email address. Error description:
.
text(conn, "Unable to verify your email address. " <> description) | ||
|
||
nil -> | ||
text(conn, "Unable to verify your email address. An unknown error occured.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We were unable to verify your email address.
Renders the page to show when email verification is successful. | ||
""" | ||
def success(conn, _attrs) do | ||
text(conn, "Your email address is successfully verified!") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your email address has been successfully verified!
@@ -259,14 +259,30 @@ defmodule EWalletDB.Account do | |||
@doc """ | |||
Get the master account for the current wallet setup. | |||
""" | |||
@spec get_preloaded_primary_wallet(keyword()) :: %Account{} | nil | |||
@spec get_master_account(keyword()) :: %Account{} | nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
|
||
case User.update_without_password(invite.user, %{invite_uuid: nil}) do | ||
{:ok, _user} -> | ||
Repo.delete(invite) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we actually delete invites or is it soft-delete? Can't we just set them to used
or something and keep them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s hard delete right now. To do soft delete, I'll do this:
- Disassociate the invite from the user (
user.invite_uuid
) after verification. Souser.invite_uuid == nil
. - Add a new
invite.user_uuid
column to keep the user->invite trace after the disassociation fromuser.invite_uuid
. - Add a new
invite.verified_at
column so it’s easy to see if and when the invite was used.
Updated with @T-Dnzt 's comments as well as @mederic-p 's suggestion to auto-include |
Closes #357.
Overview
This PR allows an eWallet provider to setup and uses the eWallet without having to implement or integrate into their own solutions.
Changes
Email configs
AdminAPI
toEWallet
app (to share withEWalletAPI
).AdminAPI.Inviter
intoEWallet.Web.Inviter
so that it accepts the email template as a parameter. So AdminAPI and EWalletAPI can send the confirmation email with different template/messages.Authentication
AdminAPI.V1.AdminUserAuthenticator
to reject authentication of non-admin usersEWalletAPI.V1.EndUserAuthenticator
to authenticate a user with email and passwordError handling
:invalid_login_credentials
,:invalid_email
,:invite_not_found
,:passwords_mismatch
fromAdminAPI.V1.ErrorHandler
toEWallet.V1.ErrorHandler
Signup flow
/api/client/user.signup
to support user signup with an email through the newEWalletAPI.V1.SignupController.signup/2
/api/client.user.verify_email
to support user email verification through the newEWalletAPI.V1.SignupController.verify_email/2
/api/client/user.login
to support user login with email/password through the newEWalletAPI.V1.AuthController.login/2
Supporting signup flow
EWalletAPI.VerificationEmail
template, separated fromAdminAPI.InviteEmail
EWallet.Web.V1.UserAuthTokenSerializer.serialize/1
to return user along with the authentication token:invite_uuid
toUser.update_changeset/2
so that invites can be removed after verificationEWalletDB.User.has_membership?/1
to also handle user's UUID as the parameter.EWalletDB.User.admin?/1
Docs
/user.login
,/user.signup
,/user.verify_email
docs/design.users.md
to explain different user types and the checker functionsEWalletDB.User
functionsImplementation Details
Since this functionality can be built on the existing invitation system for admins (and admins are also users), most of the changes are for reusing the existing code.
Usage
Try sign up with
/api/client/user.signup
followed by/api/client.user/verify_email
, then/api/client/user.login
.Example curl for /user.sign_up
Example curl for /user.verify_email
Example curl for /user.login
Impact
mix ecto.migrate
to migrate the databaseENABLE_STANDALONE=true
to enable standaloneREDIRECT_URL_PREFIXES
as a comma-separated string to allow redirect URLs that are external to the eWallet. If a redirect URL is already prefixed withBASE_URL
, it does not need to be added here.