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

Laravel Octane support #164

Open
nie7321 opened this issue Aug 16, 2022 · 4 comments
Open

Laravel Octane support #164

nie7321 opened this issue Aug 16, 2022 · 4 comments

Comments

@nie7321
Copy link

nie7321 commented Aug 16, 2022

Laravel Impersonate does not work with Laravel Octane. This manifests as several different errors concerning the $app container missing expected values when impersonating and leaving impersonation, from both the ImpersonateManager and ImpersonateController.

I'm seeing two points where this is an issue:

  1. The singleton in the ImpersonateServiceProvider has the app container passed to the constructor. This doesn't work in the Octane paradigm -- the container is adjusted for every request, so a fresh copy is needed.
    • I'm not sure if the singleton is strictly needed? I've worked around this by extending the service provider and reimplementing register() without that.
  2. The constructor in ImpersonateController sets the manager property. It seems like the constructors for controllers are only called once when Octane boots, which is leading to the app container inside that manager instance being stale.
    • Injecting it into the take/leave methods individually instead of setting it in the constructor should avoid the issue.

If you are open to a PR, I would be happy to send one.

@Ostojic-96
Copy link

I can confirm this, too.
When I try to leave impersonalization I am still logged with the same user.

@khalidmaquilang
Copy link

any update on this?

@OstojicI
Copy link

OstojicI commented Nov 2, 2023

any update on this?

@khalidmaquilang for me logout was working with these changes

@khalidmaquilang
Copy link

any update on this?

@khalidmaquilang for me logout was working with these changes

this is good.. i hope they will merge this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants