Page MenuHomePhabricator

Begin adding more guidance to the "One-Time Login" flow
ClosedPublic

Authored by epriestley on Tue, Feb 5, 1:33 PM.

Details

Summary

Ref T13244. See PHI774. If an install does not use password auth, the "one-time login" flow (via "Welcome" email or "bin/auth recover") is pretty rough. Current behavior:

  • If an install uses passwords, the user is prompted to set a password.
  • If an install does not use passwords, you're dumped to /settings/external/ to link an external account. This is pretty sketchy and this UI does not make it clear what users are expected to do (link an account) or why (so they can log in).

Instead, improve this flow:

  • Password reset flow is fine.
  • (Future Change) If there are external linkable accounts (like Google) and the user doesn't have any linked, I want to give users a flow like a password reset flow that says "link to an external account".
  • (This Change) If you're an administrator and there are no providers at all, go to "/auth/" so you can set something up.
  • (This Change) If we don't hit on any other rules, just go home?

This may be tweaked a bit as we go, but basically I want to refine the "/settings/external/" case into a more useful flow which gives users more of a chance of surviving it.

Test Plan

Logged in with passwords enabled (got password reset), with nothing enabled as an admin (got sent to Auth), and with something other than passwords enabled (got sent home).

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Tue, Feb 5, 1:33 PM
epriestley requested review of this revision.Tue, Feb 5, 1:34 PM
epriestley added inline comments.Tue, Feb 5, 1:36 PM
src/applications/auth/query/PhabricatorAuthProviderConfigQuery.php
25–28

This method has no callers, and the associated STATUS_ constants have no use sites.

This calling convention is also very out of date, modern calls are withStatuses(<list>) instead of withStatus(<some constant>) because a handful of hard-coded status constants didn't cover use cases in Maniphest/Differential, particularly once statuses became configurable.

amckinley accepted this revision.Tue, Feb 5, 8:32 PM
This revision is now accepted and ready to land.Tue, Feb 5, 8:32 PM
This revision was automatically updated to reflect the committed changes.