From 19716405b8038ba6ec0b01d5ada40f161f60d4c1 Mon Sep 17 00:00:00 2001
From: Kevin Thompson <antiduh@gmail.com>
Date: Sun, 1 Apr 2018 18:05:00 -0400
Subject: [PATCH] Moved SetThreadIdentity into the ServerContext

The ImpersonationHandle class should probably stay as bare as possible since it's provided to the caller and we don't want to expose anything more than we need. Since the ServerContext is capable of performing this as a private method, move it there.
Updated the comments and variable names on the constructor to better indicate what exactly the option does, especially since modification of this property seems to be such a touchy subject.
---
 NSspi/Contexts/ImpersonationHandle.cs | 10 +-------
 NSspi/Contexts/ServerContext.cs       | 33 +++++++++++++++++++--------
 2 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/NSspi/Contexts/ImpersonationHandle.cs b/NSspi/Contexts/ImpersonationHandle.cs
index 477702b..8bdbdf0 100644
--- a/NSspi/Contexts/ImpersonationHandle.cs
+++ b/NSspi/Contexts/ImpersonationHandle.cs
@@ -31,14 +31,6 @@ namespace NSspi.Contexts
             this.disposed = false;
         }
 
-        /// <summary>
-        /// Set the current thread security context to the impersonated identity
-        /// </summary>
-        public void SetThreadIdentity()
-        {
-            Thread.CurrentPrincipal = new WindowsPrincipal(WindowsIdentity.GetCurrent(TokenAccessLevels.AllAccess));
-        }
-
         ~ImpersonationHandle()
         {
             Dispose( false );
@@ -55,7 +47,7 @@ namespace NSspi.Contexts
 
         protected virtual void Dispose( bool disposing )
         {
-            if ( disposing && this.disposed == false  && this.server != null && this.server.Disposed == false )
+            if( disposing && this.disposed == false && this.server != null && this.server.Disposed == false )
             {
                 this.server.RevertImpersonate();
             }
diff --git a/NSspi/Contexts/ServerContext.cs b/NSspi/Contexts/ServerContext.cs
index 65bcfef..686b4ee 100644
--- a/NSspi/Contexts/ServerContext.cs
+++ b/NSspi/Contexts/ServerContext.cs
@@ -1,5 +1,7 @@
 using System;
 using System.Runtime.CompilerServices;
+using System.Security.Principal;
+using System.Threading;
 using NSspi.Buffers;
 using NSspi.Credentials;
 
@@ -14,22 +16,25 @@ namespace NSspi.Contexts
         private ContextAttrib finalAttribs;
 
         private bool impersonating;
-        private bool setThreadIdentity;
+        private bool impersonationSetsThreadPrinciple;
 
         /// <summary>
-        /// Performs basic initialization of a new instance of the ServerContext class. The ServerContext
-        /// is not ready for message manipulation until a security context has been established with a client.
+        /// Performs basic initialization of a new instance of the ServerContext class. The
+        /// ServerContext is not ready for message manipulation until a security context has been
+        /// established with a client.
         /// </summary>
         /// <param name="cred"></param>
         /// <param name="requestedAttribs"></param>
-        /// <param name="setThreadIdentity">True to automatically set the thread identity while impersonating</param>
-        public ServerContext( Credential cred, ContextAttrib requestedAttribs, bool setThreadIdentity = false ) : base( cred )
+        /// <param name="impersonationSetsThreadPrinciple">
+        /// If true, the `Thread.CurrentPrinciple` property will be modified by successful impersonation.
+        /// </param>
+        public ServerContext( Credential cred, ContextAttrib requestedAttribs, bool impersonationSetsThreadPrinciple = false ) : base( cred )
         {
             this.requestedAttribs = requestedAttribs;
             this.finalAttribs = ContextAttrib.Zero;
 
             this.impersonating = false;
-            this.setThreadIdentity = setThreadIdentity;
+            this.impersonationSetsThreadPrinciple = impersonationSetsThreadPrinciple;
 
             this.SupportsImpersonate = this.Credential.PackageInfo.Capabilities.HasFlag( SecPkgCapability.Impersonation );
         }
@@ -223,7 +228,7 @@ namespace NSspi.Contexts
 
                     this.ContextHandle.DangerousRelease();
 
-                    this.impersonating = true;
+                    this.impersonating = status == SecurityStatus.OK;
                 }
             }
 
@@ -240,9 +245,9 @@ namespace NSspi.Contexts
                 throw new SSPIException( "Failed to impersonate the client", status );
             }
 
-            if ( this.impersonating && this.setThreadIdentity )
+            if( this.impersonating && this.impersonationSetsThreadPrinciple )
             {
-                handle.SetThreadIdentity();
+                SetThreadPrinciple();
             }
 
             return handle;
@@ -307,5 +312,15 @@ namespace NSspi.Contexts
 
             base.Dispose( disposing );
         }
+
+        /// <summary>
+        /// Set the current thread security context to the impersonated identity.
+        /// </summary>
+        private void SetThreadPrinciple()
+        {
+            Thread.CurrentPrincipal = new WindowsPrincipal(
+                WindowsIdentity.GetCurrent( TokenAccessLevels.AllAccess ) 
+            );
+        }
     }
 }
\ No newline at end of file