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

[2.x] Environment APP_URL added into the default sanctum.stateful configuration #264

Merged
merged 3 commits into from
Mar 26, 2021

Conversation

polakjan
Copy link
Contributor

What?

Added the application's URL from .env's APP_URL to the default value of sanctum.stateful configuration.

Why?

In my experience (teaching new Laravel developers) not having the application's URL among default stateful domains leads to a lot of headache when deploying to a production environment. The URL changes from e.g. localhost:3000 to www.test.com and suddenly a new configuration value must be added into .env which was not needed until now.

While everyone expects they should change the APP_URL when moving to a new environment, few also remember to add SANCTUM_STATEFUL_DOMAINS

This is also hard to notice because the application works, only the requests which should be authenticated are not.

How?

Added the host part of the APP_URL environment value to /config/sanctum.php

Testing

I needed to modify the phpunit.xml.dist file to provide a sample env APP_URL configuration that could be loaded with the env() function.

Tests test the presence of the host of env('APP_URL') in config('sanctum.stateful') and that a request coming from the environment APP_URL configuration passes the Laravel\Sanctum\Http\Middleware\EnsureFrontendRequestsAreStateful::fromFrontend() check.

Duplicite PR?

This may seem like a duplicite of #243 but does the same thing with much simpler code and the test suite passes.

@driesvints driesvints changed the title Environment APP_URL added into the default sanctum.stateful configuration [2.x] Environment APP_URL added into the default sanctum.stateful configuration Mar 26, 2021
@driesvints
Copy link
Member

This shouldn't be needed I believe. Why can't you just set SANCTUM_STATEFUL_DOMAINS? Why is the change to the phpunit config file needed?

@polakjan
Copy link
Contributor Author

You can just set SANCTUM_STATEFUL_DOMAINS. This is more a convenience thing. The default stateful configuration already contains "traditional", common values like localhost:3000 while the most common value - the APP_URL set up in .env is not there.

When you forget to set the SANCTUM_STATEFUL_DOMAINS configuration which you don't need for local development, you don't get any warning that something is wrong, only SPA requests fail to be authenticated. Hours of head scratching. This simple adjustment could help avoid that.

The change to phpunit config file was needed to provide a sample a sample env APP_URL configuration that could be loaded with the env() function to test if it appears in sanctum's stateful configuration. I used information from orchestral/testbench Is there any other way to write that test?

@taylorotwell taylorotwell merged commit 209c68b into laravel:2.x Mar 26, 2021
dshoreman added a commit to dshoreman/servidor that referenced this pull request Apr 19, 2021
Version 2.9.3 includes laravel/sanctum#264 which introduces a new
null-passing error in the updated sanctum.stateful config value, which
assumes the `APP_URL` env var is set. We only set url in config, so even
though we have our own config/sanctum.php, the default file is included
in CI because config can't be cached before composer install was run.
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

Successfully merging this pull request may close these issues.

None yet

3 participants