bakerkretzmar / nova-settings-tool

Laravel Nova tool to view and edit application settings.
MIT License
167 stars 32 forks source link

Added middleware #54

Closed ramonrietdijk closed 1 year ago

ramonrietdijk commented 1 year ago

I've added additional middleware to all routes as they can currently be accessed without authentication at nova-vendor/nova-settings-tool.

bakerkretzmar commented 1 year ago

Can you please describe exactly the issue you're seeing, including how your environment is set up, and how this PR fixes it? The correct way to authorize all access to Nova is using the viewNova gate as described in the docs, which will also cover the routes in this package. Any auth you need that's custom to this tool should be added by calling ->canSee() after registering this tool.

ramonrietdijk commented 1 year ago

Hi @bakerkretzmar,

The described issue can be reproduced by the following steps:

Gate::define('viewNova', function ($user) {
    return false;
});

At this point, we expect that no user has access to anything within Nova after they sign in. And this is partly true, because after signing in to Nova we get a 403 - perfect.

However, the routes that the API has registered do not have this protection, unfortunately. When accessing the url http://[...]/nova-vendor/nova-settings-tool, while incognito, we can read the content of the settings file.

Note that the gate viewNova is never used, even on a non-local environment.

It is indeed true that we can modify the behaviour by supplying a canSee-callback to the tool, and this works, but if we do not supply this callback, everyone without authentication can access the route.

By adding the Authenticate middleware to both groups, the routes are protected.

Also notice that inertia routes have this middleware by default if you inspect the stub of the ToolServiceProvider in Nova.

bakerkretzmar commented 1 year ago

@ramonrietdijk thanks, let me look into this. I think you're right, but I'm really thrown off by the fact that this is Nova's default behavior. I'm going to file a bug and reach out to their support.

bakerkretzmar commented 1 year ago

Thanks for contributing this, it definitely should have been this way all along.

Very unreassuring response from the Nova team, for reference: laravel/nova-issues#5496.

ramonrietdijk commented 1 year ago

Agreed! I did not expect such response either, as API routes are exposed by default.