Summary:
Ref T4195. To implement the "Author" and "Committer" rules, I need to resolve author/committer strings into Phabricator users.
The code to do this is currently buried in the daemons. Extract it into a standalone query.
I also added `bin/repository lookup-users <commit>` to test this query, both to improve confidence I'm getting this right and to provide a diagnostic command for users, since there's occasionally some confusion over how author/committer strings resolve into valid users.
Test Plan:
I tested this using `bin/repository lookup-users` and `reparse.php --message` on Git, Mercurial and SVN commits. Here's the `lookup-users` output:
>>> orbital ~/devtools/phabricator $ ./bin/repository lookup-users rINIS3
Examining commit rINIS3...
Raw author string: epriestley
Phabricator user: epriestley (Evan Priestley )
Raw committer string: null
>>> orbital ~/devtools/phabricator $ ./bin/repository lookup-users rPOEMS165b6c54f487c8
Examining commit rPOEMS165b6c54f487...
Raw author string: epriestley <git@epriestley.com>
Phabricator user: epriestley (Evan Priestley )
Raw committer string: epriestley <git@epriestley.com>
Phabricator user: epriestley (Evan Priestley )
>>> orbital ~/devtools/phabricator $ ./bin/repository lookup-users rINIH6d24c1aee7741e
Examining commit rINIH6d24c1aee774...
Raw author string: epriestley <hg@yghe.net>
Phabricator user: epriestley (Evan Priestley )
Raw committer string: null
>>> orbital ~/devtools/phabricator $
The `reparse.php` output was similar, and all VCSes resolved authors correctly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1731, T4195
Differential Revision: https://secure.phabricator.com/D7801
Summary:
Ref T4107. Two issues:
- With strict MySQL settings, we try to insert `null` into the non-nullable `messageCount` field. Add an `initializeNew...` method.
- If we don't create a new conpherence (for example, because the message body is empty), we fatal on `getPHID()` right now.
Also, make this stuff a little easier to test.
Test Plan: Used `mail_handler.php` to receive empty conpherence mail, and new-thread conpherence mail.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4107
Differential Revision: https://secure.phabricator.com/D7760
Summary:
See <https://github.com/facebook/phabricator/issues/467>. @dctrwatson also ran into an issue where we were trying to `setPass()` a GitURI.
- For Git and Mercurial, properly generate credential URIs where relevant.
- Don't try to `setPass()` on Git-style URIs.
This isn't perfect but should clean things up a bit.
Test Plan: Added unit tests. Lots of `grep`.
Reviewers: btrahan
Reviewed By: btrahan
CC: dctrwatson, aran
Differential Revision: https://secure.phabricator.com/D7759
Summary: This adds a handful of 'Main Header' colors to change the look of Phabricator very slightly. I know I would probably set my dev header to a different color.
Test Plan: Tested each css class and color, can add more in the future.
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D7731
Summary: Ref T4195. Stores remote address and protocol in the logs, where possible.
Test Plan: Pushed some stuff, looked at the log, saw data.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4195
Differential Revision: https://secure.phabricator.com/D7711
Summary: Ref T4189. Fixes T2066. Mercurial has a //lot// of hooks so I'm not 100% sure this is all we need to install (we may need separate hooks for tags/bookmarks) but it should cover most of what we're after at least.
Test Plan:
- `bin/repository pull`'d a Mercurial repo and got a hook install.
- Pushed to a Mercurial repository over SSH and HTTP, with good/bad hooks. Saw hooks fire.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2066, T4189
Differential Revision: https://secure.phabricator.com/D7685
Summary:
Ref T4189. This adds SVN support, which was a little more messy than I though. Principally, we can not use `PHABRICATOR_USER` for Subversion, because it strips away the entire environment for "security reasons".
Instead, use `--tunnel-user` plus `svnlook author` to figure out the author.
Also fix "ssh://" clone URIs, which needs to be "svn+ssh://".
Test Plan:
- Made SVN commits through the hook.
- Made Git commits, too, to make sure I didn't break anything.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4189
Differential Revision: https://secure.phabricator.com/D7683
Summary:
Ref T4189. T4189 describes most of the intent here:
- When updating hosted repositories, sync a pre-commit hook into them instead of doing a `git fetch`.
- The hook calls into Phabricator. The acting Phabricator user is sent via PHABRICATOR_USER in the environment. The active repository is sent via CLI.
- The hook doesn't do anything useful yet; it just veifies basic parameters, does a little parsing, and exits 0 to allow the commit.
Test Plan:
- Performed Git pushes and pulls over SSH and HTTP.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4189
Differential Revision: https://secure.phabricator.com/D7682
Summary:
The documentation is explicit that Git does not pass this flag:
The $GIT_SSH command will be given exactly two arguments: the username@host (or just host) from the URL and the shell command to execute on that remote system.
This isn't true; it does. Accommodate it.
I'll see if I can fix this in the upstream, too.
Test Plan: Ran various `ssh-connect` commands with -p, etc.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7680
Summary: The "easy setup" is sort of an exaggeration since it basically just amounts to getting you logged in correctly, but it will probably be more true in the future.
Test Plan: Ran `bin/accountadmin` with zero and more than zero accounts.
Reviewers: asherkin, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7678
Summary: Some oldschool users create accounts via bin/accountadmin, which
messes up the first-time setup process. Auto-approve these accounts, as this
better aligns with user expectation.
Auditors: btrahan
Summary: If no ssh keys are in phabricator, bin/ssh-auth errors with undefined `$lines`. This fixes that case and explicitly tells the user no rows were found.
Test Plan: Ran bin/ssh-auth before and after change with no ssh keys in the system. Error goes away after change.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: epriestley
CC: epriestley, aran, Korvin
Differential Revision: https://secure.phabricator.com/D7675
Apparently we've got Unknown Object (Phabricator User) in
translation project now.
Not sure how it became invalid, but automated scripts
better be robust for this.
- Make it work with repositories outside of svn.b.o.
Useful for local debugging.
- Make it aware of svn subpath repository setting.
So now it's possible to restrict commit rights
to a subpath in svn repository.
- Make SVN repositories always available for
read-only access;
Summary: Fixes T2230. This isn't a total walk in the park to configure, but should work for early adopters now.
Test Plan: Read documentation, browsed UI.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7634
Summary: Fixes T4122. Ref T2230. Instead of storing credentials on each repository, store them in Passphrase. This allows easy creation/management of many repositories which share credentials.
Test Plan:
- Upgraded repositories.
- Created and edited repositories.
- Pulled HTTP and SSH repositories.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2230, T4122
Differential Revision: https://secure.phabricator.com/D7629
Summary:
This adds a Drydock blueprint for preallocated, remote hosts. This will be used by the Harbormaster interface to allow users to specify remote hosts that builds can be run on.
This adds a `canAllocateResource` method to Drydock blueprints; it is used to detect whether a blueprint can allocate a resource for the given type and attributes.
Test Plan:
Ran:
```
bin/drydock lease --type host --attributes remote=true,preallocated=true,host=192.168.56.101,port=22,user=james,keyfile=,path=C:\\Build\\,platform=windows
```
and saw the "C:\Build\<id>" folder appear on the remote Windows machine. Viewed the lease and resource in Drydock as well.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley
CC: Korvin, epriestley, aran, jamesr
Maniphest Tasks: T4111
Differential Revision: https://secure.phabricator.com/D7593
Summary:
This cleans up some garbage:
- We were specifying environmental variables with `X=y git ...`, but now have `setEnv()` on both `ExecFuture` and `PhutilExecPassthru`. Use `setEnv()`.
- We were specifying the working directory with `(cd %s && git ...)`, but now have `setCWD()` on both `ExecFuture` and `PhutilExecPassthru`. Use `setCWD()`.
- We were specifying the Git credentials with `ssh-agent -c (ssh-add ... && git ...)`. We can do this more cleanly with `GIT_SSH`. Use `GIT_SSH`.
- Since we have to write a script for `GIT_SSH` anyway, use the same script for Subversion and Mercurial.
This fixes two specific issues:
- Previously, we were not able to set `-o StrictHostKeyChecking=no` on Git commands, so the first time you cloned a git repo the daemons would generally prompt you to add `github.com` or whatever to `known_hosts`. Since this was non-interactive, things would mysteriously hang, in effect. With `GIT_SSH`, we can specify the flag, reducing the number of ways things can go wrong.
- This adds `LANG=C`, which probably (?) forces the language to English for all commands. Apparently you need to install special language packs or something, so I don't know that this actually works, but at least two users with non-English languages have claimed it does (see <https://github.com/facebook/arcanist/pull/114> for a similar issue in Arcanist).
At some point in the future I might want to combine the Arcanist code for command execution with the Phabricator code for command execution (they share some stuff like LANG and HGPLAIN). However, credential management is kind of messy, so I'm adopting a "wait and see" approach for now. I expect to split this at least somewhat in the future, for Drydock/Automerge if nothing else.
Also I'm not sure if we use the passthru stuff at all anymore, I may just be able to delete that. I'll check in a future diff.
Test Plan: Browsed and pulled Git, Subversion and Mercurial repositories.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7600
Summary: Fixed a small bug that caused the catch-all commit to purge previously added symbols in that session.
Test Plan: Re-ran the script, observed corrected behavior.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T4117
Differential Revision: https://secure.phabricator.com/D7597
Summary:
Modified the import script so it will only try to load a configurable
number of symbols at a time to avoid exhausting memory for large project
imports.
I haven't written a line of PHP in more than a decade, so please forgive
any stylistic or technical errors.
Test Plan: Ran the script on symbol table generated from linux kernel.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T4117
Differential Revision: https://secure.phabricator.com/D7596
Summary:
Ref T4039. This is mostly to deal with that, to prevent the security issues associated with mutable local paths. The next diff will lock them in the web UI.
I also added a confirmation prompt to `bin/repository delete`, which was a little scary without one.
See one comment inline about the `--as` flag. I don't love this, but when I started adding all the stuff we'd need to let this transaction show up as "Administrator" it quickly got pretty big.
Test Plan: Ran `bin/repository edit ...`, saw an edit with a transaction show up on the web UI.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4039
Differential Revision: https://secure.phabricator.com/D7579
Summary:
- Add an option for the queue.
- By default, enable it.
- Dump new users into the queue.
- Send admins an email to approve them.
Test Plan:
- Registered new accounts with queue on and off.
- As an admin, approved accounts and disabled the queue from email.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7576
Summary:
Small step forward which improves existing stuff or lays groudwork for future stuff:
- Currently, to check for email verification, we have to single-query the email address on every page. Instead, denoramlize it into the user object.
- Migrate all the existing users.
- When the user verifies an email, mark them as `isEmailVerified` if the email is their primary email.
- Just make the checks look at the `isEmailVerified` field.
- Add a new check, `isUserActivated()`, to cover email-verified plus disabled. Currently, a non-verified-but-not-disabled user could theoretically use Conduit over SSH, if anyone deployed it. Tighten that up.
- Add an `isApproved` flag, which is always true for now. In a future diff, I want to add a default-on admin approval queue for new accounts, to prevent configuration mistakes. The way it will work is:
- When the queue is enabled, registering users are created with `isApproved = false`.
- Admins are sent an email, "[Phabricator] New User Approval (alincoln)", telling them that a new user is waiting for approval.
- They go to the web UI and approve the user.
- Manually-created accounts are auto-approved.
- The email will have instructions for disabling the queue.
I think this queue will be helpful for new installs and give them peace of mind, and when you go to disable it we have a better opportunity to warn you about exactly what that means.
Generally, I want to improve the default safety of registration, since if you just blindly coast through the path of least resistance right now your install ends up pretty open, and realistically few installs are on VPNs.
Test Plan:
- Ran migration, verified `isEmailVerified` populated correctly.
- Created a new user, checked DB for verified (not verified).
- Verified, checked DB (now verified).
- Used Conduit, People, Diffusion.
Reviewers: btrahan
Reviewed By: btrahan
CC: chad, aran
Differential Revision: https://secure.phabricator.com/D7572
This script only generates content of either svnroot-access
or svnroot-authfile (depending on command line arguments).
It's not directly usable yet and would need some magic to
make it able to pass the data to svn.b.o. But this is up to
machines setup, not to the script.
Summary:
Ref T2230. The SVN protocol has a sensible protocol format with a good spec here:
http://svn.apache.org/repos/asf/subversion/trunk/subversion/libsvn_ra_svn/protocol
Particularly, compare this statement to the clown show that is the Mercurial wire protocol:
> It is possible to parse an item without knowing its type in advance.
WHAT A REASONABLE STATEMENT TO BE ABLE TO MAKE ABOUT A WIRE PROTOCOL
Although it makes substantially more sense than Mercurial, it's much heavier-weight than the Git or Mercurial protocols, since it isn't distributed.
It's also not possible to figure out if a request is a write request (or even which repository it is against) without proxying some of the protocol frames. Finally, several protocol commands embed repository URLs, and we need to reach into the protocol and translate them.
Test Plan: Ran various SVN commands over SSH (`svn log`, `svn up`, `svn commit`, etc).
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7556
