Opened 2 years ago

Closed 23 months ago

Last modified 23 months ago

#2963 closed Bug (fixed)

If mysql user does not have sufficient privileges, Piwik install fails "website time zone is not valid"

Reported by: matt Owned by:
Priority: major Milestone: Piwik 1.8
Component: Core Keywords:
Cc: Sensitive: no

Description

Reported in forum:

Rather than failing with error message "timezone not valid"

  • we should display a proper error message "User does not have privilege"
  • update the requirement documentation to detail which privileges are required

Problem in code:

This issue arises from a call to Piwik_SetOption (when it's called from setDefaultTimezone) that causes an exception to be thrown because the public function "set" in Option.php fails at it's database call. This is due to insufficient database user privileges. Granting the piwik DB user more privileges allows the setup to continue.

Maybe we could have a page that checks the database user grants before continuing the setup. Or at-least some docs on the needed permissions for the piwik DB user since not everyone will be doing a "grant all".

Attachments (1)

2963.diff.tar.gz (3.4 KB) - added by capedfuzz 2 years ago.
Patch for this issue.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 2 years ago by vipsoft (robocoder)

Checking the privileges by looking at SHOW GRANTS, or mysql.user and mysql.db tables, is non-trivial.

It would probably be easier (and more database-agnostic) to explicitly test the various operations used by Piwik, e.g.,

  • CREATE/ALTER, SELECT, INSERT, UPDATE, DELETE, etc

comment:2 Changed 2 years ago by matt (mattab)

  • Priority changed from normal to major

comment:3 Changed 2 years ago by matt (mattab)

  • Milestone changed from 1.7.x - Piwik 1.7.x to 1.7.2 - Piwik 1.7.2

Changed 2 years ago by capedfuzz (diosmosis)

Patch for this issue.

comment:4 Changed 2 years ago by capedfuzz (diosmosis)

I uploaded a patch for this issue. This is the first time I've looked at the installation code, so I believe it would be good to review them.

Notes on the patch:

  • I use the strategy @vipsoft mentioned.
  • The check is provided as an extra form validation for the database setup page. If the user doesn't have the right privileges, a message displaying all the required privileges is displayed. As far as I can tell, its not possible to make a validation error message depend on the form values themselves, so that's as specific as it gets.

Let me know what you think.

comment:5 Changed 23 months ago by matt (mattab)

Thanks vipsoft for the suggestion, and Kuddos capedfuzz for this clean patch!
Code review:

  • I would maybe detail the error a bit more in Installation_InsufficientPrivileges maybe you can add something along the lines of "You can use a tool such as phpMyAdmin (or a SQL query). If you do not know what this means, please ask your sysadmin to grant these privileges to the %s user".
  • TEST_TABLE_NAME and TEST_TEMP_TABLE_NAME couild be "piwik_test_table_tmp" and "piwik_test_table" just in case they're somehow left in there
  • can you confirm you checked individually each missing right and that the code works in all the cases? (I'm thinking in particular if the error codes in isAccessDenied() would cover all use cases of missing permissions)


comment:6 Changed 23 months ago by capedfuzz (diosmosis)

  • Resolution set to fixed
  • Status changed from new to closed

(In [6371]) Fixes #2963, added installation check for needed DB user privileges.

comment:7 Changed 23 months ago by capedfuzz (diosmosis)

Regarding my commit: I tested w/ every privilege, every needed privilege, w/ every needed privilege except one (for each privilege), and w/ no privileges. Also tested to make sure other fields were checked properly. Everything worked.

Note: See TracTickets for help on using tickets.