@eprodromou I wonder if this fits into the REST API roadmap? Seems like a logical thing to do, to make our OAuth REST API more standards-compliant.

From a quick read of the OpenID Connect Core 1.0 spec, the differences with our current implementation are:

• we have an oauth2/resource/profile endpoint, which matches what OIDC calls the UserInfo endpoint. In OIDC however the same data should also be returned alongside the access token (in the id_token field of the response) as a signed JWT when the scope=openid request parameter was passed to the authorize endpoint. (AIUI this is because UserInfo is less secure: the user is being identified by the access token, so an attacker can obtain an access token from the user by legitmate means, and then use that to pass off as the user in some poorly implemented app that relies on OAuth for user identification.)
• OIDC defines a bunch of extra parameters the authorize endpoint must handle:
  • nonce: passed through to the ID token
  • prompt=none: lets the client instruct the server to skip the authorization dialog (or error out when that's not permitted). We just need to implement the error, we skip whenever possible anyway.
  • prompt=consent: force always showing the authorization dialog.
  • prompt=login: force reauthentication.
  • prompt=select_account: show an account selection UI.
  • max_age: require reauthentication if user's last login was more than this many seconds ago (also the ID token must include an auth_time field with a timestamp of when the user authenticated)
  • request and request_uri (alternative ways to make the authorization request): only required to the extent that lack of support needs to be indicated with a specific error message.
• Other parameters which the server might or might not be required to handle (OIDC Core 1.0 is an infuriatingly poorly written spec):
  • response_type: A standard OAuth parameter, but the OIDC spec spends a lot of space on how multi-valued response types (aka hybrid flow; not part of the core OAuth spec) should be handled. But nowhere does it explicitly say that the server is required to handle them.
  • response_mode: Whether to return the token in the query or the fragment part of the URL.
• Some parameters are technically required, but only to the extent that they shouldn't cause the server to error out; no specific behavior is required and the server can in practice ignore them:
  • ui_locales: UI language; more or less the same as MediaWiki's uselang.
  • claims_locales: language used in the JWT user data fields. Which are mostly language-independent but I think it could be meaningful in some exotic cases involving language variants which do script conversions.
  • display: lets the client instruct the server to render the authorization format in some special way, e.g. as appropriate inside a popup.
  • complicated things related to user identity that aren't really relevant to a simple identity provider: id_token_hint, login_hint, acr_values, claims.
• There are naming conventions for the user identity JWT fields, which don't quite match our fields names (e.g. name vs. realname) but these are only recommendations.
• 9.: This section defines a set of Client Authentication methods that are used by Clients to authenticate to the Authorization Server when using the Token Endpoint. During Client Registration, the RP (Client) MAY register a Client Authentication method. If no method is registered, the default method is client_secret_basic. (sending the client secret via HTTP Basic auth), while we require the client secret to be sent in the client_secret field of the body, which is referred to the spec as the client_secret_post authentication method. This is probably inconsequential unless we want to support dynamic client registration.

There's an automated conformance test suite at https://openid.net/certification/connect_op_testing/

The OpenID Connect Discovery 1.0 spec is an optional extension to the core spec, which lets the client detect things like the API endpoints, without having to be hand-tuned to MediaWiki / Wikimedia. It consists of two parts:

• A JSON document served at .well-known/openid-configuration containing the authorization, access token and user info API URLs, and a bunch of other metadata. Ideally served by a MediaWiki API, but a minimal enough configuration could be represented by a static document. The only tricky field that's required is jwks_uri (the signing keys for the JWTs), the others are all API URLs or simple site-independent constants. (I suppose it would be technically valid to use the none signing algorithm for the JWT and return an empty set of singing keys.)
• Issuer discovery: an API that takes a user identifier in a certain format, and returns the OpenID provider responsible for that user. This is optional, and entirely pointless for the average wiki and for Wikimedia (I guess it's meant for some kind of federated identity system).

The OpenID Connect Dynamic Client Registration 1.0 spec spec is an optional extension to the core spec, which provides a standard way to register new OIDC clients (apps) - the rough equivalent of Special:OAuthRegisterConsumer and the oauth2/client MediaWiki REST API endpoint, and also to some extent of Special:OAuthListConsumers / oauth2/consumers. It defines two endpoints: one for registering a new client, one for getting information about a client by ID. They are fairly straightforward, just need a mapping to/from the MediaWiki OAuth extension's field names, plus throwing errors when the registration request includes something we don't support (like an indication that the client will communicate via encrypted JWTs). The core spec requires providers which allow dynamic registration to support the request_uri parameter for making authorization requests (15.2.), ie. letting the client tell the server where to fetch the registration request from, instead of just sending the registration request directly.

All in all, these seem reasonably straightforward to implement, especially the .well-known endpoint. Not sure if there's enough client support for either to justify the work, though.

I think the MVP version here is:

• Handle the openid value for the scope parameter in the oauth2/authorize endpoint; when present, stash the fact that OpenID data was requested along with the other authorization data, and when that data is retrieved in oauth2/access_token, generate the same data as oauth2/resource/profile, turn it into a JWT, and add it to the response. This is a bit tricky as stashing the data is handled by a third-party library (league/oauth2-server).
  • The extremely dirty way here is to just stash the flag somewhere (Redis?) keyed by the authorization code, then check that in the access token API and modify the response appropriately.
  • The nice way would be to integrate with the League OAuth server. Not sure how feasible this is. There's oauth2-openid-connect-server which tries just that but it looks pretty minimal (it doesn't really anything beyond declaring an id_token response type, which we probably don't need as it's for the implicit flow).
• Add a nonce parameter to the authorize API, and pass it through to the access token API. Again, either a hack must be used, or the League server wrangled to do it (it doesn't currently support nonces, and its OIDC plugin doesn't seem to either).

This does not result in a spec-compliant implementation, but covers everything an actual client is likely to use.

A fully compliant implemntation would take significantly more effort:

• Implement prompt=consent / prompt=none: skip the $authProvider->needsUserApproval() shortcut or fail when the shortcut is not followed, respectively. In the first case, probably need to devise a way to skip the similar shortcut inside SpecialMWOAuth as well.
• Implement prompt=login (probably by defining a "reauth" security level, and using it along the lines of SpecialPage::checkLoginSecurityLevel()).
• Implement prompt=select_account; the lazy approach is to just make it an alias for login.
• Implement max_age and the auth_time ID token field. Auth age is available as the AuthManager:lastAuthTimestamp session field but only in sessions which were created following a login, so the best that could be done here without a MediaWiki core change is a very aggressive implementation that almost always reauthenticates. Also, there are significant privacy implications to exposing this information to the client.
• Implement the "not supported" errors for request and request_uri.
• Mark response_mode, ui_locales, claims_locales, display, id_token_hint, login_hint, acr_values and claims as known API parameters, but ignore them otherwise.

Arguably, the OAuth extension as it is now already implements a functional subset of the OIDC spec. You can register an application with the "authorization code" grant type, get an access token (without using either the openid scope or the id_token response type), and then use the access token to access oauth2/resource/profile, which will have a return value compatible with UserInfo endpoint. The spec implies this is insecure (5.3.2, 16.11) but as far as I can see there is no realistic vulnerability there. So as long as an OIDC client does not insist on getting the ID token, or on using some of the extra options for the authorization endpoint, it can probably be used with the current implementation.

In OIDC however the same data should also be returned alongside the access token

In fact it is slightly more complicated. In full-blown OIDC implementation you can select which fields you want to have in ID token, which available in userinfo endpoint, and which you want in both. I agree that we could just simplify and simply return same in both.

response_type, response_mode

Interestingly, since OIDC came out, web evolved. A lot of OIDC handles the fact that at the point there was no CORS so for browsers to get access to tokens you had to play with hash/query string. These days all those other flows (implicit, etc.) are deprecated and considered insecure and one should just do the explicit flow (also called authorization code flow): you get authorization code from authorization endpoint in the query string which you then exchange for the access token with the token endpoint. Everything else can be simply marked as not supported and this is good (more secure).

One important security-wise feature which one should implement these days for OIDC providers is PKCE.

display

We might use this one for app to be able to instruct Mediawiki if it wants confirmation dialog to be full blown Mediawiki page (with sidebar, etc.) or without them.

id_token_hint

That is useful so that you can verify if current user you have ID token of is still signed-in. But I agree it is not critical and we could skip it (initially).

This is probably inconsequential unless we want to support dynamic client registration.

Not really. All calls to the token endpoint should be authenticated in some way when client is trusted (e.g., not a frontend-only client, so a client which can holds secrets).

I suppose it would be technically valid to use the none signing algorithm for the JWT and return an empty set of singing keys.

Please do sign access and ID tokens. Otherwise there is no way for apps to trust those tokens. So yea, JWKS management will be needed for this. Probably a database table with automatically generated key is a good start. And some documentation how can admins rotate keys. (This is why there can be multiple keys, you can add a key while keep the old key around for the maximum expiration time of tokens you issued, and then you removed the old key.)

.well-known/openid-configuration

It is really simple to provide that. But also there is /.well-known/oauth-authorization-server. They can be literally the same. See the comparison I have done here.

In https://github.com/fedwiki/wiki-security-passportjs/pull/43 there was a slight issue with Extension:OAuth requiring the HTTP Authorization header, as opposed to passing an access token in the query string.