Skip to content

Fix PATCH /Users/{userId}/status ignoring caller's Accept header - #4053

Merged
duanemay merged 2 commits into
developfrom
fix/user-status-endpoint-missing-responsebody
Aug 25, 2026
Merged

Fix PATCH /Users/{userId}/status ignoring caller's Accept header#4053
duanemay merged 2 commits into
developfrom
fix/user-status-endpoint-missing-responsebody

Conversation

@duanemay

Copy link
Copy Markdown
Member

Summary

  • updateAccountStatus (backing account-unlock and force-password-change, PATCH /Users/{userId}/status in ScimUserEndpoints) is missing @ResponseBody — accidentally dropped in 6159e2f4 (2016) when the endpoint moved from PUT to PATCH, while every sibling endpoint in this @Controller class kept it.
  • Without @ResponseBody, Spring falls back to view-name resolution instead of the normal HttpMessageConverter path. A client that doesn't send an explicit Accept: application/json header gets routed into Thymeleaf trying (and failing) to resolve a template named after the request path, and the call 500s.
  • Found via cloudfoundry/uaa-cli#333: uaa-cli's unlock-user command issues a raw PATCH without an Accept header and always gets a 500 against current develop.
  • The existing MockMvc tests for this endpoint never caught it because they explicitly set Accept: application/json, which happens to trigger a ContentNegotiatingViewResolver JSON fallback view that papers over the missing annotation.

Restoring @ResponseBody makes the endpoint correctly return JSON regardless of the caller's Accept header, consistent with every other endpoint in this file.

Side effect: the response Content-Type changes from the JSON fallback view's application/json;charset=UTF-8 to the standard HttpMessageConverter's application/json (no charset param) — matching every other endpoint here. Updated the two affected MockMvc test classes (ScimUserEndpointsMockMvcTests, ScimUserEndpointsMockMvcZonePathTests) to match; removed the now-unused APPLICATION_JSON_UTF8 test constant that existed only for this endpoint.

Test plan

  • ./gradlew :cloudfoundry-identity-uaa:test --tests ScimUserEndpointsMockMvcTests --tests ScimUserEndpointsMockMvcZonePathTests — 150/150 passing
  • Manually reproduced against a freshly-booted instance: PATCH /Users/{id}/status without an Accept header 500s on develop, returns 200 with correct JSON after this fix
  • Confirmed with Accept: application/json explicitly set, develop already returns 200 (explains why existing tests didn't catch this)

updateAccountStatus (backing unlock-account and force-password-change)
was missing @responsebody -- dropped by accident in 6159e2f (2016) when
this endpoint moved from PUT to PATCH, while every sibling endpoint in
this @controller class kept it. Without @responsebody, Spring falls back
to view-name resolution instead of the normal HttpMessageConverter path;
a client that omits an explicit `Accept: application/json` header (e.g.
uaa-cli's unlock-user, which uses a raw PATCH via its curl helper) gets
routed to Thymeleaf trying to resolve a nonexistent template, and 500s.

With Accept: application/json explicitly sent, a ContentNegotiatingView
Resolver JSON fallback view papered over the missing annotation, which is
why the existing MockMvc tests (which do set that header) never caught
this. Restoring @responsebody makes the endpoint correctly return JSON
regardless of the caller's Accept header, like every other endpoint here.

Side effect: the response Content-Type changes from the JSON view's
`application/json;charset=UTF-8` to the standard HttpMessageConverter's
`application/json` (no charset param), matching every other endpoint in
this file -- the two affected MockMvc test classes are updated to match.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 24, 2026 19:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes PATCH /Users/{userId}/status response handling in ScimUserEndpoints so it consistently returns a JSON body via Spring’s HttpMessageConverter path (instead of falling back to view-name resolution when Accept is missing), aligning this endpoint with its siblings in the same controller.

Changes:

  • Restores @ResponseBody on ScimUserEndpoints#updateAccountStatus to avoid view resolution and ensure JSON responses.
  • Updates MockMvc assertions to expect application/json (no charset parameter) for the status PATCH response.
  • Removes the now-unneeded APPLICATION_JSON_UTF8 test constant from the two affected MockMvc test classes.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
server/src/main/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpoints.java Restores @ResponseBody on the status PATCH endpoint to ensure JSON response serialization.
uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsMockMvcTests.java Updates expected response Content-Type from JSON+UTF8 to APPLICATION_JSON and removes unused UTF8 constant.
uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsMockMvcZonePathTests.java Same test expectation update/removal as the non-zone-path test class.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot review feedback on #4053: the existing MockMvc helper always sets
Accept: application/json, so it never exercised the view-resolution vs
HttpMessageConverter behavior that the missing @responsebody bug actually
depended on. Add a variant that omits the Accept header and assert 200 +
correct JSON, matching how uaa-cli's unlock-user issues this request.

Confirmed this test fails without @responsebody (reverted it locally,
re-ran, saw the failure) and passes with it restored.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@duanemay
duanemay requested a review from strehle August 25, 2026 14:01
@github-project-automation github-project-automation Bot moved this from Inbox to Pending Merge | Prioritized in Foundational Infrastructure Working Group Aug 25, 2026
@duanemay
duanemay merged commit 785891e into develop Aug 25, 2026
27 checks passed
@duanemay
duanemay deleted the fix/user-status-endpoint-missing-responsebody branch August 25, 2026 18:21
@github-project-automation github-project-automation Bot moved this from Pending Merge | Prioritized to Done in Foundational Infrastructure Working Group Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants