Skip to content

fix: send raw array body for remove_role_users/remove_role_admins [no-ado]#135

Merged
Subterrane merged 3 commits into
mainfrom
fix/remove-role-users-raw-array
Jul 2, 2026
Merged

fix: send raw array body for remove_role_users/remove_role_admins [no-ado]#135
Subterrane merged 3 commits into
mainfrom
fix/remove-role-users-raw-array

Conversation

@Subterrane

@Subterrane Subterrane commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Problem

Fixes #134.

remove_role_users and remove_role_admins have been broken since the SDK was generated. They serialize the request body as {"user_id": [123, 456]}, but the API parses DELETE /api/2/roles/{role_id}/users (and /admins) with a strict array parser and rejects anything that isn't a raw JSON array:

400 {"status":400,"error":"BadRequestError","description":"Expected array in request"}

The sibling add_role_users has always sent the correct raw-array body — the OpenAPI spec transcribed the wrong shape for the two DELETE operations only (likely from the docs page's parameter table, whose curl example correctly shows -d '[123, 456, 789]').

Fix

  • Both methods now accept a plain List[int] (matching add_role_users) and always serialize the body as a raw JSON array.
  • Passing a RemoveRoleUsersRequest still works — it is unwrapped to the raw array before sending — so the exact code from Remove role users failing #134 now succeeds without changes. The class is documented as deprecated.
  • Regression tests assert the actual wire body (via the existing pool_manager.request mock pattern) for the plain-list and legacy-model call styles on both endpoints.
  • CHANGELOG entry + version bump to 4.0.1.

Testing

  • pytest test/: 73 passed (including 3 new regression tests).
  • ruff check .: clean.

Note: the Node SDK had the same bug but was already fixed (March 2025, sends userIds directly). The stale onelogin-openapi spec repo also carries the wrong shape, but it is unmaintained and no longer feeds this SDK, so it is intentionally left alone.

…-ado]

The API parses DELETE /api/2/roles/{role_id}/users and /admins with a
strict array parser and rejects the object form with 400 'Expected
array in request' (issue #134). Both methods now send a raw JSON array
and accept a plain List[int], matching add_role_users. The deprecated
RemoveRoleUsersRequest form is still accepted and unwrapped.

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

This PR fixes the remove_role_users / remove_role_admins Roles API methods to send the request body as a raw JSON array of user IDs (matching the OneLogin API’s expectations), while keeping backward compatibility with the legacy RemoveRoleUsersRequest model by unwrapping it before sending.

Changes:

  • Update RolesApi.remove_role_users and RolesApi.remove_role_admins to accept List[int]-style input (and unwrap RemoveRoleUsersRequest), ensuring the wire body is a raw JSON array.
  • Add regression tests that assert the actual HTTP-layer body sent for both list and legacy-model call styles.
  • Update docs + changelog and bump the project version to 4.0.1.

Reviewed changes

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

Show a summary per file
File Description
onelogin/api/roles_api.py Accept list input (or legacy model) and ensure DELETE bodies serialize as raw JSON arrays.
onelogin/models/remove_role_users_request.py Document RemoveRoleUsersRequest as deprecated and clarify expected raw-array behavior.
test/test_roles_api.py Add regression tests asserting the exact serialized request body for both endpoints.
docs/RolesApi.md Update parameter docs/examples to use List[int] and note legacy-model deprecation.
docs/RemoveRoleUsersRequest.md Mark the model as deprecated and document unwrapping behavior.
CHANGELOG.md Add 4.0.1 entry describing the fix and compatibility behavior.
pyproject.toml Bump package metadata version to 4.0.1.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pyproject.toml

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 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread test/test_roles_api.py

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 8 out of 8 changed files in this pull request and generated no new comments.

@Subterrane Subterrane merged commit 491e13f into main Jul 2, 2026
9 checks passed
@Subterrane Subterrane deleted the fix/remove-role-users-raw-array branch July 2, 2026 22:42
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.

Remove role users failing

2 participants