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

`GRANT ALL` on Amazon RDS with MariaDB does not work #660

Closed
mnapoli opened this issue Nov 8, 2018 · 12 comments

Comments

Projects
None yet
6 participants
@mnapoli
Copy link
Contributor

commented Nov 8, 2018

Description

Creating a new tenant with auto-create-tenant-database-user = true does not work on Amazon RDS with MariaDB.

Per the discussion below it seems this problem only appears with MariaDB, not with MySQL.

The problem is because the root MySQL user doesn't have SUPER privileges (see https://stackoverflow.com/a/44441811/245552).

You can't grant SUPER privilege on RDS, so you can't use the ALL PRIVILEGES shortcut.

Sorry, you must name the privileges you want to grant explicitly, even if it means listing every privilege except SUPER.

This piece of code fails because of the GRANT ALL:

https://github.com/hyn/multi-tenant/blob/37cf25b9810123643aa3f24f6bb9e8420ab13e42/src/Generators/Webserver/Database/Drivers/MariaDB.php#L52

Here is the official RDS documentation mentioning the privilege limits: https://aws.amazon.com/fr/premiumsupport/knowledge-center/duplicate-master-user-mysql/

mysql> SHOW GRANTS FOR 'master_user';

GRANT SELECT, INSERT, UPDATE, DELETE, CREATE, DROP, RELOAD, PROCESS, REFERENCES, INDEX, ALTER, SHOW DATABASES, CREATE TEMPORARY TABLES, LOCK TABLES, EXECUTE, REPLICATION SLAVE, REPLICATION CLIENT, CREATE VIEW, SHOW VIEW, CREATE ROUTINE, ALTER ROUTINE, CREATE USER, EVENT, TRIGGER ON *.* TO 'master_user'@'%' IDENTIFIED BY PASSWORD '' WITH GRANT OPTION

Actual behavior

MySQL error.

Expected behavior

User is created and has the correct privileges.


Information

  • hyn/multi-tenant version: 5
  • laravel version: 5.6
  • database driver and version: RDS, MariaDB
  • webserver software and version: AWS Lambda / Bref
  • php version: 7.2

Suggested solution

I don't think the created users need all those privileges. We could fix this problem and increase security at the same time by listing the privileges actually needed.

Do you know which privileges we should give the created users?

In the meantime as a workaround we'll create the users manually with the specific privilege list (as explained here).

@mnapoli

This comment has been minimized.

Copy link
Contributor Author

commented Nov 8, 2018

Opened #663 with the list of privileges that exist on RDS. We could restrict that list further if you are sure we don't need one of those privileges.

@luceos luceos closed this in #665 Nov 12, 2018

luceos added a commit that referenced this issue Nov 12, 2018

Allow to configure database privileges for the tenant DB user (#665)
* Allow to configure database privileges for the tenant DB user

Fix #660

Some databases like RDS do not allow granting all privileges. This new configuration option will let user provide a custom privilege list.
@kynetiv

This comment has been minimized.

Copy link

commented Feb 27, 2019

FWIW, I'm using RDS with MySQL 5.6... Now 5.6 is not supported (so I've learned!), however, I'm able to create a user and GRANT ALL with the following command.

return $connection->statement("GRANT ALL ON `{$event->config['database']}`.* TO `{$event->config['username']}`@'{$event->config['host']}' IDENTIFIED BY '{$event->config['password']}'");

I'm wondering if perhaps the OP's error was related to a missing/necessary IDENTIFIED BY cause?

@mnapoli

This comment has been minimized.

Copy link
Contributor Author

commented Feb 27, 2019

@kynetiv thank you for the heads up! It seems the IDENTIFIED BY is not present in the code here, do you mean that you are making this package work with AWS RDS? Are you changing anything in the code (are you adding IDENTIFIED BY manually)?

ping @HpiGuillaume for information

@kynetiv

This comment has been minimized.

Copy link

commented Feb 28, 2019

