1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-18 21:02:41 +01:00

Make bin/storage upgrade and bin/storage adjust emit detailed messages if the user has no access to databases

Summary:
Ref T10195. Distinguish between "database does not exist" and "database exists, you just don't have permission to access it".

We can't easily get this information out of INFORMATION_SCHEMA but can just `SHOW TABLES IN ...` every database that looks like it's missing and then look at the error code.

Test Plan:
  - Created a user `limited` with limited access.
  - Ran `bin/storage adjust`.
  - Got hopefully more helpful messages about access problems, instead of "Missing" errors.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10195

Differential Revision: https://secure.phabricator.com/D15079
This commit is contained in:
epriestley 2016-01-21 12:59:19 -08:00
parent 7bf4ef451b
commit d37153f003
5 changed files with 94 additions and 6 deletions

View file

@ -6,6 +6,7 @@ final class PhabricatorConfigDatabaseSchema
private $characterSet; private $characterSet;
private $collation; private $collation;
private $tables = array(); private $tables = array();
private $accessDenied;
public function addTable(PhabricatorConfigTableSchema $table) { public function addTable(PhabricatorConfigTableSchema $table) {
$key = $table->getName(); $key = $table->getName();
@ -33,12 +34,16 @@ final class PhabricatorConfigDatabaseSchema
PhabricatorConfigStorageSchema $expect) { PhabricatorConfigStorageSchema $expect) {
$issues = array(); $issues = array();
if ($this->getCharacterSet() != $expect->getCharacterSet()) { if ($this->getAccessDenied()) {
$issues[] = self::ISSUE_CHARSET; $issues[] = self::ISSUE_ACCESSDENIED;
} } else {
if ($this->getCharacterSet() != $expect->getCharacterSet()) {
$issues[] = self::ISSUE_CHARSET;
}
if ($this->getCollation() != $expect->getCollation()) { if ($this->getCollation() != $expect->getCollation()) {
$issues[] = self::ISSUE_COLLATION; $issues[] = self::ISSUE_COLLATION;
}
} }
return $issues; return $issues;
@ -68,4 +73,13 @@ final class PhabricatorConfigDatabaseSchema
return $this->characterSet; return $this->characterSet;
} }
public function setAccessDenied($access_denied) {
$this->accessDenied = $access_denied;
return $this;
}
public function getAccessDenied() {
return $this->accessDenied;
}
} }

View file

