Skip to content

Commit 7e3d3dc

Browse files
Seggrclaude
andcommitted
v0.2.2: log viewer redesign, PIM activation fix, host permission audit, crash fix
Fixed - Log Viewer wouldn't open in v0.2.1 due to ctor ordering NRE; ALSO rebuilt the layout from DataGrid → virtualized ListBox with a 3-char severity glyph + colored accent stripe, inline wrapping messages, inline exception text, right-aligned click-to-filter class label. Dropped the date pickers (not useful for a 500-entry ring buffer) and the two-row filter is now one row. New LevelToGlyphConverter and NullToVisibilityConverter. - TrayIcon.Dispose() crashed at shutdown with ObjectDisposedException because the IServiceProvider was disposed before singleton Dispose ran. Cached the IPluginLoader at Start() time so Dispose() doesn't re-resolve. - PIM activation returned 400 Bad Request because both Graph and ARM clients sent `startDateTime = DateTimeOffset.UtcNow.ToString("o")` — by the time the request reached the service, the timestamp was a few hundred ms in the past, and Graph/ARM reject any schedule starting in the past. Now passes null (omitted via WhenWritingNull) so the service computes the start instant itself. Matches legacy Azure.PIM.Tray behaviour. - AppRegistrationGraphClient now captures Graph error response bodies on 4xx/5xx via EnsureSuccessOrThrowWithBodyAsync (same pattern as the PIM clients). Fix Permissions / Create App Registration failures now surface "Authorization_RequestDenied: Insufficient privileges..." instead of bare 403 text. Changed - HostRequiredPermissions split into Baseline (User.Read — what every signed-in user needs) and AdminTools (Application.ReadWrite.All + DelegatedPermissionGrant.ReadWrite.All — only needed by Fix Permissions / Create App Registration / App Reg search). `All` is still the union for the existing aggregation flow. No behavioural change; the declaration of intent is now legible. Plugins continue to declare their own runtime scopes via ITrayPlugin.RequiredPermissions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 537fbb1 commit 7e3d3dc

12 files changed

Lines changed: 433 additions & 314 deletions

CHANGELOG.md

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
## [0.2.2] — 2026-05-12
11+
12+
### Changed
13+
14+
- **Host permission audit + restructure.** `HostRequiredPermissions` split into two layered sets:
15+
- `Baseline``User.Read` only. What the runtime host actually exercises (sign-in identity, /me lookup, tenant display name resolution).
16+
- `AdminTools``Application.ReadWrite.All` + `DelegatedPermissionGrant.ReadWrite.All`. Only needed by the in-app admin features (Fix Permissions, Create App Registration, App Registration search). A user who never invokes those features doesn't need them functionally.
17+
- `All` = `Baseline ∪ AdminTools`, used by the aggregation that drives Create App Registration / Fix Permissions provisioning. No behavioural change vs. v0.2.1; the split makes intent legible. Plugins continue to declare their own runtime scopes via `ITrayPlugin.RequiredPermissions`.
18+
19+
### Fixed
20+
21+
- **Log Viewer redesign.** Replaced the `DataGrid` (which truncated "Information" to "Informatio", horizontally scrolled, and made selected rows unreadable) with a virtualized `ListBox` whose rows lead with a 3-char severity glyph (`ERR`/`WRN`/`INF`/`DBG`/`VRB`/`FTL`) and an accent-colored stripe. Messages wrap. Exception text renders inline under the message in muted monospace. Class label is right-aligned and click-to-filter. Dropped the date pickers (not useful for a 500-entry ring) and collapsed the two filter rows to one. New `LevelToGlyphConverter` + `NullToVisibilityConverter`.
22+
- **Host crash on shutdown.** `TrayIcon.Dispose()` called `_services.GetService<IPluginLoader>()` to unsubscribe `PluginsChanged`, but the IServiceProvider is disposed *before* singleton `Dispose()` runs. Cached the loader at `Start()` time and use that reference in `Dispose()` — no more `ObjectDisposedException` at exit (the `[FTL] Host terminated unexpectedly` entries).
23+
- **PIM activation 400 Bad Request.** Both `GraphPimClient.ActivateRoleAsync` and `ArmPimClient.ActivateRoleAsync` were sending `startDateTime = DateTimeOffset.UtcNow.ToString("o")`. By the time the request reached the server, that timestamp was a few hundred milliseconds in the past, and Graph/ARM 400 any schedule with a past start. Now passes `null` (omitted via `WhenWritingNull`) so the service computes the instant itself. Matches legacy `Azure.PIM.Tray` behaviour.
24+
- **Graph error response bodies now visible.** `AppRegistrationGraphClient` previously used `EnsureSuccessStatusCode` which discarded the response body, leaving Fix Permissions / Create App Registration failures as opaque "403 Forbidden". Now uses `EnsureSuccessOrThrowWithBodyAsync` (same pattern already in the PIM clients) so the user sees `Authorization_RequestDenied: Insufficient privileges...` and similar Graph error codes in the log.
25+
1026
## [0.2.1] — 2026-05-12
1127

