Skip to content

WebApi: apply the REST auth opt-ins to the opcua+openapi WebSocket upgrade - #4699

Merged
romanett merged 5 commits into
masterfrom
romanett/webapi-wss-openapi-auth
Oct 8, 2026
Merged

romanett merged 5 commits into
masterfrom
romanett/webapi-wss-openapi-auth

Conversation

@romanett

@romanett romanett commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to #4674 for the WebSocket side of the OpenAPI binding.

  • Plain opcua+openapi upgrade uses the REST auth. When AddWebApiBasicAuth / AddWebApiBearerAuth / AddWebApiMutualTlsAuth is configured, the upgrade runs the same default authorization policy as the REST route group (RequireAuthorization()), because the upgrade is dispatched by the listener after routing. Requests that don't satisfy it get the schemes' challenge (401 + WWW-Authenticate) or 403. Without an auth opt-in the behaviour is unchanged.
  • Upstream identity on the channel. For both opcua+openapi and opcua+openapi+<accesstoken>, the authenticated principal is mapped through ISessionlessIdentityProvider into SecureChannelContext.UpstreamIdentity, as the REST routes already do per request.
  • Token lifetime. A credential whose exp claim has already passed is refused at upgrade, also within the JwtBearer clock skew, and the socket is closed when the token expires.
  • Refused upgrades complete their response. Before, the admission lease aborted the connection when an upgrade was refused, so clients could see a connection reset instead of the 401/503. Refused upgrades on HTTP/1.x now carry Connection: close and Kestrel closes the connection after sending the response.

The other WebSocket sub-protocols (opcua+uacp, opcua+uajson) and the HTTPS binary/JSON endpoint are unchanged: per Part 6 §7.4 they authenticate through the OPC UA session layer, not HTTP credentials.

Tests

New cases in RealHttpsListenerIntegrationTests.Authentication.cs against a real HttpsTransportListener:

  • the plain upgrade for Basic, Bearer and mTLS with no, wrong, wrong-scheme and valid credentials (status, challenge, and that nothing reaches the server callback);
  • UpstreamIdentity set for both opcua+openapi variants;
  • an expired token, in the Authorization header and in the sub-protocol;
  • the socket closing when the token expires.

Locally on net10.0: Opc.Ua.Bindings.Https.WebApi.Tests 499/499, Opc.Ua.Core.Tests (Wss/OpenApi/Https) 243/243, Opc.Ua.Sessions.Tests (Wss/Https/WebApi) 53/53. Opc.Ua.Bindings.Https builds on all TFMs with 0 warnings.

Related Issues

Checklist

  • I have signed the CLA and read the CONTRIBUTING doc.
  • I have added tests that prove my fix is effective or that my feature works and increased code coverage.
  • I have added all necessary documentation.
  • I have verified that my changes do not introduce (new) build or analyzer warnings.
  • I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed.
  • I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings.
  • I have addressed all PR feedback received.

🤖 Generated with Claude Code

Copilot AI balanced review requested due to automatic review settings October 7, 2026 13:39
…grade

The AddWebApi*Auth opt-ins are enforced on the REST routes; the plain
opcua+openapi WebSocket upgrade is dispatched by the listener after
routing and now runs the same default authorization policy before it is
accepted. The principal of either opcua+openapi variant is mapped through
ISessionlessIdentityProvider into SecureChannelContext.UpstreamIdentity,
a credential whose exp has passed is refused at upgrade, and the socket
is closed when it expires. Refused upgrades now complete their HTTP
response (Connection: close) instead of resetting the connection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Authorization behavior can diverge from REST policies, and long-lived credentials bypass socket expiry enforcement.

Review effort: Balanced
Findings: 1 High severity · 3 Medium severity

Open (4)
What changed in this PR

Extends Web API authentication to OpenAPI WebSocket upgrades and propagates authenticated identities into channel contexts.

Changes:

  • Enforces REST authorization policies during plain OpenAPI upgrades.
  • Propagates upstream identity and enforces token expiry.
  • Adds real-listener authentication and expiry integration tests.
