Skip to content

[To dev/1.3] [CQ] Check READ_DATA permissions during continuous query execution (#19234) - #19260

Merged
JackieTien97 merged 1 commit into
dev/1.3from
ty/cp-19234-dev-1.3
Nov 13, 2026
Merged

JackieTien97 merged 1 commit into
dev/1.3from
ty/cp-19234-dev-1.3

Conversation

@JackieTien97

Copy link
Copy Markdown
Contributor

Description

Backport of #19234 to dev/1.3.

Tree-model CQ execution skips the query authorization step, so a CQ copies series on which its owner has no READ_DATA privilege. For example, an owner with READ_DATA only on root.cq_read_auth.source.s1 and WRITE_DATA on root.cq_read_auth.target.** runs:

SELECT * INTO root.cq_read_auth.target(::) FROM root.cq_read_auth.source

Before this change, the CQ also copied source.s2. Now only s1 is copied. As on master, CQs with explicit target columns keep SELECT INTO's positional mapping: under partial READ_DATA, the window fails with 701 without writing any column, and the scheduler retries it until access is restored. If the owner is deleted, authorization fails with a message that includes the username, and the DataNode logs a warning with the CQ ID and owner.

Adaptations for dev/1.3

  • Authorization uses AuthorityChecker.checkAuthority(statement, session), which applies QueryStatement.checkPermissionBeforeProcess on this branch. The AuthException message is kept there, because TreeAccessCheckVisitor does not exist on dev/1.3.
  • Sessions carry no user ID on dev/1.3, so the master change that resolves the owner's user ID has no counterpart. Supplying the session still moves into the try block, so a failure there is cleaned up.
  • The warning constant, with the same name and en/zh text as master, goes into the existing DataNodeQueryMessages because DataNodeMiscMessages does not exist on this branch.
  • IT: CREATE CQ requires USE_CQ on dev/1.3, so the test grants USE_CQ instead of SYSTEM. Otherwise the IT is identical to master.
  • Unit test: dev/1.3 has no AccessControl hook, so the test replaces AuthorityChecker's memoized authority fetcher through Whitebox and drops the user-ID assertions. The scenarios are unchanged: a failed authorization returns before schema analysis, the owner reaches the READ_DATA lookup and the query session, and the session and query executions are cleaned up.

Validation

  • DataNodeInternalRPCServiceImplCQTest: 2 tests passed. With the authorization block removed, both fail.
  • IoTDBCQExecIT with SimpleIT (1C1D): 9 tests passed. Against the unpatched dev/1.3 code, the 4 new cases fail: the wildcard CQ copies s2, and explicit-target windows return 200 instead of 701.
  • IoTDBCQExecIT with ClusterIT (1C3D): 9 tests passed.
  • mvn test-compile -pl iotdb-core/datanode -am -P with-zh-locale -DskipTests passed.
  • Spotless applied.

This PR has:

  • been self-reviewed.
  • added unit and integration tests.
  • been tested in 1 ConfigNode + 1 DataNode and 1 ConfigNode + 3 DataNode clusters.
Key changed classes
  • DataNodeInternalRPCServiceImpl
  • QueryStatement
  • DataNodeQueryMessages (en/zh)
  • DataNodeInternalRPCServiceImplCQTest
  • IoTDBCQExecIT

🤖 Generated with Claude Code

…19234)

Backport of #19234 to dev/1.3.

Tree-model CQ execution skipped the query authorization step, so a CQ
could copy series on which its owner had no READ_DATA privilege. Check
the owner's current READ_DATA scope before each execution, keep the
authorization failure's message, and log a warning with the CQ ID and
owner when authorization fails.

Adaptations for dev/1.3:
- Authorize with AuthorityChecker.checkAuthority(statement, session),
  which applies QueryStatement.checkPermissionBeforeProcess on this
  branch, and keep the AuthException message there instead of in
  TreeAccessCheckVisitor.
- Sessions carry no user ID on this branch, so resolving the owner's
  user ID has no counterpart. Supplying the session still moves into the
  try block, so a failure there is cleaned up.
- Add the warning constant to the existing DataNodeQueryMessages (en/zh)
  because DataNodeMiscMessages does not exist on this branch.
- IT: grant USE_CQ instead of SYSTEM for CREATE CQ.
- Unit test: replace AuthorityChecker's memoized authority fetcher, since
  there is no AccessControl hook, and drop the user-ID assertions.

(cherry picked from commit cca1dbb)
@sonarqubecloud

Copy link
Copy Markdown

@JackieTien97
JackieTien97 merged commit 0136228 into dev/1.3 Nov 13, 2026
17 of 18 checks passed
@JackieTien97
JackieTien97 deleted the ty/cp-19234-dev-1.3 branch November 13, 2026 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant