Skip to content
This repository

Attempt user/pass authentication for git repos not on github #1162

Merged
merged 4 commits into from about 1 year ago

9 participants

Mark Achée Jordi Boggiano Chris Smith Till! James Bench Edi Modrić Benjamin Lévêque Mike Brown Johan Lopes
Mark Achée

If git clone fails because of authentication, the repo isn't on github, and the url has http or https scheme composer should attempt http basic authentication.

Also, ask if the user would like to reuse the user/pass for repos at the same scheme://domain

Jordi Boggiano
Owner

This looks good but could you also fix https://github.com/machee/composer/blob/9d03dc5a899a1e6f4c1215034a509c681f4aa023/src/Composer/Downloader/GitDownloader.php#L54 so that it detects the user/pass at the next run and doesn't prompt every time it needs an update?

Jordi Boggiano
Owner

Also I'd say the authorization storage calls should only use $match[2] or the hostname instead of including the scheme.

Mark Achée

Added the GIT_ASKPASS env var because git was picking up on the need for user/pass during updates.

I added "origin" to the regex searching for user/pass in git remote because at one point when I was testing, the user and pass weren't present for the "composer" remote. Later, user/pass was present for both and I couldn't reproduce the original output.

I ran into the need for urlencode() because I happened to have a special character in my password. It should probably be added for the previous github section?

That also led me to an issue where escapeshellarg removes the % on Windows. Not sure of a solution to that or how much of a concern it is.

Chris Smith

@Seldaek That means a credential saved for a secure channel could be unintentionally transmitted over an insecure channel.

Mark Achée

@cs278 I suppose that's basically what I had in mind when originally writing it that way. Given the hubbub in the MITM PR #1092 I'm definitely for writing it with security in mind.

Should I warn the user when a repo is insecure and confirm they'd like to continue? Seems to maintain usability while encouraging security.

Also, I was surprised to see credentials stored in the git remotes. I'm not sure of a way to prevent that while running git non-interactively, but it seems like there should be an option if possible.

Jordi Boggiano
Owner

@machee @cs278 true it might be safer to just save it including the scheme, though I'd guess if one same host has both http & https available, security conscious people would only use the safer option. Anyway I'm fine with reverting that change to how it originally was.

As for storing the credentials in the remote, yeah it's "ugly", but it's that or prompting at every update/install which sucks too. I'm really not sure what's best.. We could encrypt it in a global cache so you would just have to type your password once to unlock it, but that still sucks a bit if you update frequently.

@machee Regarding escapeshellarg removing %, no idea how to fix that.

Till!

Couldn't you store that stuff in $COMPOSER_HOME?

I think if it's not readable by anyone but the current $USER then this is fine in terms of convenience and security. People should be aware of what they do but e.g. I bet most people using git, cache the passphrase anyway and don't type it in each time.

When you start encrypting stuff you likely create a circle of events where you need another password to decrypt the passwords that have been encrypted.

At least it sounds like there is no gain — it just looks more obscure.

Jordi Boggiano
Owner

@till agreed. I would say either store in the config like the github oauth token, or in another file.. either way this stuff should be chmod'ed to 600.

James Bench

Any idea when this will be merged into a release?

Edi Modrić

I would also like to see this merged as we use bitbucket for our private repos.

Mike Brown

This would be VERY handy

Jordi Boggiano Seldaek merged commit 5ed5f13 into from March 10, 2013
Jordi Boggiano Seldaek closed this March 10, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 35 additions and 3 deletions. Show diff stats Hide diff stats

  1. 38  src/Composer/Downloader/GitDownloader.php
38  src/Composer/Downloader/GitDownloader.php
@@ -49,12 +49,14 @@ public function doUpdate(PackageInterface $initial, PackageInterface $target, $p
49 49
         $this->io->write("    Checking out ".$ref);
50 50
         $command = 'cd %s && git remote set-url composer %s && git fetch composer && git fetch --tags composer';
51 51
 
52  
-        // capture username/password from github URL if there is one
  52
+        // capture username/password from URL if there is one
53 53
         $this->process->execute(sprintf('cd %s && git remote -v', escapeshellarg($path)), $output);
54  
-        if (preg_match('{^composer\s+https://(.+):(.+)@github.com/}im', $output, $match)) {
55  
-            $this->io->setAuthorization('github.com', $match[1], $match[2]);
  54
+        if (preg_match('{^(?:composer|origin)\s+https?://(.+):(.+)@([^/]+)/}im', $output, $match)) {
  55
+            $this->io->setAuthorization($match[3], urldecode($match[1]), urldecode($match[2]));
56 56
         }
57 57
 
  58
+        // added in git 1.7.1, prevents prompting the user
  59
+        putenv('GIT_ASKPASS=echo');
58 60
         $commandCallable = function($url) use ($ref, $path, $command) {
59 61
             return sprintf($command, escapeshellarg($path), escapeshellarg($url), escapeshellarg($ref));
60 62
         };
@@ -199,6 +201,36 @@ protected function runCommand($commandCallable, $url, $path = null)
199 201
                     }
200 202
                     $retrying = true;
201 203
                 } while (--$retries);
  204
+            } elseif (
  205
+                $this->io->isInteractive() &&
  206
+                preg_match('{(https?://)([^/]+)(.*)$}i', $url, $match) &&
  207
+                strpos($this->process->getErrorOutput(), 'fatal: Authentication failed') !== false
  208
+            ) {
  209
+                if ($saved = $this->io->hasAuthorization($match[2])) {
  210
+                    $auth = $this->io->getAuthorization($match[2]);
  211
+                } else {
  212
+                    $this->io->write($match[1].$match[2].$match[3].' requires Authentication');
  213
+                    $auth = array(
  214
+                        'username'  => $this->io->ask('Username: '),
  215
+                        'password'  => $this->io->askAndHideAnswer('Password: '),
  216
+                    );
  217
+                }
  218
+
  219
+                $url = $match[1].urlencode($auth['username']).':'.
  220
+                    urlencode($auth['password']).'@'.$match[2].$match[3];
  221
+
  222
+                $command = call_user_func($commandCallable, $url);
  223
+                if (0 === $this->process->execute($command, $handler)) {
  224
+                    if (!$saved) {
  225
+                        $saved = $this->io->ask('Save user/pass for other requests to '.
  226
+                            $match[2].' ? [y]/n: ');
  227
+                        if (in_array($saved, array('y', 'Y', null), true)) {
  228
+                            $this->io->setAuthorization($match[2], $auth['username'], $auth['password']);
  229
+                            $this->io->write('saved...');
  230
+                        }
  231
+                    }
  232
+                    return;
  233
+                }
202 234
             }
203 235
 
204 236
             if (null !== $path) {
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.