-
Notifications
You must be signed in to change notification settings - Fork 22
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
Remove bootstrap level from db commands #280
Comments
PR added. This passes all tests has also been tested by the following:
Database is imported |
Caution, there's a possible pitfall: to drop and create a database, the user has to have the "DROP" and "CREATE" permission, which might not be the case for unprivileged (shared hosting) accounts. I'm aware, this is not topic for this issue and PR, but something - in my opinion - that should be considered, to prevent odd errors. |
Hi @indigoxela - yes, I did this just for confirming that this would import in to a blank newly created database and I did it in lando where I wasn't worried about losing anything. Yes, it is surprising how limited some shared hosting is, though I imagine if they are limited enough that they don't have these rights, the changes are also high that they don't have command line access too. In case it is not obvious, don't test this unless you are sure you have the necessary permissions. |
To clarify: it's not an unusual limitation, but the norm. The backdrop db user (or whatever the name is) doesn't need such global privileges to do a proper job for the CMS. Giving every db user the global drop and create permission doesn't make sense. It's a bit like always running a web browser as administrator user. 😨 So, maybe check with "SHOW GRANTS" before trying to drop? I understand, that might be exaggerated. Just brainstorming here. 😉 |
OK - that will be relevant in #202 but it is not needed here. All I'm doing is removing a level of bootstrap elevation that is not needed for the commands; the commands are still doing the same things and work fine in the tests. This was an extra manual test to check that import now worked on a newly created database. Incidentally, the Backdrop docs state that having permission to create is common:
Also, it recommends giving ALL privileges to the Backdrop user:
Out of interest, what permissions do you give to the backdrop db user on the sites you create and manage? |
Sorry for derailing this issue a bit. Yes, should have posted my concern somewhere else. 😉 The Backdrop docs recommend some odd stuff. 🤷, but the "grant all" is only for the newly created database, NOT global. It's important to distinguish between local permissions (on that single db) and global permissions (server-wide). Personally, I give users (db or whatever) always only the permissions they need to do their actual job. So I grant the db user (for any CMS) full permissions on their db, but zero outside. I agree, we might discuss that topic somewhere else. Just let me know. |
OK, I think we're on the same page here; I and the lando setup script grant the backdrop user all privileges on just the database, which is in line with the docs and in line with how you do it If the backdrop db user has full permissions on that database, then they should be able to DROP and CREATE (permissions for database don't go away on dropping the database). However, if the shared host has given much more limited permissions then they may not be able to do it. Again, such a host is going to be less likely to grant command line permission anyway. For the record, I run my sites on shared hosting, and these are included but some permissions aren't which causes issues for |
FTR: just did a drop and create from a user without global perms - and dropping plus creating just worked. Sorry for the noise. 🙊 It's always better to test before posting. 😆 |
Each of the current db commands (
db-import
,db-export
,sql
) have the bootstrap level set toThis means that if the database is empty and you try to import a database, then it will give the error:
The main use case for this is where you've setup a database on hosting, transferred the files to the host, and you want to import the database. At present, you have to do a basic install before you can import the database, or import via another tool (e.g. phpMyAdmin).
Looking at each of the commands in
commands/db.bee.inc
none of them actually need this level of bootstrap as they all pass commands to the database through themysql
andmysqldump
commands rather than using backdrop database functions.The Database::getConnectionInfo still gets retrieved because we still run
includes/miscellaneous.inc > bee_initialize_console()
I therefore propose to remove the bootstrap requirement from each of the
db.bee.inc
commands.PR to follow shortly
The text was updated successfully, but these errors were encountered: