Problem
I have source code for execute a command (ProcessStart) using Impersonate, and I want apply best practices and good design pattern, and good performance if possible.
I also want to use good Logging pattern. I use the Log pattern for logging my “Executor”.
Any suggestions?
Source code (partial)
public partial class AgentServerExecutor
{
// TODO: Attention static multi-thread
static StringBuilder sbLog = new StringBuilder();
static Action<string> logTracer = null;
private static void Log(string msg)
{
Trace.WriteLine(msg);
sbLog.AppendLine(msg);
if (logTracer == null) return;
logTracer(msg);
}
public static bool DeployUsingCommandExecutor(Action<string> tracer, ArgumentsDeploymentExecutor arguments, out string errorDeploy)
{
logTracer = tracer;
try
{
var command = arguments.ExecutorCommand;
var empaquetadoXml = arguments.PackageXmlPath;
var projectName = arguments.ProjectName;
var projectGuid = arguments.ProjectGuid;
Log("Deploying command " + command);
var cmdDeploy = ConfigurationManager.AppSettings.Get(ConfigConsola.ComandoDeployKeyAppSettings);
Log("Cmd: " + cmdDeploy);
cmdDeploy = command;
var f = new System.IO.FileInfo(cmdDeploy);
Log("Cmd Fullname: " + f.FullName);
if (!f.Exists) { errorDeploy = "Not exists " + f.FullName; return false; } // Logging ??
Action<object, DataReceivedEventArgs> actionWrite = (sender, e) =>
{
Log(" DataReceived " + e.Data);
};
Log("File: " + empaquetadoXml);
GestorDatosDespliegue.FicheroDatosDespliegue = empaquetadoXml;
GestorDatosDespliegue.CargarDespliegueActual();
GestorDatosDespliegue.ObtenerListaDespliegues();
List<DatosDespliegueBase> listaDatosDespliegue = GestorDatosDespliegue.ListaProyectos;
Guid guid;
var assertGuid = Guid.TryParse(projectGuid, out guid);
if (!assertGuid) throw new InvalidOperationException("Guid no se considera válido: " + projectGuid);
DatosDespliegueBase dd = null;
foreach (var dd2 in listaDatosDespliegue)
{
if (!(dd2.Name.Equals(projectName, StringComparison.InvariantCulture) && dd2.GuidProyecto.Equals(guid))) continue;
dd = dd2;
}
var ficheroSecurity = ConfigConsola.FicheroSecurity;
Log("ficheroSecurity " + ficheroSecurity);
Log("MapPath " + System.Web.Hosting.HostingEnvironment.MapPath("~/"));
var rutaFicheroKey = System.Web.Hosting.HostingEnvironment.MapPath("~/" + ficheroSecurity);
Log("rutaFicheroKey " + rutaFicheroKey);
Log(System.Diagnostics.Process.GetCurrentProcess().ProcessName);
if (AssemblyHelper.WebDevServerMode && !File.Exists(rutaFicheroKey))
{
rutaFicheroKey = System.Web.Hosting.HostingEnvironment.MapPath("~/bin/" + ficheroSecurity);
Log("rutaFicheroKey " + rutaFicheroKey);
}
if (AssemblyHelper.UnitTestMode && !File.Exists(rutaFicheroKey))
{
rutaFicheroKey = Path.Combine(Path.GetDirectoryName(AssemblyHelper.ApplicationLocation), ficheroSecurity);
Log("rutaFicheroKey " + rutaFicheroKey);
}
// TODO: If is Windows Service
if (!File.Exists(rutaFicheroKey))
{
rutaFicheroKey = Path.Combine(Path.GetDirectoryName(AssemblyHelper.ApplicationLocation), ficheroSecurity);
Log("rutaFicheroKey " + rutaFicheroKey);
}
var userPassDto = PasswordsManager.ObtenerUsuarioInstalador(dd.TipoAplicacionDespliegue, dd.EntornoActual.ToString(), rutaFicheroKey);
if (!userPassDto.UsuarioObtenido) throw new InvalidOperationException("Not found credentials for " + dd.EntornoActual + " y Tipo aplicación " + dd.TipoAplicacionDespliegue);
IssuesAboutSecurity();
Log(
string.Format("•————————————————————————————————————————————————————————————————————————•")
+ Environment.NewLine
+ string.Format("| Credentials for deployment in '{0}' environment |", dd.EntornoActual)
+ Environment.NewLine
+ string.Format("| User '{0}' for DeploymentType '{1}' - '{2}' |", userPassDto.Usuario, dd.TipoDespliegue, dd.TipoAplicacionDespliegue)
+ Environment.NewLine
+ string.Format("| User '{0}' - UserDomainName '{1}' |", Environment.UserName, Environment.UserDomainName)
+ Environment.NewLine
+ string.Format("| WindowsIdentity '{0}' |", WindowsIdentity.GetCurrent().Name)
+ Environment.NewLine
+ string.Format("•————————————————————————————————————————————————————————————————————————•")
+ Environment.NewLine
);
//LocalSystem Usuario 'SYSTEM' UserDomainName 'MYDOMAIN'
//LocalSystem WindowsIdentity NT AUTHORITYSYSTEM
//LocalService | Usuario 'LOCAL SERVICE' - UserDomainName 'NT AUTHORITY' |
//| WindowsIdentity 'NT AUTHORITYLOCAL SERVICE' |
// Datos de usuario
Log(string.Format("Actual User '{0}\{1}'. Client MachineName '{2}'", Environment.UserDomainName, Environment.UserName, Environment.MachineName));
Log(string.Format("ProcessName '{0}'. CurrentProcess MachineName '{1}'rn", Process.GetCurrentProcess().ProcessName, Process.GetCurrentProcess().MachineName));
var destino = Path.GetDirectoryName(empaquetadoXml);
DeployManager.GenerarSustitucionesDeFicherosEnDirectorioDestino(destino, dd.EntornoActual, dd.SustitucionesPorFichero);
string output = null, errors = null;
int exitCode = int.MinValue;
if (!Environment.MachineName.Equals(Environment.UserDomainName, StringComparison.InvariantCulture))
{
var t1 = "MachineName " + Environment.MachineName + " NOT EQUALS UserDomainName " + Environment.UserDomainName
+ Environment.NewLine;
Log(t1);
Log("Security Issues Asproc Library " + userPassDto.Usuario);
SecurityIssuesAsproLibrary(userPassDto.Usuario);
Log("Admin ??????" + userPassDto.Usuario);
//var u1 = new System.Security.Principal.WindowsIdentity(userPassDto.Usuario);
var u2 = System.Security.Principal.WindowsIdentity.GetCurrent();
Log(" Admin " + u2.Name + " ? " + SecurityHelper.IsAdmin());
ActionHelper.SafeExecutor( () => {
string[] du = userPassDto.Usuario.Split('\');
Log(" Admin " + userPassDto.Usuario + " ? " + SecurityHelper.IsDomainAdmin(du[0], du[1])
+ Environment.NewLine + " Validate " + SecurityHelper.ValidateCredentials(du[0], du[1], userPassDto.Password));
});
}
var logsFolder = Path.GetDirectoryName(empaquetadoXml).CombinePathWith("Logs");
IOHelper.TryCreateDirectory(logsFolder);
Log("Creado " + logsFolder);
var args = arguments.ToArgumentsConsole();
Action<ProcessStartInfo> actionForImpersonate = (startInfo) =>
{
var usuario = userPassDto.Usuario;
var pwd = userPassDto.Password;
bool usuarioConDominio = usuario.IndexOf("\") > -1;
var wi = WindowsIdentity.GetCurrent().Name;
usuarioConDominio = usuarioConDominio && !usuario.Equals(wi, StringComparison.InvariantCultureIgnoreCase);
Log("IMPERSONATE: User: " + usuario + Environment.NewLine
+ "WindowsIdentity " + wi + Environment.NewLine);
string[] du = usuario.Split('\');
var codePwd = GetInfoPassword(pwd);
if (usuarioConDominio)
{
Log("IMPERSONATE: Usuario con dominio. Se procede a IMPERSONATE el proceso remoto con " + du[0] + "\" + du[1]);
Log(codePwd);
startInfo.Domain = du[0];
startInfo.UserName = du[1];
startInfo.Password = SecureStringHelper.ToSecureString(pwd);
startInfo.LoadUserProfile = true;
}
if (!usuarioConDominio)
{
Log("IMPERSONATE: Usuario SIN dominio. Se procede a IMPERSONATE el proceso remoto con " + Environment.MachineName + "\" + du[0]);
Log(codePwd);
startInfo.Domain = Environment.MachineName;
startInfo.UserName = du[0];
startInfo.Password = SecureStringHelper.ToSecureString(pwd);
startInfo.LoadUserProfile = true;
}
};
ExecuteCommand(guid, f.FullName, args, actionWrite, actionForImpersonate, userPassDto.Usuario, userPassDto.Password, out output, out errors, out exitCode);
Log("Output:rn" + output + "rn"
+ "Errors:rn" + errors + "rn"
+ "ExitCode: " + exitCode + "rn");
var success = string.IsNullOrEmpty(errors)
&& exitCode.Equals(0);
errorDeploy = errors;
return success;
}
finally
{
logTracer("*** LOG *** " + Environment.NewLine
+ sbLog.ToString() + Environment.NewLine
+ "*** LOG *** " + Environment.NewLine );
}
}
Solution
This is bad way to write code , I could not even read the code without headache. Please separate out your logging functionality, you function should not be more than 30 lines per method, so first
Log(
string.Format("•————————————————————————————————————————————————————————————————————————•")
+ Environment.NewLine
+ string.Format("| Credentials for deployment in '{0}' environment |", dd.EntornoActual)
+ Environment.NewLine
+
string.Format("| User '{0}' for DeploymentType '{1}' - '{2}' |", userPassDto.Usuario,
dd.TipoDespliegue, dd.TipoAplicacionDespliegue)
+ Environment.NewLine
+
string.Format("| User '{0}' - UserDomainName '{1}' |", Environment.UserName,
Environment.UserDomainName)
+ Environment.NewLine
+ string.Format("| WindowsIdentity '{0}' |", WindowsIdentity.GetCurrent().Name)
+ Environment.NewLine
+ string.Format("•————————————————————————————————————————————————————————————————————————•")
+ Environment.NewLine
);
you dont need this whole code in this way , rather create a logger class and put this whole line of code there. Pass only dynamic parameter.
Once you are able to cleanup , pattern can be applied easily