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

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

Closed
onairmarc opened this issue Apr 10, 2024 · 21 comments
Closed

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

onairmarc opened this issue Apr 10, 2024 · 21 comments

Comments

@onairmarc
Copy link

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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

Copy link

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
Copy link
Author

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
Copy link
Author

@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
Copy link
Member

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

@onairmarc
Copy link
Author

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
Copy link

@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
Copy link
Author

@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
Copy link
Member

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
Copy link
Author

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

@milwad-dev
Copy link
Contributor

milwad-dev commented Apr 17, 2024

You can load routes like this:

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

@onairmarc
Copy link
Author

@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
Copy link
Author

onairmarc commented Apr 17, 2024

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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

@taylorotwell
Copy link
Member

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
Copy link
Member

Urgh, bashing myself for missing this obvious one 😅

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

5 participants