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

Adding windows user to db_owner role fails if user name has backslash #3377

Conversation

thephantomthief
Copy link
Contributor

Description

Previously, if we create a user for windows login containing a
backslash and add that user to db_owner role, we were throwing an error
saying the internal role name is invalid.

To fix this, we will not check for backslash for the internal role
created when adding user db_owner role. Essentially, the check is
redundant since we already check if a user is a valid user during create
user and if user name has backslash and is not associated with windows
login we will throw an error at create time itself.

Task: BABEL-5539
Signed-off-by: Sharu Goel [email protected]

Check List

  • Commits are signed per the DCO using --signoff

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.

thephantomthief and others added 4 commits January 8, 2025 05:55
This commit addresses the following issues in the implementation of
supporting adding of multiple users to db_owner role:
 - Name clash around internal db_owner role: We will now check if the
   internal role we are about to create already exists or not, if it
   does, we throw an error. Conversely, if a server/database principal
   exists with the same name as the internal role we are about to
   create, we will throw an appropriate error.
 - Extra rows for sp_helpuser: Added a check for RoleName column to
   only contain T-SQL roles.
 - Unable to make db_owner role schema owner: We will not change owner
   of schema if it's authorization lies with db_owner role.
 - Adding/dropping of valid users to db_owner role again should not
   throw error: During addition, if user is already member of db_owner,
   we will omit creating internal role. During drop, if user is not
   member of db_owner role, we will omit dropping the internal role.

Task: BABEL-4899, BABEL-5491, BABEL-5502

Signed-off-by: Sharu Goel <[email protected]>
Previously, if we create a user for windows login containing a
backslash and add that user to db_owner role, we were throwing an error
saying the internal role name is invalid.

To fix this, we will not check for backslash for the internal role
created when adding user db_owner role. Essentially, the check is
redundant since we already check if a user is a valid user during create
user and if user name has backslash and is not associated with windows
login we will throw an error at create time itself.

Task: BABEL-5539
Signed-off-by: Sharu Goel <[email protected]>
@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 12668497862

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 74.924%

Totals Coverage Status
Change from base Build 12667297796: 0.0%
Covered Lines: 46906
Relevant Lines: 62605

💛 - Coveralls

@shardgupta shardgupta merged commit f975bb1 into babelfish-for-postgresql:BABEL_4_X_DEV Jan 8, 2025
47 checks passed
@shardgupta shardgupta deleted the sharu-dev-db_owner-fixes-4-x branch January 8, 2025 11:46
thephantomthief added a commit to amazon-aurora/babelfish_extensions that referenced this pull request Jan 8, 2025
…babelfish-for-postgresql#3377)

Previously, if we create a user for windows login containing a
backslash and add that user to db_owner role, we were throwing an error
saying the internal role name is invalid.

To fix this, we will not check for backslash for the internal role
created when adding user db_owner role. Essentially, the check is
redundant since we already check if a user is a valid user during create
user and if user name has backslash and is not associated with windows
login we will throw an error at create time itself.

Task: BABEL-5539
Signed-off-by: Sharu Goel [email protected]
thephantomthief added a commit to amazon-aurora/babelfish_extensions that referenced this pull request Jan 8, 2025
…babelfish-for-postgresql#3377)

Previously, if we create a user for windows login containing a
backslash and add that user to db_owner role, we were throwing an error
saying the internal role name is invalid.

To fix this, we will not check for backslash for the internal role
created when adding user db_owner role. Essentially, the check is
redundant since we already check if a user is a valid user during create
user and if user name has backslash and is not associated with windows
login we will throw an error at create time itself.

Task: BABEL-5539
Signed-off-by: Sharu Goel [email protected]
shardgupta pushed a commit that referenced this pull request Jan 9, 2025
…#3377) (#3379)

Previously, if we create a user for windows login containing a
backslash and add that user to db_owner role, we were throwing an error
saying the internal role name is invalid.

To fix this, we will not check for backslash for the internal role
created when adding user db_owner role. Essentially, the check is
redundant since we already check if a user is a valid user during create
user and if user name has backslash and is not associated with windows
login we will throw an error at create time itself.

Task: BABEL-5539
Signed-off-by: Sharu Goel [email protected]
shardgupta pushed a commit that referenced this pull request Jan 9, 2025
…#3377) (#3380)

Previously, if we create a user for windows login containing a
backslash and add that user to db_owner role, we were throwing an error
saying the internal role name is invalid.

To fix this, we will not check for backslash for the internal role
created when adding user db_owner role. Essentially, the check is
redundant since we already check if a user is a valid user during create
user and if user name has backslash and is not associated with windows
login we will throw an error at create time itself.

Task: BABEL-5539
Signed-off-by: Sharu Goel [email protected]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants