Skip to content

fix: retain existing RegistrationAccessTokenSignature when PATCHing an OAuth2 Client#4094

Open
OMBradF wants to merge 2 commits intoory:masterfrom
One-Model:fix/client-patch-registration-access-token
Open

fix: retain existing RegistrationAccessTokenSignature when PATCHing an OAuth2 Client#4094
OMBradF wants to merge 2 commits intoory:masterfrom
One-Model:fix/client-patch-registration-access-token

Conversation

@OMBradF
Copy link
Copy Markdown

@OMBradF OMBradF commented Apr 22, 2026

Fixes #4093

patchOAuth2Client applies the JSON patch by marshaling the Client struct to JSON, applying patch operations, then unmarshaling back. The RegistrationAccessTokenSignature field is tagged json:"-", so it is excluded from both the marshal and unmarshal steps. The resulting struct has an empty signature, which is then persisted to the database via UpdateClient, permanently overwriting the stored signature with an empty string.

Related issue(s)

See #4093

Checklist

  • I have read the contributing guidelines.
  • [ ] I have referenced an issue containing the design document if my change
    introduces a new feature.
    N/A - updating existing feature
  • I am following the
    contributing code guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security vulnerability, I
    confirm that I got the approval (please contact
    security@ory.com) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • [ ] I have added or changed the documentation. N/A

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue where registration access tokens were being invalidated when updating OAuth2 client configurations. Tokens now remain valid after administrative patches are applied.

OMBradF added 2 commits April 22, 2026 11:45
… client

patchOAuth2Client applies the JSON patch by marshaling the Client struct to JSON, applying patch operations, then unmarshaling back. The RegistrationAccessTokenSignature field is tagged json:"-", so it is excluded from both the marshal and unmarshal steps. The resulting struct has an empty signature, which is then persisted to the database via UpdateClient, permanently overwriting the stored signature with an empty string.

Fixes ory#4093
@OMBradF OMBradF requested review from a team and aeneasr as code owners April 22, 2026 02:10
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 22, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8e6dc9e6-790a-4c8e-9210-3ae920358436

📥 Commits

Reviewing files that changed from the base of the PR and between a54baeb and 9afd4f2.

📒 Files selected for processing (2)
  • client/handler.go
  • client/sdk_test.go

📝 Walkthrough

Walkthrough

The PR fixes a bug where the RegistrationAccessTokenSignature field was being cleared during OAuth2 client patching. The handler now explicitly preserves this value before applying the JSON patch and restores it afterward. A test case validates that the registration access token remains valid after admin PATCH operations.

Changes

Cohort / File(s) Summary
Registration Access Token Preservation
client/handler.go
Modified patchOAuth2Client to store and restore client.RegistrationAccessTokenSignature around the JSON patch operation, preventing the signature from being lost during marshaling.
Validation Test
client/sdk_test.go
Added test case patch preserves registration access token that verifies the registration access token remains functional after an admin PATCH that modifies client fields.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the main fix: retaining RegistrationAccessTokenSignature during OAuth2 Client PATCH operations.
Description check ✅ Passed The description clearly explains the bug, its root cause, and references issue #4093. Most checklist items are completed with appropriate markings for N/A items. Tests were added to verify the fix.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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.

PATCHing an OAuth Client clears the registration access token

1 participant