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

makeservices: makemysql automatic wp db pass #129

Merged
merged 8 commits into from
Nov 26, 2019
Merged

Conversation

gundralaa
Copy link
Contributor

@gundralaa gundralaa commented Oct 4, 2019

Modified makemysql and makemysql-real to automatically update wp db pass if pass is reset

@gundralaa gundralaa changed the title Modified makemysql to automatically update db password for wp makeservices: makemysql automatic wp db pass Oct 4, 2019
@dkess
Copy link
Member

dkess commented Oct 5, 2019

This approach breaks an important constraint of our current system-- we don't want users to be able to choose their database passwords.

How I would recommend doing it: makemysql-real generate the password, but capture the password from stdout (see https://github.com/ocf/utils/blob/master/makeservices/easywp#L60 for an example of how to do this), and pass that into wp config. This also means you won't have to juggle temporary files.

Copy link
Member

@jvperrin jvperrin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's also a SQL injection issue here, in that accepting user input and then running a SQL query without using a prepared statement or other safeguard means that arbitrary queries can be made, which would be a big problem.


PASS=$(sudo -u mysql /opt/share/utils/makeservices/makemysql-real | tee /dev/tty | tail -n 1 | grep -Po '(?<=: )([0-9a-zA-Z]){24,}$')
cd ~/public_html/
wp config set DB_PASSWORD "$PASS" > /dev/null 2>&1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you throwing out this error? Silent failures can create a lot of confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to make sure the script didn't interfere with other scripts that use the makemysql command. Ill modify to only throw out the stdout

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that's a good point, stripping stdout is probably OK here.

@fydai fydai requested a review from jvperrin October 30, 2019 05:43
Copy link
Contributor

@fydai fydai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me, I would add a comment before the cd ~/public_html line saying that you are changing the wordpress database password.

@gundralaa gundralaa merged commit 44d88a0 into master Nov 26, 2019
@abizer
Copy link
Member

abizer commented Nov 27, 2019

please make sure to squash commits before merging them.

also, makemysql is used in more contexts than /just/ updating WordPress passwords - I don't use WordPress for example, but I do use my OCF MySQL instance for other reasons. This is nominally safe, but should probably prompt to accept before running commands against someone's WordPress install.

@fawaf fawaf deleted the wp-automatic-db-pass branch November 28, 2019 01:45
@cg505 cg505 mentioned this pull request Feb 18, 2020
axmmisaka added a commit that referenced this pull request Oct 19, 2021
This commit consists 18 commits in total, earliest one dating back to May 2020, intended to fix chain reactions caused by #129.
This commit:
1. Changed `makemysql-real`, allowing it to output only the password and nothing else if a specific argument is given
2. Changed `makemysql`, allowing it to correctly fetch password printed by `makemysql-real`, allowing it to be silent if no wordpress installation is found, allowing it to not change wordpress password if a specific argument is given, and allowing it to output only the password and nothing else if a specific argument is given.
3. Changed `easywp` so that it is compatible with the updated `makemysql`.
Hopefully, this will not break ocf infra.

18 commits:
* rewrote so that it might work
* idk wat autopep8 changed/suggested i followed its advice
* forgot to add the messages
* Added argument parsing and non-human-friendly output
* Squashed two commits
redirected some stuff to stderr
fix stupid mistakes for makemysql-real
* updated all three scripts so that they support some silent arguments and make things fancy but they might not work as I didn't test it
* fix stupid bugs made in 3cac9d4
* fix pre-commit problem made in 3cac9d4
* Wrapper for if silent
* Fixed a stupid logical mistake and added some stuff in bash scripts; did not run pre commit yet
* Applied @kpengboy's suggestions
1. Changed --silent to --quiet
2. Disable `set -e` at places where error-handling exists
3. Added some more instructions
4. Removed some redundant stuff, but idk if this will blow stuff up
* Bug: if quite is specified, do not ask if user wants to proceed.
* Indentation Errors
* Fix, silent should be global variable
* Fixed some bugs in easywp
* fix comment
* fix so precommit pass
* more to squash
jyxzhang pushed a commit that referenced this pull request Oct 23, 2022
This commit consists 18 commits in total, earliest one dating back to May 2020, intended to fix chain reactions caused by #129.
This commit:
1. Changed `makemysql-real`, allowing it to output only the password and nothing else if a specific argument is given
2. Changed `makemysql`, allowing it to correctly fetch password printed by `makemysql-real`, allowing it to be silent if no wordpress installation is found, allowing it to not change wordpress password if a specific argument is given, and allowing it to output only the password and nothing else if a specific argument is given.
3. Changed `easywp` so that it is compatible with the updated `makemysql`.
Hopefully, this will not break ocf infra.

18 commits:
* rewrote so that it might work
* idk wat autopep8 changed/suggested i followed its advice
* forgot to add the messages
* Added argument parsing and non-human-friendly output
* Squashed two commits
redirected some stuff to stderr
fix stupid mistakes for makemysql-real
* updated all three scripts so that they support some silent arguments and make things fancy but they might not work as I didn't test it
* fix stupid bugs made in 3cac9d4
* fix pre-commit problem made in 3cac9d4
* Wrapper for if silent
* Fixed a stupid logical mistake and added some stuff in bash scripts; did not run pre commit yet
* Applied @kpengboy's suggestions
1. Changed --silent to --quiet
2. Disable `set -e` at places where error-handling exists
3. Added some more instructions
4. Removed some redundant stuff, but idk if this will blow stuff up
* Bug: if quite is specified, do not ask if user wants to proceed.
* Indentation Errors
* Fix, silent should be global variable
* Fixed some bugs in easywp
* fix comment
* fix so precommit pass
* more to squash
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.

None yet

5 participants