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

Added DDEV command to create/update the administrator account #3256

Merged
merged 8 commits into from
Jul 7, 2023
Merged

Added DDEV command to create/update the administrator account #3256

merged 8 commits into from
Jul 7, 2023

Conversation

ADDISON74
Copy link
Contributor

@ADDISON74 ADDISON74 commented May 14, 2023

The ddev config command doesn't create the administrator account, as a consequence the Backend cannot be accessed. In this case, either a query is run in the MySQL prompt getting the console ddev mysql, or the same query is run in phpMyAdmin interface ddev launch -p, so that data is inserted into the admin_role and admin_user tables. I raised this issue, but nothing was done about it.

This PR solves this issue and it adds a new DDEV command to create/update the administrator account. Usage is very simple, in terminal use this command ddev openmage-admin, choose your action then answer to the questions.

I use the update or just u action when I wanted to change the initial password, in my test environment it is much easier to enter 123, instead of a string of at least 14 characters. I didn't use the create or c action because I didn't need it, but it works. I deleted the records from the two previously mentioned tables and the script created the account. I also took in consideration the fact that the account exists.

This PR comes as an accessory to the one created here #3248 and through which OpenMage w/o Sample Data can be installed.

@github-actions github-actions bot added the ddev label May 14, 2023
@sreichel
Copy link
Contributor

Nice work, but there are some limitations. E.g. with table prefix it will not work.

I'd leave complex tasks to n98. Most commands work and just need to be verified ... netz98/n98-magerun#1275

Will continue there these days. Feel free to test.

ddev composer config repositories.n98-magerun git https://github.com/sreichel/n98-magerun
ddev composer require n98/magerun:dev-feature/php81
ddev . vendor/bin/n98-magerun admin:user:create

@ADDISON74
Copy link
Contributor Author

The tables prefix is solved by adding a question and then it is used in the table name. I will change the command to cover this issue.

@ADDISON74
Copy link
Contributor Author

ADDISON74 commented May 16, 2023

The table prefix is taken into account now and I replaced the string of characters in the password with a randomly generated one.

@ADDISON74
Copy link
Contributor Author

Please approve this PR on trust. I have been using it for some time and there are no issues. I became a fan of the development environment (Windows + WSL/Ubuntu + Docker + DDEV + PhpStorm).

@fballiano
Copy link
Contributor

why not use n98-magerun for this? asking cause I know almost nothing about ddev

@ADDISON74
Copy link
Contributor Author

ADDISON74 commented Jul 3, 2023

I don't use n98-magerun and I am not in a hurry to install it either. It needs updates, I followed the discussions between Sven and the project maintainer and there are many things to solve and I don't think it will be too soon.

DDEV has radically changed the way I test OpenMage, without needing Linux anymore. The ddev config command creates the project but not the administrator account. If you access the Backend, you don't have an account, you have to either install OM or run an SQL query. We discussed this topic in the past, but no decision was made because only me and Sven were using DDEV as far as I could see.

This PR solves two issues, it creates an administrator account and changes the administrator's password, if the account exists in the database. For example, during installation you have to add a long password of minimum 14 characters, but this command allows you to put whatever password you want.

This command along with the Sample Data install command are very important tools for using OM with DDEV. If you work in Windows with Phpstorm, I encourage you all to use Docker + WSL + DDEV. You can change the PHP version immediately, you can save the database, you can import it, you have phpMyAdmin, MailHog and tons of tools.

@fballiano
Copy link
Contributor

I use magerun daily with no issue but I don't know what discussion was made with the author. But I don't use ddev cause I prefer homebrew or devenv, which don't involve docker, which despite my high end machine, works awful.

Copy link
Contributor

@fballiano fballiano left a comment

Choose a reason for hiding this comment

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

not tested

@colinmollenhour
Copy link
Member

I agree that wrapping an invocation of n98 magerun is a much better solution as it is the defacto CLI tool (also because it uses Mage code so PHP's password_hash() which is way more secure, the single md5 is just supported for BC).

I don't see the harm of accepting this PR if it worked for everyone (within reason) but currently it appears to make these assumptions that seem like they might not be safe to make for more users:

  • requires "mysql" client installed locally (versus using the one built into docker container)
  • assumes hostname is "db"
  • assumes database name is "db"

I think those can be fixed by using ddev mysql in place of mysql as described here.

However, why not make a wrapper for n98 that can run all n98 commands? For example it might look like this:

ddev n98 admin:user:create [username] [email] [password] [firstname] [lastname] [role]

I also did not test but otherwise it looks ok.

@ADDISON74
Copy link
Contributor Author

@colinmollenhour - regarding the 3 bullets. Running DDEV requires the following commands:

ddev config
ddev start

By default, DDEV sets the name of the host and the database as db/db. You have to accept them as they are.

The command I created is run in the container named web, there the mysql command is available. DDEV does this "miracle". Running ddev mysql is a run outside the container.

I am an advocate of this command run in the DDEV web container because I use it. See also the command Sven created for installing sample data. I understand that you want to link n98 to do this job, but there are issues with it, it is not updated, On the other side his command does its job just put it on work.

@colinmollenhour
Copy link
Member

Thanks for the explanation @ADDISON74 - my apologies for doubting. 😄

@ADDISON74
Copy link
Contributor Author

I always appreciate a constructive discussion based on arguments. I am not a Docker fan like you are, but DDEV brought me closer to Docker and radically changed my testing environment. Do I want Apache or Nginx? No problem, I edit the configuration file and restart DDEV. Do I need to switch PHP 7.3 - 8.2? No problem, I edit the configuration file.

In addition, it also has nice add-ons such as ddev-cron which must be installed, ddev-browsersync. Composer is updated with ddev composer update. Everything is so simple that as an engineer I am ashamed to say it because many people try to complicate things rather than simplify them.

Through custom commands like this one we can solve a lot of issues that arise. Sven took a step after which I was inspired, the installation of the Sample Data pack. Running an OM instance doesn't take me very long now. Soon it will be 1 year since I haven't run a test in Linux.

@sreichel
Copy link
Contributor

sreichel commented Jul 7, 2023

Nice guys. :)

As soon i've time i'll finish n98 support for php8 (yeah, its far betten then custon commands).

@fballiano fballiano merged commit 4bdb36f into OpenMage:main Jul 7, 2023
3 checks passed
@fballiano fballiano changed the title DDEV command to create/update the administrator account Added DDEV command to create/update the administrator account Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants