Merge pull request #91 from alexandre-daubois/auth-header-process

Better authorization header handling in `AddAuthenticationHeader`

Author: asgrim

src/Downloading/AddAuthenticationHeader.php

@@ -9,10 +9,11 @@
 use Psr\Http\Message\RequestInterface;
 use RuntimeException;
 
+use function array_map;
 use function array_walk;
+use function count;
 use function explode;
 use function sprintf;
-use function trim;
 
 /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */
 final class AddAuthenticationHeader
@@ -27,9 +28,13 @@ public static function withAuthHeaderFromComposer(RequestInterface $request, Pac
         array_walk(
             $authHeaders,
             static function (string $v) use (&$request): void {
-                // @todo probably process this better
-                $headerParts = explode(':', $v);
-                $request     = $request->withHeader(trim($headerParts[0]), trim($headerParts[1]));
+                $headerParts = array_map('trim', explode(':', $v, 2));
+
+                if (count($headerParts) !== 2 || ! $headerParts[0] || ! $headerParts[1]) {
+                    throw new RuntimeException('Authorization header is malformed, it should contain a non-empty key and a non-empty value.');
+                }
+
+                $request = $request->withHeader($headerParts[0], $headerParts[1]);
             },
         );
 

test/unit/Downloading/AddAuthenticationHeaderTest.php

@@ -5,12 +5,14 @@
 namespace Php\PieUnitTest\Downloading;
 
 use Composer\Util\AuthHelper;
+use Generator;
 use GuzzleHttp\Psr7\Request;
 use Php\Pie\DependencyResolver\Package;
 use Php\Pie\Downloading\AddAuthenticationHeader;
 use Php\Pie\ExtensionName;
 use Php\Pie\ExtensionType;
 use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 use RuntimeException;
 
@@ -33,24 +35,48 @@ public function testAuthorizationHeaderIsAdded(): void
 
         $requestWithAuthHeader = (new AddAuthenticationHeader())->withAuthHeaderFromComposer(
             $request,
-            new Package(
-                ExtensionType::PhpModule,
-                ExtensionName::normaliseFromString('foo'),
-                'foo/bar',
-                '1.2.3',
-                $downloadUrl,
-                [],
-                null,
-                '1.2.3.0',
-                true,
-                true,
-            ),
+            $this->createDummyPackage($downloadUrl),
             $authHelper,
         );
 
         self::assertSame('whatever ABC123', $requestWithAuthHeader->getHeaderLine('Authorization'));
     }
 
+    #[DataProvider('provideInvalidAuthorizationHeaders')]
+    public function testEmptyValueInAuthorizationHeaderThrowsException(string $rawHeader): void
+    {
+        $downloadUrl = 'http://test-uri/' . uniqid('path', true);
+
+        $request = new Request('GET', $downloadUrl);
+
+        $authHelper = $this->createMock(AuthHelper::class);
+        $authHelper->expects(self::once())
+            ->method('addAuthenticationHeader')
+            ->with([], 'github.com', $downloadUrl)
+            ->willReturn([$rawHeader]);
+
+        $addAuthenticationHeader = new AddAuthenticationHeader();
+
+        $this->expectException(RuntimeException::class);
+        $this->expectExceptionMessage('Authorization header is malformed, it should contain a non-empty key and a non-empty value.');
+        $addAuthenticationHeader->withAuthHeaderFromComposer($request, $this->createDummyPackage($downloadUrl), $authHelper);
+    }
+
+    /**
+     * @return Generator<string[]>
+     *
+     * @psalm-suppress PossiblyUnusedMethod https://github.com/psalm/psalm-plugin-phpunit/issues/131
+     */
+    public static function provideInvalidAuthorizationHeaders(): Generator
+    {
+        yield ['Authorization:'];
+        yield [': Bearer'];
+        yield [' : Bearer'];
+        yield ['Authorization: '];
+        yield [':'];
+        yield ['Authorization MyToken'];
+    }
+
     public function testExceptionIsThrownWhenPackageDoesNotHaveDownloadUrl(): void
     {
         $downloadUrl = 'http://test-uri/' . uniqid('path', true);
@@ -60,21 +86,26 @@ public function testExceptionIsThrownWhenPackageDoesNotHaveDownloadUrl(): void
         $authHelper = $this->createMock(AuthHelper::class);
 
         $addAuthenticationHeader = new AddAuthenticationHeader();
-        $package                 = new Package(
+        $package                 = $this->createDummyPackage();
+
+        $this->expectException(RuntimeException::class);
+        $this->expectExceptionMessage('The package foo/bar does not have a download URL');
+        $addAuthenticationHeader->withAuthHeaderFromComposer($request, $package, $authHelper);
+    }
+
+    private function createDummyPackage(string|null $downloadUrl = null): Package
+    {
+        return new Package(
             ExtensionType::PhpModule,
             ExtensionName::normaliseFromString('foo'),
             'foo/bar',
             '1.2.3',
-            null,
+            $downloadUrl,
             [],
             null,
             '1.2.3.0',
             true,
             true,
         );
-
-        $this->expectException(RuntimeException::class);
-        $this->expectExceptionMessage('The package foo/bar does not have a download URL');
-        $addAuthenticationHeader->withAuthHeaderFromComposer($request, $package, $authHelper);
     }
 }