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

[Nextcloud] Fix database creation and Nginx config #1226

Merged
merged 9 commits into from
Nov 10, 2017
Merged

[Nextcloud] Fix database creation and Nginx config #1226

merged 9 commits into from
Nov 10, 2017

Conversation

MichaIng
Copy link
Owner

@MichaIng MichaIng commented Nov 5, 2017

  • Fix MySQL database creation
  • Remove database user on uninstallation
  • Fix Nginx config for non-SSL environment
  • Check for SSL connectivity and adjust Nginx config
  • Allow self-signed certificates on SSL check

  • Database was created with wrong privileges for user root, thus user oc_admin is not able to access.
  • On top, occ maintenance:install creates the database and user automatically, thus if create_mysql_db is used to create database with correct user oc_admin (_$username), Nextcloud will create oc_admin1 as user via occ maintenance:install and use it.
    => Removing create_mysql_db works fine for me, Nextcloud is accessible.

Actually I would also like to remove the user on Nextcloud uninstallation, but I am not too good in creating the right command to get the right string from nextcloud/config/config.php 😅.

- Database was created with wrong privileges for user "root", thus user "oc_admin" is not able to access.
- On top, "occ maintenance:install" creates the database and user automatically, thus if "create_mysql_db" is used to create database with correct user oc_admin (<prefix>_$username), Nextcloud will create "oc_admin1" as user via "occ maintenance:install" and use it.
=> Remove "create_mysql_db" works fine for me, Nextcloud is accessible.
@MichaIng
Copy link
Owner Author

MichaIng commented Nov 5, 2017

Tested with VirtualBox VMs on Stretch (MariaDB) and Jessie (MySQL).

- Put "fastcgi_param PHP_ADMIN_VALUE ..." into \.php location, where is works.
- Add X-Headers to the location base, so they are active for the whole location and prevent Nextcloud admin panel warnings.
- Comment out "fastcgi_param HTTPS on;" to prevent auto redirect to https:// on non-ssl environments => Uncommenting on SSL activation needs to be added.
- Uncommenting "fastcgi_request_buffering off;" on Stretch.

Tested on Jessie and Stretch VM.
@MichaIng MichaIng changed the title [Nextcloud] Fix database creation [Nextcloud] Fix database creation and Nginx config Nov 5, 2017
@MichaIng
Copy link
Owner Author

MichaIng commented Nov 5, 2017

I added fixes for Nginx configuration.

@Fourdee
Now we need to reliably check, if we have a SSL environment to uncomment the HTTPS directive. curl -v https://localhost and checking for "port 443 failed" would be a way I guess?
dietpi-letsencrypt on Nginx, if ran without error, should also check for Nextcloud installation and uncomment the line.

@Fourdee
Copy link
Collaborator

Fourdee commented Nov 6, 2017

@MichaIng

Looks good, i'll do a full review when I can 👍

the right command to get the right string from nextcloud/config/config.php

If you can give me example of line/value that exists and what you want it to be pulled in as, i'll see if I can assist?

Now we need to reliably check, if we have a SSL environment to uncomment the HTTPS directive. curl -v https://localhost and checking for "port 443 failed" would be a way I guess?

root@DietPi:~# /DietPi/dietpi/func/check_connection http://localhost;echo $?
 [Info] Testing connection to http://localhost
 [Info] Max duration of 20 seconds, please wait...
 [Ok] Connection test | Completed

0

root@DietPi:~# /DietPi/dietpi/func/check_connection https://localhost;echo $?
 [Info] Testing connection to https://localhost
 [Info] Max duration of 20 seconds, please wait...
 [Failed] Connection test | An issue has occured

4

In theory:

