Add design spec for phpMyAdmin all-databases top-nav access
Documents the approved design for restoring "grant access to every owned database" behavior specifically for the top-nav PHPMYADMIN button, while keeping the per-row "Zaloguj do bazy" button scoped to one database.
This commit is contained in:
@@ -0,0 +1,144 @@
|
||||
# phpMyAdmin: all-databases access from the top-nav button
|
||||
|
||||
## Context
|
||||
|
||||
The plugin's page header (`AppContext::renderTopActions()`) renders a
|
||||
top-level "PHPMYADMIN" button, separate from the per-row "Zaloguj do
|
||||
bazy" button in the database table. Both POST to the same
|
||||
`open_phpmyadmin.raw` action and go through `PhpMyAdminSso`, but they
|
||||
differ in one respect: the row button always sends `adminer_db=<name>`;
|
||||
the top-nav button never sends `adminer_db` at all.
|
||||
|
||||
A recent security fix (scoping the temporary SSO MySQL role to only the
|
||||
requested database instead of every database the account owns) treated
|
||||
both cases identically: when no database was requested, it defaulted to
|
||||
the first owned database and granted only that one. This is correct for
|
||||
the row button, but broke the top-nav button's intent, which is to open
|
||||
phpMyAdmin with access to every database the DirectAdmin user owns.
|
||||
|
||||
## Goal
|
||||
|
||||
Restore "access to all owned databases" for the top-nav PHPMYADMIN
|
||||
button, without reintroducing over-broad access for the per-row button.
|
||||
|
||||
## Design
|
||||
|
||||
### `PhpMyAdminSso::issueLoginPayload()`
|
||||
|
||||
Treat "no database requested" (`$requestedDatabase === null` or empty
|
||||
after trim) as its own explicit case, not as "default to the first
|
||||
database and scope to it."
|
||||
|
||||
Extract a pure, testable decision function:
|
||||
|
||||
```php
|
||||
/**
|
||||
* @param string[] $ownedDatabases
|
||||
* @return string[]
|
||||
*/
|
||||
private static function determineGrantTargets(?string $requestedDatabase, array $ownedDatabases): array
|
||||
{
|
||||
if ($requestedDatabase === null || $requestedDatabase === '') {
|
||||
return $ownedDatabases;
|
||||
}
|
||||
return in_array($requestedDatabase, $ownedDatabases, true) ? [$requestedDatabase] : [];
|
||||
}
|
||||
```
|
||||
|
||||
This replaces the current `determineGrantTargets(string $targetDatabase, array $ownedDatabases)`,
|
||||
which took the already-resolved target database and could no longer
|
||||
distinguish "explicitly requested this one" from "nothing was requested."
|
||||
|
||||
`issueLoginPayload()` keeps resolving a concrete `$targetDatabase` for
|
||||
the phpMyAdmin *landing page* route (`route=/database/structure&db=...`)
|
||||
exactly as it does today — defaulting to the first owned database when
|
||||
none was requested. That landing behavior is unaffected; only the grant
|
||||
scope and the `only_db` restriction change.
|
||||
|
||||
A new local value, `$restrictToDatabase`, is derived alongside the
|
||||
grant targets: empty string when granting all databases, or the single
|
||||
target database name when scoped to one. This value is written into the
|
||||
ticket payload as a new field, `restrict_db`, separate from the
|
||||
existing `db` field (which continues to mean "which database to land
|
||||
on," not "which database(s) to allow").
|
||||
|
||||
### Ticket payload
|
||||
|
||||
```json
|
||||
{
|
||||
"version": 1,
|
||||
"created_at": 0,
|
||||
"expires_at": 0,
|
||||
"auth": { "user": "...", "password": "...", "host": "...", "port": 0, "db": "..." },
|
||||
"route": "/database/structure",
|
||||
"db": "landing_database",
|
||||
"restrict_db": ""
|
||||
}
|
||||
```
|
||||
|
||||
`restrict_db` is `""` for the top-nav (all-databases) case, or the
|
||||
specific database name for the per-row case.
|
||||
|
||||
### `da_login.php` (generated by `phpmyadmin_install.sh`)
|
||||
|
||||
When consuming a ticket, store `restrict_db` into the SSO session under
|
||||
a new key, alongside the existing auth payload:
|
||||
|
||||
```php
|
||||
$_SESSION['DA_MYSQL_PHPMYADMIN_AUTH'] = $ticket['auth'];
|
||||
$_SESSION['DA_MYSQL_PHPMYADMIN_RESTRICT_DB'] = (string)($ticket['restrict_db'] ?? '');
|
||||
```
|
||||
|
||||
### `config.inc.php`'s `only_db` builder
|
||||
|
||||
The existing `da_mysql_phpmyadmin_only_db()` function currently reads
|
||||
`$_SESSION['DA_MYSQL_PHPMYADMIN_AUTH']['db']`. It will instead read the
|
||||
new `DA_MYSQL_PHPMYADMIN_RESTRICT_DB` session key, defaulting to `''`
|
||||
(no restriction) when absent. An empty value means phpMyAdmin's
|
||||
`only_db` stays unset, showing every database the temporary role has
|
||||
been granted.
|
||||
|
||||
## Data flow
|
||||
|
||||
- Row button: POST includes `adminer_db=admin_wiesio` → handler passes
|
||||
`'admin_wiesio'` to `issueLoginPayload()` → grants only `admin_wiesio`,
|
||||
`restrict_db=admin_wiesio` → phpMyAdmin shows only that database.
|
||||
- Top-nav button: POST has no `adminer_db` field → handler's existing
|
||||
`if ($requestedDb === '') { $requestedDb = null; }` passes `null` →
|
||||
`issueLoginPayload(null)` → grants every owned database,
|
||||
`restrict_db=''` → phpMyAdmin shows all databases, landing on the
|
||||
first one's structure page (unchanged from today).
|
||||
|
||||
## Error handling
|
||||
|
||||
Unchanged. If the DirectAdmin user owns zero databases, both entry
|
||||
points still throw "Użytkownik nie ma żadnej bazy MySQL do otwarcia w
|
||||
phpMyAdmin." before reaching the grant-scope logic.
|
||||
|
||||
## Naming
|
||||
|
||||
No new "Adminer"-branded identifiers. New field/session key names
|
||||
(`restrict_db`, `DA_MYSQL_PHPMYADMIN_RESTRICT_DB`) follow the existing
|
||||
`da_mysql_*` / `DA_MYSQL_*` convention already used throughout the
|
||||
generated phpMyAdmin integration files. Pre-existing legacy names like
|
||||
`MySQLService::grantTemporaryAdminerAccess()` are out of scope for this
|
||||
change.
|
||||
|
||||
## Testing
|
||||
|
||||
- `tests/phpmyadmin_sso_test.php`: update the existing
|
||||
`determineGrantTargets` assertions for the new `?string` signature,
|
||||
and add a case asserting that `null`/`''` returns every owned
|
||||
database.
|
||||
- `tests/phpmyadmin_install_test.sh`: extend the existing `only_db`
|
||||
PHP-fixture check with a second scenario — a session with
|
||||
`DA_MYSQL_PHPMYADMIN_RESTRICT_DB` unset/empty — asserting
|
||||
`$cfg['Servers'][1]['only_db']` is `''` (not the landing `db` value).
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Renaming legacy "Adminer"-named methods/settings elsewhere in the
|
||||
codebase.
|
||||
- Any change to the row button's behavior (already correct).
|
||||
- Any new UI element — the top-nav button already exists and already
|
||||
omits `adminer_db`; no markup changes are needed.
|
||||
Reference in New Issue
Block a user