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

Wrong intent is stored by Laravel because it ignores the proxies that are not direct clients and there is no fix. #83

Open
RobIsHere opened this issue Oct 18, 2022 · 3 comments

Comments

@RobIsHere
Copy link

RobIsHere commented Oct 18, 2022

Here https://github.com/mariovalney/laravel-keycloak-web-guard/blob/master/src/Middleware/KeycloakAuthenticated.php#L17 the redirection is done with the redirect method which does not preserve the intent. For preserving the intent the special redirect()->guest($target) method exists in laravel.

In my case the app always redirects to some "old" stored intent, which redirects the user to / while the whole app is located under /dashboard

See also accepted answer here:
https://stackoverflow.com/questions/51882925/laravel-redirect-intended-is-not-working-in-custom-login-controller

@RobIsHere RobIsHere changed the title Intent is not stored before redirection, so wrong intent is pulled after callback from keycloak Wrong intent is stored by Laravel because it ignores the proxies that are not direct clients and there is no fix. Oct 18, 2022
@RobIsHere
Copy link
Author

Looks like the intent is implicitly overwritten by Laravel calling the guest method internally but with the wrong url because it ignores the Proxy rewrites of downstream proxies.

So the intent is practically not usable.

An Opt-out from using this intent would be necessary here.

@RobIsHere
Copy link
Author

To get the intent right you have to set trusted proxy and headers. The necessary header 'HEADER_X_FORWARDED_PREFIX' is missing from Laravels internal getTrustedHeaderNames(), so it silently discards this header from trusted ones and it will not work. You have to overwrite the method and add the header like below to get the bug fixed.

<?php

namespace App\Http\Middleware;

use Illuminate\Http\Middleware\TrustProxies as Middleware;
use Illuminate\Http\Request;

class TrustProxies extends Middleware
{
    /**
     * The trusted proxies for this application.
     *
     * @var array<int, string>|string|null
     */
    protected $proxies = "*";

    /**
     * The headers that should be used to detect proxies.
     *
     * @var int
     */
    protected $headers =
        Request::HEADER_X_FORWARDED_FOR |
        Request::HEADER_X_FORWARDED_HOST |
        Request::HEADER_X_FORWARDED_PORT |
        Request::HEADER_X_FORWARDED_PROTO |
        Request::HEADER_X_FORWARDED_PREFIX;

    /**
     * Retrieve trusted header name(s), falling back to defaults if config not set.
     *
     * @return int A bit field of Request::HEADER_*, to set which headers to trust from your proxies.
     */
    protected function getTrustedHeaderNames()
    {
        switch ($this->headers) {
            case 'HEADER_X_FORWARDED_AWS_ELB':
            case Request::HEADER_X_FORWARDED_AWS_ELB:
                return Request::HEADER_X_FORWARDED_AWS_ELB;

            case 'HEADER_FORWARDED':
            case Request::HEADER_FORWARDED:
                return Request::HEADER_FORWARDED;

            case 'HEADER_X_FORWARDED_FOR':
            case Request::HEADER_X_FORWARDED_FOR:
                return Request::HEADER_X_FORWARDED_FOR;

            case 'HEADER_X_FORWARDED_HOST':
            case Request::HEADER_X_FORWARDED_HOST:
                return Request::HEADER_X_FORWARDED_HOST;

            case 'HEADER_X_FORWARDED_PORT':
            case Request::HEADER_X_FORWARDED_PORT:
                return Request::HEADER_X_FORWARDED_PORT;

            case 'HEADER_X_FORWARDED_PROTO':
            case Request::HEADER_X_FORWARDED_PROTO:
                return Request::HEADER_X_FORWARDED_PROTO;

            case 'HEADER_X_FORWARDED_PREFIX':
            case Request::HEADER_X_FORWARDED_PREFIX:
                return Request::HEADER_X_FORWARDED_PREFIX;

            default:
                return Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_HOST | Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO | Request::HEADER_X_FORWARDED_AWS_ELB | Request::HEADER_X_FORWARDED_PREFIX;
        }
    }

}

@RobIsHere
Copy link
Author

laravel/framework#44633

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

No branches or pull requests

1 participant