Skip to content
Snippets Groups Projects
user avatar
Xavier Morel authored
The current implementation is rather non-standard and largely an
ad-hoc pre-RFC implementation, with a number of incompatibilities with
the standard & actual real-world identity providers (IDP).

Tested with the following IDP:

- google oauth v1
- google oauth v3
- auth0
- okta

Add support to bearer Authorization
===================================

Sending the access token via "Authorization: Bearer $TOK" is strongly
recommended by the RFC, and required for all IDP to support. The query
parameter method is a legacy compatibility method and should be
avoided.

Query parameter access tokens are supported by Google (both v1 and
v3), and auth0, but not okta. All three support bearer tokens. However
making this the default is complicated by compatibility issues with
current behavior.

Use standard `sub`ject for identity
===================================

The specification defines `sub` as the userinfo key providing the user
identifier at the IDP.

- auth0, okta, and google v3 use `sub`
- google v1 uses `id`
- google v1's `tokeninfo` (possibly v3 as well, not tested) uses
  `user_id`
- odoo replicates the google v1 tokeninfo behavior, using `user_id`

All the code is now standardised on `sub`, with `_auth_oauth_validate`
performing unification under that key.

Support non-json error bodies and WWW-Authenticate
==================================================

Per-spec, there is no requirement for error (userinfo) responses to
return any body, and all error information can be returned via
`WWW-Authenticate`.

Both auth0 and okta return empty bodies on error, though only okta
returns a useful www-authenticate, or relevant 40x statuses (auth0
seems to always return 400, okta has been observed to return both 400
and 401 depending on client error).

Error handling in `_auth_oauth_rpc` has been updated to only parse the
body as json on success (200), and fallback on a generic error payload
if `WWW-Authenticate` doesn't contain relevant information.

Nonce
=====

Okta requires a nonce to be provided.

Misc
====

A few improvements which are in no way required but should make things
simpler / clearer:

- update the default scope to match the standard for the implicit
  flow's values (intersected with our requirements)
- update the default google configuration to use the v3 endpoints and
  drop the tokeninfo request, remove the explicit scopes
- update the label of `validation_endpoint` to match the official
  terminology, same with `auth_endpoint`
- add a label to `body` in order to explain what it's for (as that's
  really confusing when the form just says `body` until you hover the
  field)

Expected future updates
=======================

These issues were left out and may lead to degraded security, but were
considered too large changes fora stable compatibility-oriented
update:

* store and validate the nonce
* request and properly validate the id token, as well as validate the
  access token (implicit guide sections 2.2.1, 2.2.2)
* implement "basic" flow[^basic], and / or "hybrid" flow, the implicit
  flow[^implicit] is intended for purely client-side applications
  (SPAs), the "authorization code" flow is intended as the primary
  flow for normal web applications involving a server component,
  the main advantage of the hybrid flow is that the id token *can*
  contain the claims selected by `scope`, avoiding the need for the
  userinfo request[^idtoken]
* remove support for query parameter requests
* remove support for Google's v1 oauth and subject identifiers other
  than `sub`, facebook has not been tested but looks to support that
  key as well in the OpenGraph API[^fb], this will require migrating
  existing google providers to v3 implicitly (but would allow
  simplifying their configuration)

References: RFC 6749, RFC 6750, Implicit Client Implementer's Guide
1.0 draft 23[^implicit]

Closes #88618, closes #64348, fixes #63963, closes #63970,
closes #69568

[^implicit]: https://openid.net/specs/openid-connect-implicit-1_0.html
[^basic]: https://openid.net/specs/openid-connect-basic-1_0.html also
          known as "authorization code" flow
[^fb]: https://www.facebook.com/.well-known/openid-configuration


[^idtoken]: during testing, only auth0 returned the additional claims
            as part of the id token, but this may be a configuration
            issue

closes odoo/odoo#91191

X-original-commit: 56fe16bd
Signed-off-by: default avatarXavier Morel (xmo) <xmo@odoo.com>
fb3c4845
History
Name Last commit Last update