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.

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

Not throwing exception for missing header with custom name #156

Closed
wallacio opened this issue Feb 26, 2019 · 2 comments
Closed

Not throwing exception for missing header with custom name #156

wallacio opened this issue Feb 26, 2019 · 2 comments
Assignees

Comments

@wallacio
Copy link

I'm using a custom header name, and I've set middleware options accordingly. I'd like the middleware to accept a token sent in a cookie if there's no header present, so I've defined a cookie with a custom name to check too.

        $app->add(new JwtAuthentication([
            "header"    => "X-OCA",
            "cookie"    => "X_OCA",
            "regexp"    => "/(.*)/"
        ]...

I've discovered that the cookie is never checked for a token if a header with a custom name isn't present. This block of code in fetchToken() in src/JwtAuthentication always returns true, even if the header isn't found when the regexp is also "/(.*)/"

        /* Check for token in header. */
        $headers = $request->getHeader($this->options["header"]);
        $header = isset($headers[0]) ? $headers[0] : "";
        if (preg_match($this->options["regexp"], $header, $matches)) {
            $this->log(LogLevel::DEBUG, $message);
            return $matches[1];
        }

Would it be more appropriate to check if there's anything in $header before even attempting a preg_match? Such a fix works great for my particular case:

        if ($header !== "" && preg_match($this->options["regexp"], $header, $matches)) {
            $this->log(LogLevel::DEBUG, $message);
            return $matches[1];
        }
@tuupola
Copy link
Owner

tuupola commented Feb 26, 2019

Seems you are correct. If the header does not exist the middleware should continue directly to checking if the cookie exists. Will fix.

@wallacio
Copy link
Author

wallacio commented Mar 1, 2019

Appreciated - thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants