skyline: Replaced IdentityModel's TokenResponse with an owned waters_connect type

[brendanx67]

Summary

• Replaced IdentityModel's TokenResponse with WatersConnectTokenResponse, parsed with JsonConvert behind static factories - the pattern the Ardia response types alongside it already use (StorageInfoResponse.Create, GetParentFolderResponse.FromJson).
• ResponseErrorType becomes a local TokenErrorType, which is only ever compared against Exception.
• No behavior change intended. The eight members waters_connect actually uses are reproduced, and the two semantics that matter were measured against the checked-in IdentityModel 3.9 rather than recalled: a non-JSON body is an error rather than a throw, and an HTTP-error response keeps the body verbatim as Raw.

Why

#4613 moved waters_connect onto HttpClientWithProgress. The token request now does its own form POST, its own RFC 6749 credential encoding, and its own error classification - what it still took from IdentityModel was one thing, a JSON-parsed shape.

That leftover is what makes master and the .NET 10 port branch (#4619) disagree. Master compiles against the checked-in IdentityModel 3.9, whose TokenResponse has public constructors taking a raw body, an HTTP status, or an exception. The port branch references IdentityModel 7.0.0 from NuGet, where it has exactly one:

IdentityModel 7.0.0  IdentityModel.Client.TokenResponse
Public constructors:
Void .ctor()

So merging master into Skyline/work/20260612_net8_port gives three compile errors in RequestToken, with no resolution that satisfies both sides while either still depends on IdentityModel's type - and the conflict returns on every subsequent merge. The port branch handles it by using IdentityModel 7's RequestPasswordTokenAsync / RequestRefreshTokenAsync extensions, which take a raw HttpClient; that is the usage the port to HttpClientWithProgress exists to remove, so adopting it is not an option.

Owning the shape makes this file compile identically under both, permanently.

Why the risk is low

HandleAuthenticationException - the classification that turns invalid_scope, invalid_client and invalid_grant into user-facing messages - does not reference this type at all. It re-parses ex.Data[TOKEN_DATA] with JObject.Parse and reads the raw JSON fields directly. So that path is untouched as long as Raw still carries the body verbatim, which the factories preserve and their contracts state.

Test plan

• Solution builds clean
• TestWatersConnectExportMethodDlg en + fr - covers both the success path and the authentication-failure path, which is the part most changed
• CodeInspection - 0 failures

Scope

waters_connect only. UnifiAccount.Authenticate() still returns IdentityModel's TokenResponse, so the package reference stays in CommonMsData regardless. Unifi is the obvious follow-on, and wants this together with the HttpClientWithProgress port rather than on its own - UnifiAccount.cs:168 is still new HttpClient() per call, the same raw-client usage #4613 removed from waters_connect.

Raised deliberately as a PR to master rather than as a resolution inside the #4619 merge, so it gets net472 CI where TestWatersConnectExportMethodDlg actually runs, and so it is a reviewable diff rather than something buried in a conflict resolution.

@chambm @rita-gwen - flagging you both since this touches the token path you have each worked in recently, and the alternative reading is that we should keep IdentityModel's type and adapt per branch instead.

See ai/todos/active/TODO-20260901_watersconnect_token_poco.md

Co-Authored-By: Claude noreply@anthropic.com

[Copilot]

🟡 Changes recommended

FromJson currently classifies non-JSON/deserialize failures as TokenErrorType.Exception despite docs/enum semantics indicating these should be Http, which can change error-path behavior and messaging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR removes the remaining IdentityModel.Client.TokenResponse dependency from waters_connect by introducing an owned WatersConnectTokenResponse POCO (with JSON parsing via JsonConvert) and updating WatersConnectAccount to use it, allowing the code to compile consistently across IdentityModel 3.9 (checked-in) and IdentityModel 7 (NuGet) without changing the token-request wire behavior.

Changes:

• Added WatersConnectTokenResponse (+ TokenErrorType) with static factory methods to parse JSON, represent HTTP errors, and represent exception-only failures.
• Updated WatersConnectAccount token caching and token request logic to use WatersConnectTokenResponse instead of IdentityModel’s TokenResponse.
• Added the new source file to CommonMsData.csproj.

File summaries

File	Description
pwiz_tools/Shared/CommonMsData/RemoteApi/WatersConnect/WatersConnectTokenResponse.cs	New owned token-response shape + parsing/error classification factories.
pwiz_tools/Shared/CommonMsData/RemoteApi/WatersConnect/WatersConnectAccount.cs	Switched authentication/token request plumbing from IdentityModel TokenResponse to the owned response type.
pwiz_tools/Shared/CommonMsData/CommonMsData.csproj	Included the new token response file in the build.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[brendanx67]

Claude proposed this as a simplified solution for OAuth requirements rather than pulling in the entire Identity Apache project for parsing a single JSON object. Unsure about this. What do you two (@rita-gwen @chambm) think about this as a potential solution? Otherwise, we probably want to perform the same upgrade Matt just made in the .NET 10.0 branch in Rita's HttpClientWithProgress port because Matt's upgrade of Identity has fallout for using HttpClientWithProgress which needs to be worked out.

[rita-gwen]

Superseded by #4637, which extracts a shared OAuthPasswordGrantClient into CommonUtil for both the managed and native readers - a more general solution to the same IdentityModel coupling this PR addressed. Branch and local WIP (UNIFI extension) discarded.