nova-issues icon indicating copy to clipboard operation
nova-issues copied to clipboard

Nova overrides custom authentication pipeline when using Octane and not being the default authentication prrovider

Open PNardman opened this issue 8 months ago • 4 comments

  • Laravel Version: 11.43.2
  • Nova Version: 5.4.2
  • PHP Version: 8.3.19
  • Database Driver & Version: MySql 8.0
  • Operating System and Version: Unrelated
  • Browser type and version: Unrelated
  • Reproduction Repository: Not needed

Description:

When having a custom login pipeline by defining them via Fortify::authenticateThrough and having Nova installed but not set to Nova being the default authentication provider and using Octane, Nova does not respect the customized authentication order and falls back to the default one for fortify.

This happens due to two things:

  1. When Nova is not the default authentication provider in the flush/ syncmethods of the PendingFortifyConfiguration basically set the $authenticateThrough callbacks of Fortify.
  2. Due to using Octane the declared service providers, do not boot up with every request and refresh the custom callbacks anymore, so the information gets lost.

Detailed steps to reproduce the issue on a fresh Nova installation:

  1. Have your application running on Octane.
  2. Use Fortify within your main application.
  3. Customize your authentication pipeline by calling Fortify::authenticateThrough and passing the custom steps.
  4. Do NOT use nova as your default authentication provider.
  5. Custom authentication steps will be skipped.

For reference the related NovaServiceProvider and FortifyServiceProvider

NovaServiceProvider:

class NovaServiceProvider extends NovaApplicationServiceProvider
{
    /**
     * Bootstrap any application services.
     */
    public function boot(): void
    {
        parent::boot();

        // here we declare all the menu items for nova 

        Nova::withoutNotificationCenter();
    }

    /**
     * Register the configurations for Laravel Fortify.
     */
    protected function fortify(): void
    {
        // we set the features to null so that nova does not override our 2FA
        Nova::fortify()
            ->features(features: null)
            ->register();
    }

    /**
     * Register the Nova routes.
     */
    protected function routes(): void
    {
        Nova::routes()
            ->withoutAuthenticationRoutes()
            ->withoutPasswordResetRoutes()
            ->withoutEmailVerificationRoutes()
            ->register();
    }

    /**
     * Get the dashboards that should be listed in the Nova sidebar.
     *
     * @return array<int, \Laravel\Nova\Dashboard>
     */
    protected function dashboards(): array
    {
        return [
            new \App\Nova\Dashboards\Main,
        ];
    }

    /**
     * Get the tools that should be listed in the Nova sidebar.
     *
     * @return array<int, \Laravel\Nova\Tool>
     */
    public function tools(): array
    {
        return [
            \Vyuldashev\NovaPermission\NovaPermissionTool::make()
                ->roleResource(Role::class),
            new \Tighten\NovaStripe\NovaStripe,
        ];
    }

    /**
     * Register any application services.
     */
    public function register(): void
    {
        parent::register();
    }

    /**
     * We are overrideing this in order NOT to be able to go into the admin panel locally and always checking the gate, to be more secure in a potential security risk.
     */
    protected function authorization(): void
    {
        Nova::auth(static function (Request $request): bool {
            return Gate::check('viewNova', [Nova::user($request)]);
        });
    }
}

FortifyServiceProvider


class FortifyServiceProvider extends ServiceProvider
{
    public static $loginLimitPerMinute = 5;

    /**
     * Register any application services.
     */
    public function register(): void
    {
    }

    /**
     * Bootstrap any application services.
     */
    public function boot(): void
    {
        Fortify::authenticateThrough(function (Request $request): array {
            return [
                EnsureLoginIsNotThrottled::class,
                CanonicalizeUsername::class,
                //our custom middleware that should be passed during login
                AttemptToAuthenticate::class,
                PrepareAuthenticatedSession::class,
            ];
        });

        Fortify::loginView(function () {
            return Inertia::render('Auth/Login', [
                'canResetPassword' => Route::has('password.request'),
                'status' => session('status'),
            ]);
        });

        Fortify::confirmPasswordView(function () {
            return Inertia::render('Auth/ConfirmPassword');
        });

        Fortify::twoFactorChallengeView(function (Request $request) {
            return inertia('Auth/2FA')->toResponse($request);
        });

        RateLimiter::for('login', function (Request $request) {
            $throttleKey = Str::transliterate(Str::lower($request->input(Fortify::email())).'|'.$request->ip());

            return Limit::perMinute(FortifyServiceProvider::$loginLimitPerMinute)->by($throttleKey)->response(function () {
                return back()->withErrors([
                    'email' => 'Too many login attempts. Please try again later.',
                ]);
            });
        });
    }
}

One possible solution that I tried was creating a sub class of PendingFortifyConfiguration that would override the sync and flush methods. This fixes the issues. But I am not sure if this is the correct way.

I have a couple of questions regarding this:

  1. Is it intentional behavior that the cache and pipelines should be cleared, when Nova is not the default behavior?
  2. Is there a setting that we can set to not let this happen?
  3. Is it okay to override PendingFortifyConfiguration or would it have any side effects?

PNardman avatar Mar 26 '25 09:03 PNardman

The custom PendingFortifyConfiguration that would fix this:

class PendingFortifyConfiguration extends BasePendingFortifyConfiguration
{
    public function flush(): void
    {
    }

    public function sync(): void
    {
    }
}

And then in the NovaServiceProvider I add it like this:

    protected function fortify(): void
    {
        Nova::$fortifyResolver = new PendingFortifyConfiguration();

        // we set the features to null so that nova does not override our 2FA
        Nova::fortify()
            ->features(features: null)
            ->register();
    }
``

PNardman avatar Mar 26 '25 09:03 PNardman

Please provide full reproducing repository based on fresh installation as suggested in the bug report template (or you can refer to https://github.com/nova-issues for example)

crynobone avatar Mar 26 '25 09:03 crynobone

Please provide full reproducing repository based on fresh installation as suggested in the bug report template (or you can refer to https://github.com/nova-issues for example)

I am not sure if that would be of that much value, since the issue is not really in custom code itself but the fact that any differences when creating your own fortify pipeline - like it is mentioned in the fortify documentation - just get disregarded when Nova is installed and not used for default authentication. It can even be reproduced by just duplicating one of the default steps and adding an exit statement.

I know that there is also some dependencies here on octane. But imo it is just not correctly implemented that if Nova is not the default authentication method Nova just basically resets any authentication - which causes issues if the providers are using the singleton pattern.

Especially for the fact that there is not way to disable the syncing / flushing mechanism in the config. Please correct me if I am wrong and there is a setting and I just overlooked it.

Could you maybe also elaborate if it is a valid strategy to set the $pendingFortifyResolver?

PNardman avatar Mar 26 '25 10:03 PNardman

I wouldn't be able to provide any useful feedback until I see the actual reproducing repository.

crynobone avatar Mar 26 '25 11:03 crynobone

Reproducing repository is on its way!

PNardman avatar Apr 02 '25 13:04 PNardman

@crynobone I added a reproducing repository with steps to replicate. First with the application having a custom pipeline and exitting when everything works as expected and then with octane using laravel sail, where you can clearly see that this gets skipped, when using nova. Let me know if something is still unclear.

PNardman avatar Apr 02 '25 17:04 PNardman

This should been fixed via 5.4.4

crynobone avatar Apr 08 '25 23:04 crynobone

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

github-actions[bot] avatar Apr 14 '25 00:04 github-actions[bot]