feat(graphile-test): add setUserContext utility for RLS testing#1094
Closed
pyramation wants to merge 2 commits intomainfrom
Closed
feat(graphile-test): add setUserContext utility for RLS testing#1094pyramation wants to merge 2 commits intomainfrom
pyramation wants to merge 2 commits intomainfrom
Conversation
In production, @dataplan/pg provides withTransaction on the pgClient. In tests, the raw pg Client lacks it, causing plugins that call pgClient.withTransaction() (e.g. graphile-presigned-url-plugin) to fail unless tests manually monkey-patch the method. This adds a savepoint-based withTransaction implementation to the client inside the withPgClient callback. Savepoints nest cleanly inside the test harness's outer transaction, matching production behavior. Eliminates the need for per-test monkey patches.
Convenience wrapper around db.setContext() with standard constructive JWT claims (role: authenticated, jwt.claims.user_id, jwt.claims.database_id). Eliminates the need to copy-paste the same setContext pattern in every test file.
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
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.
Summary
Two additions to
graphile-test:1.
withTransactionon test pg clients (context.ts)In production,
@dataplan/pgprovidespgClient.withTransaction(). The raw pgClientused in tests lacks it, which causes plugins that callwithTransaction(e.g. the presigned-url plugin's upload mutations) to fail withwithTransaction is not a function. This adds a savepoint-based polyfill that is injected once per client insidewithPgClient, nesting cleanly inside the test harness's outer transaction.2.
setUserContextutility (utils.ts)Every RLS integration test in constructive-db copies the same
db.setContext({ role: 'authenticated', 'jwt.claims.user_id': ..., 'jwt.claims.database_id': ... })pattern. This adds a typed convenience function so tests can callsetUserContext(db, userId, databaseId)instead.Review & Testing Checklist for Human
withTransactionsavepoint semantics: The polyfill usesSAVEPOINT/RELEASE/ROLLBACK TOinstead of a real nested transaction. Verify this is sufficient for the plugins that depend onwithTransaction— in particular, confirm the presigned-url plugin's usage doesn't rely on true transaction isolation (it shouldn't, since PG savepoints share the parent transaction's isolation).as anycast and mutation of pgClient: The polyfill monkey-patches the pgClientinstance. Thetypeofguard prevents re-patching, but confirm this doesn't interfere with other test utilities that may also augment the client.setUserContextclaim names: Hardcodesjwt.claims.user_idandjwt.claims.database_id. Confirm these are the canonical claim names across all constructive RLS policies and won't diverge.Recommended test: run the storage integration tests in constructive-db (
app-integration-storageCI job) after publishing this version — those tests exercise bothwithTransaction(via upload mutations) and the RLS context pattern thatsetUserContextstandardizes.Notes
withTransactionpolyfill was previously monkey-patched inline inminio-multi-scope-graphql.integration.test.tsin constructive-db. This moves it into the framework so all tests get it automatically and the monkey-patch can be removed.setUserContextis intentionally a standalone function (not a method onPgTestClient) because the claim names are constructive-specific, whilePgTestClientlives in the genericpgsql-testpackage.Link to Devin session: https://app.devin.ai/sessions/7903d2a3e7a34c6daa605e12d6b80d9e
Requested by: @pyramation