[Client] Fix closing of shared Transport Channel in Recreate Scenario / Add missing event Handler to Session Constructor / Call RenewUserIdentity on Session.ReCreate (#3055)

* Extend Session.Open Method with new overload allowing to specify if the used channel is closed when the Session.Open fails
Add Missing Event Handler m_RenewUserIdentity om Session constructor taking a template

* Implement Traceable Session
Create Async open overload

* keep channel open on ReCreateAsync with provided channel

* Add Test executing Recreate using existing channel

* fix test

* Add RenewUserIdentity Handler in case of a RecreateSession

* Add Test, fix Recreate to use new Token

* Improve code coverage with async test overload

* fix Parameter checkDomain in Open Session overload

Author: romanett

Libraries/Opc.Ua.Client/Session/ISession.cs

@@ -607,6 +607,17 @@ Task<Dictionary<NodeId, DataDictionary>> LoadDataTypeSystem(
         /// <param name="checkDomain">If set to <c>true</c> then the domain in the certificate must match the endpoint used.</param>
         void Open(string sessionName, uint sessionTimeout, IUserIdentity identity, IList<string> preferredLocales, bool checkDomain);
 
+        /// <summary>
+        /// Establishes a session with the server.
+        /// </summary>
+        /// <param name="sessionName">The name to assign to the session.</param>
+        /// <param name="sessionTimeout">The session timeout.</param>
+        /// <param name="identity">The user identity.</param>
+        /// <param name="preferredLocales">The list of preferred locales.</param>
+        /// <param name="checkDomain">If set to <c>true</c> then the domain in the certificate must match the endpoint used.</param>
+        /// <param name="closeChannel">If set to <c>true</c> then the channel is closed when the Open fails.</param>
+        void Open(string sessionName, uint sessionTimeout, IUserIdentity identity, IList<string> preferredLocales, bool checkDomain, bool closeChannel);
+
         /// <summary>
         /// Updates the preferred locales used for the session.
         /// </summary>
@@ -669,6 +680,18 @@ Task<Dictionary<NodeId, DataDictionary>> LoadDataTypeSystem(
         /// <param name="ct">The cancellation token.</param>
         Task OpenAsync(string sessionName, uint sessionTimeout, IUserIdentity identity, IList<string> preferredLocales, bool checkDomain, CancellationToken ct);
 
+        /// <summary>
+        /// Establishes a session with the server.
+        /// </summary>
+        /// <param name="sessionName">The name to assign to the session.</param>
+        /// <param name="sessionTimeout">The session timeout.</param>
+        /// <param name="identity">The user identity.</param>
+        /// <param name="preferredLocales">The list of preferred locales.</param>
+        /// <param name="checkDomain">If set to <c>true</c> then the domain in the certificate must match the endpoint used.</param>
+        /// <param name="closeChannel">If set to <c>true</c> then the channel is closed when the Open fails.</param>
+        /// <param name="ct">The cancellation token.</param>
+        Task OpenAsync(string sessionName, uint sessionTimeout, IUserIdentity identity, IList<string> preferredLocales, bool checkDomain, bool closeChannel, CancellationToken ct);
+
         /// <summary>
         /// Reads the values for the node attributes and returns a node object collection.
         /// </summary>

Libraries/Opc.Ua.Client/Session/Session.cs

@@ -153,6 +153,7 @@ public Session(ITransportChannel channel, Session template, bool copyEventHandle
                 m_SubscriptionsChanged = template.m_SubscriptionsChanged;
                 m_SessionClosing = template.m_SessionClosing;
                 m_SessionConfigurationChanged = template.m_SessionConfigurationChanged;
+                m_RenewUserIdentity = template.m_RenewUserIdentity;
             }
 
             foreach (Subscription subscription in template.Subscriptions)
@@ -1268,11 +1269,12 @@ public static Session Recreate(Session template)
 
             try
             {
+                session.RecreateRenewUserIdentity();
                 // open the session.
                 session.Open(
                     template.SessionName,
                     (uint)template.SessionTimeout,
-                    template.Identity,
+                    session.Identity,
                     template.PreferredLocales,
                     template.m_checkDomain);
 
@@ -1314,11 +1316,12 @@ public static Session Recreate(Session template, ITransportWaitingConnection con
 
             try
             {
+                session.RecreateRenewUserIdentity();
                 // open the session.
                 session.Open(
                     template.m_sessionName,
                     (uint)template.m_sessionTimeout,
-                    template.m_identity,
+                    session.Identity,
                     template.m_preferredLocales,
                     template.m_checkDomain);
 
@@ -1349,13 +1352,15 @@ public static Session Recreate(Session template, ITransportChannel transportChan
 
             try
             {
+                session.RecreateRenewUserIdentity();
                 // open the session.
                 session.Open(
                     template.m_sessionName,
                     (uint)template.m_sessionTimeout,
-                    template.m_identity,
+                    session.Identity,
                     template.m_preferredLocales,
-                    template.m_checkDomain);
+                    template.m_checkDomain,
+                    false);
 
                 // create the subscriptions.
                 foreach (Subscription subscription in session.Subscriptions)
@@ -2288,6 +2293,16 @@ public void Open(
         {
             Open(sessionName, sessionTimeout, identity, preferredLocales, true);
         }
+        /// <inheritdoc/>
+        public void Open(
+            string sessionName,
+            uint sessionTimeout,
+            IUserIdentity identity,
+            IList<string> preferredLocales,
+            bool checkDomain)
+        {
+            Open(sessionName, sessionTimeout, identity, preferredLocales, checkDomain, true);
+        }
 
         /// <inheritdoc/>
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Maintainability", "CA1506:AvoidExcessiveClassCoupling")]
@@ -2296,7 +2311,8 @@ public void Open(
             uint sessionTimeout,
             IUserIdentity identity,
             IList<string> preferredLocales,
-            bool checkDomain)
+            bool checkDomain,
+            bool closeChannel)
         {
             OpenValidateIdentity(ref identity, out var identityToken, out var identityPolicy, out string securityPolicyUri, out bool requireEncryption);
 
@@ -2555,7 +2571,7 @@ public void Open(
             {
                 try
                 {
-                    Close(true);
+                    Close(closeChannel);
                 }
                 catch (Exception e)
                 {
@@ -5346,6 +5362,17 @@ public bool ResendData(IEnumerable<Subscription> subscriptions, out IList<Servic
 
             return false;
         }
+
+        /// <summary>
+        /// Helper to refresh the identity (reprompt for password, refresh token) in case of a Recreate of the Session.
+        /// </summary>
+        public virtual void RecreateRenewUserIdentity()
+        {
+            if (m_RenewUserIdentity != null)
+            {
+                m_identity = m_RenewUserIdentity(this, m_identity);
+            }
+        }
         #endregion
 
         #region Private Methods

Libraries/Opc.Ua.Client/Session/SessionAsync.cs

@@ -67,13 +67,26 @@ public Task OpenAsync(
             return OpenAsync(sessionName, sessionTimeout, identity, preferredLocales, true, ct);
         }
 
+        /// <inheritdoc/>
+        public Task OpenAsync(
+            string sessionName,
+            uint sessionTimeout,
+            IUserIdentity identity,
+            IList<string> preferredLocales,
+            bool checkDomain,
+            CancellationToken ct)
+        {
+            return OpenAsync(sessionName, sessionTimeout, identity, preferredLocales, checkDomain, true, ct);
+        }
+
         /// <inheritdoc/>
         public async Task OpenAsync(
             string sessionName,
             uint sessionTimeout,
             IUserIdentity identity,
             IList<string> preferredLocales,
             bool checkDomain,
+            bool closeChannel,
             CancellationToken ct)
         {
             OpenValidateIdentity(ref identity, out var identityToken, out var identityPolicy, out string securityPolicyUri, out bool requireEncryption);
@@ -322,7 +335,10 @@ public async Task OpenAsync(
                 try
                 {
                     await base.CloseSessionAsync(null, false, CancellationToken.None).ConfigureAwait(false);
-                    await CloseChannelAsync(CancellationToken.None).ConfigureAwait(false);
+                    if (closeChannel)
+                    {
+                        await CloseChannelAsync(CancellationToken.None).ConfigureAwait(false);
+                    }
                 }
                 catch (Exception e)
                 {
@@ -1514,11 +1530,12 @@ public static async Task<Session> RecreateAsync(Session sessionTemplate, Cancell
 
             try
             {
+                session.RecreateRenewUserIdentity();
                 // open the session.
                 await session.OpenAsync(
                     sessionTemplate.SessionName,
                     (uint)sessionTemplate.SessionTimeout,
-                    sessionTemplate.Identity,
+                    session.Identity,
                     sessionTemplate.PreferredLocales,
                     sessionTemplate.m_checkDomain,
                     ct).ConfigureAwait(false);
@@ -1562,11 +1579,12 @@ public static async Task<Session> RecreateAsync(Session sessionTemplate, ITransp
 
             try
             {
+                session.RecreateRenewUserIdentity();
                 // open the session.
                 await session.OpenAsync(
                     sessionTemplate.m_sessionName,
                     (uint)sessionTemplate.m_sessionTimeout,
-                    sessionTemplate.m_identity,
+                    session.Identity,
                     sessionTemplate.m_preferredLocales,
                     sessionTemplate.m_checkDomain,
                     ct).ConfigureAwait(false);
@@ -1604,13 +1622,15 @@ public static async Task<Session> RecreateAsync(Session sessionTemplate, ITransp
 
             try
             {
+                session.RecreateRenewUserIdentity();
                 // open the session.
                 await session.OpenAsync(
                     sessionTemplate.m_sessionName,
                     (uint)sessionTemplate.m_sessionTimeout,
-                    sessionTemplate.m_identity,
+                    session.Identity,
                     sessionTemplate.m_preferredLocales,
                     sessionTemplate.m_checkDomain,
+                    false,
                     ct).ConfigureAwait(false);
 
                 // create the subscriptions.

Libraries/Opc.Ua.Client/Session/TraceableSession.cs

@@ -626,6 +626,15 @@ public void Open(string sessionName, uint sessionTimeout, IUserIdentity identity
             }
         }
 
+        /// <inheritdoc/>
+        public void Open(string sessionName, uint sessionTimeout, IUserIdentity identity, IList<string> preferredLocales, bool checkDomain, bool closeChannel)
+        {
+            using (Activity activity = ActivitySource.StartActivity())
+            {
+                m_session.Open(sessionName, sessionTimeout, identity, preferredLocales, checkDomain, closeChannel);
+            }
+        }
+
         /// <inheritdoc/>
         public void ChangePreferredLocales(StringCollection preferredLocales)
         {
@@ -707,6 +716,15 @@ public async Task OpenAsync(string sessionName, uint sessionTimeout, IUserIdenti
             }
         }
 
+        /// <inheritdoc/>
+        public async Task OpenAsync(string sessionName, uint sessionTimeout, IUserIdentity identity, IList<string> preferredLocales, bool checkDomain, bool closeChannel, CancellationToken ct)
+        {
+            using (Activity activity = ActivitySource.StartActivity())
+            {
+                await m_session.OpenAsync(sessionName, sessionTimeout, identity, preferredLocales, checkDomain, closeChannel, ct).ConfigureAwait(false);
+            }
+        }
+
 
         /// <inheritdoc/>
         public async Task FetchNamespaceTablesAsync(CancellationToken ct = default)

Tests/Opc.Ua.Client.Tests/ClientTest.cs

@@ -625,6 +625,12 @@ public async Task ConnectMultipleSessionsAsync()
             session2.DetachChannel();
             session2.Dispose();
 
+
+            //Recreate session using same channel
+            var session3 = await ClientFixture.SessionFactory.RecreateAsync(session1, channel).ConfigureAwait(false);
+
+            _ = session3.ReadValue(VariableIds.Server_ServerStatus, typeof(ServerStatusDataType));
+
             channel.Dispose();
         }
 
@@ -851,6 +857,89 @@ public async Task ReconnectSessionOnAlternateChannelWithSavedSessionSecrets(stri
             Utils.SilentDispose(session2);
         }
 
+        /// <summary>
+        /// Open a session on a channel, then recreate using the session as a template, verify the renewUserIdentityHandler is brought to the new session and called before Session.Open
+        /// /// </summary>
+        [Test, Order(270)]
+        [TestCase(false)]
+        [TestCase(true)]
+        public async Task RecreateSessionWithRenewUserIdentity(bool async)
+        {
+            IUserIdentity userIdentityAnonymous = new UserIdentity() ;
+            IUserIdentity userIdentityPW = new UserIdentity("user1", "password");
+
+            // the first channel determines the endpoint
+            ConfiguredEndpoint endpoint = await ClientFixture.GetEndpointAsync(ServerUrl, SecurityPolicies.Basic256Sha256, Endpoints).ConfigureAwait(false);
+            Assert.NotNull(endpoint);
+
+            UserTokenPolicy identityPolicy = endpoint.Description.FindUserTokenPolicy(userIdentityAnonymous.TokenType,
+                userIdentityAnonymous.IssuedTokenType,
+                endpoint.Description.SecurityPolicyUri);
+            if (identityPolicy == null)
+            {
+                Assert.Ignore($"No UserTokenPolicy found for {userIdentityAnonymous.TokenType} / {userIdentityAnonymous.IssuedTokenType}");
+            }
+
+            // the active channel
+            ISession session1 = await ClientFixture.ConnectAsync(endpoint, userIdentityAnonymous).ConfigureAwait(false);
+            Assert.NotNull(session1);
+
+            ServerStatusDataType value1 = (ServerStatusDataType)session1.ReadValue(VariableIds.Server_ServerStatus, typeof(ServerStatusDataType));
+            Assert.NotNull(value1);
+
+            // hook callback to renew the user identity
+            session1.RenewUserIdentity += (session, identity) => {
+                return userIdentityPW;
+            };
+
+            Client.Session session2;
+            if (async)
+            {
+                session2 = await Client.Session.RecreateAsync((Session)((TraceableSession)session1).Session).ConfigureAwait(false);
+            }
+            else
+            {
+                session2 = Client.Session.Recreate((Session)((TraceableSession)session1).Session);
+            }
+
+            // create new channel
+            ITransportChannel channel2 = await ClientFixture.CreateChannelAsync(session1.ConfiguredEndpoint, false).ConfigureAwait(false);
+            Assert.NotNull(channel2);
+
+            Client.Session session3;
+            if (async)
+            {
+                session3 = await Client.Session.RecreateAsync((Session)((TraceableSession)session1).Session, channel2).ConfigureAwait(false);
+
+            }
+            else
+            {
+                session3 = Client.Session.Recreate((Session)((TraceableSession)session1).Session, channel2);
+            }
+
+            //validate new Session Ids are used and also UserName PW identity token is injected as renewed token
+            Assert.AreNotEqual(session1.SessionId, session2.SessionId);
+            Assert.AreEqual(userIdentityPW.TokenType, session2.Identity.TokenType);
+            Assert.AreNotEqual(session1.SessionId, session3.SessionId);
+            Assert.AreEqual(userIdentityPW.TokenType, session3.Identity.TokenType);
+
+            ServerStatusDataType value2 = (ServerStatusDataType)session2.ReadValue(VariableIds.Server_ServerStatus, typeof(ServerStatusDataType));
+            Assert.NotNull(value2);
+
+
+            session1.DeleteSubscriptionsOnClose = true;
+            session1.Close(1000);
+            Utils.SilentDispose(session1);
+
+            session2.DeleteSubscriptionsOnClose = true;
+            session2.Close(1000);
+            Utils.SilentDispose(session2);
+
+            session3.DeleteSubscriptionsOnClose = true;
+            session3.Close(1000);
+            Utils.SilentDispose(session3);
+        }
+
         [Test, Order(300)]
         public new void GetOperationLimits()
         {