Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Preserve git+https auth when provided #61

Closed
wants to merge 1 commit into from
Closed

Preserve git+https auth when provided #61

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 31 additions & 13 deletions lib/git.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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']
}
Expand All @@ -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)
})
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)
})
Expand Down
69 changes: 67 additions & 2 deletions test/git.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}`,
Expand All @@ -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}`,
Expand All @@ -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`
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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:[email protected]/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:[email protected]/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 = () =>
Expand Down Expand Up @@ -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:[email protected]/${proj}`)
const { hosted: noAuth } = npa(`git+https://github.com/${proj}`)
const { hosted: ssh } = npa(`git+ssh://[email protected]/${proj}`)
const { hosted: git } = npa(`git://github.com/${proj}`)
const { repoUrl } = GitFetcher
const expectNoAuth = `git+ssh://[email protected]/${proj}`
const expectAuth = `git+https://user:[email protected]/${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)
})