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

Multiple error in REVOKE operator [CORE5804] #6066

Closed
firebird-issue-importer opened this issue Apr 23, 2018 · 11 comments
Closed

Multiple error in REVOKE operator [CORE5804] #6066

firebird-issue-importer opened this issue Apr 23, 2018 · 11 comments

Comments

@firebird-issue-importer

Submitted by: @romansimakov

Attachments:
revoke-test.sql
revoke-bug.txt
revoke-fixed.txt

A logic of rvoking options, especially for field permissions is very complicated and wrong. Several examples:
================EXAMPLE 1====================
grant update(f1, f2) on table t to u with grant option;
commit;show grants;

/* Grant permissions for this database */
GRANT UPDATE (F1) ON T TO USER U WITH GRANT OPTION
GRANT UPDATE (F2) ON T TO USER U WITH GRANT OPTION

revoke grant option for update on table t from u;
commit;show grants;

/* Grant permissions for this database */
GRANT UPDATE (F2) ON T TO USER U

But should be:

/* Grant permissions for this database */
GRANT UPDATE (F1) ON T TO USER U
GRANT UPDATE (F2) ON T TO USER U

================EXAMPLE 2====================
grant update(f1, f2) on table t to u with grant option;
commit;show grants;

/* Grant permissions for this database */
GRANT UPDATE (F1) ON T TO USER U WITH GRANT OPTION
GRANT UPDATE (F2) ON T TO USER U WITH GRANT OPTION

revoke grant option for update(f1) on table t from u;
commit;show grants;

/* Grant permissions for this database */
GRANT UPDATE ON T TO USER U
GRANT UPDATE (F2) ON T TO USER U WITH GRANT OPTION

But should be

/* Grant permissions for this database */
GRANT UPDATE (F1) ON T TO USER U
GRANT UPDATE (F2) ON T TO USER U WITH GRANT OPTION

=====================EXAMPLE 3================
grant default r1 to role r2;
commit; show grants;

/* Grant permissions for this database */
GRANT DEFAULT R1 TO R2

revoke default r1 from role r2;-- revoke only default option
commit; show grants;

/* Grant permissions for this database */
GRANT DEFAULT R1 TO R2

But should be:

/* Grant permissions for this database */
GRANT R1 TO R2

ETC.

It's necessary to fix a logic and make a code more readable in this place.

Commits: 6019498 ea69393

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Apr 23, 2018

Modified by: @romansimakov

assignee: Roman Simakov [ roman-simakov ]

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Apr 23, 2018

Commented by: @romansimakov

revoke-test.sql is a test script
revoke-bug.txt - a result of test script before fix
revoke-fixed.txt - a result of test script after fix

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Apr 23, 2018

Modified by: @romansimakov

Attachment: revoke-test.sql [ 13238 ]

Attachment: revoke-bug.txt [ 13239 ]

Attachment: revoke-fixed.txt [ 13240 ]

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Apr 23, 2018

Modified by: @romansimakov

status: Open [ 1 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

Fix Version: 4.0 Beta 1 [ 10750 ]

Fix Version: 3.0.4 [ 10863 ]

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Apr 24, 2018

Commented by: @pavel-zotov

Please look in file "revoke-fixed.txt", lines 311...323:

===
grant default r1 to role r2 with admin option;
show grants;

/* Grant permissions for this database */
GRANT SELECT ON T TO USER U WITH GRANT OPTION
GRANT DEFAULT R1 TO R2 WITH ADMIN OPTION

grant default r1 to role r2;
show grants;

/* Grant permissions for this database */
GRANT SELECT ON T TO USER U WITH GRANT OPTION
GRANT DEFAULT R1 TO R2 WITH ADMIN OPTION -- <<<<<<<<<<<<<< [ ? ] Why "WITH ADMIN OPTION" is still here ?

We:
1) gave to role R2 ability to work with role R1 (+ set is as default) with ADMIN option;
2) gave to role R2 ability to work with role R1 (+ set is as default) but *WITHOUT* specifying admin option.

My question: should "WITH GRANT OPTION" still be displayed after action "2)" ? I thought (earlier) that any grant statement first implicitly revokes any other options that could present before it (i mean options that could be used with the same grant, of course)

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Apr 24, 2018

Commented by: @pavel-zotov

IMO, something like 'REVOKE ADMIN OPTION' will be useful for GRANT ROLE statement - just like we can do it in CREATE USER for explicit prohibition to use admin privileges:

SQL> create or alter user foo password '123' using plugin srp grant admin role;
SQL> commit;
SQL> set list on;
SQL> select * from sec$users where sec$user_name='FOO';

SEC$USER_NAME FOO
...
SEC$ADMIN <true>
SEC$DESCRIPTION <null>
SEC$PLUGIN Srp

SQL> commit;

SQL> create or alter user foo password '456' using plugin srp; ------ here we do NOT specify anything related to admin role
SQL> commit;
SQL> select * from sec$users where sec$user_name='FOO';

SEC$USER_NAME FOO
. . .
SEC$ADMIN <true> -- it remains the same; Ok, because we have ability to drop it (see next statement)
SEC$DESCRIPTION <null>
SEC$PLUGIN Srp

SQL> commit;
SQL> create or alter user foo password '789' using plugin srp revoke admin role;
SQL> commit;
SQL> select * from sec$users where sec$user_name='FOO';

SEC$USER_NAME FOO
. . .
SEC$ADMIN <false>
SEC$DESCRIPTION <null>
SEC$PLUGIN Srp

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Apr 24, 2018

Commented by: @romansimakov

It's correct from my POV and I've discussed it with Alex. It's clear even in code implementation. All versions of Firebird check if this privilege even with more wide rights already granted and SKIP grant in such case. For example if a privilege on an object is already granted to a subject with grant option, granting the same privilege on the same object to the same subject WITHOUT GRANT OPTION just will be ignored. And I find it logical. There are no implicit revokes.

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Apr 24, 2018

Commented by: @romansimakov

Granting privilege is not regular DDL operation. Maybe this job for some potential grant or alter operator:)

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Apr 24, 2018

Commented by: @romansimakov

Also remember it's a legacy behaviour and changing it may break backward compatibility

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Apr 24, 2018

Commented by: @pavel-zotov

> just will be ignored. And I find it logical. There are no implicit revokes.

OK, thank you for explanation.

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Apr 24, 2018

Modified by: @pavel-zotov

status: Resolved [ 5 ] => Resolved [ 5 ]

QA Status: No test => Done successfully

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

No branches or pull requests

2 participants