Marek Miklewicz
722ce5beaa
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.
145 lines
5.5 KiB
Markdown
145 lines
5.5 KiB
Markdown
# 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.
|