Ref D20645. Start making this view a little more useful:
Details
Mk. 1 eyeball
Diff Detail
- Repository
- rP Phabricator
- Branch
- auth-view
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 23121 Build 31746: Run Core Tests Build 31745: 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.