diff --git a/lib/git.js b/lib/git.js index b9665b5f..406ab5c6 100644 --- a/lib/git.js +++ b/lib/git.js @@ -24,13 +24,16 @@ const _cloneRepo = Symbol('_cloneRepo') const _setResolvedWithSha = Symbol('_setResolvedWithSha') const _prepareDir = Symbol('_prepareDir') -// get the repository url. prefer ssh, fall back to git:// +// get the repository url. +// prefer https if there's auth, since ssh will drop that. +// otherwise, prefer ssh if available (more secure). // We have to add the git+ back because npa suppresses it. -const repoUrl = (hosted, opts) => - hosted.sshurl && addGitPlus(hosted.sshurl(opts)) || - hosted.https && addGitPlus(hosted.https(opts)) +const repoUrl = (h, opts) => + h.sshurl && !(h.https && h.auth) && addGitPlus(h.sshurl(opts)) || + h.https && addGitPlus(h.https(opts)) -const addGitPlus = url => url && `git+${url}` +// add git+ to the url, but only one time. +const addGitPlus = url => url && `git+${url}`.replace(/^(git\+)+/, 'git+') class GitFetcher extends Fetcher { constructor (spec, opts) { @@ -51,6 +54,11 @@ class GitFetcher extends Fetcher { this.resolvedSha = '' } + // just exposed to make it easier to test all the combinations + static repoUrl (hosted, opts) { + return repoUrl(hosted, opts) + } + get types () { return ['git'] } @@ -69,13 +77,16 @@ class GitFetcher extends Fetcher { } // first try https, since that's faster and passphrase-less for - // public repos. Fall back to SSH to support private repos. - // NB: we always store the SSH url in the 'resolved' field. + // public repos, and supports private repos when auth is provided. + // Fall back to SSH to support private repos + // NB: we always store the https url in resolved field if auth + // is present, otherwise ssh if the hosted type provides it [_resolvedFromHosted] (hosted) { return this[_resolvedFromRepo](hosted.https && hosted.https()) .catch(er => { const ssh = hosted.sshurl && hosted.sshurl() - if (!ssh) + // no fallthrough if we can't fall through or have https auth + if (!ssh || hosted.auth) throw er return this[_resolvedFromRepo](ssh) }) @@ -121,9 +132,11 @@ class GitFetcher extends Fetcher { // either a git url with a hash, or a tarball download URL [_addGitSha] (sha) { if (this.spec.hosted) { - this[_setResolvedWithSha]( - this.spec.hosted.shortcut({ noCommittish: true }) + '#' + sha - ) + const h = this.spec.hosted + const opt = { noCommittish: true } + const base = h.https && h.auth ? h.https(opt) : h.shortcut(opt) + + this[_setResolvedWithSha](`${base}#${sha}`) } else { const u = url.format(new url.URL(`#${sha}`, this.spec.rawSpec)) this[_setResolvedWithSha](url.format(u)) @@ -232,14 +245,19 @@ class GitFetcher extends Fetcher { }) } + // first try https, since that's faster and passphrase-less for + // public repos, and supports private repos when auth is provided. + // Fall back to SSH to support private repos + // NB: we always store the https url in resolved field if auth + // is present, otherwise ssh if the hosted type provides it [_cloneHosted] (ref, tmp) { const hosted = this.spec.hosted const https = hosted.https() return this[_cloneRepo](hosted.https({ noCommittish: true }), ref, tmp) .catch(er => { const ssh = hosted.sshurl && hosted.sshurl({ noCommittish: true }) - /* istanbul ignore if - should be covered by the resolve() call */ - if (!ssh) + // no fallthrough if we can't fall through or have https auth + if (!ssh || hosted.auth) throw er return this[_cloneRepo](ssh, ref, tmp) }) diff --git a/test/git.js b/test/git.js index cde1fc28..5019fcad 100644 --- a/test/git.js +++ b/test/git.js @@ -3,6 +3,7 @@ const httpPort = 18000 + (+process.env.TAP_CHILD_ID || 0) const hostedUrl = `http://localhost:${httpPort}` const ghi = require('hosted-git-info/git-host-info.js') const gitPort = 12345 + (+process.env.TAP_CHILD_ID || 0) + ghi.localhost = { protocols: [ 'git+https', 'git+ssh' ], domain: `127.0.0.1:${gitPort}`, @@ -16,6 +17,24 @@ ghi.localhost = { hashformat: h => h, protocols_re: /^(git):$/ } + +ghi.localhosthttps = { + protocols: [ 'git+https', 'git+ssh' ], + domain: `127.0.0.1`, + + httpstemplate: `git://127.0.0.1:${gitPort}/{user}{#committish}`, + sshtemplate: 'git://nope this should never be used', + sshurltemplate: 'git://nope this should never be used either', + + treepath: 'not-implemented', + // shortcut MUST have a user and project, at least + shortcuttemplate: '{type}:{user}/x{#committish}', + pathtemplate: '/{user}{#committish}', + pathmatch: /^\/(repo|submodule-repo|no-repo-here)/, + hashformat: h => h, + protocols_re: /^(git|git\+https):|$/ +} + ghi.localhostssh = { protocols: [ 'git+ssh' ], domain: `localhostssh:${gitPort}`, @@ -31,7 +50,6 @@ ghi.localhostssh = { protocols_re: /^(git):$/ } - const remote = `git://localhost:${gitPort}/repo` const remoteHosted = `git://127.0.0.1:${gitPort}/repo` const submodsRemote = `git://localhost:${gitPort}/submodule-repo` @@ -153,7 +171,7 @@ t.test('setup', { bail: true }, t => { '--verbose', '--informative-errors', '--reuseaddr', - `--base-path=.`, + '--base-path=.', '--listen=localhost', ], { cwd: me, stdio: ['pipe', 1, 'pipe' ] }) const onDaemonData = c => { @@ -398,6 +416,36 @@ t.test('extract from tarball from hosted git service', t => { })) }) +t.test('include auth with hosted https when provided', async t => { + const opt = {cache} + const spec = `git+https://user:pass@127.0.0.1/repo` + const g = new GitFetcher(spec, opt) + const resolved = await g.resolve() + // this weird url is because our fakey mock hosted service's + // https() method returns a git:// url, since the git daemon we + // spun up only responds to the git:// protocol, not https. + // But! we are verifying that it IS using the https() method, + // and not the sshurl() method, because that one returns a + // 'do not use this' string. + t.equal(resolved, `git+git://127.0.0.1:${gitPort}/repo#${REPO_HEAD}`) + t.equal(g.spec.hosted.shortcut(), 'localhosthttps:repo/x', + 'using the correct dummy hosted service') + + t.test('fail, but do not fall through to sshurl', async t => { + const spec = `git+https://user:pass@127.0.0.1/no-repo-here` + const failer = new GitFetcher(spec, { + cache, + resolved: resolved.replace(/\/repo/, '/no-repo-here'), + }) + t.equal(failer.spec.hosted.shortcut(), 'localhosthttps:no-repo-here/x', + 'using the correct dummy hosted service') + const path = t.testdir({}) + await t.rejects(failer.extract(path), { + args: [ 'ls-remote', `git://127.0.0.1:${gitPort}/no-repo-here` ], + }) + }) +}) + t.test('include .gitignore in hosted tarballs for preparation', async t => { const spec = npa(`localhost:foo/y#${REPO_HEAD}`) spec.hosted.tarball = () => @@ -465,3 +513,20 @@ t.test('fall back to ssh url if https url fails or is missing', t => { return gf.extract(`${me}/localhostssh`).then(({resolved}) => t.equal(resolved, `git+git://127.0.0.1:${gitPort}/repo#${REPO_HEAD}`)) }) + +t.test('repoUrl function', async t => { + const proj = 'isaacs/abbrev-js' + const { hosted: shortcut } = npa(`github:${proj}`) + const { hosted: hasAuth } = npa(`git+https://user:pass@github.com/${proj}`) + const { hosted: noAuth } = npa(`git+https://github.com/${proj}`) + const { hosted: ssh } = npa(`git+ssh://git@github.com/${proj}`) + const { hosted: git } = npa(`git://github.com/${proj}`) + const { repoUrl } = GitFetcher + const expectNoAuth = `git+ssh://git@github.com/${proj}` + const expectAuth = `git+https://user:pass@github.com/${proj}` + t.match(repoUrl(shortcut), expectNoAuth) + t.match(repoUrl(hasAuth), expectAuth) + t.match(repoUrl(noAuth), expectNoAuth) + t.match(repoUrl(ssh), expectNoAuth) + t.match(repoUrl(git), expectNoAuth) +})