Skip to content

[18.0][FIX] auth_oidc: use a single client authentication method on the token request - #1044

Open
SamyShaawat wants to merge 1 commit into
OCA:18.0from
SamyShaawat:fix/18.0-auth_oidc-single-client-auth
Open

SamyShaawat wants to merge 1 commit into
OCA:18.0from
SamyShaawat:fix/18.0-auth_oidc-single-client-auth

Conversation

@SamyShaawat

Copy link
Copy Markdown

Fixes #1037

Problem

The authorization code exchange sends client_id in the request body and, when
a client secret is configured, also authenticates with HTTP Basic. A client must
not use more than one authentication method per request (RFC 6749, 2.3.1), and
providers such as Nextcloud reject it:

{"error":"invalid_request","error_description":"Use exactly one client authentication method."}

Fix

  • With a client secret: authenticate with HTTP Basic only (client_secret_basic),
    without client_id in the body.
  • Without client secret (public client using PKCE): send client_id in the body,
    as before.

Tests

test_token_request_confidential_client and test_token_request_public_client
check what is sent to the token endpoint in both cases. The first one fails
without the fix; the module suite passes on Odoo 18.0.

Not covered

The issue suggests making the method configurable (client_secret_basic or
client_secret_post). This PR only removes the duplicate authentication and
keeps client_secret_basic, which is what the module already sent.

…en request

The authorization code exchange sent `client_id` in the request body and, when
a client secret is configured, also authenticated with HTTP Basic. A client
must not use more than one authentication method per request (RFC 6749,
section 2.3.1), and providers such as Nextcloud reject it with "Use exactly
one client authentication method".

* With a client secret, authenticate with HTTP Basic only.
* Without client secret (public client using PKCE), send `client_id` in the
  body, as it is not authenticated otherwise.
@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hi @sbidoul,
some modules you are maintaining are being modified, check this out!

@SamyShaawat

Copy link
Copy Markdown
Author

@sbidoul could you please review this fix of the auth_oidc token request (single client authentication method, as required by RFC 6749 2.3.1)? Thanks!

@SamyShaawat

Copy link
Copy Markdown
Author

CI note: the failing test jobs here are not caused by this change. The test database cannot be initialized because extendable_fastapi cannot be imported with pydantic 2.14 (get_type_ref was removed, see #1045 for the fix). The module tests of this PR pass locally (12 tests). Once #1045 is merged, the jobs just need a re-run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants