404labfr / 404labfr/laravel-impersonate

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

Đang mở
#241 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
PHP
Star
2.3k
Fork
235
Chỉ số merge pull request
Không có pull request nào được merge trong 30 ngày

Mô tả

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.

Hướng dẫn đóng góp

Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.