{{ message }}
fix(auth): guard FGAC checker build in SessionHL::Pull on valid license#4322
Draft
andrejtonev wants to merge 1 commit into
Draft
fix(auth): guard FGAC checker build in SessionHL::Pull on valid license#4322andrejtonev wants to merge 1 commit into
andrejtonev wants to merge 1 commit into
Conversation
SessionHL::Pull built a fine-grained auth checker whenever an authenticated user was present, but -- unlike the three call sites in interpreter.cpp -- did not first check that the enterprise license is valid. AuthChecker::GetFineGrainedAuthChecker returns a null unique_ptr the moment the license is invalid, so on an enterprise binary running without a valid license the following `auth_checker->NeedsFineGrainedAuthChecker()` dereferenced null and crashed the server (the DMG_ASSERT guarding it is compiled out in release builds). Restore the invariant the DMG_ASSERT documents by gating the block on license::global_license_checker.IsEnterpriseValidFast(), matching the other call sites. When the license is invalid, fine-grained filtering is simply not applied (community-legal) and the query returns cleanly. The crash reproduces whenever an enterprise binary runs unlicensed and any authenticated user (QueryUserOrRole is truthy iff a username is set -- a role is not required) runs a data query. QA hit it by creating a role under a valid license and then dropping the license; that role and license drop are incidental to the crash. Regression test: fine_grained_access/unlicensed_user_query_crash_tests.py. It cannot use a runner-managed `cluster:` instance because CI supplies the license via env (MEMGRAPH_ENTERPRISE_LICENSE / MEMGRAPH_ORGANIZATION_NAME) and the license resolver keeps the valid ENV candidate even after `SET DATABASE SETTING 'enterprise.license' TO ''`, so the license can't be dropped at runtime. Instead the test self-manages its instance via interactive_mg_runner and pops the license env vars before start (Popen inherits the mutated os.environ), so Memgraph boots unlicensed. It then creates a user, runs a data query as that user, and asserts the query returns cleanly and the server stays alive. Claude-Session: https://claude.ai/code/session_01S7bHLYsqWYDtsuR9oK3K37
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.




SessionHL::Pull built a fine-grained auth checker whenever an authenticated user was present, but -- unlike the three call sites in interpreter.cpp -- did not first check that the enterprise license is valid. AuthChecker::GetFineGrainedAuthChecker returns a null unique_ptr the moment the license is invalid, so on an enterprise binary running without a valid license the following
auth_checker->NeedsFineGrainedAuthChecker()dereferenced null and crashed the server (the DMG_ASSERT guarding it is compiled out in release builds).