Clear invalid session cookies

[paescuj]

Scope

In some cases tokens can become invalid. For example, if the signing secret has changed since the token was issued, which will especially be the case in dev/test environments after merging #22320.

A client with an invalid token won't be able to perform any requests. This is particularly cumbersome in browsers using the session cookie mode (Data Studio) as the cookie would have to be deleted manually, in order to be able to refresh the session (login) and get a valid token again.

This PR ensures such invalid session cookies are cleared from the client's cookie store. For the Data Studio, this means the login page will be shown instead of a reoccurring error message (for open tabs, a page refresh might be necessary).

Potential Risks / Drawbacks

• This solution feels like the simplest one, but could perhaps present a somewhat unexpected / not that intuitive behavior. That seems negligible to me, as we are dealing with an edge case here respectively a case which is only to be expected in dev/tests environments.
• The affected request will still fail, the clearing of the cookie will only take effect in subsequent requests.

Review Notes / Questions

None

[changeset-bot]

🦋 Changeset detected

Latest commit: 3ae3b1c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@directus/api	Patch
directus	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[br41nslug]

I feel like it may be overkill to introduce a tokenSource for all the sources purely to check for a session token here 🤔 perhaps we can simplify it to something like this:

Suggested change

	} catch (error) {
	// Clear the session cookie if the provided token is invalid,
	// allowing the client to login again
	if (isDirectusError(error, ErrorCode.InvalidToken) && req.tokenSource === 'cookie') {
	const env = useEnv();
	res.clearCookie(env['SESSION_COOKIE_NAME'] as string, SESSION_COOKIE_OPTIONS);
	}
	} catch (error) {
	// Clear the session cookie if the provided token is invalid, allowing the client to login again
	const env = useEnv();
	const cookie_name = env['SESSION_COOKIE_NAME'] as string;
	if (isDirectusError(error, ErrorCode.InvalidToken) && req.cookies[cookie_name]) {
	res.clearCookie(cookie_name, SESSION_COOKIE_OPTIONS);
	}

Or we could check the actual token to determine if it is a session token but i think this can suffice 🤔

[paescuj]

Wasn't a fan of having it in the request either 👍 But I'd like to be certain about the token source to prevent any (malicious or unintended) logouts via invalid access_token queries.

Therefore, I've now combined the extractToken and getAccountability functions in the authenticate middleware, so we can easily access the source where we need it.
IMO that structure makes more sense anyway - we do not need to distribute these two functions, which both serve the purpose of authentication, across two middleware.
This also makes testing easier/cleaner (which I've also cleaned-up now).

On that note, we could now also drop req.token, as it is not used anywhere in the API. However, there could be extensions that make use of this... Would certainly be a separate PR.

[br41nslug]

This solution feels like the simplest one

I don't feel this review statement still holds after the latest refactor. While im not against the merging of these 2 middle wares it looks like some other unrelated (maybe unintended) things were refactored here as well.

Will have to review from scratch again.