Page MenuHomePhabricator

Start fleshing out PhabricatorAuthProviderViewController
ClosedPublic

Authored by amckinley on Jul 10 2019, 4:28 PM.
Tags
None
Referenced Files
F14076319: D20646.diff
Thu, Nov 21, 4:48 PM
Unknown Object (File)
Tue, Nov 19, 6:06 PM
Unknown Object (File)
Sat, Nov 16, 1:59 PM
Unknown Object (File)
Tue, Nov 12, 2:04 AM
Unknown Object (File)
Thu, Nov 7, 11:50 PM
Unknown Object (File)
Oct 16 2024, 10:05 AM
Unknown Object (File)
Oct 15 2024, 6:19 PM
Unknown Object (File)
Oct 14 2024, 7:26 PM
Subscribers
Tokens
"Pterodactyl" token, awarded by epriestley.

Details

Summary

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

Screen Shot 2019-07-10 at 9.27.35 AM.png (496×586 px, 45 KB)

Test Plan

Mk. 1 eyeball

Diff Detail

Repository
rP Phabricator
Branch
auth-view
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 23122
Build 31748: Run Core Tests
Build 31747: arc lint + arc unit

Event Timeline

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

Gate auto-login and trust emails.