1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-27 16:00:59 +01:00
Commit graph

854 commits

Author SHA1 Message Date
epriestley
ebd8f3c987 Make translation, timezone and pronoun into real settings
Summary:
Ref T4103. These are currently stored on the user, for historic/performance reasons.

Since I want administrators to be able to set defaults for translations and timezones at a minimum and there's no longer a meaningful performance penalty for moving them off the user record, turn them into real preferences and then nuke the columns.

Test Plan:
  - Set settings to unusual values.
  - Ran migrations.
  - Verified my unusual settings survived.
  - Created a new user.
  - Edited all settings with old and new UIs.
  - Reconciled client/server timezone disagreement.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4103

Differential Revision: https://secure.phabricator.com/D16005
2016-06-02 06:29:47 -07:00
epriestley
edfc6a6934 Convert some loadPreferences() to getUserSetting()
Summary:
Ref T4103. This doesn't get everything, but takes care of most of the easy stuff.

The tricky-ish bit here is that I need to move timezones, pronouns and translations to proper settings. I expect to pursue that next.

Test Plan:
- Grepped for `loadPreferences` to identify callsites.
- Changed start-of-week setting, loaded Calendar, saw correct start.
- Visited welcome page, read "Adjust Settings" point.
- Loaded Conpherence -- I changed behavior here slightly (switching threads drops the title glyph) but it wasn't consistent to start with and this seems like a good thing to push to the next version of Conpherence.
- Enabled Filetree, toggled in Differential.
- Disabled Filetree, no longer visible in Differential.
- Changed "Unified Diffs" preference to "Small Screens" vs "Always".
- Toggled filetree in Diffusion.
- Edited a task, saw sensible projects in policy dropdown.
- Viewed user profile, uncollapsed/collapsed side nav, reloaded page, sticky'd.
- Toggled "monospaced textareas", used a comment box, got appropriate fonts.
- Toggled durable column.
- Disabled title glyphs.
- Changed monospaced font to 18px/36px impact.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4103

Differential Revision: https://secure.phabricator.com/D16004
2016-06-02 06:29:20 -07:00
epriestley
9180f429eb Provide a general-purpose, modular user cache for settings and other similar data
Summary:
Ref T4103. Currently, we issue a `SELECT * FROM user_preferences ... WHERE userPHID = ...` on every page to load the viewer's settings.