1228
### Added
@@ -133,7 +149,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
133149

134150
- This release is foundation only; no features from `Azure.PIM.Tray` have been ported yet.
135151

136-
[Unreleased]: https://github.com/Proxylayer/AzureTray/compare/v0.2.1...HEAD
152+
[Unreleased]: https://github.com/Proxylayer/AzureTray/compare/v0.2.2...HEAD
153+
[0.2.2]: https://github.com/Proxylayer/AzureTray/compare/v0.2.1...v0.2.2
137154
[0.2.1]: https://github.com/Proxylayer/AzureTray/compare/v0.2.0...v0.2.1
138155
[0.2.0]: https://github.com/Proxylayer/AzureTray/compare/v0.1.0...v0.2.0
139156
[0.1.0]: https://github.com/Proxylayer/AzureTray/releases/tag/v0.1.0

Directory.Build.props

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
<PropertyGroup>
1818
<!-- Host EXE version. Plugin packages each declare their own
1919
<Version> in their csproj so they're decoupled from this. -->
20-
<Version>0.2.1</Version>
20+
<Version>0.2.2</Version>
2121
<Authors>ProxyLayer</Authors>
2222
<Product>AzureTray</Product>
2323
<Copyright>Copyright (c) ProxyLayer</Copyright>

src/AzureTray.Plugin.PIM/Arm/ArmPimClient.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,12 @@ public async Task<ArmRoleAssignmentScheduleRequest> ActivateRoleAsync(
144144
: linkedRoleEligibilityScheduleId,
145145
scheduleInfo = new
146146
{
147-
startDateTime = DateTimeOffset.UtcNow.ToString("o", CultureInfo.InvariantCulture),
147+
// See GraphPimClient.ActivateRoleAsync — sending a UtcNow
148+
// timestamp here is racy because by the time ARM
149+
// evaluates the request, the moment is already in the
150+
// past and ARM rejects past start times. Null (omitted
151+
// via WhenWritingNull) means "start now".
152+
startDateTime = (string?)null,
148153
expiration = new
149154
{
150155
type = "AfterDuration",

src/AzureTray.Plugin.PIM/Graph/GraphPimClient.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,13 @@ public async Task<EntraScheduleRequest> ActivateRoleAsync(
123123
justification,
124124
scheduleInfo = new
125125
{
126-
startDateTime = DateTimeOffset.UtcNow.ToString("o", CultureInfo.InvariantCulture),
126+
// Pass null (omitted from the JSON by WhenWritingNull) so
127+
// Graph defaults to "activate now". Sending DateTimeOffset.UtcNow
128+
// here causes intermittent 400s — by the time the request
129+
// lands on Graph's server, the timestamp is already in the
130+
// past by network-latency milliseconds, and Graph rejects
131+
// any startDateTime in the past. Matches legacy behaviour.
132+
startDateTime = (string?)null,
127133
expiration = new
128134
{
129135
type = "afterDuration",
Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,72 @@
11
using System.Collections.Generic;
2+
using System.Linq;
23
using AzureTray.Plugin.Contracts;
34

45
namespace AzureTray.AppRegistration;
56

6-
// The minimum delegated permissions the host itself needs, independent
7-
// of any loaded plugin. Currently just User.Read so the host can call
8-
// /me to resolve the signed-in user's tenant/display name. Plugins add
9-
// their own scopes via ITrayPlugin.RequiredPermissions and are
10-
// aggregated alongside these in Fix Permissions / Create App Registration.
7+
// Splits the host's declared scopes into two layers so the consent
8+
// surface stays honest:
9+
//
10+
// Baseline — what every signed-in user needs just to operate the
11+
// tray (sign-in identity, tenant display name resolution).
12+
// This is the only set used by the runtime host code that
13+
// a non-admin user actually exercises.
14+
//
15+
// AdminTools — what the in-app admin features (Fix Permissions,
16+
// Create App Registration, App Registration search)
17+
// additionally need to function. A user who never uses
18+
// those features doesn't need these scopes. We still
19+
// add them to the app registration at provision time
20+
// so administrators can self-heal, but users with no
21+
// directory role to back them up get a clean "you
22+
// don't have authority" failure instead of a confusing
23+
// "Insufficient privileges" 403 with no context.
24+
//
25+
// All = Baseline ∪ AdminTools and is what AggregateRequiredPermissions()
26+
// in SettingsViewModel.cs / TestRegistry.cs already consumes, so this
27+
// reshuffle is a pure rename/documentation change at the call sites.
28+
//
29+
// Plugins declare their own scopes via ITrayPlugin.RequiredPermissions
30+
// and are folded into the same aggregate downstream — those are the
31+
// host's only "what does the runtime need" inputs aside from Baseline.
1132
internal static class HostRequiredPermissions
1233
{
13-
public static IReadOnlyList<PluginPermissionRequirement> All { get; } = new[]
34+
// Always needed. Pure read of the signed-in user; required by /me
35+
// and by the /organization → /me companyName fallback.
36+
public static IReadOnlyList<PluginPermissionRequirement> Baseline { get; } = new[]
1437
{
1538
new PluginPermissionRequirement(
1639
PermissionApi.MicrosoftGraph,
1740
"User.Read",
1841
"e1fe6dd8-ba31-4d61-89e7-88639da4683d",
1942
"Sign in and read user profile (host /me lookup)"),
2043
};
44+
45+
// Only needed for in-app administration of tenant app registrations.
46+
// The two scopes together let a holder of an appropriate directory
47+
// role (Application Admin / Cloud Application Admin / Privileged
48+
// Role Admin / Global Admin) run Fix Permissions and Create App
49+
// Registration end-to-end. Without these scopes consented, those
50+
// features return 403 with the underlying Authorization_RequestDenied
51+
// — surfaced to the user via the response-body capture in
52+
// AppRegistrationGraphClient.
53+
public static IReadOnlyList<PluginPermissionRequirement> AdminTools { get; } = new[]
54+
{
55+
new PluginPermissionRequirement(
56+
PermissionApi.MicrosoftGraph,
57+
"Application.ReadWrite.All",
58+
"1bfefb4e-e0b5-418b-a88f-73c46d2cc8e9",
59+
"Read, search, and update tenant app registrations (in-app Fix Permissions / Create App Registration)"),
60+
new PluginPermissionRequirement(
61+
PermissionApi.MicrosoftGraph,
62+
"DelegatedPermissionGrant.ReadWrite.All",
63+
"41ce6ca6-6826-4807-84f1-1c82854f7ee5",
64+
"Grant tenant-wide admin consent for plugin scopes (in-app Fix Permissions / Create App Registration)"),
65+
};
66+
67+
// Union used by Create App Registration / Fix Permissions to provision
68+
// the full host-side scope set on the app reg. Plugins' own
69+
// requirements are appended downstream — this list is host-only.
70+
public static IReadOnlyList<PluginPermissionRequirement> All { get; } =
71+
Baseline.Concat(AdminTools).ToArray();
2172
}

src/AzureTray/AppRegistration/Internal/AppRegistrationGraphClient.cs

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public AppRegistrationGraphClient(
4848
public async Task<T?> GetAsync<T>(string tenantId, string url, CancellationToken ct)
4949
{
5050
using var response = await SendAsync(tenantId, HttpMethod.Get, url, body: null, ct);
51-
response.EnsureSuccessStatusCode();
51+
await EnsureSuccessOrThrowWithBodyAsync(response, ct).ConfigureAwait(false);
5252
return await response.Content.ReadFromJsonAsync<T>(JsonOptions, ct);
5353
}
5454

@@ -69,20 +69,48 @@ public async Task<List<T>> ListAsync<T>(string tenantId, string firstUrl, Cancel
6969
public async Task<T?> PostAsync<T>(string tenantId, string url, object body, CancellationToken ct)
7070
{
7171
using var response = await SendAsync(tenantId, HttpMethod.Post, url, body, ct);
72-
response.EnsureSuccessStatusCode();
72+
await EnsureSuccessOrThrowWithBodyAsync(response, ct).ConfigureAwait(false);
7373
return await response.Content.ReadFromJsonAsync<T>(JsonOptions, ct);
7474
}
7575

7676
public async Task PatchAsync(string tenantId, string url, object body, CancellationToken ct)
7777
{
7878
using var response = await SendAsync(tenantId, HttpMethod.Patch, url, body, ct);
79-
response.EnsureSuccessStatusCode();
79+
await EnsureSuccessOrThrowWithBodyAsync(response, ct).ConfigureAwait(false);
8080
}
8181

8282
public async Task DeleteAsync(string tenantId, string url, CancellationToken ct)
8383
{
8484
using var response = await SendAsync(tenantId, HttpMethod.Delete, url, body: null, ct);
85-
response.EnsureSuccessStatusCode();
85+
await EnsureSuccessOrThrowWithBodyAsync(response, ct).ConfigureAwait(false);
86+
}
87+
88+
// Microsoft Graph returns rich error JSON on 4xx/5xx — code, message,
89+
// request-id, often a helpful inner-error block. EnsureSuccessStatusCode
90+
// throws but discards the body, so the user / log sees just "403
91+
// Forbidden" with no clue what scope was insufficient. This helper
92+
// preserves the body in the exception Message so the caller (Settings
93+
// Fix Permissions, etc.) can surface "Authorization_RequestDenied:
94+
// Insufficient privileges to complete the operation." instead.
95+
private static async Task EnsureSuccessOrThrowWithBodyAsync(HttpResponseMessage response, CancellationToken ct)
96+
{
97+
if (response.IsSuccessStatusCode) return;
98+
99+
string body;
100+
try
101+
{
102+
body = await response.Content.ReadAsStringAsync(ct).ConfigureAwait(false);
103+
}
104+
catch
105+
{
106+
body = "(body unreadable)";
107+
}
108+
if (body.Length > 1500) body = body[..1500] + "…(truncated)";
109+
110+
throw new HttpRequestException(
111+
$"Graph {response.RequestMessage?.Method} {response.RequestMessage?.RequestUri} returned {(int)response.StatusCode} {response.ReasonPhrase}. Body: {body}",
112+
inner: null,
113+
statusCode: response.StatusCode);
86114
}
87115

88116
public async Task<HttpResponseMessage> SendAsync(

0 commit comments

Comments
 (0)