404labfr / 404labfr/laravel-impersonate

take() deletes the impersonator's remember-me cookie, so an expired session logs them out for good

Aberta
#241 0 comentários 0 reações 0 responsáveis Ver no GitHub
Linguagem predominante
PHP
Estrelas
2.3k
Forks
235
Métricas de merge de PRs
Nenhum PR com merge em 30d

Descrição

Impersonating clears the impersonator's remember-me cookie, so an expired session logs them out permanently instead of returning them to their own account.

Version 1.7.8, Laravel 13, PHP 8.5, `session.driver = database`, single `web` guard.

### Symptom

An admin logs in with remember-me, impersonates a user, then leaves the tab idle past `session.lifetime`. Clicking "leave impersonation" hits the `auth` middleware as a guest and redirects to the login page. Their own account is unreachable, and there is nothing in the browser to recover it from.

The same admin, doing the same thing without impersonating, is silently restored from the recaller cookie. Impersonation is what removes the safety net.

### Mechanism

Two separate defects, both in `ImpersonateManager`:

**1. `take()` deletes the impersonator's recaller cookie.**

`take()` calls `quietLogout()` on the current guard ([ImpersonateManager.php#L120](https://github.com/404labfr/laravel-impersonate/blob/master/src/Services/ImpersonateManager.php#L120)), which reaches `Illuminate\Auth\SessionGuard::clearUserDataFromStorage()`. That queues `forget()` on the recaller cookie, so the browser drops it. For the whole duration of the impersonation, the impersonator's only anchor is `saveAuthCookieInSession()`'s copy in the session, which is exactly the thing that expires.

This defeats the intent stated in `Guard\SessionGuard::quietLogout()`'s own docblock ("Logout the user without updating remember_token"). The token is preserved, but the cookie carrying it is not.

**2. `leave()` restores the cookie without a lifetime.**

`extractAuthCookieFromSession()` restores it with `$this->app['cookie']->queue($session[0], $session[1])` ([ImpersonateManager.php#L250](https://github.com/404labfr/laravel-impersonate/blob/master/src/Services/ImpersonateManager.php#L250)). `CookieJar::queue()` forwards to `make($name, $value, $minutes = 0)`, so the cookie comes back with `expires=0`, a session cookie. A remember-me cookie that survives a browser restart goes in and a session-scoped one comes out.

### Reproduction

Verified over HTTP with a cookie jar, not in a test harness, because the failure is invisible to anything using `actingAs()`.

| step | recaller cookie in browser |
| --- | --- |
| log in with `Auth::attempt($credentials, remember: true)` | `remember_web_...` expires in 5 years |
| `GET /impersonate/take/{id}` | **deleted** |
| session expires or is garbage collected | none |
| `GET /impersonate/leave` | 302 to `/login`, logged out |

And for the second defect, without any session expiry:

| step | recaller cookie in browser |
| --- | --- |
| log in | expires in 5 years |
| take | deleted |
| leave | restored, `expires=0` |

### Suggested fix

`quietLogout()` should not touch the recaller cookie. If it leaves the cookie alone, the impersonator stays recoverable for the whole impersonation, an expired session falls back to the recaller and lands them in their own account, and the `saveAuthCookieInSession()` / `extractAuthCookieFromSession()` pair becomes unnecessary rather than merely lossy:

```php
// Guard/SessionGuard.php
public function quietLogout()
{
$this->session->remove($this->getName());

$this->user = null;
$this->loggedOut = true;
}
```

One consequence worth calling out: the impersonator's recaller cookie stays in the browser while impersonating. It is the same person's browser and it grants nothing to the impersonated account, since `SessionGuard::user()` prefers the session over the recaller. It only takes effect once the session is gone, which is precisely the case that is broken today.

If the stash/restore pair is kept instead, `extractAuthCookieFromSession()` needs `forever()` rather than a zero-minute `queue()`, but that still leaves the window during the impersonation uncovered.

Related to #208, which reports the same symptom but proposes adding remember-me to `quietLogin()` for the impersonated user. That is a different concern from the impersonator's own cookie being destroyed.

I am working around this in application code with a listener on `TakeImpersonation` and `LeaveImpersonation` that re-queues the incoming recaller cookie with its full lifetime. Happy to raise a PR for the guard change if the approach looks right to you.

Guia de contribuição

Nenhum guia de contribuição indexado para este repositório

Avaliação

Esta issue ainda não foi avaliada.

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.