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

Improve UX for unverified accounts on CLI #751

Open
jcraigk opened this issue Feb 5, 2020 · 6 comments
Open

Improve UX for unverified accounts on CLI #751

jcraigk opened this issue Feb 5, 2020 · 6 comments

Comments

@jcraigk
Copy link

jcraigk commented Feb 5, 2020

When creating a new account on hex.pm and running mix hex.user auth, there is no mention of the verification email in the CLI feedback. For example:

ᐅ mix hex.user auth
Username: my-username-here
Account password:
Generating keys...
You have authenticated on Hex using your account password. However, Hex requires you to have a local password that applies only to this machine for security purposes. Please enter it.
Local password:
Local password (confirm):

This feedback leads me to believe that I can readily be added to organizations and then download their dependencies, for example:

mix hex.organization auth my-organization-here

But when running mix deps.get against a project in said organization, I get a failure with the message:

This could be because the package does not exist, it was spelled incorrectly or you don't have permissions to it

Debugging, using mix hex.user whoami reveals the problem:

ᐅ mix hex.user whoami
Failed to auth
email not verified

It would be helpful for new users coming to the platform to make this email verification step more transparent through the CLI by doing one or both:

  • During mix hex.user auth and on mix deps.get failures, provide clear messaging about verification step
  • Bail out at username/pass entry if email is unverified

Even if the second one were implemented, the first would still be helpful in instances where an email address was changed and needed to be re-verified.

@jcraigk jcraigk changed the title Suggestion: Improve messaging for unverified accounts via CLI Suggestion: Improve UX for unverified accounts on CLI Feb 5, 2020
@supersimple
Copy link
Member

Thanks for the contribution JCK.
I agree, it would be nice to send feedback when auth'ing that your email has not been verified.
We do show this message when you register, but as you pointed out, it can be missed
You are required to confirm your email to access your account, a confirmation email has been sent to <email+address>

@supersimple supersimple changed the title Suggestion: Improve UX for unverified accounts on CLI Improve UX for unverified accounts on CLI Feb 15, 2020
@b-hass
Copy link

b-hass commented Mar 2, 2020

I would be excited to open a PR for this as my first issue. Should I follow any guidelines?

@supersimple
Copy link
Member

Hi @b-hass. There are no specific guidelines outside if the readme. I think this will involve changes both to hex and hexpm API.
If you run into questions, post them here or in the hex channel on the elixir slack.

@b-hass
Copy link

b-hass commented Mar 2, 2020

Hi @supersimple, thanks for the reply. Wouldn't a call to the current Hex.API.User.me inform us if the user email is confirmed or not (like it's done for whoami)?

defp whoami() do
auth = Mix.Tasks.Hex.auth_info(:read)
case Hex.API.User.me(auth) do
{:ok, {code, body, _}} when code in 200..299 ->
Hex.Shell.info(body["username"])
other ->
Hex.Shell.error("Failed to auth")
Hex.Utils.print_error_result(other)
end
end

What would we need to change in the hexpm API?

@supersimple
Copy link
Member

@b-hass yes, that might work. I was assuming there would be changes in the API, but you make a good point. Let me know if you need help once you get started. 💛

@ericmj
Copy link
Member

ericmj commented Mar 3, 2020

We shouldn't hit the API endpoint directly when fetching dependencies because the API is less reliable than the repository and doesn't have the same ability as the repository to handle peaks of 1000 requests/sec. So unfortunately calling Hex.API.User.me/1 is not a viable solution.

Looking into this issue more it seems more complex than we first anticipated, the current behavior is dictated by the interaction of four different systems: the client, API server, our CDN, and S3.

To authenticate requests for private packages the CDN does a cached preflight request to the authentication endpoint on the API [1] to verify the access. The authentication endpoint on the API actually returns an error response that includes the reason why the request failed (unverified email, not a valid API key, etc.). The CDN unfortunately hides this response, the reason being that it is by default JSON encoded and this client doesn't have a JSON library. The client could set accept: application/vnd.hex+erlang but that would be an invalid content-type when we get a registry file or package tarball. I am not sure how we should handle different content types for the success case and the error message.

Additionally we don't display the error message on the client either, the reason being that the error message from the S3 bucket is confusing to most users [2]. We might be able to hide this response on the CDN instead.

In the future we may want to change the authentication for private packages so that we do not hit the API at all and instead move to a static system that does not rely on the availability of the API. It's hard to imagine a static authentication system that can check if the user's email is verified so the best solution may be to drop this check completely.

[1] https://github.com/hexpm/hexpm/blob/master/lib/hexpm_web/controllers/api/auth_controller.ex#L7
[2] https://repo.hex.pm/do-not-have-permission

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants