Fix SSO wire format (id key, ms timestamp, camelCase payload)#6
Merged
Conversation
The hand-written sso module produced tokens the FastComments server rejects (verified against server util/sso-utils.ts decryptSSOPayload): - SecureSSOUserData serialized `user_id` instead of `id`, so tokens failed with missing-user-id - FastCommentsSSO.new_secure used epoch seconds instead of milliseconds - SecureSSOPayload emitted snake_case keys (user_data_json_base64) instead of the widget/server format userDataJSONBase64 / verificationHash - SimpleSSOUserData omitted username entirely Also: - Add the optional Secure SSO fields (avatar, displayName, websiteUrl, groupIds, isAdmin, isModerator, ...), null-stripping, and login/logout URLs; add SecureSSOPayload.to_widget_dict() for embedding in a widget config - Move the generated client's runtime deps (pydantic, urllib3, ...) into a `client` extra so the base install is pure-stdlib SSO - Fix README examples (id kwarg, api_key auth key, host without /api) - Rewrite the unit tests to assert the real wire format and add a pinned cross-SDK parity vector; make unit tests run offline and gate the integration tests on real credentials
Correcting the DefaultApi auth key means the request now authenticates and reaches user validation, which returns 422 for the made-up context_user_id (previously the wrong auth key returned 401). Accept 422 alongside the other expected statuses.
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
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.
What
Fixes the hand-written
sso/module so it produces tokens the FastComments server actually accepts. The generatedclient/code is untouched.The bugs (all in
sso/, not generated code)Verified against the server verifier
fastcomments/util/sso-utils.ts(decryptSSOPayload):SecureSSOUserDataserialized the id asuser_idinstead ofid-> tokens failed withmissing-user-id.FastCommentsSSO.new_secureusedint(time.time())(seconds) instead of milliseconds -> outside the server's timestamp window.SecureSSOPayload.toJSONemitted snake_case keys (user_data_json_base64,verification_hash) instead of the wire formatuserDataJSONBase64/verificationHash/timestamp.SimpleSSOUserDataomittedusernameentirely (thesimpleSSOobject is keyed on it).Why the tests didn't catch it
The old
tests/test_sso.pyasserted the buggy behavior against itself: it checkedparsed["user_id"]and snake-caseparsed["user_data_json_base64"], testedcreate_verification_hashwith an arbitrary timestamp (so ms-vs-seconds never surfaced), and built simple SSO without a username. The only test that exercises the real end-to-end path (posting a token to the server) is the integration suite, which is skipped without live credentials. The unit tests were internally consistent with the wrong implementation.Changes
idis stringified and null fields are dropped.avatar,displayName,websiteUrl,groupIds,isAdmin,isModerator, ...), pluslogin_url/logout_urlplumbing andSecureSSOPayload.to_widget_dict()for embedding directly in a widget config.pydantic,urllib3,python-dateutil,typing-extensions) into aclientextra, so the base install is pure-stdlib SSO. Install the client withpip install "fastcomments[client] @ git+...".id=kwarg,api_keyauth key, host without the erroneous/apisuffix).Verification
pytest tests/: 22 passed, 7 skipped (integration needs credentials).id(notuser_id) and a millisecond timestamp.This is a prerequisite for the new
fastcomments-djangointegration, which depends on this module for SSO signing.