Skip to content

Jpfr/fix wss session discovery - #4135

Merged
marcschier merged 2 commits into
OPCFoundation:masterfrom
jpfr:jpfr/fix-wss-session-discovery
Aug 4, 2026
Merged

Jpfr/fix wss session discovery#4135
marcschier merged 2 commits into
OPCFoundation:masterfrom
jpfr:jpfr/fix-wss-session-discovery

Conversation

@jpfr

@jpfr jpfr commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Description

WSS binary endpoints use the opcua+uacp sub-protocol and carry the normal OPC UA Connection Protocol, SecureChannel, and Session layers. User authentication should therefore behave like opc.tcp, independently of whether mutual TLS is enabled for the WebSocket connection.

HttpsServiceHost previously shared an HTTPS-specific rule with WSS: when HttpsMutualTls was disabled, it removed the Anonymous user-token policy from the endpoint description. This prevented an anonymous OPC UA session even when anonymous access was explicitly configured by the server.

This PR:

  • Preserves the server’s configured OPC UA user-token policies for WSS UA-SC UA-Binary endpoints.
  • Retains the existing anonymous-token filtering for HTTPS transports.
  • Adds an integration test that disables mutual TLS, verifies that the WSS endpoint advertises Anonymous, and opens a complete anonymous OPC UA session.
  • Passes ApplicationConfiguration into endpoint discovery performed by updateBeforeConnect, allowing WSS discovery to use the application’s certificate manager.
  • Applies certificate-aware discovery to both the default and channel-manager session paths while preserving the existing endpoint-refresh APIs.

Verification

  • Anonymous WSS binary session with HttpsMutualTls = false: passed.
  • WSS updateBeforeConnect using application certificate validation: passed.
  • Null application-configuration validation: passed.
  • Affected projects build without new warnings.

Related Issues

Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here.

If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR)).

  • Fixes #github-issue-number, ...

Checklist

Put an x in the boxes that apply. You can complete these step by step after opening the PR.

  • 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.

o6-jpfr added 2 commits July 30, 2026 20:25
HttpsMutualTls controls authentication for HTTPS transports, but WSS binary endpoints carry normal UA Secure Conversation sessions. Filtering anonymous user tokens when mutual TLS was disabled left those WSS endpoints unable to open a configured anonymous session.

Retain the server's configured OPC UA user-token policies for the WSS UA-SC UA-Binary profile while preserving the existing HTTPS behavior.

Add an integration test that disables mutual TLS, verifies the anonymous token is advertised, and opens an anonymous WSS binary session.

Specification: https://reference.opcfoundation.org/Core/Part6/v105/docs/7.5

Tests: dotnet test tests/Opc.Ua.Sessions.Tests/Opc.Ua.Sessions.Tests.csproj -f net10.0 --filter FullyQualifiedName~AnonymousSessionOverWssBinaryOpensWithoutMutualTlsAsync (1 passed)
ConfiguredEndpoint refreshed endpoint descriptions with a DiscoveryClient
created only from EndpointConfiguration. That path did not receive the
application certificate manager, so WSS discovery could not validate a
server certificate trusted by the application before opening a session.

Add an ApplicationConfiguration-aware endpoint refresh overload and use it
from the default and channel-manager session paths. Preserve the existing
overloads for compatibility.

The application-aware DiscoveryClient now supplies the configured
certificate validation for secure discovery transports while TCP and legacy
callers retain their existing behavior.

Specification:
https://reference.opcfoundation.org/Core/Part6/v105/docs/7.5.3

Tests: UpdateFromServerWithNullApplicationConfigurationThrowsArgumentNullException
(1 passed, net10.0)

Tests: UpdateBeforeConnectUsesApplicationCertificateValidationAsync
(1 passed, net10.0)
Copilot AI review requested due to automatic review settings July 30, 2026 18:50

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.

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR adjusts WSS endpoint discovery and authentication behavior so WSS UA-Binary sessions respect the server’s configured OPC UA user-token policies (including Anonymous), and so endpoint discovery can use the application’s certificate validation.

Changes:

  • Stop filtering out Anonymous user-token policies for WSS UA-Binary endpoints when HTTPS mutual TLS is disabled; keep the filtering for HTTPS.
  • Pass ApplicationConfiguration into ConfiguredEndpoint.UpdateFromServerAsync(...) to enable certificate-aware discovery (used by updateBeforeConnect).
  • Add/extend integration and unit tests for anonymous WSS sessions and null ApplicationConfiguration handling.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/Opc.Ua.Sessions.Tests/WssTransportIntegrationTests.cs Adds a test exercising updateBeforeConnect path for WSS sessions.
tests/Opc.Ua.Sessions.Tests/WssJsonTransportIntegrationTests.cs Adds an integration test asserting Anonymous is advertised and a session can open.
tests/Opc.Ua.Core.Tests/Stack/Client/ConfiguredEndpointTests.cs Adds unit coverage for null ApplicationConfiguration validation.
src/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs Changes endpoint update APIs to accept ApplicationConfiguration and uses it to create a discovery client.
src/Opc.Ua.Client/Session/Session.ChannelManager.cs Updates updateBeforeConnect to call the new endpoint update overload with configuration.
src/Opc.Ua.Client/Session/DefaultSessionFactory.cs Updates channel creation path to call the new endpoint update overload with configuration.
src/Opc.Ua.Client/Session/ChannelManagerSessionFactory.cs Updates manager preparation path to call the new endpoint update overload with configuration.
src/Opc.Ua.Bindings.Https/Https/HttpsServiceHost.cs Limits anonymous-token filtering to HTTPS (excludes WSS UA-Binary).

Comment thread src/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs
Comment thread tests/Opc.Ua.Sessions.Tests/WssTransportIntegrationTests.cs
Comment thread tests/Opc.Ua.Sessions.Tests/WssJsonTransportIntegrationTests.cs
@marcschier

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@marcschier marcschier added the ready Ready to merge once CI Passes label Jul 31, 2026
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.84848% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.90%. Comparing base (8a10139) to head (76a3563).
⚠️ Report is 9 commits behind head on master.

Files with missing lines Patch % Lines
....Ua.Client/Session/ChannelManagerSessionFactory.cs 0.00% 3 Missing ⚠️
...Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs 92.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4135      +/-   ##
==========================================
- Coverage   80.23%   79.90%   -0.33%     
==========================================
  Files        1515     1515              
  Lines      209980   210008      +28     
  Branches    36213    36215       +2     
==========================================
- Hits       168479   167812     -667     
- Misses      28867    29596     +729     
+ Partials    12634    12600      -34     
Files with missing lines Coverage Δ
...rc/Opc.Ua.Bindings.Https/Https/HttpsServiceHost.cs 92.15% <100.00%> (ø)
src/Opc.Ua.Client/Session/DefaultSessionFactory.cs 87.30% <100.00%> (+0.13%) ⬆️
...rc/Opc.Ua.Client/Session/Session.ChannelManager.cs 79.23% <100.00%> (ø)
...Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs 74.76% <92.00%> (-2.78%) ⬇️
....Ua.Client/Session/ChannelManagerSessionFactory.cs 35.86% <0.00%> (-0.40%) ⬇️

... and 59 files with indirect coverage changes

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

@marcschier

Copy link
Copy Markdown
Collaborator

@jpfr - reminder to address the review feedback in both pull requests so this can be merged. Thanks!

Copilot AI added a commit that referenced this pull request Aug 4, 2026
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
@marcschier
marcschier merged commit da3358d into OPCFoundation:master Aug 4, 2026
220 of 222 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready Ready to merge once CI Passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants