Authentification error

[willoucom]

Hello,

I found a bug in the authentification, it may be complicated to reproduce, but i will try to provides as many details as possible.

I am using this package in a php-slim4 application, the application is hosted on AWS with an ALB (application load balancer) in a very standard format. For strange reasons, the Authorization header is duplicated and $request->getHeaderLine('Authorization') return Basic cm9vdDp0MDBy,Basic cm9vdDp0MDBy.

I use a Psr\Http\Message\MessageInterface and the getHeaderLine() description state: Retrieves a comma-separated string of the values for a single header.

Here is the function in slim/psr7/message.php

    public function getHeaderLine($name): string
    {
        $values = $this->headers->getHeader($name);
        return implode(',', $values);
    }

In this (rare) case, the following code doesn't match anything and the authorization fail

        if (preg_match("/Basic\s+(.*)$/i", $request->getHeaderLine("Authorization"), $matches)) {
            $explodedCredential = explode(":", base64_decode($matches[1]), 2);
            if (count($explodedCredential) == 2) {
                list($params["user"], $params["password"]) = $explodedCredential;
            }
        }

I believe i can find a workaround to prevent the duplication of the header, however i think the authorization in the package can be broken because the getHeaderLine can return a coma separated string instead of the header.

The solution can be: explode(",", getHeaderLine('Authorization')) and loop through the result

I have made some research and it seems the RFC (https://tools.ietf.org/html/rfc7235#appendix-C) permit multiple entries in the authorization header, separated by a coma.

If the problem/solution seems relevant, i can make a PR

[tuupola]

If comma separated authorization is allowed by RFC I agree it should be fixed. If not I think it is not this middlewares responsibility to fix broken requests. Need to read about this a bit but for example rfc2616 says:

https://www.ietf.org/rfc/rfc2616.txt

"Multiple message-header fields with the same field-name MAY be present in a message if and only if the entire field-value for that header field is defined as a comma-separated list [i.e., #(values)]. It MUST be possible to combine the multiple header fields into one "field-name: field-value" pair, without changing the semantics of the message, by appending each subsequent field-value to the first, each separated by a comma."

[tuupola]

Apparently also author of the spec says it is not valid to specify multiple Authorization fields.

"You can only use multiple header fields when they are defined using list syntax."

I see two options. Basic Auth middleware could use getHeader() instead and use the first result in the array. Other option is to add an ad hoc middleware which removes the extra headers before passing the request to Basic Auth middleware.

[torgie]

I also encountered this bug, but in the context of an nginx reverse proxy Docker container in front of my Slim app.

I should note, however, that the bug might be in the $request->getHeaderLine("Authorization") call. If I call PHP's native getallheaders() function, the "authorization" field appears correctly. Immediately afterward, if I call $request->getHeaderLine("Authorization"), I get the repeated comma-separated version.

echo getallheaders()["authorization"];
// Basic a1b2c3
echo $request->getHeaderLine("Authorization");
// Basic a1b2c3,Basic a1b2c3

A simple and reasonable fix for this is to constrain the expression in the preg_match call on line 155 to base64 characters:

if (preg_match("|^Basic\s+([A-Za-z0-9+/]+={0,2})|i", $request->getHeaderLine("Authorization"), $matches)) {

This change swaps out the "lazy" .* match with explicit base64 characters. It also favors the left side of the string with ^ instead of $. Grabbing non-base64 characters with .* would make the decode fail, anyway.

I can make a PR if this looks acceptable to you.

[tuupola]

It seems to be an issue with slim/psr-7. In the meanwhile possible workaround is:

$ composer remove slim/psr7
$ composer require nyholm/psr7
$ composer require nyholm/psr7-server