File Description
WebApiHttpsStartupContributor.cs Adds upgrade authentication and identity resolution.
HttpsTransportListener.cs Handles authorization failures, identity propagation, and credential expiry.
RealHttpsListenerIntegrationTests.Authentication.cs Tests WebSocket authentication, identity, and expiry behavior.

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

Comment thread src/Opc.Ua.Bindings.Https/Https/HttpsTransportListener.cs Outdated
Comment thread src/Opc.Ua.Bindings.Https/WebApi/WebApiHttpsStartupContributor.cs Outdated
Comment thread src/Opc.Ua.Bindings.Https/WebApi/WebApiHttpsStartupContributor.cs Outdated
- Authorize the upgrade with the HttpContext as the resource and answer a
  failure through IAuthorizationMiddlewareResultHandler, so challenge and
  forbid use the default policy's schemes exactly as on the REST routes.
- Re-arm the credential expiry wait until the actual expiry instead of
  skipping lifetimes beyond a single timer wait.
- Run the expiry on an injectable TimeProvider; the test drives it with a
  FakeTimeProvider instead of racing the handshake against a short token,
  and covers a lifetime beyond a single timer wait.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Code coverage

✅ Coverage gate passed.

Check Result Threshold
✅ Project line rate 88.59% (305422/344776 lines) >= 70.00%
✅ Project branch rate 79.02% >= 60.00%
✅ Patch coverage 97.46% (115/118 changed lines) >= 75.00% (> 100 changed lines)
ℹ️ Baseline delta (advisory) +14.99 pp 73.60% recorded
Uncovered changed lines
  • src/Opc.Ua.Bindings.Https/WebApi/WebApiHttpsStartupContributor.cs: 387
  • src/Opc.Ua.Bindings.Https/Https/HttpsTransportListener.cs: 1932, 1933

Coverage is above the recorded baseline - consider ratcheting coverage-thresholds.json.

Thresholds live in coverage-thresholds.json. Whole report before exclusions: line 87.72%, branch 78.22%.

@romanett
romanett requested a review from marcschier October 7, 2026 14:47
@codecov

codecov Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.91525% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.82%. Comparing base (e5b6d13) to head (b81baee).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
....Ua.Bindings.Https/Https/HttpsTransportListener.cs 94.62% 2 Missing and 3 partials ⚠️
...ings.Https/WebApi/WebApiHttpsStartupContributor.cs 96.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4699      +/-   ##
==========================================
- Coverage   82.83%   82.82%   -0.01%     
==========================================
  Files        2262     2262              
  Lines      344665   344776     +111     
  Branches    60057    60078      +21     
==========================================
+ Hits       285491   285556      +65     
- Misses      39150    39188      +38     
- Partials    20024    20032       +8     
Flag Coverage Δ
actions 82.82% <94.91%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ings.Https/WebApi/WebApiHttpsStartupContributor.cs 93.57% <96.00%> (+0.46%) ⬆️
....Ua.Bindings.Https/Https/HttpsTransportListener.cs 76.74% <94.62%> (+1.50%) ⬆️

... and 30 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Covers an opcua+openapi upgrade whose authenticator or identity resolver
throws, and GetCredentialExpiry for whole and fractional exp values,
out-of-range clamping, unusable claims and unauthenticated principals.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@marcschier

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts in this pull request

Copilot AI commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Merged master and resolved the WebApi listener setup conflict while preserving the injected TimeProvider and the free-port retry logic. Merge commit: 5c34c6c4.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

HTTP/2 refusal handling prematurely releases the physical admission lease while leaving the connection reusable.

1 open finding
4 resolved since last review

🧠 Review effort: Balanced

Comment thread src/Opc.Ua.Bindings.Https/Https/HttpsTransportListener.cs Outdated
romanett and others added 2 commits October 7, 2026 18:34
…-openapi-auth

# Conflicts:
#	tests/Opc.Ua.Bindings.Https.WebApi.Tests/RealHttpsListenerIntegrationTests.Authentication.cs
A refused WebSocket upgrade on a connection-admitted request closed the
connection's physical lease while leaving the connection alive: HTTP/2
keeps it usable for other streams, so it was no longer counted or closed
by admission shutdown. The refused upgrade now completes the lease's
handshake and leaves it to RunHttpsConnectionAsync, which releases it
when the connection really closes. A request-scoped lease is released
as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants