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

ensure auth0 middleware is prioritized to load before others #419

Closed
wants to merge 1 commit into from

Conversation

squpshift
Copy link

Changes

Ensure auth0 middleware is prioritized to load before other middleware that expect auth to have been completed, such as middleware for Laravel Inetrtia, Laravel Nova.

References

Fixes #414

@squpshift squpshift requested a review from a team as a code owner June 25, 2023 23:32
@squpshift squpshift mentioned this pull request Jun 25, 2023
6 tasks
@evansims
Copy link
Member

Hi @squpshift 👋 Thanks for your suggestion.

This is not an acceptable change because if you review the middleware priorities in the app/Http/Kernel.php file:

protected $middlewarePriority = [
    \Illuminate\Foundation\Http\Middleware\HandlePrecognitiveRequests::class,
    \Illuminate\Cookie\Middleware\EncryptCookies::class,
    \Illuminate\Session\Middleware\StartSession::class,
    \Illuminate\View\Middleware\ShareErrorsFromSession::class,
    \Illuminate\Contracts\Auth\Middleware\AuthenticatesRequests::class,
    \Illuminate\Routing\Middleware\ThrottleRequests::class,
    \Illuminate\Routing\Middleware\ThrottleRequestsWithRedis::class,
    \Illuminate\Contracts\Session\Middleware\AuthenticatesSessions::class,
    \Illuminate\Routing\Middleware\SubstituteBindings::class,
    \Illuminate\Auth\Middleware\Authorize::class,
];

And evaluate this against the default middleware groups themselves:

protected $middlewareGroups = [
    'web' => [
        \App\Http\Middleware\EncryptCookies::class,
        \Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse::class,
        \Illuminate\Session\Middleware\StartSession::class,
        \Illuminate\View\Middleware\ShareErrorsFromSession::class,
        \App\Http\Middleware\VerifyCsrfToken::class,
        \Illuminate\Routing\Middleware\SubstituteBindings::class,
    ],
 
    'api' => [
        \Illuminate\Routing\Middleware\ThrottleRequests::class.':api',
        \Illuminate\Routing\Middleware\SubstituteBindings::class,
    ],
];

For web, this means AuthenticatorMiddleware would run before sessions are established, CSRF tokens are verified, and cookies are decrypted — this would be problematic for obvious reasons.

For api, this means AuthorizerMiddleware would run before network requests are checked for throttling, which opens applications up to denial of service attacks.

This is why this behavior is not implemented in the SDK in the first place.

Thank you for your suggestion, but we'll need to find alternative ways of addressing this.

@evansims evansims closed this Jun 26, 2023
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

Successfully merging this pull request may close these issues.

Incorrect middleware ordering
2 participants