laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.32k stars 10.95k forks source link

Routes Provided by Package Do Not Load When Middleware Applied #50992

Closed onairmarc closed 5 months ago

onairmarc commented 6 months ago

Laravel Version

11.3

PHP Version

8.3.3

Database Driver & Version

No response

Description

Routes do not load properly when a middleware is applied in a package.

PackageServiceProvider (package_root/src/Providers/PackageServiceProvider.php)

<?php

namespace Example\Package\Providers;

use Illuminate\Support\ServiceProvider;

class ExamplePackageProvider extends ServiceProvider
{
    public function register(): void
    {
    }

    public function boot(): void
    {

        $this->loadRoutesFrom(__DIR__ . '/../../routes/web.php');

        $this->loadMigrationsFrom(__DIR__ . '/../../database/migrations');

        $this->loadViewsFrom(__DIR__ . '/../../resources/views', 'eppLogin');
    }
}

Web Routes (package_root/routes/web.php)

<?php

use Example\Package\Http\Controllers\DemoController;

Route::middleware('auth')->group(function () {
    Route::get('/dummy_package_route, [DemoController::class, 'index'])
        ->name('dummy_package_route_name.index');
});

When the routes are registered with middleware via a package, the auth middleware redirects the request to AppServiceProvider::HOME, but when the exact same routes are copied and pasted into routes/web.php in the main application, the routes function as expected. Removing the middleware when registering the routes in the package allows the routes to function properly, except for the fact that the middleware is no longer applied.

Steps To Reproduce

  1. Install a package that registers a route wrapped in the auth middleware via the package. (a simple dummy or symlinked/composer path package will work. (that's what I have))
  2. Attempt to visit that route, you will be redirected to AppServiceProvider::HOME
  3. Comment out the routes registered via the package and place the same routes with the middleware in routes/web.php. The route works as expected.
driesvints commented 6 months ago

Try applying the middleware through a route group within the service provider:

        Route::group([
            'middleware' => 'auth',
        ], function () {
            $this->loadRoutesFrom(__DIR__ . '/../../routes/web.php');
        });

Does that help?

onairmarc commented 6 months ago

I'm assuming the Route is Illuminate\Support\Facades\Route correct? I just tried this, and unfortunately, it did not work. The outcome did not change.

public function boot(): void
    {

        Route::group([
            'middleware' => 'auth',
        ], function () {
            $this->loadRoutesFrom(__DIR__ . '/../../routes/web.php');
        });

        $this->loadMigrationsFrom(__DIR__ . '/../../database/migrations');

        $this->loadViewsFrom(__DIR__ . '/../../resources/views', 'eppLogin');
    }
driesvints commented 6 months ago

Yeah, the facade. Hmm this is a weird one. Would appreciate some external help here if anyone has any insights.

github-actions[bot] commented 6 months ago

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

onairmarc commented 6 months ago

Yeah, the facade. Hmm this is a weird one. Would appreciate some external help here if anyone has any insights.

I'll try to take a deeper dive into this sometime this week. Might be able to submit a PR if I find something. I will say my skill may not be up to par for Laravel Core however. 🤓

onairmarc commented 6 months ago

@driesvints I dove a bit deeper into this and determined where the redirect to AppServiceProvider::HOME is occurring. I was able to confirm that the redirect occurs in the authenticate method. Below are my inline code comments.

Illuminate\Auth\Middleware\Authenticate

protected function authenticate($request, array $guards)
    {
        //dd('Authenticate Middleware: Authenticate Method');
        if (empty($guards)) {
            //dd('Authenticate Middleware: Authenticate Method: Empty Guards');
            $guards = [null];
        }

        foreach ($guards as $guard) {
            if ($this->auth->guard($guard)->check()) {
                //dd('Authenticate Middleware: Authenticate Method: Guard Check Passed - Should Use Guard ' . $guard); // $gaurd value is null
                return $this->auth->shouldUse($guard); // Once returned, the request redirects to AppServiceProvider::HOME
            }
        }

        $this->unauthenticated($request, $guards); // Never executed.
    }

I am not familiar with factories, so I'm unsure of what the next step should be in locating the issue. I do know that by the time Line 62 finishes executing, the redirect to AppServiceProvider::HOME has already completed. I verified this by adding dd($request) on Line 63.

driesvints commented 6 months ago

TBH: can't you just remove the HOME constant? We don't ship it any longer by default in a Laravel v11 app.

onairmarc commented 6 months ago

No such luck. Same outcome.

The only thing I've done that makes this work is commenting out the route registration in the package and adding require base_path('app_modules/_EPP/epp-login/routes/web.php'); at the bottom of routes/web.php.

It's a bit of a hacky solution, but it does allow the route to work with the middleware properly. I don't think this is the right solution, however; it feels like it's breaking a lot of Laravel rules.

sxtnmedia commented 5 months ago

@onairmarc Are you sure, it's v11?

I tried as you described and cannot reproduce the bug. Furthermore, the HOME constant is not present anywhere in the fresh install (with vendors ofc).

I could try to help you with this, but if you could provide a sample repository with a reproducible error, that would be very helpful.

onairmarc commented 5 months ago

@sxtnmedia Yes, I just double checked and laravel/framework is set to ^11.0. Looks like we're currently running v11.3.1

I did comment out AppServiceProvider::HOME and replaced it with /dashboard Here is the current bootstrap/app.php file:

<?php

use Illuminate\Auth\Middleware\Authenticate;
use Illuminate\Auth\Middleware\RedirectIfAuthenticated;
use Illuminate\Foundation\Application;
use Illuminate\Foundation\Configuration\Exceptions;
use Illuminate\Foundation\Configuration\Middleware;

return Application::configure(basePath: dirname(__DIR__))
    ->withProviders()
    ->withRouting(
        web: base_path('routes/web.php'),
        api: base_path('routes/api.php'),
        commands: base_path('routes/console.php'),
        // channels: __DIR__.'/../routes/channels.php',
        health: '/up',
    )
    ->withMiddleware(function (Middleware $middleware) {
        $middleware->redirectGuestsTo(fn() => route('login'));
        $middleware->redirectUsersTo('/dashboard');

        $middleware->throttleApi();

        $middleware->alias([
            'auth' => Authenticate::class,
            'guest' => RedirectIfAuthenticated::class,
        ]);
    })
    ->withExceptions(function (Exceptions $exceptions) {
        //
    })->create();
driesvints commented 5 months ago

Heya, thanks for reporting.

We'll need more info and/or code to debug this further. Can you please create a repository with the command below, commit the code that reproduces the issue as one separate commit on the main/master branch and share the repository here? Please make sure that you have the latest version of the Laravel installer in order to run this command. Please also make sure you have both Git & the GitHub CLI tool properly set up.

laravel new bug-report --github="--public"

Please do not amend and create a separate commit with your custom changes. After you've posted the repository, we'll try to reproduce the issue.

Thanks!

onairmarc commented 5 months ago

@driesvints Sure thing! I'll try to get this done this afternoon.

milwad-dev commented 5 months ago

You can load routes like this:

Route::middleware($this->middlewareRoute)
    ->namespace($this->namespace)
    ->group(__DIR__.$this->routePath);
onairmarc commented 5 months ago

@driesvints Here is the link to the repo: https://github.com/onairmarc/package-routing-issue

Reproduction Steps:

  1. Run composer update after cloning the repo.
  2. Run the migrations as this repo is using SQLite.
  3. Run php artisan serve
  4. Register an account
  5. Visit /bug-reports (This is where it should redirect you to /dashboard as a result of the bug)
  6. Uncomment out require base_path('app_modules/bug-reports/routes/web.php'); in routes/web.php
  7. Visit /bug-reports (You should now see that the route is working, although not through the conventional means.

Looking forward to your feedback.

onairmarc commented 5 months ago

You can load routes like this:

Route::middleware($this->middlewareRoute)
    ->namespace($this->namespace)
    ->group(__DIR__.$this->routePath);

@milwad-dev I am not familiar with this method of loading routes via packages. Can you please elaborate?

driesvints commented 5 months ago

I managed to recreate it through your repo. I have no idea what's going on but your code looks like it should work to me. I do not know why you get redirected instead of the route being shown especially since you're already logged in. Would greatly appreciate some additional help/insights with this one.

onairmarc commented 5 months ago

Yay! I'm not completely crazy! 😂 I'm happy to help where I can, but I'm unsure where to begin looking for a solution. As I continue my research, I'll post any relevant findings here.

driesvints commented 5 months ago

I think the best point to start debugging is in the auth middleware to see what the state is when you hit the package route. Obviously the middleware thinks you're not logged in at that point.

onairmarc commented 5 months ago

I've added this to my to-do list. Should have some time this weekend to do a deep dive.

taylorotwell commented 5 months ago

Your package route is never placed in the web middleware group, so sessions are never started.

Route::middleware(['web', 'auth'])->get('/package', function () {
    return 'Package route...';
});
driesvints commented 5 months ago

Urgh, bashing myself for missing this obvious one 😅