Page MenuHomePhabricator

Start fleshing out PhabricatorAuthProviderViewController
ClosedPublic

Authored by amckinley on Jul 10 2019, 4:28 PM.

Details

Summary

Ref D20645. Start making this view a little more useful:

Test Plan

Mk. 1 eyeball

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

amckinley created this revision.Jul 10 2019, 4:28 PM
amckinley requested review of this revision.Jul 10 2019, 4:30 PM
epriestley accepted this revision.Jul 10 2019, 5:08 PM

We currently have a problem where some of these settings don't make sense or aren't actually used but can be viewed/edited, and this makes it slightly worse.

For example, in Auth you can edit a "Username/Password" provider and toggle the "Allow Link" / "Allow Unlink", but these settings don't actually have any effect because the Provider has a hard-coded override which ignores the saved settings:

public function shouldAllowAccountLink() {
  return false;
}

public function shouldAllowAccountUnlink() {
  return false;
}

Mostly, this code is just old and there's no way to specify "linking doesn't make sense for this provider, so link-related options don't make sense either and shouldn't be configurable".

However, the newer options do actually have "does this option make sense?" methods, notably:

  • supportsAutoLogin()
  • shouldAllowEmailTrustConfiguration()

So I think the "don't make anything worse than it already is" change is to gate the "Auto Login" and "Trust Email" items with those calls, and leave the other items for later:

if ($config->supportsAutoLogin()) {
  $view->addItem(...);
}

The "make everything great" change is to, like, modularize all this stuff and make it consistent (e.g. supportsLinking() or somesuch), but that's a much more far-reaching change that probably breaks at least some third-party providers and runs into modular transactions and who knows what else.

This revision is now accepted and ready to land.Jul 10 2019, 5:08 PM
amckinley updated this revision to Diff 49254.Jul 10 2019, 5:31 PM

Gate auto-login and trust emails.