github-actions[bot] commented on code in PR #69617:
URL: https://github.com/apache/doris/pull/69617#discussion_r4374911821
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/StreamingInsertJob.java:
##########
@@ -429,9 +432,13 @@ private SourceOffsetProvider
createOffsetProvider(Map<String, String> jdbcSource
private void initInsertJob() {
try {
init();
+ ConnectContext ctx = ConnectContext.get();
UnboundTVFRelation currentTvf = getCurrentTvf();
this.tvfType = currentTvf.getFunctionName();
this.originTvfProps = currentTvf.getProperties().getMap();
+ if (CdcStreamTableValuedFunction.NAME.equalsIgnoreCase(tvfType)) {
+ StreamingJobUtils.checkCdcDriverPrivilege(ctx,
originTvfProps.get(DataSourceConfigKeys.DRIVER_URL));
Review Comment:
[P2] Check mapped ADMIN roles before replacing the session
A persistent user with target LOAD and ADMIN supplied by an authentication
integration's role mapping passes the initial CDC TVF check in
`CreateJobInfo.analyzeAndCreateStreamingInsertJob()`, but fails this second
check. `getCurrentTvf()` has already called `initLogicalPlan()` ->
`makeConnectContext()`, installing a new context whose `authenticatedRoles` is
empty. Saving and passing the original `ctx` does not help: the built-in
authorization path reaches `Auth.getRolesByUserWithLdap()`, which reads
authentication-issued roles from `ConnectContext.get()`. Thus valid ADMIN
sessions cannot create remote-driver TVF jobs. Perform this check before
replacing the thread context, or preserve/restore the full initiating context,
and cover session-mapped ADMIN separately from persistent grants.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterJobCommand.java:
##########
@@ -306,6 +309,8 @@ private void checkUnmodifiableProperties(String
originExecuteSql) throws Analysi
"The uri property cannot be modified in ALTER JOB");
break;
case "cdc_stream":
+ StreamingJobUtils.checkCdcDriverPrivilege(ConnectContext.get(),
Review Comment:
[P1] Authorize the original ALTER caller before rebuilding job context
For an ADMIN-created CDC TVF job restored while manually PAUSED, a LOAD-only
user can submit `ALTER JOB j PROPERTIES('offset'='<valid JSON file/pos
offset>') INSERT ...` with the same target/source identifiers and an
attacker-controlled HTTP `driver_url`. `validateProps()` runs before this check
and calls `validateAlterOffset()` -> `getOriginTvfProps()` -> `getCurrentTvf()`
-> `initLogicalPlan()`. With the SQL caches absent after image restore, that
path calls `makeConnectContext()`, which delegates to
`InsertTask.makeConnectContext(getCreateUser(), getCurrentDbName())` and
replaces the thread-local caller with the ADMIN creator. This gate consequently
passes, the replacement SQL is persisted, and subsequent tasks also use that
ADMIN creator when loading the driver. Capture the initiating `doRun` context
and authorize against it before helpers can replace the session; cover combined
offset+SQL ALTER after restoring a paused job.
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/StreamingInsertJob.java:
##########
@@ -267,6 +268,8 @@ private void initSourceJob() {
try {
init();
checkRequiredSourceProperties();
+ StreamingJobUtils.checkCdcDriverPrivilege(ConnectContext.get(),
Review Comment:
[P2] Update the limited-user CDC regression for the new driver requirement
With `enableJdbcTest=true`,
`regression-test/suites/job_p0/streaming_job/cdc/test_streaming_mysql_job_priv.groovy`
now fails before exercising its schema-change authorization checks. The
fixture uses an HTTPS driver URL (line 38), grants its new Doris user SELECT
plus LOAD/CREATE (lines 48 and 104), and expects the CREATE at lines 105-121 to
succeed without ADMIN. This new check rejects that CREATE. Switch this fixture
to a preinstalled local driver while retaining its limited grants; granting
ADMIN would invalidate the later assertions about missing ALTER privilege. Add
separate denial/acceptance cases for the new remote-driver requirement.
##########
fe/fe-core/src/main/java/org/apache/doris/tablefunction/CdcStreamTableValuedFunction.java:
##########
@@ -83,6 +84,8 @@ private void processProps(Map<String, String> properties)
throws AnalysisExcepti
StreamingJobUtils.populateDefaultSourceProperties(sourceType,
copyProps, jobId);
super.parseCommonProperties(copyProps);
+ StreamingJobUtils.checkCdcDriverPrivilege(ConnectContext.get(),
Review Comment:
[P2] Retain ADMIN approval when a changed job is planned by its creator
A non-ADMIN user can legitimately create a CDC TVF job with a preinstalled
local driver. If an ADMIN later changes that paused job's SQL to an allowed
HTTP(S) driver URL, the new ALTER check correctly passes and the change is
saved. However, `StreamingInsertJob.createStreamingInsertTask()` still passes
`getCreateUser()` to the task, and `StreamingInsertTask.before()` rebuilds and
plans the SQL under that original non-ADMIN user. This constructor check then
rejects every task, so the successful ADMIN-approved ALTER leaves an unusable
job. Preserve the trusted statement-time authorization through background
reconstruction while keeping checks for newly supplied standalone TVF/job
properties; add a local-driver owner -> ADMIN remote-driver ALTER -> RESUME
case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
