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

The default security configuration requires a trailing forward slash when accessing the profiler #1201

Open
aarong416 opened this issue May 18, 2023 · 2 comments

Comments

@aarong416
Copy link

aarong416 commented May 18, 2023

I created a new Symfony 5.4 project (5.4 specifically because I needed to use PHP 7.4), using symfony new --webapp, and the security configuration (config/packages/security.yml) generated by default has the following firewall:

dev:
    pattern: ^/(_(profiler|wdt)|css|images|js)/
    security: false

The problem with the pattern is that it doesn't match the URL localhost:8000/_profiler, but it does match the URL localhost:8000/_profiler/ (note the trailing `/). This means that when accessing the profiler, a trailing forward slash has to be added in at the end, which might not be added by the browser, otherwise you get redirected to the login screen.

But, if you navigate to localhost:8000/_profiler/ (with a trailing /), the dev firewall is used and the profiler can be accessed without authentication.

The way I came upon this problem is because I have a login screen that isn't working and so I needed to debug it. I navigated to localhost:8000/_profiler, but it redirected me to the login page, but since the login page wasn't working, I wasn't sure if it was possible to access the profiler, even though I know it should be accessible from an unauthenticated context.

So the problems here are:

1.The default security configuration leads you to think that you have to be authenticated to be able to access the profile, when it can be accessed by an unauthenticated user.
2. The default security configuration forces developers to add the trailing / in the URL when accessing the profiler.

The fix for this was to add a ? at the end of the regular expression to make the trailing / optional:

pattern: ^/(_(profiler|wdt)|css|images|js)/?

I suggest that this is the change that needs to be made, so that it doesn't need to be added by developers whenever they create new projects.

I had a look at this repo for a CONTRIBUTING.md file, but there is none, so I'm not sure if there's a way I can contribute other tha creating an issue. If there, let me know and I'd be happy to make changes and create a PR.

@weaverryan
Copy link
Member

I totally agree with your assessment! However, we might need to "live with this". The problem with making the / at the end optional is that this would now, potentially catch other paths - e.g. /profilersection. It's a total edge case, but it's possible.

If we WERE to improve this, it would need to match EXACTLY /profiler or /profiler/*, but not /profiler*.

@stof
Copy link
Member

stof commented Jun 5, 2023

This could be done by changing the end of the regex, replace / with (/|$)

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

No branches or pull requests

3 participants