externalconn: user on creation should get ALL privileges
#86261
Assignees
Labels
A-disaster-recovery
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
T-disaster-recovery
Comments
adityamaru
added
C-bug
|
cc @cockroachdb/bulk-io |
adityamaru
added a commit
to adityamaru/cockroach
that referenced
this issue
Aug 17, 2022
Previously, the user that created the external connection was not granted any privileges as the creator/owner of the object. This diff changes this by granting the user `ALL` privileges. When synthetic privileges have a concept of owners, we should also set the owner of the External Connection object to the creator. This can happen once cockroachdb#86181 is addressed. There was also a small bug in the grant/revoke code where external connection names that required to be wrapped in `""` egs: `"foo-kms"` were being incorrectly stringified when creating the synthetic privilege path. This resulted in: ``` CREATE EXTERNAL CONNECTION "foo-kms" AS ... GRANT USAGE ON EXTERNAL CONNECTION "foo-kms" TO baz ``` To fail with a "foo-kms" cannot be found, even though it was infact created before attempting the grant. Tests have been added to test this case. Fixes: cockroachdb#86261 Release note (bug fix): Users that create an external connection are now granted `ALL` privileges on the object.
craig bot
pushed a commit
that referenced
this issue
Aug 18, 2022
86055: sql: add index recommendation to execution_insights table r=j82w a=j82w Adds index recommendation to execution_insights table part of #81024 Release justification: Category 2: Bug fixes and low-risk updates to new functionality Release note (sql change): Adds index recommendation to execution_insights 86193: opt: use only required columns in provided ordering for project r=DrewKimball a=DrewKimball Project operators can only pass through their input ordering. However, the provided ordering may have to be remapped in order to ensure it only refers to output columns, since the `Project` can add and remove columns. The `Project` uses its `internalFDs` field to accomplish the remapping; these are constructed when the `Project` is added to the memo by combining the functional dependencies of the input and the projections. The problem occurs when transformation rules cause the input of the `Project` to "reveal" additional functional dependencies. For example, one side of a union may be eliminated and the FDs of the remaining side used in the result. This can cause the `Project` to output an ordering that is equivalent to the required ordering according to its own FDs, but which a parent operator cannot tell is equivalent because its FDs were calculated before the tranformation rule fired. This can cause panics later down the line when the provided ordering does not match up with the required ordering. In the following example, an exploration rule transforms the join into two joins unioned together, one over each disjunct. After the transformation, a normalization rule fires that removes the `t0.rowid IS NULL` side because rowids are non-null. This reveals the `t1.rowid = t0.rowid` FD, which later causes `t0.rowid` to be used in a provided ordering rather than `t1.rowid`. For the reasons mentioned above, this later causes a panic when a `Project` attempts to remap to the required `t1.rowid` ordering. ``` CREATE TABLE t0 (c0 INT); CREATE TABLE t1 (c0 INT); SELECT * FROM t0 CROSS JOIN t1 WHERE (t0.rowid IS NULL) OR (t1.rowid IN (t0.rowid)) ORDER BY t1.rowid; ``` This commit prevents the panic by making `Project` operators remap the input provided ordering to use columns from the required ordering (which are a subset of the output columns). This prevents the disparity between required and provided orderings that can cause panics down the line. In the example given above, the `t1.rowid` column would be chosen for the provided ordering because it is in the required ordering. Fixes #85393 Release note (bug fix): fixed a vulnerability in the optimizer that could cause a panic in rare cases when planning complex queries with `ORDER BY`. Release justification: fixes release blocker 86336: externalconn: grant `ALL` on `CREATE EXTERNAL CONNECTION` r=adityamaru a=adityamaru Previously, the user that created the external connection was not granted any privileges as the creator/owner of the object. This diff changes this by granting the user `ALL` privileges. When synthetic privileges have a concept of owners, we should also set the owner of the External Connection object to the creator. This can happen once #86181 is addressed. There was also a small bug in the grant/revoke code where external connection names that required to be wrapped in `""` egs: `"foo-kms"` were being incorrectly stringified when creating the synthetic privilege path. This resulted in: ``` CREATE EXTERNAL CONNECTION "foo-kms" AS ... GRANT USAGE ON EXTERNAL CONNECTION "foo-kms" TO baz ``` To fail with a "foo-kms" cannot be found, even though it was infact created before attempting the grant. Tests have been added to test this case. Fixes: #86261 Release note (bug fix): Users that create an external connection are now granted `ALL` privileges on the object. Release justification: bug fix for new functionality 86349: ttl: replace SPLIT AT with SplitTable in tests r=otan a=ecwall refs #85800 fixes #86376 SPLIT AT was causing occasional test failures if it did not asynchronously split the range before the TTL job started. SplitTable synchronously splits the range before the test starts the TTL job. Release justification: Fix test flake. Release note: None 86397: cluster-ui: added internal prefix to transaction workload insights view r=ericharmeling a=ericharmeling Previously, the connected component for transaction workload insights was using the selector for the sessions data slice to load the internal app name prefix. This isn't necessary; we can just use a constant, as is done on other pages. Fixes #86250. https://www.loom.com/share/18bf4edf46fb4b1dad2fe32f9d285dcd Release justification: bug fixes and low-risk updates to new functionality Release note: None Co-authored-by: j82w <[email protected]> Co-authored-by: DrewKimball <[email protected]> Co-authored-by: Aditya Maru <[email protected]> Co-authored-by: Evan Wall <[email protected]> Co-authored-by: Eric Harmeling <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The user that runs
CREATE EXTERNAL CONNECTIONshould be grantedALLprivileges on the External Connection object as the owner of that object. There seems to be a bug where even if the user is grantedALLon a non-descriptor backed synthetic privilege, a function callp.CheckPrivilege(ctx, ecPrivilege, privilege.USAGE)returns an error for that user. Once that is confirmed/resolved by @RichardJCai we should augment the create logic to grant the ownerALLon the object.Jira issue: CRDB-18653
The text was updated successfully, but these errors were encountered: