Page MenuHomePhabricator

Add a workflow and a new config option for locking authentication providers
ClosedPublic

Authored by amckinley on Apr 10 2019, 10:35 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 12, 10:47 PM
Unknown Object (File)
Thu, Dec 12, 10:47 PM
Unknown Object (File)
Thu, Dec 12, 10:47 PM
Unknown Object (File)
Thu, Dec 12, 10:47 PM
Unknown Object (File)
Sat, Dec 7, 12:01 PM
Unknown Object (File)
Thu, Dec 5, 12:46 PM
Unknown Object (File)
Fri, Nov 29, 8:22 AM
Unknown Object (File)
Wed, Nov 27, 1:07 AM
Subscribers

Details

Summary

Ref T7667. Adds new flows bin/auth lock and bin/auth unlock to prevent compromised administrator accounts from doing additional damage by altering the authentication provider configuration.

Note that this currently doesn't actually do anything because we aren't checking this config key in any of the edit controllers yet.

Test Plan

Ran lock and unlock, checked for correct DB state, observed expected setup warning.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley added inline comments.
src/applications/auth/management/PhabricatorAuthManagementLockWorkflow.php
29–31

Slightly more modern is just echo tsprintf(...) without $console, since the whole mess that required $console in the first place was basically thrown away. It's possible we might revive it in some form, but random scripts won't ever need it (it supported interrogating stdin in some parent process to get answers to prompts).

src/applications/config/check/PhabricatorAuthSetupCheck.php
82–94

I guess maybe only show this if we aren't show the other setup check (i.e., only show it if we have $configs) -- since they sort of follow naturally from one another ("No Configs" -> "Okay, you have configs, now lock them configs").

Maybe phrase this more directly toward a first-time administrator by leading with a specific call to action before motivating that action, e.g.:

Authentication configuration is currently unlocked. Once you finish setting up or modifying authentication, lock the configuration to prevent unauthorized changes.

blah blah this is a serious attack blah blah all the other text

This revision is now accepted and ready to land.Apr 10 2019, 10:53 PM
src/applications/config/option/PhabricatorAuthenticationConfigOptions.php
94

Maybe "run" vs "do" as a verb for running a CLI command.

amckinley added inline comments.
src/applications/config/check/PhabricatorAuthSetupCheck.php
82–94

Oh, good call.

amckinley marked an inline comment as done.

Requested changes.

This revision was automatically updated to reflect the committed changes.