feat: close remaining API parity gaps, fix transport-layer bugs found against rc.7 - #21
Merged
Conversation
… against rc.7 Adds the last missing server API coverage and fixes several correctness bugs found while verifying this SDK against a real quay.io/authorizer/authorizer:2.4.0-rc.7 instance across graphql/rest/grpc. New coverage: - Public: lock_mfa, email_otp_mfa_setup, sms_otp_mfa_setup, totp_mfa_setup, skip_mfa_setup, revoke, full webauthn.* (registration/login options+verify, delete, list credentials) - Admin: SAML IdP (create/update/delete/get/list service provider, rotate IdP cert, retire key, list keys, import SP metadata) and org domains (request/verify/add-verified/delete/list) for home-realm discovery - Meta response: is_org_discovery_enabled, is_totp_mfa_enabled, is_email_otp_mfa_enabled, is_sms_otp_mfa_enabled, is_webauthn_enabled, is_mfa_enforced Bug fixes: - Admin Users() GraphQL query declared a stale $data: PaginatedRequest variable type; the schema moved to ListUsersRequest (adds a query filter) a while back, silently breaking the graphql transport for user listing (rest/grpc were unaffected, masking it). - adminClientFields GraphQL fragment never requested client_id, so creating/updating/rotating/fetching a service account over graphql could never surface the OAuth client_id needed to actually use it. - Critical: NewAuthorizerClient computed x-authorizer-url/x-authorizer-client-id into a local headers map but assigned the original (often-nil) extraHeaders parameter to c.ExtraHeaders, discarding them. Any machine-only gRPC client built with nil extra headers (the natural construction for a pure S2S caller) lost x-authorizer-url, so the server's issuer-hostname binding check rejected every bearer-token-protected gRPC call with Unauthenticated. Confirmed at the wire level with grpcurl directly against the server, isolated to this one line, fixed, and re-verified end-to-end: a client_credentials token now authenticates correctly over raw gRPC metadata. - signup.go: removed IsMultiFactorAuthEnabled (server dropped it from SignUpRequest as a security fix; letting signup set MFA enforcement was an auth bypass) and fixed the nick_name JSON tag to nickname, matching the schema (also fixed in update_profile.go). Regenerated vendored proto stubs (internal/genpb) to match the current server proto. Verified: go build, go vet, and the full integration test suite all pass against the real rc.7 image, plus live smoke tests for SAML IdP, org domains, and the gRPC S2S auth round-trip.
…sumptions CI 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, is_org_discovery_enabled on Meta) -- every one of those queries failed GraphQL validation against the old schema, breaking CI. Bumped to rc.7, and added --disable-mfa to docker-up 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: make test passes end-to-end against the rc.7 image with this config.
3 tasks
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 transport-layer bugs found along the way — including a critical one that broke gRPC-native service-to-service auth entirely.New coverage
lock_mfa,email_otp_mfa_setup,sms_otp_mfa_setup,totp_mfa_setup,skip_mfa_setup,revoke, fullwebauthn.*(registration/login options+verify, delete, list credentials)Meta:is_org_discovery_enabled,is_totp_mfa_enabled,is_email_otp_mfa_enabled,is_sms_otp_mfa_enabled,is_webauthn_enabled,is_mfa_enforcedBug fixes
Users()GraphQL query declared a stale$data: PaginatedRequestvariable type; the schema moved toListUsersRequest(adds aqueryfilter) a while back, silently breaking the graphql transport for user listing (rest/grpc were unaffected, masking it).adminClientFieldsGraphQL fragment never requestedclient_id, so creating/updating/rotating/fetching a service account over graphql could never surface the OAuth client_id needed to actually use it.NewAuthorizerClientcomputedx-authorizer-url/x-authorizer-client-idinto a localheadersmap but assigned the original (often-nil)extraHeadersparameter toc.ExtraHeaders, discarding them. Any machine-only gRPC client built withnilextra headers (the natural construction for a pure S2S caller) lostx-authorizer-url, so the server's issuer-hostname binding check rejected every bearer-token-protected gRPC call withUnauthenticated. Confirmed at the wire level withgrpcurldirectly against the server, isolated to this one line, fixed, and re-verified end-to-end: a client_credentials token now authenticates correctly over raw gRPC metadata.signup.go: removedIsMultiFactorAuthEnabled(server dropped it fromSignUpRequestas a security fix — letting signup set MFA enforcement was an auth bypass) and fixed thenick_nameJSON tag tonickname, matching the schema (also fixed inupdate_profile.go).Regenerated vendored proto stubs (
internal/genpb) to match the current server proto.Test plan
go build ./...go vet ./...go test ./...against the real2.4.0-rc.7image (not a dev build)CheckPermissions, authenticates correctly; negative control (no token) correctly rejected