Page MenuHomePhabricator

Update "bin/policy unlock" to be more surgical, flexible, modular, and modern
ClosedPublic

Authored by epriestley on Mar 7 2019, 4:41 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 12, 10:40 PM
Unknown Object (File)
Thu, Dec 12, 11:57 AM
Unknown Object (File)
Sun, Dec 8, 10:44 AM
Unknown Object (File)
Sat, Dec 7, 11:47 AM
Unknown Object (File)
Tue, Dec 3, 8:24 PM
Unknown Object (File)
Sat, Nov 30, 9:42 AM
Unknown Object (File)
Thu, Nov 28, 12:10 PM
Unknown Object (File)
Tue, Nov 26, 3:42 PM
Subscribers
None

Details

Summary

See PHI1115. Ref T13249. Currently, you can bin/policy unlock objects which have become inaccessible through some sort of policy mistake.

This script uses a very blunt mechanism to perform unlocks: just manually calling setXPolicy() and then trying to save() the object. Improve things a bit:

  • More surgical: allow selection of which policies you want to adjust with "--view", "--edit", and "--owner" (potentially important for some objects like Herald rules which don't have policies, and "edit-locked" tasks which basically ignore the edit policy).
  • More flexible: Instead of unlocking into "All Users" (which could be bad for stuff like Passphrase credentials, since you create a short window where anyone can access them), take a username as a parameter and set the policy to "just that user". Normally, you'd run this as bin/policy unlock --view myself --edit myself or similar, now.
  • More modular: We can't do "owner" transactions in a generic way, but lay the groundwork for letting applications support providing an owner reassignment mechanism.
  • More modern: Use transactions, not raw set() + save().

This previously had some hard-coded logic around unlocking applications. I've removed it, and the new generic stuff doesn't actually work. It probably should be made to work at some point, but I believe it's exceptionally difficult to lock yourself out of applications, and you can unlock them with bin/config set phabricator.application-settings ... anyway so I'm not too worried about this. It's also hard to figure out the PHID of an application and no one has ever asked about this so I'd guess the reasonable use rate of bin/policy unlock to unlock applications in the wild may be zero.

Test Plan
  • Used bin/policy unlock to unlock some objects, saw sensible transactions.

Diff Detail

Repository
rP Phabricator
Branch
unlock1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 22207
Build 30359: Run Core Tests
Build 30358: arc lint + arc unit

Event Timeline

amckinley added inline comments.
src/applications/policy/management/PhabricatorPolicyManagementUnlockWorkflow.php
30–35

Maybe remove this option until PhabricatorUnlockEngine ::newUnlockOwnerTransaction() gets implemented?

This revision is now accepted and ready to land.Mar 7 2019, 8:14 PM
This revision was automatically updated to reflect the committed changes.