feat(users-roles): manage database users, roles, and privileges for MySQL and PostgreSQL#1853
Conversation
…ers and roles view model
… search to the driver contract
…a with effective-privilege resolution
…with a scope tree and privilege checklist
…nd EmptyStateView calls
… leaking passwords to query history
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
datlechin
added
the
abi-additive
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 374619cbc2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| static let defaultHost = "%" | ||
|
|
||
| private static let excludedPrivileges: Set<String> = ["GRANT OPTION", "PROXY"] |
There was a problem hiding this comment.
When SHOW PRIVILEGES includes MySQL's USAGE pseudo-privilege, this catalog only filters GRANT OPTION and PROXY, so every no-privilege account parsed from SHOW GRANTS can show a server-level USAGE grant and the UI may stage meaningless/invalid REVOKE USAGE changes. Treat USAGE like the other non-editable privilege names so it does not appear as a grantable privilege.
Useful? React with 👍 / 👎.
| if let reassignTarget = options.reassignOwnedTo { | ||
| statements.append("REASSIGN OWNED BY \(role) TO \(quoteIdentifier(reassignTarget.name))") | ||
| statements.append("DROP OWNED BY \(role)") | ||
| } else if options.dropOwned { | ||
| statements.append("DROP OWNED BY \(role)") |
There was a problem hiding this comment.
For PostgreSQL roles that own objects in another database, principalOwnsObjects can detect the ownership through pg_shdepend, but the generated REASSIGN OWNED / DROP OWNED statements here only affect the current database before DROP ROLE runs. In that scenario the review flow promises reassignment or deletion, yet DROP ROLE still fails on the remaining dependencies; scope the ownership check to the current database or perform the cleanup in every database that has owned objects.
Useful? React with 👍 / 👎.
| private var duplicateName: Bool { | ||
| viewModel.principalRows.contains { $0.info.ref.name == trimmedName } | ||
| } |
There was a problem hiding this comment.
When host scoping is enabled, MySQL allows distinct accounts such as alice@% and alice@localhost, but this duplicate check only compares the user name. Creating a valid second account for a different host is therefore blocked by the sheet; include the host in the comparison whenever supportsPrincipalHostScoping is true.
Useful? React with 👍 / 👎.
…lying on run-loop coalescing
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00786c126f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| onOpenInEditor: { | ||
| coordinator?.loadQueryIntoEditor( | ||
| viewModel.previewSQL | ||
| .map { $0.hasSuffix(";") ? $0 : $0 + ";" } | ||
| .joined(separator: "\n\n") | ||
| ) |
There was a problem hiding this comment.
When the review includes CREATE USER / ALTER USER password SQL, previewSQL contains the plaintext password; sending it to loadQueryIntoEditor puts that text in a normal query tab, and normal tabs are persisted via PersistedTab.query by the tab persistence flow. This bypasses the new carriesCredentials history protection and can write database credentials to disk as soon as tab state is saved, so disable or redact this action for credential-carrying statements.
Useful? React with 👍 / 👎.
| .onAppear { | ||
| target = viewModel.connectedPrincipal ?? prompt.reassignCandidates.first | ||
| } |
There was a problem hiding this comment.
When the connected PostgreSQL role is also one of the roles being dropped and it owns objects, requestDrop excludes it from prompt.reassignCandidates, but this still seeds target with connectedPrincipal. The picker has no matching row while disposition is non-nil, so the dialog can generate REASSIGN OWNED ... TO the same/dropped role followed by DROP ROLE, which fails; only default to the connected principal if it is actually in the candidate list.
Useful? React with 👍 / 👎.
| let owningRefs = Set(owning.map(\.ref)) | ||
| let candidates = changeManager.principals | ||
| .map(\.ref) | ||
| .filter { !owningRefs.contains($0) } |
There was a problem hiding this comment.
In a multi-drop where role A owns objects and role B does not, B is staged for drop during the ownership check but remains in this candidate list because only owning roles are filtered out. If the user reassigns A's objects to B, applying the staged changes either targets a role already being dropped or makes B own objects just before its DROP ROLE, so the batch fails; exclude all requested refs and already-pending drops from reassignment candidates.
Useful? React with 👍 / 👎.
| private func apply() { | ||
| guard let info = viewModel.changeManager.principals.first(where: { $0.ref == principal }) | ||
| else { return } |
There was a problem hiding this comment.
When editing “Member Of” for a principal that was just created in this session, it exists only in pendingCreates, so this guard returns and the Done button silently discards the selected memberships. Since stageAlter already knows how to fold edits into a staged create and the create SQL emits definition.memberOf, build the current definition from pendingCreates as well as existing principals.
Useful? React with 👍 / 👎.
| let closure = PrivilegeEffectivenessResolver.roleClosure( | ||
| for: principal, | ||
| principals: principals | ||
| ) |
There was a problem hiding this comment.
After a PostgreSQL membership edit is staged, the new memberOf list lives in pendingAlters, but effective privilege resolution still computes the role closure from the original principals snapshot only. The Effective column therefore continues to show privileges from removed roles, or omits privileges from newly added roles, until apply/reload; merge pending alters into the principal data before resolving inheritance.
Useful? React with 👍 / 👎.
1 participant
Closes #1413.
Adds a Users & Roles tab for MySQL/MariaDB and PostgreSQL: browse the accounts on a server, see and edit their privileges from the server down through databases, schemas, tables, and columns, and create, alter, drop, and set passwords. Changes are staged, undoable, and shown as the exact SQL before they run.
Open it from View > Users & Roles. The command is disabled on drivers that do not support it.
Design
The privilege editor is a scope tree plus a privilege checklist, not a matrix. A principal x privilege grid collapses at real scale (MySQL has ~30 static plus ~40 dynamic privileges), and one click on a "Server" row would grant
SUPER,SHUTDOWN,FILEand 40 more at once. Instead: anNSOutlineViewof objects, lazily loaded, beside a checklist scoped to what is actually grantable on the selected object. The tree carries one summary column, which is what the HIG allows an outline view to do, and it answers "what does this account have onshop.orders?" without a click.The UI does not lie about access. An unchecked box is not the same as "this user cannot do this". An Effective column resolves both inheritance axes:
NOINHERIT(the privilege only applies afterSET ROLE).PostgreSQL cannot read table ACLs for a database it is not connected to. Those databases still appear and still show their cluster-wide database-level grants, marked as not browsable. They are never shown as though the account had no privileges there.
Changes are a baseline plus a delta. Refresh (⌘R) re-reads the server and rebases: intent the server already satisfies quietly disappears. Unchecking a box you just checked cancels out instead of emitting a redundant
REVOKE, and a grant that was grantable to others stays grantable.Plugin API
Five new requirements on
PluginPrincipalManagement, all with default implementations:privilegeCascades(from:to:)andrestrictsGrantBrowsingToCurrentDatabasedrive the two points above.supportsGrantableScopeSearch/searchGrantableScopes(matching:limit:)back the object search.rollsBackPrincipalStatementsexists because MySQL implicit-commits DDL. Claiming a rollback that did not happen would be a lie, so a partial failure reports how many statements actually landed and rebases the diff to the truth.scripts/check-pluginkit-abi.shreports the change as purely additive: no symbol removed, no initializer signature changed. NocurrentPluginKitVersionbump and no plugin re-release.Safety
ExecutionGateProvider.authorize, so read-only connections and Safe Mode govern it exactly as they govern any other write. Drops force.destructiveQuery._and%in the database position as LIKE wildcards for global and database-level grants, but as literal characters in a table-level target. Both directions are handled, and round-tripped in tests.CREATE USER/ALTER USERstatements embed a plaintext password, so they are never written to the on-disk query history.Scope
Not covered, deliberately: editing
WITH GRANT OPTION(it is displayed, and provably never stripped),ALTER DEFAULT PRIVILEGES, MySQL 8 roles, and renaming an account. Each is documented indocs/features/users-roles.mdxwith its reason.Testing
Unit tests cover the change model (delta semantics, rebase, undo), effective-privilege resolution under both engines' cascade rules, transitive and cycle-safe role closure, scope summaries, the privilege catalog, password generation, and MySQL grant parsing and escaping round-trips.
swiftlint lint --strictis clean. No UI automation for the privilege grid yet; the existingTableProUITestshas no pattern for driving a customNSOutlineView.Note for the reviewer
TablePro/Resources/Localizable.xcstringsis not in this PR. Xcode regenerates it on build; committing the regenerated file here would have mixed in unrelated churn from the working tree. It should be regenerated and committed separately.