@mnapoli Hope I can clarify some. I hooked into the Hyn\Tenancy\Events\Database\Creating event to essentially bypass what the MariaDB database driver does to make it work for MySQL 5.6 (5.6 doesn't have CREATE USER). Here's a gist of my event listener. I have not looked into it too closely just yet but it my workaround does succeed in creating a user and the database.

This leads me to believe this package's GRANT ALL statement may not be working without out the IDENTIFIED BY clause. This package separates creating the user and then granting the user permissions in two steps where as my modification does it in one step, which is necessary for MySQL 5.6.

@ArlonAntonius

This comment has been minimized.

Copy link
Member

commented Feb 28, 2019

Can someone verify that this summary is accurate:

Using an Amazon RDS is not possible without modification
Amazon RDS is not creating the User with privileges correctly due to the "GRANT ALL" option not being available
You can fix this by setting the privileges that you can specify in the config

If this is wrong or something is missing, please post it here.

@kynetiv

This comment has been minimized.

Copy link

commented Mar 1, 2019

@ArlonAntonius @mnapoli As I mentioned previously, I was using RDS MySQL 5.6 (not supported by this package) and the modifications I made in an event listener (posted above) allowed me to create a user and GRANT ALL privileges to that user.

Now, I tested a new MySQL 5.7.23 instance on RDS, and have not encountered any issue with creating users and granting them ALL privileges as the package does currently in v5.3.1.

I'll confirm I have auto-create-tenant-database & auto-create-tenant-database-user both set to true.

I'm not sure what I'm doing differently but in my experience I'm not having this permissions issue on RDS.

@HpiGuillaume

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

Hi !
I've re-tested on MariaDB 10.3 (Free Tier AWS)
The same error raise:

SQLSTATE[42000]: Syntax error or access violation: 1044 Access denied for user 'master_user_rds'@'%' to database 'tenant_my_tenant' (SQL: GRANT ALL ON `tenant_my_tenant`.* TO `tenant_my_tenant`@'%')

But when I change the tenancy config with:

'tenant-database-user-privileges' => 'SELECT, INSERT, UPDATE, DELETE, CREATE, DROP, REFERENCES, INDEX, ALTER, CREATE TEMPORARY TABLES, LOCK TABLES, EXECUTE, CREATE VIEW, SHOW VIEW, CREATE ROUTINE, ALTER ROUTINE, EVENT, TRIGGER', 

It's works ;)

with:

  • "laravel/framework": "5.7.*",
    
  • "hyn/multi-tenant": "5.x-dev",
    

So for me, It's important to keep this tenant-database-user-privileges option in config ;)
Thanks !
G.

@luceos

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

Not sure if we need a DB driver for AWS RDS here.. It might be a good solution and it allows separation of that configuration..

In the end being able to configure user permissions is already helpful.

@kynetiv

This comment has been minimized.

Copy link

commented Mar 5, 2019

What is interesting here is that the issue appears to be specific to MariaDB on RDS whereas MySQL on RDS is not affected. After @HpiGuillaume re-test, I did a quick search and came across this blog post that also confirms this permissions issue specific to MariaDB on RDS.

Perhaps updating the issue title/description is enough since the configurable permissions is in place already.

@mnapoli mnapoli changed the title `GRANT ALL` on Amazon RDS (MySQL) does not work `GRANT ALL` on Amazon RDS with MariaDb does not work Mar 8, 2019

@mnapoli mnapoli changed the title `GRANT ALL` on Amazon RDS with MariaDb does not work `GRANT ALL` on Amazon RDS with MariaDB does not work Mar 8, 2019

@mnapoli

This comment has been minimized.

Copy link
Contributor Author

commented Mar 8, 2019

@kynetiv good call! I've updated the title and description, let me know if I made a mistake.

@arshadasgar

This comment has been minimized.

Copy link

commented May 7, 2019

@HpiGuillaume I am using 5.3 of multi-tenant and I don't see the config option tenant-database-user-privileges anywhere? Could you please help?

@luceos

This comment has been minimized.

Copy link
Member

commented May 7, 2019

@arshadasgar that might be part of 5.4:

503d5d8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.