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.

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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

amckinley updated this revision to Diff 48670.Apr 10 2019, 10:35 PM
amckinley created this revision.

Words.

Harbormaster completed remote builds in B22543: Diff 48670.
amckinley requested review of this revision.Apr 10 2019, 10:38 PM
epriestley accepted this revision.Apr 10 2019, 10:53 PM
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
epriestley added inline comments.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 marked 3 inline comments as done.Apr 10 2019, 11:11 PM
amckinley added inline comments.
src/applications/config/check/PhabricatorAuthSetupCheck.php
82–94

Oh, good call.

amckinley updated this revision to Diff 48671.Apr 10 2019, 11:11 PM
amckinley marked an inline comment as done.

Requested changes.

This revision was automatically updated to reflect the committed changes.