babelfish-for-postgresql / babelfish_extensions

Babelfish for PostgreSQL provides the capability for PostgreSQL to work with applications written for Microsoft SQL Server. Babelfish understands the SQL Server wire-protocol and T-SQL, the Microsoft SQL Server query and procedural language, so you don’t have to switch database drivers or rewrite all of your application queries.
https://babelfishpg.org/
Apache License 2.0
274 stars 93 forks source link

[Fix] Database owner login is unable to view db objects in SSMS #2876

Closed aswiniip closed 2 weeks ago

aswiniip commented 1 month ago

Description

Currently in babelfish when we create a login (e.g. postjend_svc) and a database (e.g. test) and make postjend_svc owner of test and then connect to SSMS object explorer as login "postjend_svc" to database "test" and try to expand the "Database" field to view the objects. It does not show anything. With this change the login with be able to expand the "Database" field.

The issue arised because when the databases are expanded in SSMS, a query is being executed which contains a cross-db query. Currently, we handle cross-db queries by switching the database and current user. This cross-db query is resulting into permission denied error due to lack of permission of the current user (which is switched internally to master_guest) to insert into a table created in "test" db.

The solution of the above issue is implemented in three parts:

1 - For the successful execution of the cross-db query, I switched the current user with session user (login) which resulted in the user having adequate permission of both the databases required for the execution of the script.

BABEL-5218 was found after implementing the above fix. This issue arised because when a non sysadmin login tries to run cross-db dml query like INSERT. The permission check is done through the pg_catalogue side causing a permission denied error because dbo role for that database is not given to that login during the "alter authorization".

2 - To fix the above issue, I assigned dbo role to the login when it is made the owner of the database. Since the login has the dbo role, it can perform the above queries successfully.

After implementing the above fixes, an issue was found with the has_dbaccess() function which returns incorrect value for non-sysadmin logins who are db_owners. The logins who are db_owner should get the output of has_dbaccess() as 1.

3 - To fix the above issue, I added a check in has_dbaccess() function which checks if the login is a db_owner. By doing so the function returns 1 for users who are db_owner and 0 otherwise.

Issues Resolved

BABEL-5119, BABEL-5218

Test Scenarios Covered

Check List

Signed-off-by: P Aswini Kumar aswinii@amazon.com

By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

coveralls commented 1 month ago

Pull Request Test Coverage Report for Build 10905115883

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
contrib/babelfishpg_tsql/src/dbcmds.c 15 16 93.75%
contrib/babelfishpg_tsql/src/rolecmds.c 29 30 96.67%
<!-- Total: 56 58 96.55% -->
Files with Coverage Reduction New Missed Lines %
contrib/babelfishpg_tsql/src/tsql_for/forxml.c 1 93.25%
contrib/babelfishpg_tsql/src/catalog.c 1 87.15%
contrib/babelfishpg_tsql/src/pl_handler.c 129 90.16%
<!-- Total: 131 -->
Totals Coverage Status
Change from base Build 10829093829: 0.03%
Covered Lines: 44885
Relevant Lines: 60316

💛 - Coveralls