Remove backwards-compatibility code for access tokens without an associated device

[anoadragon453]

I ran into this while implementing #11215.

Before #949, it was possible to have access tokens that were not associated with a device. This has now been deprecated for 5 years.

This possibility has now crept across the codebase, as we've marked device_id as Optional in many places, including:

synapse/synapse/handlers/sync.py

Lines 87 to 93 in d4e6f35

	@attr.s(slots=True, frozen=True, auto_attribs=True)
	class SyncConfig:
	user: UserID
	filter_collection: FilterCollection
	is_guest: bool
	request_key: SyncRequestKey
	device_id: str

This makes little sense, as you're expected to have a device if you're calling /sync. Much of the /sync handling code assumes you have a device, and would logically fail if the user didn't:

synapse/synapse/storage/databases/main/deviceinbox.py

Lines 199 to 201 in 3e0536c

	async def delete_messages_for_device(
	self, user_id: str, device_id: Optional[str], up_to_stream_id: int
	) -> int:

Passing device_id=None to this function always results in zero to-device messages being returned.

Is there anything holding us back (old access tokens?) from marking device IDs as str, not Optional[str]?

Edit: There are currently 64 entries in the access_tokens table on matrix.org that do not have an associated device ID. They are all either abuse, or go neb...

[reivilibre]

We don't know what the situation may be like on other servers, so realistically we'd have to have some kind of migration to take care of this.
That sounds like a slightly painful idea — but maybe not impossible.

For example, one idea might be to assign random device IDs to IDless devices/access tokens on-demand.
I don't know if anything would break there — presumably these devices never worked with encryption anyway (GoNEB doesn't if I recall!).

[richvdh]

I'd be up for just invalidating such access tokens (requiring the holders of them to log in again). We can include a bit of SQL in the upgrade notes to let people figure out if they are affected.

[anoadragon453]

@richvdh Likewise. Unfortunately, one of the abuse tokens was used yesterday, so we first need to figure out how these are being created.

It appears that access tokens without a device can still be created via the User Login Admin API, so that may be what's happening here.

synapse/synapse/rest/admin/users.py

Lines 939 to 944 in 7a1cefc

	token = await self.auth_handler.create_access_token_for_user_id(
	user_id=auth_user.to_string(),
	device_id=None,
	valid_until_ms=valid_until_ms,
	puppets_user_id=user_id,
	)

It'd be pretty easy to start generating device IDs, but we'd need to figure out what bit of our current infrastructure is using this.

Other quirks about this Admin API: you cannot use this endpoint to generate an access token for the current account.

[richvdh]

Unfortunately, one of the abuse tokens was used yesterday

It might just be very old?

[anoadragon453]

It might just be very old?

Definitely a possibility. We don't record when an access token was created alas.

[anoadragon453]

I just created an admin account through the EMS dashboard and it came back with an access token, but no device ID. It is presumably using the User Login Admin API to retrieve that access token.

[sandhose]

For the record, I would also like to mandate device_ids in access tokens, because I would like to remove any usage of the access_token_id, in preparation of a proper OIDC patch. Some details in #13064