diff --git a/CHANGELOG.md b/CHANGELOG.md index 29f09f21..25b43d2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,32 +9,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Windows and CLI: **the Azure tab is a scope tree, and a subtree is one click.** Azure resource - eligibility does not scale in a flat list — a platform engineer eligible for Contributor on sixty - subscriptions had sixty sibling rows to read and sixty clicks before the single activation Elevate - promised. The Windows **Azure** pivot now groups its rows by the hierarchy the ARM scope string - already describes: management group, subscription, resource group, resource, each header opening - and closing and saying how many roles sit under it. A scope that only passes through — no - eligibility of its own, one way down — is folded into the node below and named there - ("Alpha / prod"), so a narrow panel spends no row or indent on nothing. In select mode a scope - header carries its own checkbox that takes every eligibility under it at once. The search box - narrows the tree rather than sitting beside it, now reaches the whole ARM path, and keeps matches - in their place in the tree even under a header you had closed. For scripts, `elevate` gains - `--under ` (everything at or below a resource group or subscription name, a subscription - id, or a whole path, compared step by step so `/subscriptions/abc` never swallows - `/subscriptions/abcdef`) and a glob form of `--scope` where a `*` crosses slashes - (`--scope "/subscriptions/*"`); `--scope` without a `*` is the substring search it always was. - `elevate activate --all` then takes every role the filters and names match instead of insisting - each name picks exactly one, which is the scripted form of the subtree checkbox. Management - groups sit beside the subscriptions rather than above them, in the panel and for `--under`: ARM - writes a management group scope as its own flat path and never repeats it in a subscription's, so - the eligibilities alone cannot say which subscriptions belong to which management group. The - macOS Azure tab still lists flat; its tree follows. - ([#186](https://github.com/FrodeHus/elevate/issues/186)) -- macOS: the panel's search box reaches an Azure role's whole scope path, not just the caption the - row shows, so typing part of a subscription id or a resource group that is only named in the path - now finds it. The rest of the Azure tab's scope tree follows. - ([#194](https://github.com/FrodeHus/elevate/issues/194)) +- macOS, Windows and CLI: **the Azure tab is a scope tree, and a subtree is one click.** Azure + resource eligibility does not scale in a flat list — a platform engineer eligible for Contributor + on sixty subscriptions had sixty sibling rows to read and sixty clicks before the single + activation Elevate promised. The **Azure** tab now groups its rows by the hierarchy the ARM scope + string already describes: management group, subscription, resource group, resource, each header + opening and closing and saying how many roles sit under it. A scope that only passes through — no + eligibility of its own, one way down — is folded into the node below and named there ("Alpha / + prod"), and a scope leading to a single role gets no header at all, so a narrow panel spends no + row or indent on nothing. In select mode a scope header carries its own checkbox that takes every + eligibility under it at once. The search box narrows the tree rather than sitting beside it, now + reaches the whole ARM path, and keeps matches in their place in the tree even under a header you + had closed. For scripts, `elevate` gains `--under ` (everything at or below a resource + group or subscription name, a subscription id, or a whole path, compared step by step so + `/subscriptions/abc` never swallows `/subscriptions/abcdef`) and a glob form of `--scope` where a + `*` crosses slashes (`--scope "/subscriptions/*"`); `--scope` without a `*` is the substring + search it always was. `elevate activate --all` then takes every role the filters and names match + instead of insisting each name picks exactly one, which is the scripted form of the subtree + checkbox. Management groups sit beside the subscriptions rather than above them, in the panel and + for `--under`: ARM writes a management group scope as its own flat path and never repeats it in a + subscription's, so the eligibilities alone cannot say which subscriptions belong to which + management group. ([#186](https://github.com/FrodeHus/elevate/issues/186), + [#194](https://github.com/FrodeHus/elevate/issues/194)) - macOS and Windows: the activation sheet no longer signs off with "Active", the word that means only that PIM wrote the assignment down. It holds for the first effective-access check and closes on **Ready** when the access is already there, on **Activated** when the check has not answered in diff --git a/docs/activating-roles.md b/docs/activating-roles.md index b3ff1055..67440c99 100644 --- a/docs/activating-roles.md +++ b/docs/activating-roles.md @@ -110,6 +110,19 @@ The **Azure** tab lists eligibilities on management groups, subscriptions, resou individual resources. The first Azure activation in a tenant asks you to consent to Azure Service Management; no administrator is needed. +Rather than listing them flat, the tab groups them by scope: each header opens and closes with its +chevron and says how many roles sit under it. A scope that only passes through — no eligibility of +its own and one way down — is folded into the node below it and named there ("Alpha / prod"), so +the panel does not spend a row and a level of indent on nothing; a scope leading to a single role +gets no header at all, and the role row stands where the header would have. In select mode a scope +header carries its own checkbox: one press takes every eligibility under it, which is what +"Contributor across these twelve subscriptions" costs now. A half-filled checkbox means part of the +subtree is already ticked, and pressing it takes the rest. + +Management groups sit beside the subscriptions rather than above them: ARM writes a management +group scope as its own flat path and never repeats it in a subscription's, so the eligibilities +alone cannot say which subscriptions belong to which management group. + ![The Groups tab with PIM for Groups memberships](images/tutorials/panel-groups.png) The **Groups** tab lists PIM for Groups memberships and ownerships. Activating one makes you a @@ -122,6 +135,10 @@ Click the magnifier in the header and type. The panel keeps only the roles, tena that match, across all tabs. Press Escape or clear the field to see everything again; the filter never survives closing the panel. +On the Azure tab the search narrows the tree itself rather than sitting beside it, and it reaches +the whole ARM path — so a subscription id finds its roles even when no caption on the row shows it. +Matches stay in place, including under a header you had closed. + ## Notifications and the menu bar Elevate sends macOS notifications: diff --git a/macos/Sources/ElevateApp/App/AppModel+Panel.swift b/macos/Sources/ElevateApp/App/AppModel+Panel.swift index e909f859..7ee0322b 100644 --- a/macos/Sources/ElevateApp/App/AppModel+Panel.swift +++ b/macos/Sources/ElevateApp/App/AppModel+Panel.swift @@ -28,6 +28,31 @@ extension AppModel { func toggleTenant(_ key: TenantKey) { if collapsedTenants.contains(key) { collapsedTenants.remove(key) } else { collapsedTenants.insert(key) } } func toggleIdentity(_ id: String) { if collapsedIdentities.contains(id) { collapsedIdentities.remove(id) } else { collapsedIdentities.insert(id) } } + // MARK: The Azure tab's scope tree + + /// The Azure tab's eligibilities for one tenant, as the management group / subscription / + /// resource group / resource tree the scope strings already describe. Built from the filtered + /// rows, so a search narrows the tree rather than sitting beside it. + func azureTree(for tenantKey: TenantKey) -> [ScopeNode] { + ScopeTree.build(roles(for: tenantKey, tab: .azure)) + } + + /// A scope node's collapsed state is per tenant: the same scope can be reached by two accounts. + static func scopeNodeKey(_ tenantKey: TenantKey, _ node: ScopeNode) -> String { + "\(tenantKey.identityId)|\(tenantKey.tenantId)|\(node.scope)" + } + + /// While a search is running every match stays visible in its place in the tree, so a closed + /// node does not hide a row the user is looking at. + func isScopeCollapsed(_ tenantKey: TenantKey, _ node: ScopeNode) -> Bool { + !isFiltering && collapsedScopes.contains(Self.scopeNodeKey(tenantKey, node)) + } + + func toggleScope(_ tenantKey: TenantKey, _ node: ScopeNode) { + let key = Self.scopeNodeKey(tenantKey, node) + if collapsedScopes.contains(key) { collapsedScopes.remove(key) } else { collapsedScopes.insert(key) } + } + // MARK: Search var isFiltering: Bool { PanelFilter.isActive(searchQuery) } @@ -140,4 +165,49 @@ extension AppModel { guard canActivate(key) else { return } if selection.contains(key) { selection.remove(key) } else { selection.insert(key) } } + + /// Where a scope node's checkbox stands: nothing under it chosen, some of it, or all of it. + enum SubtreeSelection { case none, some, all } + + /// The roles a scope node's checkbox acts on: everything at or below it that can actually be + /// activated. A role already active, or view-only, is not something the checkbox can take, so + /// it is left out of both the count and the toggle. + func subtreeKeys(_ node: ScopeNode) -> [RoleKey] { + node.allRoles.map(\.key).filter(canSelect) + } + + /// Whether a role's own checkbox would be enabled: activatable, and not already active, + /// scheduled or awaiting approval — there is nothing for a bulk activation to do with those. + /// A failed one can be chosen again. + func canSelect(_ key: RoleKey) -> Bool { + guard canActivate(key) else { return false } + guard let assignment = assignment(for: key) else { return true } + if case .failed = assignment.status { return true } + return false + } + + func subtreeState(_ node: ScopeNode) -> SubtreeSelection { + let keys = subtreeKeys(node) + guard !keys.isEmpty else { return .none } + let chosen = keys.count { selection.contains($0) } + if chosen == 0 { return .none } + return chosen == keys.count ? .all : .some + } + + /// "Select every eligibility under this scope", and pressing it again lets them all go. A + /// partly chosen subtree fills up rather than emptying: the checkbox is offering the rest. + /// Returns how many keys the press added or removed, for the confirmation the row shows. + @discardableResult + func toggleSubtree(_ node: ScopeNode) -> Int { + let keys = subtreeKeys(node) + guard !keys.isEmpty else { return 0 } + if subtreeState(node) == .all { + let removed = keys.count { selection.contains($0) } + selection.subtract(keys) + return removed + } + let added = keys.count { !selection.contains($0) } + selection.formUnion(keys) + return added + } } diff --git a/macos/Sources/ElevateApp/App/AppModel.swift b/macos/Sources/ElevateApp/App/AppModel.swift index ca6d706b..ecd40701 100644 --- a/macos/Sources/ElevateApp/App/AppModel.swift +++ b/macos/Sources/ElevateApp/App/AppModel.swift @@ -68,6 +68,9 @@ final class AppModel { /// Collapsed state lives here, not in view @State: rows inside the lazy panel list are recreated as they scroll. var collapsedTenants: Set = [] var collapsedIdentities: Set = [] + /// Collapsed Azure scope nodes, keyed by `scopeNodeKey(_:_:)`. In-memory like the other two, + /// and scopes start open, so nothing is hidden on first sight. + var collapsedScopes: Set = [] var selection: Set = [] var startupError: String? /// Transient, dismissible message (failed sign-in, unreadable state file). Never blocks the panel. diff --git a/macos/Sources/ElevateApp/Views/ScopeRow.swift b/macos/Sources/ElevateApp/Views/ScopeRow.swift new file mode 100644 index 00000000..52f8fc43 --- /dev/null +++ b/macos/Sources/ElevateApp/Views/ScopeRow.swift @@ -0,0 +1,89 @@ +import SwiftUI +import ElevateCore + +/// How far a row of the Azure tab's scope tree is pushed in. The panel is narrow and a management +/// group path is long, so a step is small and the tree stops indenting after a few levels rather +/// than squeezing the text off the row — the path on the header says where you are. +enum PanelIndent { + static let step: CGFloat = 12 + static let maxSteps = 3 + + static func `for`(_ depth: Int) -> CGFloat { CGFloat(min(max(depth, 0), maxSteps)) * step } +} + +/// One scope in the Azure tab's tree: a management group, subscription, resource group or resource +/// that has roles beneath it. The chevron opens it and, in select mode, the checkbox takes +/// everything under it in one press. +struct ScopeRow: View { + @Environment(AppModel.self) private var model + @Environment(\.accessibilityReduceMotion) private var reduceMotion + let tenant: TenantContext + let node: ScopeNode + let depth: Int + + private var expanded: Bool { !model.isScopeCollapsed(tenant.id, node) } + /// "Alpha /" — scopes folded into this one because they only passed through. + private var ancestors: String? { + node.ancestors.isEmpty ? nil : node.ancestors.joined(separator: " / ") + " /" + } + /// What a press of the checkbox would select; the count beside the name says the same. + private var countText: String { node.roleCount == 1 ? "1 role" : "\(node.roleCount) roles" } + private var tooltip: String { + guard let ancestors else { return node.scope } + return "\(ancestors) \(node.title)\n\(node.scope)" + } + + var body: some View { + HStack(spacing: 6) { + if model.selectMode { subtreeCheckbox } + // One plain button for chevron + name, as the tenant header does: a bare tap gesture + // loses to neighbouring hit areas that stretch across the row. + Button { withAnimation(reduceMotion ? nil : .snappy) { model.toggleScope(tenant.id, node) } } label: { + HStack(spacing: 5) { + Image(systemName: "chevron.right").rotationEffect(.degrees(expanded ? 90 : 0)) + .font(.caption2.weight(.semibold)).foregroundStyle(.secondary).frame(width: 10) + if let ancestors { + Text(ancestors).font(.caption).foregroundStyle(.tertiary).lineLimit(1).layoutPriority(-1) + } + Text(node.title).font(.caption.weight(.medium)).lineLimit(1) + Text(ArmScope.label(node.kind)).font(.caption2).foregroundStyle(.secondary).lineLimit(1) + } + .contentShape(Rectangle()) + } + .buttonStyle(.plain) + .accessibilityLabel(expanded ? "Collapse \(node.title)" : "Expand \(node.title)") + Spacer(minLength: 4) + Text(countText).font(.caption2).foregroundStyle(.secondary).fixedSize() + } + .help(tooltip) + .padding(.vertical, 2) + .padding(.leading, PanelMetrics.headerInset + PanelIndent.for(depth)) + .padding(.trailing, PanelMetrics.trailingInset) + } + + /// Three states, as on Windows: everything under the node chosen, some of it, or none. Disabled + /// when there is nothing left to take — all active already, or view-only. + private var subtreeCheckbox: some View { + let state = model.subtreeState(node) + let empty = model.subtreeKeys(node).isEmpty + return Button { model.toggleSubtree(node) } label: { + Image(systemName: glyph(state)) + .font(.body) + .foregroundStyle(state == .none ? AnyShapeStyle(.secondary) : AnyShapeStyle(Color.accentColor)) + .contentShape(Rectangle()) + } + .buttonStyle(.plain) + .disabled(empty) + .accessibilityLabel(node.roleCount == 1 + ? "Select the role under \(node.title)" + : "Select all \(node.roleCount) roles under \(node.title)") + } + + private func glyph(_ state: AppModel.SubtreeSelection) -> String { + switch state { + case .all: "checkmark.square.fill" + case .some: "minus.square.fill" + case .none: "square" + } + } +} diff --git a/macos/Sources/ElevateApp/Views/TenantSection.swift b/macos/Sources/ElevateApp/Views/TenantSection.swift index da2c4881..410d3fb6 100644 --- a/macos/Sources/ElevateApp/Views/TenantSection.swift +++ b/macos/Sources/ElevateApp/Views/TenantSection.swift @@ -91,10 +91,26 @@ struct TenantRoles: View { } .padding(.leading, PanelMetrics.roleInset).padding(.vertical, 6) } - ForEach(roles) { role in RoleRow(role: role) } + if model.panelTab == .azure { + // The Azure tab reads its scope strings as the hierarchy they are; the tree is + // flattened back into rows so it still lives in the panel's one list. + ForEach(scopeTreeRows) { entry in + if let role = entry.role { + RoleRow(role: role).padding(.leading, PanelIndent.for(entry.depth)) + } else { + ScopeRow(tenant: tenant, node: entry.node, depth: entry.depth) + } + } + } else { + ForEach(roles) { role in RoleRow(role: role) } + } } } + private var scopeTreeRows: [ScopeTreeEntry] { + ScopeTree.flatten(model.azureTree(for: tenant.id), isCollapsed: { model.isScopeCollapsed(tenant.id, $0) }) + } + private var emptyText: String { switch (model.panelTab, tenant.discoveryMode) { case (.groups, _): "No eligible groups." diff --git a/macos/Tests/ElevateAppTests/AppModelScopeTreeTests.swift b/macos/Tests/ElevateAppTests/AppModelScopeTreeTests.swift new file mode 100644 index 00000000..75fb4078 --- /dev/null +++ b/macos/Tests/ElevateAppTests/AppModelScopeTreeTests.swift @@ -0,0 +1,175 @@ +import Foundation +import Testing +import ElevateCore +@testable import Elevate + +/// The Azure tab's scope tree: what it groups, what a search leaves, and what the subtree checkbox takes. +@MainActor +struct AppModelScopeTreeTests { + private static let subA = "/subscriptions/aaaaaaaa-0000-0000-0000-000000000000" + private static let subB = "/subscriptions/bbbbbbbb-0000-0000-0000-000000000000" + + private func azure(_ scope: String, _ name: String, detail: String? = nil, + identityId: String = Sample.identityId, tenantId: String = Sample.tenantId) -> EligibleRole { + EligibleRole(key: Sample.key(.azureResource(scope: scope, roleDefinitionId: "rd-" + name), + identityId: identityId, tenantId: tenantId), + displayName: name, detail: detail, source: .discovered, policy: .manualDefault) + } + + /// Two subscriptions: one branching into resource groups, one holding a single role. + private func seeded() async -> AppModel { + var state = AppState() + state.identities = [Sample.identity()] + state.tenants = [Sample.tenant()] + let model = await makeModel(state: state) + model.roles[Sample.tenantKey] = [ + azure(Self.subA, "Owner", detail: "Alpha · subscription"), + azure(Self.subA + "/resourceGroups/prod-rg", "Contributor", detail: "prod-rg · resource group"), + azure(Self.subA + "/resourceGroups/test-rg", "Reader", detail: "test-rg · resource group"), + azure(Self.subB, "Contributor", detail: "Bravo · subscription"), + Sample.role(Sample.entraKey, name: "Global Reader"), + ] + return model + } + + @Test func azureTreeGroupsResourceGroupsUnderTheirSubscriptionAndLeavesEntraOut() async { + let model = await seeded() + let tree = model.azureTree(for: Sample.tenantKey) + + #expect(tree.map(\.title) == ["Alpha", "Bravo"]) + #expect(tree[0].children.map(\.title) == ["prod-rg", "test-rg"]) + #expect(tree[0].roleCount == 3) + #expect(tree[1].roleCount == 1) + cleanup(model) + } + + @Test func azureTreeNarrowsWithTheSearchBox() async { + let model = await seeded() + model.searchQuery = "prod-rg" + + let tree = model.azureTree(for: Sample.tenantKey) + #expect(tree.count == 1) + #expect(tree[0].allRoles.map(\.displayName) == ["Contributor"]) + cleanup(model) + } + + @Test func theSearchBoxReachesTheArmPathEvenWhenNoCaptionShowsIt() async { + let model = await seeded() + model.searchQuery = "bbbbbbbb" + + #expect(model.azureTree(for: Sample.tenantKey).flatMap(\.allRoles).count == 1) + cleanup(model) + } + + @Test func aCollapsedScopeReopensWhenToggledAgain() async { + let model = await seeded() + let alpha = model.azureTree(for: Sample.tenantKey)[0] + + #expect(model.isScopeCollapsed(Sample.tenantKey, alpha) == false, "scopes start open, as tenants do") + model.toggleScope(Sample.tenantKey, alpha) + #expect(model.isScopeCollapsed(Sample.tenantKey, alpha)) + model.toggleScope(Sample.tenantKey, alpha) + #expect(model.isScopeCollapsed(Sample.tenantKey, alpha) == false) + cleanup(model) + } + + @Test func aClosedScopeStillShowsItsMatchesWhileSearching() async { + // A match must never be hidden behind a node the user closed earlier. + let model = await seeded() + model.toggleScope(Sample.tenantKey, model.azureTree(for: Sample.tenantKey)[0]) + + model.searchQuery = "prod" + let tree = model.azureTree(for: Sample.tenantKey) + #expect(!tree.isEmpty) + #expect(model.isScopeCollapsed(Sample.tenantKey, tree[0]) == false) + cleanup(model) + } + + @Test func toggleSubtreeTakesEveryEligibilityUnderTheScope() async { + let model = await seeded() + let alpha = model.azureTree(for: Sample.tenantKey)[0] + + #expect(model.subtreeState(alpha) == .none) + #expect(model.toggleSubtree(alpha) == 3) + #expect(model.subtreeState(alpha) == .all) + #expect(model.selection.count == 3) + cleanup(model) + } + + @Test func pressingAFullyChosenSubtreeAgainLetsItGo() async { + let model = await seeded() + let alpha = model.azureTree(for: Sample.tenantKey)[0] + + _ = model.toggleSubtree(alpha) + #expect(model.toggleSubtree(alpha) == 3) + #expect(model.selection.isEmpty) + cleanup(model) + } + + @Test func aPartlyChosenSubtreeFillsUpRatherThanEmptying() async { + let model = await seeded() + let alpha = model.azureTree(for: Sample.tenantKey)[0] + model.toggleSelection(alpha.roles[0].key) + + #expect(model.subtreeState(alpha) == .some) + // The one already chosen is left where it is. + #expect(model.toggleSubtree(alpha) == 2) + #expect(model.subtreeState(alpha) == .all) + cleanup(model) + } + + @Test func aSubtreeCheckboxOnlyTakesWhatCanActuallyBeActivated() async { + let model = await seeded() + let alpha = model.azureTree(for: Sample.tenantKey)[0] + model.active[alpha.roles[0].key] = Sample.assignment(alpha.roles[0].key) + + #expect(model.subtreeKeys(alpha).count == 2) + #expect(model.toggleSubtree(alpha) == 2) + #expect(model.subtreeState(alpha) == .all) + cleanup(model) + } + + @Test func aSubtreeWithNothingToTakeReportsNoSelection() async { + let model = await seeded() + let bravo = model.azureTree(for: Sample.tenantKey)[1] + model.active[bravo.roles[0].key] = Sample.assignment(bravo.roles[0].key) + + #expect(model.subtreeKeys(bravo).isEmpty) + #expect(model.subtreeState(bravo) == .none) + #expect(model.toggleSubtree(bravo) == 0) + cleanup(model) + } + + @Test func theSameScopeUnderTwoAccountsCollapsesIndependently() async { + var state = AppState() + state.identities = [Sample.identity("id-1"), Sample.identity("id-2")] + state.tenants = [Sample.tenant(identityId: "id-1"), Sample.tenant(identityId: "id-2", tenantId: "tenant-2")] + let model = await makeModel(state: state) + let first = TenantKey(identityId: "id-1", tenantId: Sample.tenantId) + let second = TenantKey(identityId: "id-2", tenantId: "tenant-2") + model.roles[first] = [ + azure(Self.subA, "Owner", identityId: "id-1"), + azure(Self.subA + "/resourceGroups/rg", "Reader", identityId: "id-1"), + ] + model.roles[second] = [ + azure(Self.subA, "Owner", identityId: "id-2", tenantId: "tenant-2"), + azure(Self.subA + "/resourceGroups/rg", "Reader", identityId: "id-2", tenantId: "tenant-2"), + ] + + model.toggleScope(first, model.azureTree(for: first)[0]) + #expect(model.isScopeCollapsed(first, model.azureTree(for: first)[0])) + #expect(model.isScopeCollapsed(second, model.azureTree(for: second)[0]) == false) + cleanup(model) + } + + @Test func theFlattenedRowsSkipWhatAClosedScopeHides() async { + let model = await seeded() + let tree = model.azureTree(for: Sample.tenantKey) + + #expect(ScopeTree.flatten(tree, isCollapsed: { model.isScopeCollapsed(Sample.tenantKey, $0) }).count == 5) + model.toggleScope(Sample.tenantKey, tree[0]) + // Alpha's header stays, its three rows go, and Bravo's single role row remains. + #expect(ScopeTree.flatten(tree, isCollapsed: { model.isScopeCollapsed(Sample.tenantKey, $0) }).count == 2) + cleanup(model) + } +}