Page MenuHomePhabricator

POC: Land a revision to Github from Web UI
AbandonedPublic

Authored by avivey on Oct 31 2013, 10:49 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jan 31, 2:05 AM
Unknown Object (File)
Tue, Jan 21, 11:37 AM
Unknown Object (File)
Tue, Jan 21, 9:06 AM
Unknown Object (File)
Sun, Jan 19, 11:41 AM
Unknown Object (File)
Jan 8 2025, 12:00 AM
Unknown Object (File)
Dec 15 2024, 6:08 AM
Unknown Object (File)
Dec 14 2024, 4:34 AM
Unknown Object (File)
Dec 12 2024, 10:12 PM

Details

Reviewers
None
Group Reviewers
Blessed Reviewers
Summary

initial approximation of how this can work.

needs https://secure.phabricator.com/differential/diff/16821/ to libphutil, and a new github token.

Then go to /magic/, put revision number and github repo, see "log" of result.

My general plan from here, in no particular order:

Test Plan

test.

Diff Detail

Branch
local
Lint
Lint Skipped
Unit
No Test Coverage

Event Timeline

My brain is pretty shot right now but I'll give you some more specific feedback when I get a chance, probably tomorrow morning. In general, this looks pretty good to me. I think we can build some pieces of it so that they're upstream-able today -- the only thing I'm really hesitant about here is the "clone a copy" instead of "use drydock", but maybe we can even clean that up enough to make it upstreamable.

I would have assumed a git push approach would be simpler than this API -- something like: have users generate a keypair, put the public key on GitHub, and give the private key to Phabricator to push on their behalf (Phabricator could even do the "generate" part, and maybe add the key over the API? Or at least say "paste this public key into your GitHub"). Then we could git origin add github ... and git push, in theory. Are there reasons this is actually way harder/worse than the low-level API? One thing I can think of is that it doesn't isolate permissions as well, but if you're pushing to GitHub enterprise/firewall edition, that seems like it shouldn't matter too much? Another is that having users generate keypairs is a huge pain, but it looks like there's an API for pushing them: http://developer.github.com/v3/users/keys/

So in theory we could skip the low-level stuff and do:

  • Prompt the user for repo/key permissions, say, "We're going to install a public key, click OK to continue..."
  • Generate a keypair and push the public key to GitHub (if we haven't already).
  • Do all the checkout/patch logic we do right now.
  • But then just git push with right key instead of shipping up all the blobs.

Would that work? Would that be better? The big downside I can see is that pushing a public key is really powerful/scary, but I'd guess most users won't actually have much of an issue with it, especially against Enterprise/Firewall edition.

We could also do a reduced strength version of that, which is to give Phabricator a GitHub private key (on some arbitrary user's account or a bot account or whatever) and have it push with that. I don't think GitHub actually cares who the pushing user is -- it doesn't alter the repository data or anything. So just having "GitHub Sync Private Key" and "GitHub Account" somewhere should be good enough to let us "git push", I think? It could even be per-repo.

The more I think about it, the more it looks like the best approach is to add an ssh key per-user.

  • It's at least as secure as OAuth tokens, because it the same per-user notation, and might actually be more limited - can only read/write stuff, but not modify too many settings. It can also be revoked just as easily from GH web.
  • Either creating a single Phabricator user (and granting it access to each repo) or creating "deploy keys" (A single-repo, read-write key) would let users bypass GitHub's permissions system, and push via Phabricator to repos they shouldn't be able to. I can see push-back on this issue.

I Initially went with OAuth because (1) I had the notion that it will somehow only be valid while inside an active user session, a bit like ssh-agent, and (2) there is already a flow and storage for OAuth tokens. Now that I'm taking a step back, I understand that (1) isn't the case, so that advantage is gone. (2) looks like it will be mostly solved anyway as part of the Hosting change, and isn't really this big anyway.

Generating a pair and showing the public end for the user sounds like the easiest thing; If my test group is having a hard time with it we can try adding it via API (It also requires a change in the OAuth Scope param).

This seems reasonably GitHub specific. Is there a reason to not just using plain old Git commands? That way it could be used to land revisions against any repository Phabricator is tracking.

Yhe, I'll abandon the api approach in favor of plain git ssh.

So, here's what I have in mind right now:

When mature, Landing (Pushing?) via ssh options would probably include these things:

  1. One key for the install (For installs that use github and have a phabricator account there)
  2. Per user (For installs that want phab to act as users - this is the one I want for my install)
  3. Per repo (Github calls these "Deploy Keys".)
  4. Other things, like per domain, per repo*user, etc.

Option 1. is easy to hack, and 3. looks like it's there already (For pulling, but it's basically the same), but I'm interested in hacking option 2.

Is "External accounts" a good candidate for this?

As for this diff - I think I'll try to replace it with a more constructive "Land to Hosted git repo", unless there's something en rout already, because it's half some way there anyway; Since it'll end with a git-push, it'd be easy to make it push to some remote instead of local.

Is drydock in a state to allocate a working copy yet?

Drydock can't do anything useful yet.

I think ExternalAccount is OK for storing this. It's a little bit awkward, but I can't think of any reasons why it's really bad, offhand. We could really migrate out of it easily enough later on, too, since everything will be well-structured and the total number of records will be small.

You should be able to add records like accountType = ssh-key, accountDomain = github.com, accountID = <github account ID>, I think, and dump the public and private keys in the metadata or properties or whatever field.

To make this even more redundant:

git push https://${access_token}:x-oauth-basic@github.com/user/repo.git HEAD:master

is the way to use oauth tokens for pushing to github...