feat: close remaining API parity gaps, fix transport-layer bugs found against rc.7#5
Merged
Merged
Conversation
… against rc.7 Verified this SDK against a real quay.io/authorizer/authorizer:2.4.0-rc.7 instance across graphql/rest/grpc, closed the remaining API coverage gaps, and fixed several bugs found along the way. New coverage: - SAML IdP admin (create/update/delete/get/list service provider, rotate IdP cert, retire key, list keys, import SP metadata) - Org domains (request/verify/add-verified/delete/list) for home-realm discovery - WebAuthn/passkey and remaining MFA-setup public methods - ListUsersRequest: the proto's UsersRequest has a query substring-filter field with no prior Python equivalent; admin.users() only accepted the plain PaginatedRequest Bug fixes: - ADMIN_USERS GraphQL query declared $data: PaginatedRequest instead of ListUsersRequest (same defect independently present in the Go SDK), breaking admin user listing over graphql specifically. Caught by the live integration suite against rc.7, not a pre-existing test. - Client dataclass and its GraphQL fragment were both missing client_id (same defect independently present in Go and JS) -- from_dict's field filtering silently dropped it even when the server sent it. - Two unannotated `list[Any]` mypy errors in webauthn_credentials (client.py, async_client.py). - Finished wiring __all__ in __init__.py for types that were already imported (SAML IdP, org domains, WebAuthn, MFA-setup) but not yet exported. Regenerated vendored grpc stubs (src/authorizer/_grpc) to match the current server proto. Verified: ruff, mypy, and 112 unit tests clean; 63/63 live integration tests passing across graphql/rest/grpc against the real rc.7 image; and a gRPC-native service-to-service auth check (client_credentials token minted over REST, presented as raw gRPC metadata to CheckPermissions, authenticates correctly; negative control with no token correctly rejected). Unlike Go's SDK, this one recomputes x-authorizer-url fresh from config on every call (_core.build_headers / _grpc_transport. grpc_metadata) rather than caching it at construction time, so it was never exposed to the equivalent failure mode -- confirmed live, not just assumed from the architecture.
CI's live-integration job was pinned to quay.io/authorizer/authorizer:2.3.0, which predates the schema fields this PR's new coverage queries (e.g. should_offer_webauthn_mfa_verify on AuthResponse, ListUsersRequest) -- every one of those queries failed GraphQL validation against the old schema, breaking CI. Bumped to rc.7, and added --disable-mfa since rc.7's default MFA-methods-enabled-by-default behavior (a separate, unrelated server-side change) would otherwise gate signup/login behind an MFA offer instead of returning an immediate access_token, which is what the existing test suite assumes. Verified locally: the live suite passes end-to-end (63/63) against the rc.7 image with this config.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verified this SDK against a real
quay.io/authorizer/authorizer:2.4.0-rc.7instance across graphql/rest/grpc, closed the remaining API coverage gaps, and fixed several bugs found along the way.New coverage
ListUsersRequest: the proto'sUsersRequesthas aquerysubstring-filter field with no prior Python equivalent;admin.users()only accepted the plainPaginatedRequestBug fixes
ADMIN_USERSGraphQL query declared$data: PaginatedRequestinstead ofListUsersRequest(same defect independently present in the Go SDK), breaking admin user listing over graphql specifically. Caught by the live integration suite against rc.7, not a pre-existing test.Clientdataclass and its GraphQL fragment were both missingclient_id(same defect independently present in Go and JS) —from_dict's field filtering silently dropped it even when the server sent it.list[Any]mypy errors inwebauthn_credentials(client.py,async_client.py).__all__in__init__.pyfor types that were already imported (SAML IdP, org domains, WebAuthn, MFA-setup) but not yet exported.Regenerated vendored gRPC stubs (
src/authorizer/_grpc) to match the current server proto.Test plan
ruff check,mypy, 112 unit tests all cleanCheckPermissions, authenticates correctly; negative control with no token correctly rejected. Unlike Go's SDK, this one recomputesx-authorizer-urlfresh from config on every call rather than caching it at construction time, so it was never exposed to the equivalent failure mode — confirmed live, not just assumed from the architecture