if (( $(/DietPi/dietpi/func/check_connection https://localhost) > 0 )); then

echo -e "non-ssl"

fi

@MichaIng
Copy link
Owner Author

MichaIng commented Nov 6, 2017

@Fourdee

If you can give me example of line/value that exists and what you want it to be pulled in as, i'll see if I can assist?

The /var/www/nextcloud/config/config.php contains the following lines, where in this case 'oc_admin'@'localhost' is the database user, Nextcloud uses for access:

'dbhost' => 'localhost',
'dbuser' => 'oc_admin',

I just realized that occ maintenance:install by dietpi-software creates two users: oc_admin@localhost and oc_admin@%, This also happens if the option --database-host "localhost" is given, which defaults anyway. We could directly remove the non-localhost user, as it remains unused on local databases.


root@DietPi:~# /DietPi/dietpi/func/check_connection http://localhost;echo $?

Ah nice 🙂. Would implement it also in dietpi-letsencrypt. Shell we also do this on every dietpi-software run, on boot, dietpi-services or something, to recheck and in case adjust regularly, if the user enables SSL via different way? Ah just the 20 seconds are quite long, what you think? Maybe we implement an option to check_connection that checks faster, like the curl check (almost instantly), to check after dhcp IP received, modules and services related to network should be loaded?

CLI URL also needs to be switched to HTTPS on SSL environments
@MichaIng
Copy link
Owner Author

MichaIng commented Nov 7, 2017

About last commit: php7.0-opcache is dependency of every Stretch PHP package (mod-php, php-fpm, php-cgi)

@Fourdee
Copy link
Collaborator

Fourdee commented Nov 8, 2017

@MichaIng

Shell we also do this on every dietpi-software run, on boot, dietpi-services or something, to recheck and in case adjust regularly, if the user enables SSL via different way?

This check is to determine if we need to apply a SSL or non-SSL nginx configuration? If so, dietpi-software only for now.

Maybe we implement an option to check_connection that checks faster, like the curl check (almost instantly), to check after dhcp IP received, modules and services related to network should be loaded?

If you can find a quicker and stable replacement for the current wget --spider, let me know.

@MichaIng
Is this PR now completed and ready for merging?

@MichaIng
Copy link
Owner Author

MichaIng commented Nov 8, 2017

If you can find a quicker and stable replacement for the current wget --spider, let me know.

Ah no, if the check gets a (negative) response, it doesn't check for 20s but directly gives the negative result. So everything is fine. Will implement it on Nextcloud installation.

@Fourdee

Is this PR now completed and ready for merging?

Not yet, will implement SSL check + config adjustments in about 2 hours, afterwards will be ready to merge.

€: Could you give me a hint for the right command to get the database user(+host) from config.php (see above)?

€€: Corrected some comment typos and spacing here and there 😉.

@MichaIng
Copy link
Owner Author

MichaIng commented Nov 9, 2017

Ah, wget --spider also checks certificate and fails if self signed. Could simply add $2 to wget line of check_connection script to allow adding --no-check-certificate, or directly call wget here for now.

€: For now I just also allow error code 5 here.

@Fourdee
Copy link
Collaborator

Fourdee commented Nov 9, 2017

@MichaIng

Legend, thanks 👍

Ran out of time today, i'll check PR and merge in next day/two.

€: Could you give me a hint for the right command to get the database user(+host) from config.php (see above)?

Quick solution. You could probably optimize this to do it one sed.

root@DietPi:~# grep -m1 "^'dbhost'" config.php | awk '{print $3}' | sed "s/'//g" | sed "s/,//g"
localhost

root@DietPi:~# grep -m1 "^'dbuser'" config.php | awk '{print $3}' | sed "s/'//g" | sed "s/,//g"
oc_admin

- occ maintenance:install also creates obsolete 'oc_admin'@'%' mysql user, which can directly be removed. 
- The effort to get user and host is actually a bid high, as we exactly know the defaults that Nextcloud creates.
- But at least on removal we do not know, if user changed something, for whatever reason.
- Tiny database cleanup 😉
- Needed to add $GLOBAL_PW for this, copied from dietpi-software.
@MichaIng
Copy link
Owner Author

grep -m1 "^'dbuser'" config.php | awk '{print $3}' | sed "s/'//g" | sed "s/,//g"

Just realized that the ' perfectly fits into the mysql command, thus not necessary to /s it 🙂.
Added and commands tested with my VMs.

  • Ready to merge 👍


#Global Password: Exception to AUTO first run init.
GLOBAL_PW=$(cat /DietPi/dietpi.txt | grep -m1 '^AUTO_Global_Password=' | sed 's/.*=//')
if [ ! -n "$GLOBAL_PW" ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice 👍

@Fourdee
Copy link
Collaborator

Fourdee commented Nov 10, 2017

@MichaIng

Looks good, great work, many thanks 👍

@Fourdee Fourdee merged commit 7d4ef53 into MichaIng:testing Nov 10, 2017
Fourdee referenced this pull request Nov 10, 2017
+ avoid grep: /var/www/nextcloud/config/config.php: No such file or
directory during patch: https://github.com/Fourdee/DietPi/issues/1067
https://github.com/Fourdee/DietPi/pull/1226
@flappysquirrel
Copy link

flappysquirrel commented Nov 15, 2017

I apologize if this is out of place, but I'm getting an http error 500 on a fresh (single) install of Nextcloud and Dietpi v158 (main branch), Raspberry Pi 3. Is this related?

@MichaIng MichaIng deleted the patch-1 branch November 15, 2017 13:23
@MichaIng
Copy link
Owner Author

@flappysquirrel Could be related to https://github.com/Fourdee/DietPi/issues/1233

  • Are you on Jessie or Stretch?
  • Did you actively choose MariaDB database stack in the front?
  • Could you check manually if the 'nextcloud' database was actually created by installation script?

@flappysquirrel
Copy link

Thank you, I've also made another reinstall and filed a bug report about this, my reference code is: 9ef16341-5f32-4baf-acc2-7bd706d08d6d-0

Should I open an issue? I'm new to this.

To answer your questions:

Are you on Jessie or Stretch?

Jessie.

Did you actively choose MariaDB database stack in the front?

Nope, I changed the log format to full, then chose 'Nextcloud'; ran the installer, and filed the bug report.

Could you check manually if the 'nextcloud' database was actually created by installation script?

I think so. If this is right, I ran mysql -u root -p, followed by SHOW DATABASES; and got the following:

mysql> SHOW DATABASES;
+--------------------+
| Database           |
+--------------------+
| information_schema |
| mysql              |
| nextcloud          |
| performance_schema |
+--------------------+
4 rows in set (0.01 sec)

@MichaIng
Copy link
Owner Author

@flappysquirrel But as you posted it here, you chose nginx as webserver first, or didn't you switch that eather and lighttpd is installed?

I rust tried with nginx and ran into a crazy database privilege error. occ installation went successfull, right user was created and linked in config.php, just his privileges were somehow lost. Granting them again, solved the problem and enabled access to nextcloud web ui: mysql> grant all privileges on nextcloud.* to 'oc_admin'@'localhost'

So far, will create PR in the evening.

@MichaIng
Copy link
Owner Author

@flappysquirrel
Fixed by https://github.com/Fourdee/DietPi/pull/1242

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants