New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Enterprise configured for regular HTTP #488

Merged
merged 3 commits into from Feb 24, 2014

Conversation

Projects
None yet
3 participants
@mislav
Member

mislav commented Feb 16, 2014

This allows the protocol option in hub config file to specify whether an Enterprise instance is using regular (non-secure) HTTP. The only way to set the option is by manually editing the config file. This is a temporary hack to allow people to use hub with non-secure Enterprise installs.

This affects: API requests, browse & compare commands. This doesn't affect: git protocol, i.e. git clone/fetch operations.

Example ~/.config/hub:

my.example.org:
- user: myuser
  oauth_token: mytoken
  protocol: http

Alternative to #247 #485. Advantages of the approach taken here:

  • Minimal diff. As few changes to GitHubAPI as possible
  • protocol instead of uri_scheme as a more intention-revealing name
  • More solid tests

/cc @msabramo @mmrobins

mislav added some commits Feb 16, 2014

Validate that HTTPS is used by default for API requests
Since we use a test server representing the GitHub API, all requests get
transformed to HTTP in tests. Make a check that the original requests
really was intended to go over HTTPS.
Support Enterprise over HTTP via `protocol` config option
Example `~/.config/hub`:

  my.example.org:
  - user: myuser
    oauth_token: mytoken
    protocol: http

Currently there is no way to set `protocol` other than manually editing the
configuration file.
Make `browse` and `compare` respect `protocol` config option
Allows using these commands for Enterprise configurations over regular HTTP.
@mmrobins

This comment has been minimized.

mmrobins commented Feb 17, 2014

Works for me

@mislav

This comment has been minimized.

Member

mislav commented Feb 17, 2014

@jingweno This is something that I'm adding as a temp solution for people who really need to use hub for their Enterprise instances over HTTP. (Not sure why would anyone have Enterprise over HTTP, though).

I'd like to make the user experience around this a little better in the next big version of hub. Maybe something like a dedicated command to whitelist + authenticate to an Enteprise host:

hub auth http://my.git.org
@msabramo

This comment has been minimized.

Contributor

msabramo commented Feb 17, 2014

My company has GHE over HTTP.

I guess they just never bothered to get or make an SSL cert and figured it was okay because it's behind our firewall.

I probably should pester our DevOps guy again to make a self-signed cert, but this is my stop-gap solution.

@msabramo

This comment has been minimized.

Contributor

msabramo commented Feb 22, 2014

Just tested this out and it works for me.

One comment/question:

protocol instead of uri_scheme as a more intention-revealing name

Why do you think protocol is a more intention-revealing name? My thought is that protocol sounds more generic -- it could be specifying whether to use HTTP or SSH for git operations for example, whereas uri_scheme sounds more specific -- like it's referring specifically to the part before the "://" in an HTTP URL. But probably since you picked protocol, you probably are seeing it from a different angle that I'm not seeing.

Just curious. Right now uri_scheme seems more intuitive the way I'm thinking about it, but if you think protocol is better, that's fine too and the important thing is that I tried out what you have here and it works and I'd love to have some kind of support for accessing GHE over HTTP. And it looks like we're almost there, so that's awesome.

Thanks for your work on this!

@mislav

This comment has been minimized.

Member

mislav commented Feb 24, 2014

Why do you think protocol is a more intention-revealing name?

  1. A "scheme" is only a part of a URI. While technically we change schemes for different protocols, our intent wasn't to “switch to another URI scheme”. Our intent is to “opt-in to an insecure protocol”. Changing the scheme is the means, an implementation detail—not the goal.
  2. Neither users nor I will be sure in 3 weeks whether it was uri_scheme or url_scheme.
  3. Maps closer to how we think about it/speak. You won't ask your colleague, “hey, which URI scheme have we set up our Enterprise instance on?”. You will ask “have we set up GitHub Enterprise over secure protocol?”
@msabramo

This comment has been minimized.

Contributor

msabramo commented Feb 24, 2014

Cool. Thanks for the explanation! This looks good to me.

mislav added a commit that referenced this pull request Feb 24, 2014

Merge pull request #488 from github/enterprise-http
Allow Enterprise configured for regular HTTP

@mislav mislav merged commit b78dd75 into master Feb 24, 2014

1 check passed

default The Travis CI build passed
Details

@mislav mislav deleted the enterprise-http branch Feb 24, 2014

@msabramo

This comment has been minimized.

Contributor

msabramo commented Feb 24, 2014

Nice! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment