Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Branch-qualified DSN gets different branch permissions #8623

Closed
pilt opened this issue Dec 2, 2024 · 4 comments · Fixed by #8630 or #8639
Closed

Branch-qualified DSN gets different branch permissions #8623

pilt opened this issue Dec 2, 2024 · 4 comments · Fixed by #8630 or #8639
Labels

Comments

@pilt
Copy link

pilt commented Dec 2, 2024

Version: 1.43.19.

Setup:

-- New, see comment below:
DELETE FROM dolt_branch_namespace_control WHERE true;
DELETE FROM dolt_branch_control WHERE true;

DROP USER IF EXISTS 'tadmin'@'%';
DROP USER IF EXISTS 'ttask'@'%';
DROP DATABASE IF EXISTS t;

-- Like before from now on:
CREATE DATABASE t;
CREATE USER 'ttask'@'%' IDENTIFIED BY 't';
GRANT ALL ON t.* TO 'ttask'@'%';

INSERT INTO t.dolt_branch_control (`database`, `branch`, `user`, `host`, `permissions`) VALUES ('t', 'task%', 'ttask', '%', 'admin');

USE t;
CALL DOLT_BRANCH('task_feature');

Connect with "/t" in DSN:

CALL DOLT_CHECKOUT('task_feature');
CALL DOLT_ADD('-A');
-- status=0

Connect with "/t/task_feature" in DSN:

SELECT user(), database(), active_branch();
-- user(),database(),active_branch()
-- ttask@%,t/task_feature,task_feature

CALL DOLT_ADD('-A');
-- [HY000][1105] `ttask`@`%` does not have the correct permissions on branch `task_feature`

-- Doesn't matter if we call dolt_checkout:
CALL DOLT_CHECKOUT('task_feature'); 
-- status,message
-- 0,Already on branch 'task_feature'
CALL DOLT_ADD('-A');
@pilt pilt changed the title Branch-qualified DSNs gets different branch permissions Branch-qualified DSN gets different branch permissions Dec 2, 2024
@timsehn timsehn added bug Something isn't working version control labels Dec 2, 2024
@Hydrocharged Hydrocharged linked a pull request Dec 3, 2024 that will close this issue
@Hydrocharged
Copy link
Contributor

The exact details are mentioned in the linked PR. There was a bug with where

INSERT INTO t.dolt_branch_control (`database`, `branch`, `user`, `host`, `permissions`) VALUES ('t', 'task%', 'ttask', '%', 'admin');

was being considered a subset of the default entry, which is:

database branch user host permissions
% % % % write

It's not a subset since admin permissions allow for more operations than write, so that bug is now fixed in the linked PR. That incorrect subset issue seemed to be behind the errors, as my local testing (and added testing to Dolt itself) seem to work just fine now.

@pilt
Copy link
Author

pilt commented Dec 3, 2024

I forgot that default permission! Actually I delete it before the test is run (unless it's a rule that cannot be seen in dolt_branch_control or something). Sorry about that.

Here's some additional test setup and what mysql.user looks like before the database and other resources are created:

DELETE FROM dolt_branch_namespace_control WHERE true;
DELETE FROM dolt_branch_control WHERE true;

DROP USER IF EXISTS 'tadmin'@'%';
DROP USER IF EXISTS 'ttask'@'%';
DROP DATABASE IF EXISTS t;

SELECT * FROM mysql.user;
Host User Select_priv Insert_priv Update_priv Delete_priv Create_priv Drop_priv Reload_priv Shutdown_priv Process_priv File_priv Grant_priv References_priv Index_priv Alter_priv Show_db_priv Super_priv Create_tmp_table_priv Lock_tables_priv Execute_priv Repl_slave_priv Repl_client_priv Create_view_priv Show_view_priv Create_routine_priv Alter_routine_priv Create_user_priv Event_priv Trigger_priv Create_tablespace_priv ssl_type ssl_cipher x509_issuer x509_subject max_questions max_updates max_connections max_user_connections plugin authentication_string password_expired password_last_changed password_lifetime account_locked Create_role_priv Drop_role_priv Password_reuse_history Password_reuse_time Password_require_current User_attributes identity
% root Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y 0 0 0 0 mysql_native_password N 1970-01-01 00:00:01 null N Y Y null null null null
localhost __dolt_local_user__ Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y 0 0 0 0 mysql_native_password *62C192B5E705F8CEA7D0430715495D259608575B N 1970-01-01 00:00:01 null N Y Y null null null null

Here's the result of select * from dolt_branch_control just after root has created the branch and the additional wiping of defaults:

CREATE DATABASE t;
CREATE USER 'ttask'@'%' IDENTIFIED BY 't';
GRANT ALL ON t.* TO 'ttask'@'%';

INSERT INTO t.dolt_branch_control (`database`, `branch`, `user`, `host`, `permissions`) VALUES ('t', 'task%', 'ttask', '%', 'admin');

USE t;
CALL DOLT_BRANCH('task_feature');

select * from dolt_branch_control;
database branch user host permissions
t task_feature root % admin
t task% ttask % admin

I also forgot that the permission is added to root automatically, as the branch creator, even though it's stated in the documentation. I presume this explains why DOLT_ADD('-A') fails when I connect with a branch-qualified DSN as ttask, as the longest match is task_feature and not task%?

But in that case I don't understand why I can connect as ttask without a branch-qualified DSN and have DOLT_ADD('-A') work after DOLT_CHECKOUT('task_feature').

@Hydrocharged
Copy link
Contributor

Thank you for that additional information! My initial testing seemed to point to that insertion failing, but this points to a potentially different issue that I need to investigate.

@Hydrocharged
Copy link
Contributor

I'm glad you provided that extra detail, as it uncovered another bug (and the actual one you were running into)! I put out another PR with the fix for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants