Skip to content

Comments

feat(oauth-server): allow updating token_endpoint_auth_method for OAuth clients#2391

Open
cemalkilic wants to merge 2 commits intomasterfrom
cemal/fix-token-auth-endpoint-method-update
Open

feat(oauth-server): allow updating token_endpoint_auth_method for OAuth clients#2391
cemalkilic wants to merge 2 commits intomasterfrom
cemal/fix-token-auth-endpoint-method-update

Conversation

@cemalkilic
Copy link
Contributor

Summary

The OAuth client update endpoint (PUT) now accepts token_endpoint_auth_method, allowing admins to change how a client authenticates at the token endpoint without deleting and re-creating it. Cross-type changes are rejected (e.g., setting a confidential client to 'none').

@cemalkilic cemalkilic requested a review from a team as a code owner February 24, 2026 11:38
@coderabbitai
Copy link

coderabbitai bot commented Feb 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Cache: Disabled due to Reviews > Disable Cache setting

Disabled knowledge base sources:

  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3582607 and c1ebd21.

📒 Files selected for processing (1)
  • internal/api/oauthserver/service.go

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Admin OAuth client endpoints now accept and update a token endpoint authentication method (none, client_secret_basic, client_secret_post).
    • Validation ensures the chosen method is allowed for the client's type.
  • Tests

    • Added tests covering updates to the token endpoint authentication method, including valid, invalid, and combined-field update scenarios.

Walkthrough

This pull request adds support for updating the token_endpoint_auth_method field when managing OAuth clients through the admin API. A new optional field TokenEndpointAuthMethod is added to the client update params, with validation ensuring the provided method is among allowed values and compatible with the client's client_type. The OpenAPI spec is updated to document the token_endpoint_auth_method enum (none, client_secret_basic, client_secret_post) for POST and PUT admin client endpoints. Tests were added for successful updates, invalid-method errors, and combined-field updates.

Sequence Diagram(s)

sequenceDiagram
    participant Admin as Admin Client
    participant API as Admin API Handler
    participant Svc as OAuthServer Service
    participant DB as Data Store

    Admin->>API: PUT /admin/oauth/clients/{id} with token_endpoint_auth_method
    API->>Svc: Build OAuthServerClientUpdateParams (includes token_endpoint_auth_method)
    Svc->>Svc: Validate token_endpoint_auth_method in allowed methods
    alt method valid for client_type
        Svc->>DB: Update client record (apply new method and other fields)
        DB-->>Svc: Updated client
        Svc-->>API: Return updated client payload
        API-->>Admin: 200 OK with updated client
    else invalid for client_type
        Svc-->>API: Return validation error
        API-->>Admin: 4xx error with message
    end
Loading

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/api/oauthserver/service.go`:
- Around line 432-437: The validation for p.TokenEndpointAuthMethod currently
returns a plain fmt.Errorf when the method is invalid; update the check in the
function containing p.TokenEndpointAuthMethod to return an
apierrors.NewBadRequestError (with validation_failed error_code) instead of
fmt.Errorf so the caller receives a 400/validation_failed response. Keep the
logic that compares *p.TokenEndpointAuthMethod against GetAllValidAuthMethods()
(using slices.Contains) but replace the error construction with an
apierrors.NewBadRequestError that includes a clear message listing validMethods,
mirroring other validations in this file.

ℹ️ Review info

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Cache: Disabled due to Reviews > Disable Cache setting

Disabled knowledge base sources:

  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 36d712d and 3582607.

📒 Files selected for processing (3)
  • internal/api/oauthserver/handlers_test.go
  • internal/api/oauthserver/service.go
  • openapi.yaml

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants