Docs should make clear that setting `oidc_providers.allow_existing_users: true` has security implications

The `oidc_providers.allow_existing_users` documentation is pretty sparse:

> `allow_existing_users`: set to true to allow a user logging in via OIDC to match a pre-existing account instead of failing. This could be used if switching from password logins to OIDC. Defaults to false.

The way [the sample configurations](https://element-hq.github.io/synapse/latest/openid.html) do this makes assumptions *contrary* to [OIDC claim stability guidelines](https://openid.net/specs/openid-connect-basic-1_0.html#ClaimStability). If a Synapse instance was configured in this way, it *could* allow account take-overs[^1] by an attacker with an unknown-to-Synapse IdP account.

The documentation should describe the security implications of enabling the option.

While there are some weaknesses in Synapse that make this possible at all, the ability to exploit it is mostly configuration dependant – so I feel this is a "security-related docs bug" rather than "docs-related security bug".

[^1]: If a user has enabled end-to-end encryption and uses it in all channels, I suspect it would prevent an attacker from accessing prior messages or impersonating the user on those channels.

### Background

From [OpenID Connect Basic Client Implementer's Guide 1.0, Section 2.5.3, Claim Stability and Uniqueness](https://openid.net/specs/openid-connect-basic-1_0.html#ClaimStability), emphasis added:

> The sub (subject) and iss (issuer) Claims, used together, are the only Claims that an RP can rely upon as a stable identifier for the End-User, since the sub Claim MUST be locally unique and never reassigned within the Issuer for a particular End-User, as described in [Section 2.2](https://openid.net/specs/openid-connect-basic-1_0.html#IDToken). ***Therefore, the only guaranteed unique identifier for a given End-User is the combination of the iss Claim and the sub Claim.***
>
> ***All other Claims carry no such guarantees across different issuers in terms of stability over time or uniqueness across users, and Issuers are permitted to apply local restrictions and policies.*** For instance, an Issuer MAY re-use an email Claim Value across different End-Users at different points in time, and the claimed email address for a given End-User MAY change over time. ***Therefore, other Claims such as email, phone_number, and preferred_username and MUST NOT be used as unique identifiers for the End-User.***

By default, Synapse uses the `sub` claim to uniquely identify a user, which is in-line with OIDC recommendations.

### Detail

Setting `oidc_providers.allow_existing_users: true` triggers an extra code path in [`grandfather_existing_users()`](https://github.com/element-hq/synapse/blob/2ce7a1edf7291cfda14bb975135eb7c989d910bc/synapse/handlers/oidc.py#L1243) on login if [lookup by external user ID fails](https://github.com/element-hq/synapse/blob/2ce7a1edf7291cfda14bb975135eb7c989d910bc/synapse/handlers/sso.py#L466-L470).

This maps an OIDC user to a Matrix user based on `localpart_template`:

https://github.com/element-hq/synapse/blob/2ce7a1edf7291cfda14bb975135eb7c989d910bc/synapse/handlers/oidc.py#L1247-L1255

If there is an existing Matrix user which maps, the new `sub` is added to the database:

https://github.com/element-hq/synapse/blob/2ce7a1edf7291cfda14bb975135eb7c989d910bc/synapse/handlers/sso.py#L474-L479

As there doesn't seem to be any *further* authentication (such as requesting a user login with a password, or with another configured external identity provider), my working assumption (from static analysis) is that enabling `allow_existing_users` allows an [unknown-to-Synapse](https://github.com/element-hq/synapse/blob/2ce7a1edf7291cfda14bb975135eb7c989d910bc/synapse/handlers/sso.py#L466-L470) OIDC user to permanently link their OIDC identity to an existing Matrix account that matches by `localpart_template`.

I couldn't find anywhere that branch checks to see if a matched account *already* has an `user_external_id` associated with it (only that [an external ID can't be re-used](https://github.com/element-hq/synapse/blob/2ce7a1edf7291cfda14bb975135eb7c989d910bc/synapse/storage/schema/main/delta/56/user_external_ids.sql#L42)), so it looks like this would allow two OIDC users to "claim" the same Matrix account.

One caveat is that if an instance sets `enable_registration: true` (the default), an attacker would need a new IdP account for every attempt they made to take over an account.

### The `localpart_template`

The impact of the issue has depends on the `localpart_template`.

There is no *default* `localpart_template`, but the [example OIDC configs doc have some suggestions](https://element-hq.github.io/synapse/latest/openid.html), and many of them look insecure:

* Entra ID (Azure AD): Using everything before an `@` character in `preferred_username`.

Entra ID sets the `preferred_username` to the account email address, so this would treat `user@example.com` and `user@example.net` as the same user.

Entra ID also has a tenancy ID attribute, but this doesn't seem to be used.

* Django OAuth Toolkit: Using everything before an `@` character in `email`.

Similar issue to Entra ID.

* Shibboleth: Using everything before an `@` character in `user.sub`, and also sets `allow_existing_users: true`.

Shibboleth has [custom mapping rules for `sub`](https://shibboleth.atlassian.net/wiki/spaces/IDPPLUGINS/pages/1376879208/OPSubClaim), but this example seems to suggest `sub` is an `email`-like field.

If it is an email field, then this would have a similar issue to Entra ID, with **an insecure-by-default config**.

* Auth0, Authentik, Keycloak, LemonLDAP: Using `preferred_username` as-is – this is an unstable claim.

* Twitch: Using `preferred_username` as-is.

This is [the display name](https://dev.twitch.tv/docs/authentication/getting-tokens-oidc/#requesting-claims), and [can be changed by users, or recycled by Twitch](https://help.twitch.tv/s/article/username-rename-and-recycling-policies?language=en_US).

* GitHub, GitLab, Gitea, Mastodon, Twitter: Using `user.login` / `user.nickname` / `user.username` as-is.

Those accounts may be renamed.

* Google: Using `user.given_name` as-is.

This may be freely changed by the user.

If an attacker with an unknown-to-Synapse IdP account can discover a Synapse server's `localpart_template`, and find that it is under user control, they could exploit a server with `oidc_providers.allow_existing_users: true`:

1. Identify a target user's localpart (this could be learned by federation).
2. Get a new account on the Synapse instance's IdP. If `enable_registration: true` (the default), an attacker would need a new IdP account for each takeover attempt, though this is a low bar for a public identity provider.
3. Attacker sets the field used in `localpart_template` to match the target user.
4. Attacker logs in to the target's Synapse instance with OIDC for the first time.
5. Synapse links the attacker's external user ID to the target account.
6. Attacker is now authenticated as the target user.

It's not clear to me (from static analysis) whether a user can discover what external user IDs are associated with their account; though it would appear as another unverified session (as they wouldn't have the E2EE keys).

### What could change in the docs?

`oidc_config.allow_existing_users` should describe *how* it maps existing users (with `localpart_template`), and that enabling the option has significant security implications.

It should also note that enabling the option adds extra constraints to any claim used by `localpart_template`:

* the identifier **must** be unique among all users, stable and never re-issued
* violating those additional constraints may allow account take-overs
* it's best to set up a custom claim (eg: `matrix_username`) in the IdP to map users, rather than using other, [potentially-unstable identifiers](https://openid.net/specs/openid-connect-basic-1_0.html#ClaimStability) (like `email` or `username`)

### What could change in Synapse?

When "grandfathering" existing Matrix user accounts, Synapse should also authenticate the user by some *other* method, even if those methods are not normally available (eg: password backend). This would prevent account take-overs during the migration process.

There should be a way to prevent more than one external user ID being assigned to the same Matrix user. This would prevent account take-overs for accounts which are already migrated to OIDC.