There are several other questionable data accesses on every page too, most of which could benefit from improved caching strategies (see T4103#178122).

This query will soon get more expensive, since it may need to load several objects (e.g., the user's settings and their "role profile" settings). Although we could put that data on the User and do both in one query, it's nicer to put it on the Preferences object ("This inherits from profile X") which means we need to do several queries.

Rather than paying a greater price, we can cheat this stuff into the existing query where we load the user's session by providing a user cache table and doing some JOIN magic. This lets us issue one query and try to get cache hits on a bunch of caches cheaply (well, we'll be in trouble at the MySQL JOIN limit of 61 tables, but have some headroom).

For now, just get it working:

  - Add the table.
  - Try to get user settings "for free" when we load the session.
  - If we miss, fill user settings into the cache on-demand.
  - We only use this in one place (DarkConsole) for now. I'll use it more widely in the next diff.

Test Plan:
  - Loaded page as logged-in user.
  - Loaded page as logged-out user.
  - Examined session query to see cache joins.
  - Changed settings, saw database cache fill.
  - Toggled DarkConsole on and off.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4103

Differential Revision: https://secure.phabricator.com/D16001
2016-06-02 06:28:56 -07:00
epriestley
b256f2d7b2 Prepare UserPreferences for transactions
Summary:
Ref T4103. This give preferences a PHID, policy/transaction interfaces, a transaction table, and a Query class.

This doesn't actually change how they're edited, yet.

Test Plan:
- Ran migrations.
- Inspected database for date created, date modified, PHIDs.
- Changed some of my preferences.
- Deleted a user's preferences, verified they reset properly.
- Set some preferences as a new user, got a new row.
- Destroyed a user, verified their preferences were destroyed.
- Sent Conpherence messages.
- Send mail.
- Tried to edit another user's settings.
- Tried to edit a bot's settings as a non-admin.
- Edited a bot's settings as an admin (technically, none of the editable settings are actually stored in the settings table, currently).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4103

Differential Revision: https://secure.phabricator.com/D15991
2016-05-31 12:28:44 -07:00
epriestley
da6b3de65c Use transactions to apply web UI SSH key edits
Summary:
Ref T10917. Converts web UI edits to transactions.

This is about 95% "the right way", and then I cheated on the last 5% instead of building a real EditEngine. We don't need it for anything else right now and some of the dialog workflows here are a little weird so I'm just planning to skip it for the moment unless it ends up being easier to do after the next phase (mail notifications) or something like that.

Test Plan: {F1652160}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10917

Differential Revision: https://secure.phabricator.com/D15947
2016-05-19 15:00:18 -07:00
epriestley
0308d580d7 Deactivate SSH keys instead of destroying them completely
Summary:
Ref T10917. Currently, when you delete an SSH key, we really truly delete it forever.

This isn't very consistent with other applications, but we built this stuff a long time ago before we were as rigorous about retaining data and making it auditable.

In partiular, destroying data isn't good for auditing after security issues, since it means we can't show you logs of any changes an attacker might have made to your keys.

To prepare to improve this, stop destoying data. This will allow later changes to become transaction-oriented and show normal transaction logs.

The tricky part here is that we have a `UNIQUE KEY` on the public key part of the key.

Instead, I changed this to `UNIQUE (key, isActive)`, where `isActive` is a nullable boolean column. This works because MySQL does not enforce "unique" if part of the key is `NULL`.

So you can't have two rows with `("A", 1)`, but you can have as many rows as you want with `("A", null)`. This lets us keep the "each key may only be active for one user/object" rule without requiring us to delete any data.

Test Plan:
- Ran schema changes.
- Viewed public keys.
- Tried to add a duplicate key, got rejected (already associated with another object).
- Deleted SSH key.
- Verified that the key was no longer actually deleted from the database, just marked inactive (in future changes, I'll update the UI to be more clear about this).
- Uploaded a new copy of the same public key, worked fine (no duplicate key rejection).
- Tried to upload yet another copy, got rejected.
- Generated a new keypair.
- Tried to upload a duplicate to an Almanac device, got rejected.
- Generated a new pair for a device.
- Trusted a device key.
- Untrusted a device key.
- "Deleted" a device key.
- Tried to trust a deleted device key, got "inactive" message.
- Ran `bin/ssh-auth`, got good output with unique keys.
- Ran `cat ~/.ssh/id_rsa.pub | ./bin/ssh-auth-key`, got good output with one key.
- Used `auth.querypublickeys` Conduit method to query keys, got good active keys.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10917

Differential Revision: https://secure.phabricator.com/D15943
2016-05-18 14:54:28 -07:00
epriestley
809c7bf996 Allow users to manage package dominion rules
Summary: Ref T10939. This adds UI, transactions, etc, to adjust dominion rules.

Test Plan:
  - Read documentation.
  - Changed dominion rules.
  - Created packages on `/` ("A") and `/x` ("B") with "Auto Review: Review".
  - Touched `/x`.
  - Verified that A and B were added with strong dominion.
  - Verified that only B was added when A was set to weak dominion.
  - Viewed file in Diffusion, saw correct ownership with strong/weak dominion rules.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10939

Differential Revision: https://secure.phabricator.com/D15936
2016-05-17 10:57:43 -07:00
epriestley
875b866715 Add missing "oauth_server_edge" tables
Summary: Fixes T10975. The "scramble attached file permissions when an object is saved" code is misfiring here too. See T10778 + D15803 for prior work.

Test Plan:
  - Ran `bin/storage upgrade -f`.
  - Edited the view policy of an OAuth server (prepatch: fatal; postpatch: worked great).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10975

Differential Revision: https://secure.phabricator.com/D15938
2016-05-17 08:50:27 -07:00
epriestley
52ac242eb3 Implement "Auto Review" in packages with a "Subscribe" option
Summary:
Ref T10939. Ref T8887. This moves toward letting packages automatically become reviewers or blocking reviewers of owned code.

This change adds an "Auto Review" option to packages. Because adding reviewers/blocking reviewers is a little tricky, it doesn't actually have these options yet -- just a "subscribe" option. I'll do the reviewer work in the next update.

Test Plan:
Created a revision in a package with "Auto Review: Subscribe to Changes". The package got subscribed.

{F1311677}

{F1311678}

{F1311679}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8887, T10939

Differential Revision: https://secure.phabricator.com/D15915
2016-05-13 17:21:58 -07:00
epriestley
576b73dc53 Index all repository URIs, not just the "primary" repository URI
Summary:
Ref T10923. When regenerating the URI index for a repository, index every URI.

  - Also, make the index slightly stricter (domain + path instead of just path). Excluding the domain made more sense when we were generating only first-party URIs.
  - Make the index smarter about `/diffusion/123/` URIs.
  - Show normalized URIs in `diffusion.repository.search` results.

Test Plan:
  - Ran migration.
  - Verified sensible-looking results in database.
  - Searched for a repository URI by first-party clone URI.
  - Searched for a repository URI by mirror URI.
  - Used `diffusion.repository.search` to get information about repository URIs.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10923

Differential Revision: https://secure.phabricator.com/D15876
2016-05-11 06:36:06 -07:00
epriestley
29d1115037 Swap Repository Edit UI to new code
Summary:
Ref T10748. This needs more extensive testing and is sure to have some rough edges, but seems to basically work so far.

Throwing this up so I can work through it more deliberately and make notes.

Test Plan:
- Ran migration.
- Used `bin/repository list` to list existing repositories.
- Used `bin/repository update <repository>` to update various repositories.
- Updated a migrated, hosted Git repository.
- Updated a migrated, observed Git repository.
- Converted an observed repository into a hosted repository by toggling the I/O mode of the URI.
- Conveted a hosted repository into an observed repository by toggling it back.
- Created and activated a new empty hosted Git repository.
- Created and activated an observed Git repository.
- Updated a mirrored repository.
- Cloned and pushed over HTTP.
- Tried to HTTP push a read-only repository.
- Cloned and pushed over SSH.
- Tried to SSH push a read-only repository.
- Updated several Mercurial repositories.
- Updated several Subversion repositories.
- Created and edited repositories via the API.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10748

Differential Revision: https://secure.phabricator.com/D15842
2016-05-04 16:19:57 -07:00
epriestley
42eaa88f80 Cut mirroring over to new URIs
Summary:
Ref T10748. This migrates and swaps mirroring to `PhabricatorRepositoryURI`, obsoleting `PhabricatorRepositoryMirror`.

This prevents you from editing, adding or disabling mirrors unless you know a secret URI (until the UI cuts over fully), but existing mirroring is not affected.

Test Plan:
  - Added a mirroring URI to an old repository.
  - Verified it worked with `bin/repository mirror`.
  - Migrated forward.
  - Verified it still worked with `bin/repository mirror`.
  - Wow, mirroring: https://github.com/epriestley/locktopia-mirror

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10748

Differential Revision: https://secure.phabricator.com/D15841
2016-05-04 16:16:16 -07:00
epriestley
dd2b10b8f8 Guarantee repositories have unique local paths
Summary:
Ref T4039. Long ago these were more freely editable and there were some security concerns around creating a repository, then setting its local path to point somewhere it shouldn't.

Local paths are no longer editable so there's no real reason we need to provide a uniqueness guarantee anymore, but you could still make a mistake with `bin/repository move-paths` by accident, and it's a little cleaner to pull them out into their own column with a key.

(We still don't -- and, largely can't -- guarantee that two paths aren't //equivalent// since one might be symlinked to the other, or symlinked only on some hosts, or whatever, but the primary value here is as a sanity check that you aren't goofing things up and pointing a bunch of repositories at the same working copy by mistake.)

Test Plan:
  - Ran migrations.
  - Grepped for `local-path`.
  - Listed and moved paths with `bin/repository`.
  - Created a new repository, verified its local path populated correctly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4039

Differential Revision: https://secure.phabricator.com/D15837
2016-05-04 16:09:52 -07:00
epriestley
616c9ae887 Rough sketch of new repository URI editing
Summary:
Ref T10748. Ref T10366. This adds a new EditEngine, EditController, Editor, Query, and Transaction for RepositoryURIs.

None of these really do anything helpful yet, and these URIs are still unused in the actual application.

Test Plan: {F1249794}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10366, T10748

Differential Revision: https://secure.phabricator.com/D15815
2016-04-29 09:21:00 -07:00
epriestley
467c4e84e5 Add an edge table to the search database
Summary:
Fixes T10778. This is a result of T10262: when we save a form configuration and adjust the policy, we try to scramble attached file secrets.

There aren't going to be any attached files, but there's also no edge table, so we fail.

We could skip this code, but we'll likely need an edge table here sooner or later so it's probably simpler in the long run to just add an empty one.

Test Plan:
   - Ran `bin/storage upgrade`, got a clean bill of health.
   - Saved a form configuration after making a policy edit, no more `edge` exception.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10778

Differential Revision: https://secure.phabricator.com/D15803
2016-04-26 11:26:26 -07:00
epriestley
892a9a1f07 Make cluster repositories more resistant to freezing
Summary:
Ref T10860. This allows us to recover if the connection to the database is lost during a push.

If we lose the connection to the master database during a push, we would previously freeze the repository. This is very safe, but not very operator-friendly since you have to go manually unfreeze it.

We don't need to be quite this aggressive about freezing things. The repository state is still consistent after we've "upgraded" the lock by setting `isWriting = 1`, so we're actually fine even if we lost the global lock.

Instead of just freezing the repository immediately, sit there in a loop waiting for the master to come back up for a few minutes. If it recovers, we can release the lock and everything will be OK again.

Basically, the changes are:

  - If we can't release the lock at first, sit in a loop trying really hard to release it for a while.
  - Add a unique lock identifier so we can be certain we're only releasing //our// lock no matter what else is going on.
  - Do the version reads on the same connection holding the lock, so we can be sure we haven't lost the lock before we do that read.

Test Plan:
  - Added a `sleep(10)` after accepting the write but before releasing the lock so I could run `mysqld stop` and force this issue to occur.
  - Pushed like this:

```
$ echo D >> record && git commit -am D && git push
[master 707ecc3] D
 1 file changed, 1 insertion(+)
# Push received by "local001.phacility.net", forwarding to cluster host.
# Waiting up to 120 second(s) for a cluster write lock...
# Acquired write lock immediately.
# Waiting up to 120 second(s) for a cluster read lock on "local001.phacility.net"...
# Acquired read lock immediately.
# Device "local001.phacility.net" is already a cluster leader and does not need to be synchronized.
# Ready to receive on cluster host "local001.phacility.net".
Counting objects: 3, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (3/3), 254 bytes | 0 bytes/s, done.
Total 3 (delta 1), reused 0 (delta 0)
BEGIN SLEEP
```

  - Here, I stopped `mysqld` from the CLI in another terminal window.

```
END SLEEP
# CRITICAL. Failed to release cluster write lock!
# The connection to the master database was lost while receiving the write.
# This process will spend 300 more second(s) attempting to recover, then give up.
```

  - Here, I started `mysqld` again.

```
# RECOVERED. Link to master database was restored.
# Released cluster write lock.
To ssh://local@localvault.phacility.com/diffusion/26/locktopia.git
   2cbf87c..707ecc3  master -> master
```

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10860

Differential Revision: https://secure.phabricator.com/D15792
2016-04-25 11:37:31 -07:00
epriestley
6edf181a7e Record which cluster host received a push
Summary: Ref T4292. When we write a push log, also log which node received the request.

Test Plan: {F1230467}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4292

Differential Revision: https://secure.phabricator.com/D15759
2016-04-19 13:06:30 -07:00
epriestley
f424f9f2d2 Record more details about where a write is taking place while holding a cluster lock
Summary: Ref T4292. This will let the UI and future `bin/repository` tools give administrators more tools to understand problems when reporting or resolving them.

Test Plan:
  - Pushed fully clean repository.
  - Pushed previously-pushed repository.
  - Forced write to abort, inspected useful information in the database.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4292

Differential Revision: https://secure.phabricator.com/D15748
2016-04-18 11:55:27 -07:00
epriestley
595f203816 Correct RepositoryURI schema and propagate adjust exit code correctly
Summary:
Fixes T10830.

  - The return code from `storage adjust` did not propagate correct.
  - There was one column issue which I missed the first time around because I had a bunch of unrelated stuff locally.

Test Plan:
  - Ran `bin/storage upgrade -f` with failures, used `echo $?` to make sure it exited nonzero.
  - Got fully clean `bin/storage adjust` by dropping all my extra local tables.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10830

Differential Revision: https://secure.phabricator.com/D15746
2016-04-18 08:11:22 -07:00
epriestley
9352ed8abb Add missing RepositoryURI table + run storage adjustments in tests
Summary:
Fixes T10830. Ref T10366. I wasn't writing to this table yet so I didn't build it, but the fact that `bin/storage adjust` would complain slipped my mind.

  - Add the table.
  - Make the tests run `adjust`. This is a little slow (a few extra seconds) but we could eventually move some steps like this to run server-side only.

Test Plan: Ran `bin/storage upgrade -f`, got a clean `adjust`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10366, T10830

Differential Revision: https://secure.phabricator.com/D15744
2016-04-18 07:54:02 -07:00
epriestley
4244cad990 Move toward multi-master replicated repositories
Summary:
Ref T4292. This mostly implements the locking/versioning logic for multi-master repositories. It is only active on Git SSH pathways, and doesn't actually do anything useful yet: it just does bookkeeping so far.

When we read (e.g., `git fetch`) the logic goes like this:

- Get the read lock (unique to device + repository).
  - Read all the versions of the repository on every other device.
  - If any node has a newer version:
    - Fetch the newer version.
    - Increment our version to be the same as the version we fetched.
- Release the read lock.
- Actually do the fetch.

This makes sure that any time you do a read, you always read the most recently acknowledged write. You may have to wait for an internal fetch to happen (this isn't actually implemented yet) but the operation will always work like you expect it to.

When we write (e.g., `git push`) the logic goes like this:

- Get the write lock (unique to the repository).
  - Do all the read steps so we're up to date.
  - Mark a write pending.
    - Do the actual write.
  - Bump our version and mark our write finished.
- Release the write lock.

This allows you to write to any replica. Again, you might have to wait for a fetch first, but everything will work like you expect.

There's one notable failure mode here: if the network connection between the repository node and the database fails during the write, the write lock might be released even though a write is ongoing.

The "isWriting" column protects against that, by staying locked if we lose our connection to the database. This will currently "freeze" the repository (prevent any new writes) until an administrator can sort things out, since it'd dangerous to continue doing writes (we may lose data).

(Since we won't actually acknowledge the write, I think, we could probably smooth this out a bit and make it self-healing //most// of the time: basically, have the broken node rewind itself by updating from another good node. But that's a little more complex.)

Test Plan:
  - Pushed changes to a cluster-mode repository.
  - Viewed web interface, saw "writing" flag and version changes.
  - Pulled changes.
  - Faked various failures, got sensible states.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4292

Differential Revision: https://secure.phabricator.com/D15688
2016-04-12 08:57:57 -07:00
lkassianik
1f423c3bd1 Make badges searchable by name
Summary: Closes T10690

Test Plan: Open Badges application, go to Advanced Search, search for a badge by its name and see result.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Maniphest Tasks: T10690

Differential Revision: https://secure.phabricator.com/D15656
2016-04-07 12:25:11 -07:00
epriestley
8bca296ac1 Migrate old task transactions to use new display code
Summary: Ref T6027. This converts the old transaction records to the new format so we don't have to keep legacy code around.

Test Plan: Migrated tasks, browsed around, looked at transaction records, didn't see any issues.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6027

Differential Revision: https://secure.phabricator.com/D15637
2016-04-06 09:14:41 -07:00
epriestley
8dfc7d4201 Allow OAuth applications to be disabled instead of destroyed
Summary: Ref T7303. This interaction is very oldschool; modernize it to enable/disable instead of "nuke from orbit".

Test Plan:
  - Enabled applications.
  - Disabled applications.
  - Viewed applications in list view.
  - Generated new tokens.
  - Tried to use a token from a disabled application (got rebuffed).
  - Tried to use a token from an enabled application (worked fine).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7303

Differential Revision: https://secure.phabricator.com/D15620
2016-04-05 13:22:05 -07:00
epriestley
57f016b166 Convert OAuthServer to Transactions + EditEngine
Summary: Ref T7303. This application is currently stone-age tech (no transactions, hard "delete" action). Bring it up to modern specs.

Test Plan:
  - Created and edited an OAuth application.
  - Viewed transaction record.
  - Tried to create something with no name, invalid redirect URI, etc. Was gently rebuffed with detailed explanatory errors.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7303

Differential Revision: https://secure.phabricator.com/D15609
2016-04-05 01:55:49 -07:00
Chad Little
dc2dab94bb Add commenting to Fund
Summary: Adds basic commenting to Fund Initiatives.

Test Plan: Leave a comment, see comment.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D15554
2016-03-31 16:01:15 -07:00
lkassianik
00425cac94 Converting badge quality property from color to an integer representation for later sorting purposes
Summary: Ref T9007

Test Plan: Create badges, update quality, search by quality without change of functionality.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Maniphest Tasks: T9007

Differential Revision: https://secure.phabricator.com/D15551
2016-03-30 17:28:34 -07:00
epriestley
f50693de61 Remove dedicated storage for NuanceRequestor
Summary:
Ref T10537. Currently, Nuance has a `NuanceRequestor` object, intended to represent the external user who created content (e.g., a GitHub account or a Twitter account or whatever).

This object is currently almost unused, and its design predates Doorkeeper. In D15541, I chose to use doorkeeper objects instead of NuanceRequestor objects to represent requestors.

I don't currently anticipate a need for such an object, given that we have Doorkeeper. If we do need it in the future for some reason, it would be fairly easy to restore it, create a requestor type which wraps a Doorkeeper object, and then migrate. Not super thrilling to do that, but not a huge mess.

`NuanceItem` still has a `requestorPHID`, but this is now a less formal object PHID instead of a more formal Requestor-object PHID, and holds a doorkeeper exeternal object PHID for GitHub events.

Test Plan:
  - Grepped for `nuancerequestor`.
  - Ran `bin/storage upgrade -f`.
  - Grepped for `requestor`, remaining uses of this term seem reasonable/correct.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10537

Differential Revision: https://secure.phabricator.com/D15546
2016-03-29 08:53:35 -07:00
lkassianik
0330ea575d Converting badge recipients from Edge to BadgeAward table
Summary: Ref T8996, Convert badge recipients from Edges to actual BadgeAward objects

Test Plan: Create badge, award it to recipient. Make sure adding/removing recipients works. (Still need to migrate exisiting recipients to new table and need to create activity feed blurbs)

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: chad, Korvin

Maniphest Tasks: T8996

Differential Revision: https://secure.phabricator.com/D15014
2016-03-26 14:03:48 -07:00
epriestley
601aaa5a86 Modularize content sources
Summary:
Ref T10537. For Nuance, I want to introduce new sources (like "GitHub" or "GitHub via Nuance" or something) but this needs to modularize eventually.

Split ContentSource apart so applications can add new content sources.

Test Plan:
This change has huge surface area, so I'll hold it until post-release. I think it's fairly safe (and if it does break anything, the breaks should be fatals, not anything subtle or difficult to fix), there's just no reason not to hold it for a few hours.

- Viewed new module page.
- Grepped for all removed functions/constants.
- Viewed some transactions.
- Hovered over timestamps to get content source details.
- Added a comment via Conduit.
- Added a comment via web.
- Ran `bin/storage upgrade --namespace XXXXX --no-quickstart -f` to re-run all historic migrations.
- Generated some objects with `bin/lipsum`.
- Ran a bulk job on some tasks.
- Ran unit tests.

{F1190182}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10537

Differential Revision: https://secure.phabricator.com/D15521
2016-03-26 11:59:45 -07:00
epriestley
1885c4e03b Add an ItemCommand queue to Nuance
Summary:
Ref T10537. Generally, when users interact with Nuance items we'll dump a command into a queue and apply it in the background. This avoids race conditions with multiple users interacting with an item, which Nuance is more subject to than other applications because it has an import/external component.

The "sync" command doesn't actually do anything yet.

Test Plan: {F1186365}

Reviewers: chad

Reviewed By: chad

Subscribers: Luke081515.2

Maniphest Tasks: T10537

Differential Revision: https://secure.phabricator.com/D15506
2016-03-22 15:08:23 -07:00
epriestley
47dedfb152 Introduce "bridged" objects
Summary:
Ref T10537. These are objects which are bound to some external object, like a Maniphest task which is a representation of a GitHub issue.

This doesn't do much yet and may change, but my thinking is:

  - I'm putting these on-object instead of on edges because I think we want to actively change the UI for them (e.g., clearly call out that the object is bridged) but don't want every page to need to do extra queries in the common case where zero bridged objects exist anywhere in the system.
  - I'm making these one-to-one, more or less: an issue can't be bridged to a bunch of tasks, nor can a bunch of tasks be bridged to a single issue. Pretty sure this makes sense? I can't come up with any reasonable, realistic cases where you want a single GitHub issue to publish to multiple different tasks in Maniphest.
  - Technically, one type of each bridgable object could be bridged, but I expect this to never actually occur. Hopefully.

Test Plan: Ran storage upgrade, loaded some pages.

Reviewers: chad

Reviewed By: chad

Subscribers: Luke081515.2

Maniphest Tasks: T10537

Differential Revision: https://secure.phabricator.com/D15502
2016-03-22 15:06:57 -07:00
epriestley
f46686ff58 Implement a Git LFS link table and basic batch API
Summary:
Ref T7789. This implements:

  - A new table to store the `<objectHash, filePHID>` relationship between Git LFS files and Phabricator file objects.
  - A basic response to `batch` commands, which return actions for a list of files.

Test Plan:
Ran `git lfs push origin master`, got a little further than previously:

```
epriestley@orbital ~/dev/scratch/poemslocal $ git lfs push origin master
Git LFS: (2 of 1 files) 174.24 KB / 87.12 KB
Git LFS operation "upload/b7e0aeb82a03d627c6aa5fc1bbfd454b6789d9d9affc8607d40168fa18cf6c69" is not supported by this server.
Git LFS operation "upload/b7e0aeb82a03d627c6aa5fc1bbfd454b6789d9d9affc8607d40168fa18cf6c69" is not supported by this server.
```

With `GIT_TRACE=1`, this shows the batch part of the API going through.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7789

Differential Revision: https://secure.phabricator.com/D15489
2016-03-17 17:15:20 -07:00
epriestley
a837c3d73e Make temporary token storage/schema more flexible
Summary:
Ref T10603. This makes minor updates to temporary tokens:

  - Rename `objectPHID` (which is sometimes used to store some other kind of identifier instead of a PHID) to `tokenResource` (i.e., which resource does this token permit access to?).
  - Add a `userPHID` column. For LFS tokens and some other types of tokens, I want to bind the token to both a resource (like a repository) and a user.
  - Add a `properties` column. This makes tokens more flexible and supports custom behavior (like scoping LFS tokens even more tightly).

Test Plan:
- Ran `bin/storage upgrade -f`, got a clean upgrade.
- Viewed one-time tokens.
- Revoked one token.
- Revoked all tokens.
- Performed a one-time login.
- Performed a password reset.
- Added an MFA token.
- Removed an MFA token.
- Used a file token to view a file.
- Verified file token was removed after viewing file.
- Linked my account to an OAuth1 account (Twitter).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10603

Differential Revision: https://secure.phabricator.com/D15478
2016-03-16 09:33:38 -07:00
epriestley
5d6bb0ffeb Import raw GitHub event data into Nuance
Summary:
Ref T10537. Ref T10538. This polls the GitHub events API and creates Nuance items from the raw data.

It does nothing useful with them.

Test Plan:
  - Polled GitHub.
  - Saw some items get created.
  - X-Poll-Interval seemed to work.
  - ETag seemed to work.
  - Recognizing when we hit items we've already seen seemed to work.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10537, T10538

Differential Revision: https://secure.phabricator.com/D15440
2016-03-08 12:03:11 -08:00
epriestley
2a3c3b2b98 Provide bin/nuance import and ngram indexes for sources
Summary:
Ref T10537. More infrastructure:

  - Put a `bin/nuance` in place with `bin/nuance import`. This has no useful behavior yet.
  - Allow sources to be searched by substring. This supports `bin/nuance import --source whatever` so you don't have to dig up PHIDs.

Test Plan:
  - Applied migrations.
  - Ran `bin/nuance import --source ...` (no meaningful effect, but works fine).
  - Searched for sources by substring in the UI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10537

Differential Revision: https://secure.phabricator.com/D15436
2016-03-08 10:30:24 -08:00
epriestley
3f4cc3ad6e Allow Nuances sources to provide import cursors
Summary:
Ref T10537. Some sources (like the future "GitHub Repository" source) need to poll remotes.

  - Provide a mechanism for sources to emit import cursors.
  - Hook them into the trigger daemon so they'll fire periodically.
  - Provide some storage.

This diff does nothing useful or interesting, and is pure infrastructure.

Test Plan:
  - Ran `bin/storage upgrade -f`, no adjustment issues.
  - Poked around Nuance.
  - Ran the trigger daemon, verified it didn't crash and checked for Nuance stuff to do.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10537

Differential Revision: https://secure.phabricator.com/D15435
2016-03-08 10:30:04 -08:00
epriestley
fc0dc02bb9 Allow Drydock blueprints to be tagged and searched, and give types some little icons
Summary:
Ref T10457.

  - Let blueprints be tagged so you can search and annotate them a little more easily.
  - Give each blueprint type an optional icon to make things a little easier to parse visually.

Test Plan:
  - Tagged blueprints.
  - Searched by tags.
  - Looked at nice little icons.

{F1139712}

Reviewers: chad

Reviewed By: chad

Subscribers: yelirekim

Maniphest Tasks: T10457

Differential Revision: https://secure.phabricator.com/D15392
2016-03-03 15:21:58 -08:00
epriestley
01379958fa Allow Drydock blueprints to be searched by name
Summary:
Ref T10457. The ngram indexing seems to be working well; extend it into Drydock.

Also clean up the list controller a little bit.

Test Plan:
  - Ran migrations.
  - Searched for blueprints by name.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10457

Differential Revision: https://secure.phabricator.com/D15389
2016-03-03 15:21:12 -08:00
epriestley
f078fd98d7 Support searching for Harbormater build plans by name substring
Summary: Ref T10457. Allow build plans to be queried by name.

Test Plan:
  - Searched for plans by name.
  - Renamed a plan, searched for new name.

{F1133085}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10457

Differential Revision: https://secure.phabricator.com/D15359
2016-02-29 05:22:24 -08:00
epriestley
7d4b323da2 Store Almanac "service types" instead of "service classes"
Summary:
Ref T10449. Currently, we store classes (like "AlmanacClusterRepositoryServiceType") in the database.

Instead, store types (like "cluster.repository").

This is a small change, but types are a little more flexible (they let us freely reanme classes), a little cleaner (fewer magic strings in the codebase), and a little better for API usage (they're more human readable).

Make this minor usability change now, before we unprototype.

Also make services searchable by type.

Also remove old Almanac API endpoints.

Test Plan:
  - Ran migration, verified all data migrated properly.
  - Created, edited, rebound, and changed properties of services.
  - Searched for services by service type.
  - Reviewed available Conduit methods.

Reviewers: chad

Reviewed By: chad

Subscribers: yelirekim

Maniphest Tasks: T10449

Differential Revision: https://secure.phabricator.com/D15346
2016-02-26 06:21:50 -08:00
epriestley
4c97d88aa4 Allow Almanac bindings to be disabled and unused interfaces to be removed
Summary:
Fixes T9762. Ref T10246.

**Disabling Bindings**: Previously, there was no formal way to disable bindings. The internal callers sometimes check some informal property on the binding, but this is a common need and deserves first-class support in the UI. Allow bindings to be disabled.

**Deleting Interfaces**: Previously, you could not delete interfaces. Now, you can delete unused interfaces.

Also some minor cleanup and slightly less mysterious documentation.

Test Plan: Disabled bindings and deleted interfaces.

Reviewers: chad

Reviewed By: chad

Subscribers: yelirekim

Maniphest Tasks: T9762, T10246

Differential Revision: https://secure.phabricator.com/D15345
2016-02-26 06:21:23 -08:00
epriestley
944539a786 Simplify locking of Almanac cluster services
Summary:
Fixes T6741. Ref T10246. Broadly, we want to protect Almanac cluster services:

  - Today, against users in the Phacility cluster accidentally breaking their own instances.
  - In the future, against attackers compromising administrative accounts and adding a new "cluster database" which points at hardware they control.

The way this works right now is really complicated: there's a global "can create cluster services" setting, and then separate per-service and per-device locks.

Instead, change "Can Create Cluster Services" into "Can Manage Cluster Services". Require this permission (in addition to normal permissions) to edit or create any cluster service.

This permission can be locked to "No One" via config (as we do in the Phacility cluster) so we only need this one simple setting.

There's also zero reason to individually lock //some// of the cluster services.

Also improve extended policy errors.

The UI here is still a little heavy-handed, but should be good enough for the moment.

Test Plan:
  - Ran migrations.
  - Verified that cluster services and bindings reported that they belonged to the cluster.
  - Edited a cluster binding.
  - Verified that the bound device was marked as a cluster device
  - Moved a cluster binding, verified the old device was unmarked as a cluster device.
  - Tried to edit a cluster device as an unprivileged user, got a sensible error.

{F1126552}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6741, T10246

Differential Revision: https://secure.phabricator.com/D15339
2016-02-25 03:38:39 -08:00
epriestley
03d6e7f1b6 Correct an old issue with Paste by restoring the file attachment edge
Summary:
I'm having trouble figuring out exactly what the timeframe on this was, but for a while in November we were not writing edges between pastes and their attached files correctly.

An example of this on this install is here:

https://secure.phabricator.com/P1893

That will start working once the migration runs, but until it does it shows this:

{F1126605}

This got fixed so recent stuff works fine, but it looks like WMF updated while the bug was active so they have more affected pastes than we do (we only have about 10).

Test Plan:
Ran this query to find pastes with missing edges:

```
select id, FROM_UNIXTIME(p.dateCreated) from pastebin_paste p LEFT JOIN edge ON edge.src = p.phid AND edge.type = 25 WHERE edge.dst IS NULL order by id;
```

Ran the migration.

Verified the edges were fixed.

Viewed one of the affected pastes, things now worked properly.

Reviewers: chad

Reviewed By: chad

Subscribers: 20after4

Differential Revision: https://secure.phabricator.com/D15340
2016-02-23 17:31:12 -08:00
epriestley
ab86523ac4 Allow Almanac properties to be deleted, use EditEngine instead of CustomField
Summary:
Fixes T10410. Immediate impact of this is that you can now actually delete properties from Almanac services, devices and bindings.

The meat of the change is switching from CustomField to EditEngine for most of the actual editing logic. CustomField creates a lot of problems with using EditEngine for everything else (D15326), and weird, hard-to-resolve bugs like this one (not being able to delete stuff).

Using EditEngine to do this stuff instead seems like it works out much better -- I did this in ProfilePanel first and am happy with how it looks.

This also makes the internal storage for properties JSON instead of raw text.

Test Plan:
  - Created, edited and deleted properties on services, devices and bindings.
  - Edited and reset builtin properties on repository services.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10410

Differential Revision: https://secure.phabricator.com/D15327
2016-02-22 11:28:26 -08:00
epriestley
db50d0fb11 Rough-in Almanac namespaces
Summary:
Ref T6741. Ref T10246.

Root problem: to provide Drydock in the cluster, we need to expose Almanac, and doing so would let users accidentally or intentionally create a bunch of `repo006.phacility.net` devices/services which could conflict with the real ones we manage.

There's currently no way to say "you can't create anything named `*.blah.net`". This adds "namespaces", which let you do that (well, not yet, but they will after the next diff).

After the next diff, if you try to create `repo003.phacility.net`, but the namespace `phacility.net` already exists and you don't have permission to edit it, you'll be asked to choose a different name.

Also various modernizations and some new docs.

Test Plan:
  - Created cool namespaces like `this.computer`.
  - Almanac namespaces don't actually enforce policies yet.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6741, T10246

Differential Revision: https://secure.phabricator.com/D15324
2016-02-22 04:58:35 -08:00
epriestley
50debecf52 Allow Almanac namespaces to be searched by ngram index
Summary: Ref T6741. Ref T10246. This is largely modernization, but will partially support namespace locking in Almanac.

Test Plan:
Searched for Almanac networks by name substring.

{F1121740}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6741, T10246

Differential Revision: https://secure.phabricator.com/D15322
2016-02-22 04:58:18 -08:00
epriestley
959bb16d0f Allow Almanac services to be searched by substring
Summary: Ref T10246. Build an ngram index for Almanac services, and use it to support improved search.

Test Plan: {F1121725}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10246

Differential Revision: https://secure.phabricator.com/D15321
2016-02-22 04:58:03 -08:00
epriestley
1b6ddae6b2 Allow Almanac devices to be queried and sorted by name
Summary:
Ref T10205. Ref T10246. This is general modernization, but also supports fixing the interface datasource in T10205.

  - Update Query.
  - Update SearchEngine.
  - Use an ngrams index for searching names efficiently.

Test Plan:
  - Ran migrations.
  - Searched Almanac devices by name.
  - Created a new device, searched for it by name.

{F1121303}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10205, T10246

Differential Revision: https://secure.phabricator.com/D15319
2016-02-21 10:44:46 -08:00
epriestley
dc7d0b4a56 Make repository callsigns optional
Summary:
Ref T4245. This could still use a little UI smoothing, but:

  - Don't require a callsign on the create flow (you can add one later in "Edit Basic Information" if you want).
  - Allow existing callsigns to be removed.

Test Plan:
  - Created a new repository with no callsign.
  - Cloned it; pushed to it.
  - Browsed around Diffusion a bunch.
  - Visited a commit URI.
  - Added a callsign to it.
  - Removed the callsign again.
  - Referenced it with `R22` in remarkup.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D15305
2016-02-18 10:36:13 -08:00