@ -47,6 +47,25 @@ final class PhabricatorConfigSchemaQuery extends Phobject {
$databases); $databases);
$database_info = ipull($database_info, null, 'SCHEMA_NAME'); $database_info = ipull($database_info, null, 'SCHEMA_NAME');
// Find databases which exist, but which the user does not have permission
// to see.
$invisible_databases = array();
foreach ($databases as $database_name) {
if (isset($database_info[$database_name])) {
continue;
}
try {
queryfx($conn, 'SHOW TABLES IN %T', $database_name);
} catch (AphrontAccessDeniedQueryException $ex) {
// This database exists, the user just doesn't have permission to
// see it.
$invisible_databases[] = $database_name;
} catch (AphrontSchemaQueryException $ex) {
// This database is legitimately missing.
}
}
$sql = array(); $sql = array();
foreach ($tables as $table) { foreach ($tables as $table) {
$sql[] = qsprintf( $sql[] = qsprintf(
@ -148,6 +167,13 @@ final class PhabricatorConfigSchemaQuery extends Phobject {
$server_schema->addDatabase($database_schema); $server_schema->addDatabase($database_schema);
} }
foreach ($invisible_databases as $database_name) {
$server_schema->addDatabase(
id(new PhabricatorConfigDatabaseSchema())
->setName($database_name)
->setAccessDenied(true));
}
return $server_schema; return $server_schema;
} }
@ -194,6 +220,7 @@ final class PhabricatorConfigSchemaQuery extends Phobject {
if (!$actual_database) { if (!$actual_database) {
$actual_database = $expect_database->newEmptyClone(); $actual_database = $expect_database->newEmptyClone();
} }
if (!$expect_database) { if (!$expect_database) {
$expect_database = $actual_database->newEmptyClone(); $expect_database = $actual_database->newEmptyClone();
} }

View file

@ -17,6 +17,7 @@ abstract class PhabricatorConfigStorageSchema extends Phobject {
const ISSUE_SUBFAIL = 'subfail'; const ISSUE_SUBFAIL = 'subfail';
const ISSUE_AUTOINCREMENT = 'autoincrement'; const ISSUE_AUTOINCREMENT = 'autoincrement';
const ISSUE_UNKNOWN = 'unknown'; const ISSUE_UNKNOWN = 'unknown';
const ISSUE_ACCESSDENIED = 'accessdenied';
const STATUS_OKAY = 'okay'; const STATUS_OKAY = 'okay';
const STATUS_WARN = 'warn'; const STATUS_WARN = 'warn';
@ -130,6 +131,8 @@ abstract class PhabricatorConfigStorageSchema extends Phobject {
return pht('Column has Wrong Autoincrement'); return pht('Column has Wrong Autoincrement');
case self::ISSUE_UNKNOWN: case self::ISSUE_UNKNOWN:
return pht('Column Has No Specification'); return pht('Column Has No Specification');
case self::ISSUE_ACCESSDENIED:
return pht('Access Denied');
default: default:
throw new Exception(pht('Unknown schema issue "%s"!', $issue)); throw new Exception(pht('Unknown schema issue "%s"!', $issue));
} }
@ -175,6 +178,7 @@ abstract class PhabricatorConfigStorageSchema extends Phobject {
public static function getIssueStatus($issue) { public static function getIssueStatus($issue) {
switch ($issue) { switch ($issue) {
case self::ISSUE_MISSING: case self::ISSUE_MISSING:
case self::ISSUE_ACCESSDENIED:
case self::ISSUE_SURPLUS: case self::ISSUE_SURPLUS:
case self::ISSUE_NULLABLE: case self::ISSUE_NULLABLE:
case self::ISSUE_SUBFAIL: case self::ISSUE_SUBFAIL:

View file

@ -123,6 +123,16 @@ final class PhabricatorStorageManagementAPI extends Phobject {
'SELECT patch FROM %T', 'SELECT patch FROM %T',
self::TABLE_STATUS); self::TABLE_STATUS);
return ipull($applied, 'patch'); return ipull($applied, 'patch');
} catch (AphrontAccessDeniedQueryException $ex) {
throw new PhutilProxyException(
pht(
'Failed while trying to read schema status: the database "%s" '.
'exists, but the current user ("%s") does not have permission to '.
'access it. GRANT the current user more permissions, or use a '.
'different user.',
$this->getDatabaseName('meta_data'),
$this->getUser()),
$ex);
} catch (AphrontQueryException $ex) { } catch (AphrontQueryException $ex) {
return null; return null;
} }

View file

@ -422,6 +422,11 @@ abstract class PhabricatorStorageManagementWorkflow
continue; continue;
} }
if ($actual_database->getAccessDenied()) {
// If we can't access the database, we can't access the tables either.
continue;
}
$issues = array(); $issues = array();
if ($database->hasIssue($issue_charset)) { if ($database->hasIssue($issue_charset)) {
$issues[] = $issue_charset; $issues[] = $issue_charset;
@ -634,6 +639,8 @@ abstract class PhabricatorStorageManagementWorkflow
$any_surplus = false; $any_surplus = false;
$all_surplus = true; $all_surplus = true;
$any_access = false;
$all_access = true;
foreach ($errors as $error) { foreach ($errors as $error) {
$pieces = array_select_keys( $pieces = array_select_keys(
$error, $error,
@ -643,12 +650,20 @@ abstract class PhabricatorStorageManagementWorkflow
$name = PhabricatorConfigStorageSchema::getIssueName($error['issue']); $name = PhabricatorConfigStorageSchema::getIssueName($error['issue']);
if ($error['issue'] === PhabricatorConfigStorageSchema::ISSUE_SURPLUS) { $issue = $error['issue'];
if ($issue === PhabricatorConfigStorageSchema::ISSUE_SURPLUS) {
$any_surplus = true; $any_surplus = true;
} else { } else {
$all_surplus = false; $all_surplus = false;
} }
if ($issue === PhabricatorConfigStorageSchema::ISSUE_ACCESSDENIED) {
$any_access = true;
} else {
$all_access = false;
}
$table->addRow( $table->addRow(
array( array(
'target' => $target, 'target' => $target,
@ -668,11 +683,24 @@ abstract class PhabricatorStorageManagementWorkflow
'does not expect). For information on resolving these '. 'does not expect). For information on resolving these '.
'issues, see the "Surplus Schemata" section in the "Managing Storage '. 'issues, see the "Surplus Schemata" section in the "Managing Storage '.
'Adjustments" article in the documentation.'); 'Adjustments" article in the documentation.');
} else if ($all_access) {
$message[] = pht(
'The user you are connecting to MySQL with does not have the correct '.
'permissions, and can not access some databases or tables that it '.
'needs to be able to access. GRANT the user additional permissions.');
} else { } else {
$message[] = pht( $message[] = pht(
'The schemata have errors (detailed above) which the adjustment '. 'The schemata have errors (detailed above) which the adjustment '.
'workflow can not fix.'); 'workflow can not fix.');
if ($any_access) {
$message[] = pht(
'Some of these errors are caused by access control problems. '.
'The user you are connecting with does not have permission to see '.
'all of the database or tables that Phabricator uses. You need to '.
'GRANT the user more permission, or use a different user.');
}
if ($any_surplus) { if ($any_surplus) {
$message[] = pht( $message[] = pht(
'Some of these errors are caused by surplus schemata (extra '. 'Some of these errors are caused by surplus schemata (extra '.
@ -698,6 +726,11 @@ abstract class PhabricatorStorageManagementWorkflow
"**<bg:yellow> %s </bg>**\n\n%s\n", "**<bg:yellow> %s </bg>**\n\n%s\n",
pht('SURPLUS SCHEMATA'), pht('SURPLUS SCHEMATA'),
phutil_console_wrap($message)); phutil_console_wrap($message));
} else if ($all_access) {
$console->writeOut(
"**<bg:yellow> %s </bg>**\n\n%s\n",
pht('ACCESS DENIED'),
phutil_console_wrap($message));
} else { } else {
$console->writeOut( $console->writeOut(
"**<bg:red> %s </bg>**\n\n%s\n", "**<bg:red> %s </bg>**\n\n%s\n",