Depends on D7642. This updates the authentication logic so that HTTP writes can be made to Git repositories hosted by Phabricator.
Details
Details
- Reviewers
hach-que - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- Restricted Maniphest Task
- Commits
- Restricted Diffusion Commit
rP43fd567ef471: Implement writes over HTTP for Git.
Set the policy to allow me to push and I was able to. Changed the policy to disallow push and I was no longer able to push.
Diff Detail
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
A couple of tweaks inline if you get to this before I pull it, but this looks good to me.
| src/applications/diffusion/controller/DiffusionController.php | ||
|---|---|---|
| 75–77 | We should put a key on this in D7462 to serve this query. I'd ideally like to use a user-based salt, which would mean we'd have to load the user first instead in order to hash the password (that is, hashPassword would become hashPassword($password, $user)). Is there a reason we can't do this in the "natural" order (check for a user, then verify the password) that you're aware of? | |
| 83–84 | We should reject the viewer if their account isDisabled() here. | |
| src/applications/diffusion/controller/DiffusionController.php | ||
|---|---|---|
| 73–84 | Oh, this doesn't actually do what it says on the tin -- PHP_AUTH_USER is completely ignored. I assume that's not intentional? | |
Comment Actions
I have the password part of this working, but git-http-backend isn't working correctly for bare Git repositories. I'm going to keep digging and see if I can figure that out.
| src/applications/diffusion/controller/DiffusionController.php | ||
|---|---|---|
| 73–84 | Oh whoops! That definitely needs to have the username check in there. It's missing username = %s and $username in the query. | |
Comment Actions
- Check username.
- Clarify some 401 vs 403: if you will never be able to log in, 403 instead of 401.
- Pass HTTP_CONTENT_ENCODING so that git-http-backend knows it has to unzip stuff.
- Add some more error handling and debugging that I wrote to try to figure out the gzip thing.