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

Using Laravel's session store for the PHP SDK's sessionStorage/transientStorage #299

Closed
nie7321 opened this issue Aug 16, 2022 · 2 comments · Fixed by #307
Closed

Using Laravel's session store for the PHP SDK's sessionStorage/transientStorage #299

nie7321 opened this issue Aug 16, 2022 · 2 comments · Fixed by #307
Assignees

Comments

@nie7321
Copy link

nie7321 commented Aug 16, 2022

What should be added?

I'm developing for Laravel Octane. One issue I ran into is that the default configuration for the package is to use cookies for the sessionStorage & transientStorage. The PHP SDK's CookieStore will call setcookie(), which won't work when running on Octane1.

I didn't see anything in the Laravel package to offer another driver, so I implemented a StoreInterface that uses the the framework's sessions.

However: It was a little difficult to configure this. It didn't look like I could just give the two storage parameters the classpath in config/auth0.php; it expected instances. That's a bit of a bummer since caching the config would fail2.

I have two suggestions for enhancements of the Laravel package:

  1. When a parameter in config/auth0.php is expected to be an object, allow it to be specified as a classpath and have the service container resolve it.
  2. Add a StoreInterface implementation that uses the framework's session driver.

Footnotes

  1. See the Laravel Swoole notices, specifically point number 6.

  2. Running php artisan config:cache is what fails if you pass new MyCoolStorageDriver. I think I could have made the classes serializable to make it work, but that seems very un-Laravel-y.

@nie7321 nie7321 added the triage label Aug 16, 2022
@evansims
Copy link
Member

evansims commented Aug 17, 2022

Hey @nie7321! 👋 Thanks for raising this!

Adding support for Laravel's session store is definitely on my radar as a next feature addition. I was unaware of the Octane limitation on setcookie() (particularly since I didn't have any issues in my tests with Octane using it), but it's good to know and further underscores the need for that storage support feature. I'm on it!

@evansims evansims self-assigned this Aug 17, 2022
@nie7321
Copy link
Author

nie7321 commented Aug 17, 2022

(particularly since I didn't have any issues in my tests with Octane using it)

It was failing specifically because this script does a couple log messages by writing output when it starts Octane up.

The easiest way I found to simulate that was adding an event listener to WorkerStarting in the config/octane.php that just echos something. It prints to the console, but it seems to count as output for the purposes of breaking setcookie().

@auth0 auth0 deleted a comment from roberttolton Oct 10, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants