Skip to content

Some ImpersonationTokens are not cleaned up #1348

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

Open
nodra-vr opened this issue Apr 20, 2025 · 2 comments
Open

Some ImpersonationTokens are not cleaned up #1348

nodra-vr opened this issue Apr 20, 2025 · 2 comments
Assignees
Labels
feature New feature or request

Comments

@nodra-vr
Copy link

nodra-vr commented Apr 20, 2025

Bug description

If aborts happen in UserImpersonation::makeResponse the token records are never cleaned up.

Steps to reproduce

Create an error state, or cause a timeout on redirects and trigger make response.

Expected behavior

A method or job that cleans up old token.

Laravel version

12.9.2

stancl/tenancy version

3.9.1

Ref.

@nodra-vr nodra-vr added the bug Something isn't working label Apr 20, 2025
@stancl stancl added feature New feature or request and removed bug Something isn't working labels Apr 20, 2025
@stancl stancl changed the title Leaks ImpersonationToken records Some ImpersonationTokens are not cleaned up Apr 20, 2025
@stancl
Copy link
Member

stancl commented Apr 20, 2025

The abort probably isn't even necessary for this to happen, simply generating the URLs too generously where not all get visited (and therefore consumed) would cause this.

Probably going to add a command that cleans these up, where the created_at is older than some configurable interval (probably a CLI arg with some reasonable default/static property) which can then be scheduled as needed.

@nodra-vr
Copy link
Author

To me users not hitting the URL is outside the scope of the library, documentation is key here. A cleanup Command or Task would be a nice feature to have.

The aborts on the other hand, something like the example below feels cleaner and may solve those leaks:

        $token = $token instanceof ImpersonationToken ? $token : ImpersonationToken::findOrFail($token);
        try {
            if (((string) $token->tenant_id) !== ((string) tenant()->getTenantKey())) {
                abort(403);
            }
            if ($token->created_at->diffInSeconds(Carbon::now()) > ($ttl ?? static::$ttl)) {
                abort(403);
            }
            Auth::guard($token->auth_guard)->loginUsingId($token->user_id);
            return redirect($token->redirect_url);
        } finally {
            $token->delete();
        }

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

No branches or pull requests

2 participants