Restart Citrix session using locally stored application

Question

I have a Windows forms application which we deployed in my firm for use on thin clients. The app lives locally on each user's machine, but pretty much all of the work the users do is over a Citrix session.

One of the options (the main option) available to the user is the ability to restart their own session. I recently attempted to completely redesign this area of the program as version which is currently live is very messy despite the fact that it works quite reliably.

A brief outline:

A list of servers is retrieved from AD. It check if an appdata file (created by the logon script) is present which contains the server name. If not, or if it contains the wrong server, then it iterates through all servers and queries them through the CMD until the server is found. It then updates the session with these details. If there is no server (for example, halfway through a restart), then it opens Outlook (published) and checks if the user is logged on against our server list, then displays a loading screen until "lync.exe" is running.

The session reset is initiated inside a background worker. This keeps the UI responsive and allows for a loading GIF (on page 10):

BackgroundWorker Reset

    public void backgroundReset_DoWork(object sender, DoWorkEventArgs e)
    {
        // Allow page change when whilst on a thread other than the Form's
        pageControl1.Invoke(new Action(() =>
        {
            pageControl1.SelectedIndex = 10;
        }));

        // _currentsession is initialised right at the beginning, in a separate backgroundWorker (if there is no session, one is created and assigned to _currentsession)
        Session oldsession = _currentSession;
        Session newsession = oldsession.Reset();

        e.Result = newsession;
    }

    public void backgroundReset_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
    {
        Session newsession = (Session)e.Result;

        if (newsession.ID == null)
        {
            // No session - revert to emergency support page
            pageControl1.SelectedIndex = 5;
        }
        else
        {
            // Assign the new session to our session field, and navigate to the "work done" page
            _currentSession = newsession;
            pageControl1.SelectedIndex = 3;
        }
    }

Session

class Session
{
    #region Fields
    private string _username;
    private string _pcname;
    private int? _id; // the ? allows it to be nullable ... to make checking easier
    private string _server;
    #endregion

    #region Properties
    public string UserName
    {
        get { return _username; }
        set { _username = value; }
    }
    public string PCName
    {
        get { return _pcname; }
        set { _pcname = value; }
    }
    public int? ID
    {
        get { return _id; }
        set { _id = value; }
    } 
    public string ServerName
    {
        get { return _server; }
        set { _server = value; }
    }
    #endregion

    public Session()
    {
        // When a session is instantiated, retrieve the details immediately:
        PCName = Environment.MachineName;
        UserName = WindowsIdentity.GetCurrent().Name.Split('\\')[1];

        SessionHelper.GetSession(this);
    }

    public Session Reset()
    {
        SessionHelper.Reset(this);
        return this;
    }
}

One of my main concerns with this is my use of a SessionHelper class; is this necessary? I originally had all my methods in Session, but at the time it seemed sensible to separate the concept of a Session and the methods performed on the session in some way:

static class SessionHelper
{
    public static string AppData = Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData);
    private static string AppDataCitrix = AppData + "\\Citrix\\SelfService\\";

    //////////////////////////////////////////////////////////// Destroy - Log session off
    static public void Destroy(Session session)
    {
        string cmdIn = "Logoff " + session.ID + " /SERVER:" + session.ServerName;
        Cmd.Exec(cmdIn);
    }
    //////////////////////////////////////////////////////////// Reset
    public static Session Reset(Session oldsession)
    {
        Destroy(oldsession);

        while (oldsession.ID != null)
        {
            // If there is no session, CheckSession will update the details with null
            CheckSession(oldsession);
        }

        System.Threading.Thread.Sleep(5000);

        // Start outlook
        Start();

        Session newsession = GetSession(oldsession);

        // Query all servers until we find a session
        while (newsession.ID == null)
        {
            newsession = GetSession(oldsession);
        }

        // While lync isn't running, sleep 1 second then check again
        while (IsRunning(newsession, "lync.exe") == false)
        {
            System.Threading.Thread.Sleep(1000);
        }

        return newsession;
    }
    //////////////////////////////////////////////////////////// Is Running
    public static bool IsRunning(Session session, string processName)
    {
        string strcmdIn = "tasklist /S " + session.ServerName + " /FI \"SESSION eq " + session.ID + "\"";
        string cmdOut = Cmd.StdOutAdmin(strcmdIn);

        if (cmdOut.Contains(processName))
        {
            return true;
        }
        else
        {
            return false;
        }
    }
    //////////////////////////////////////////////////////////// Start Session
    public static void Start()
    {
        Process.Start(AppDataCitrix + "Outlook.exe");
    }
    //////////////////////////////////////////////////////////// Check Session - Checks whether the given session is active, and updates it (whether it is active or not)
    static public void CheckSession(Session session)
    {
        string queryResult = Query(session, session.ServerName);

        if (queryResult.Contains(session.UserName))
        {
            try
            {
                session.ID = Int32.Parse(queryResult.Substring(119, 4).Trim());
                session.ServerName = session.ServerName;
            }
            catch (FormatException)
            {
                // The query output did not have a session id, or the session id was at a different index in the string
            }
        }
        else
        {
            session.ID = null;
            session.ServerName = null;
        }
    }
    //////////////////////////////////////////////////////////// Query - queries the given server against the session's user name and return the results
    static private string Query(Session session, string server)
    {
        string cmdIn = "query session " + session.UserName + " /SERVER:" + server;
        string cmdOut = Cmd.StdOut(cmdIn);

        return cmdOut;
    }
    //////////////////////////////////////////////////////////// Get Session - check all servers to find the current session
    public static Session GetSession(Session session)
    {
        // Session session = new Session();
        string queryResult;

        session = CheckFile(session);

        // If the CheckFile method returned the current session ...
        if (session.ID != null)
        {
            return session;
        }

        foreach (string server in Servers.List)
        {
            queryResult = Query(session, server);

            if (queryResult.Contains(session.UserName))
            {
                try
                {
                    session.ID = Int32.Parse(queryResult.Substring(119, 4).Trim());
                    session.ServerName = server;

                    return session;
                }
                catch (FormatException)
                {
                    // ID not in valid int format
                }
            }
        }

        return session;
    }
    //////////////////////////////////////////////////////////// Revive session - when there is no session but there should be
    public static Session Revive(Session session)
    {
        Start();

        // Query all servers until we find a session
        while (session.ID == null/* && ++count < 15*/)
        {
            session = GetSession(session);
        }

        // While outlook isn't running, sleep 1 second then check again
        while (IsRunning(session, "lync.exe") == false)
        {
            //   System.Threading.Thread.Sleep(1000);
        }

        return session;
    }
    //////////////////////////////////////////////////////////// Check Server File - file created by the log on script
    private static Session CheckFile(Session session)
    {
        string[] allFiles = System.IO.Directory.GetFiles("\\\\file\\Home$\\" + session.UserName + "\\Application Data");
        string serverFileId = "qxjz";
        string servName;
        string queryResult;

        foreach (string file in allFiles)
        {
            // Found the file containing the code
            if (file.Contains(serverFileId))
            {
                string[] nameArr = file.Split('\\');
                servName = nameArr[nameArr.Length - 1];
                servName = servName.Substring(4, servName.Length - 8);

                queryResult = Query(session, servName);

                // It's the right server file!
                if (queryResult.Length != 0)
                {
                    try
                    {
                        session.ID = Int32.Parse(queryResult.Substring(119, 4).Trim());
                        session.ServerName = servName;

                        return session;
                    }
                    catch (FormatException)
                    {
                        // Wrong format for the session ID
                    }
                }

                // File exists, but its the wrong server
                break;
            }
        }

        return session;
    }
}

Please also note that I still need to build in a check where if, after a session restart, lync (Skype for business) does not open after x amount of time, the app gives up checking and either tries to start again or informs the user. This also applies to the while loop which iterates through all the servers until the correct one is found (in some cases there will be no valid server).

Please also ignore any error-handling attempts, as these are not yet fully implemented (if at all).

Cmd Class - for sending arguments to the command prompt

static class Cmd
{
    /////////////////////////////////////////////////// EXECUTE COMMAND
    public static void Exec(string args)
    {
        ProcessStartInfo startInfo = new ProcessStartInfo("cmd", "/C " + args)
        {
            WindowStyle = ProcessWindowStyle.Hidden,
            UseShellExecute = false,
            CreateNoWindow = true
        };

        Process p = Process.Start(startInfo);
        p.Start();
    }

    /////////////////////////////////////////////////// EXECUTE COMMAND AS ADMIN
    public static void ExecAdmin(string args)
    {
        ProcessStartInfo startInfo = new ProcessStartInfo("cmd", "/C " + args)
        {
            WindowStyle = ProcessWindowStyle.Hidden,
            UseShellExecute = false,
            CreateNoWindow = true,

            WorkingDirectory = @"C:\windows\system32",
            Verb = "runas",
            Domain = "myDomain",
            UserName = "myUserName",
            Password = pw()
        };

        Process p = Process.Start(startInfo);
        p.Start();
    }

    /////////////////////////////////////////////////// GET THE STDOUT
    public static string StdOut(string args)
    {
        string cmdOut = "";

        ProcessStartInfo startInfo = new ProcessStartInfo("cmd", "/C " + args)
        {
            WindowStyle = ProcessWindowStyle.Hidden,
            UseShellExecute = false,
            RedirectStandardOutput = true,
            RedirectStandardError = true,
            CreateNoWindow = true
        };

        cmdOut = ExecuteCommand(cmdOut, startInfo);

        return cmdOut;
    }

    /////////////////////////////////////////////////// GET THE STDOUT AS ADMIN
    public static string StdOutAdmin(string args)
    {
        string cmdOut = "";

        ProcessStartInfo startInfo = new ProcessStartInfo("cmd", "/C " + args)
        {
            WindowStyle = ProcessWindowStyle.Hidden,
            UseShellExecute = false,
            RedirectStandardOutput = true,
            RedirectStandardError = true,
            CreateNoWindow = true,

            WorkingDirectory = @"C:\windows\system32",
            Verb = "runas",
            Domain = "myDomain",
            UserName = "myUserName",
            Password = pw()
        };

        cmdOut = ExecuteCommand(cmdOut, startInfo);

        return cmdOut;
    }

    /////////////////////////////////////////////////// GET THE STDOUT AS ADMIN IN LIST FORMAT
    public static List<string> StdOutAdminList(string args)
    {
        List<string> cmdOut = new List<string>();

        ProcessStartInfo startInfo = new ProcessStartInfo("cmd", "/C " + args)
        {
            WindowStyle = ProcessWindowStyle.Hidden,
            UseShellExecute = false,
            RedirectStandardOutput = true,
            RedirectStandardError = true,
            CreateNoWindow = true,

            WorkingDirectory = @"C:\windows\system32",
            Verb = "runas",
            Domain = "myDomain",
            UserName = "myUserName",
            Password = pw()
        };

        cmdOut = ExecuteCommand(cmdOut, startInfo);

        return cmdOut;
    }

    /////////////////////////////////////////////////// EXECUTE COMMAND
    private static string ExecuteCommand(string cmdOut, ProcessStartInfo startInfo)
    {
        Process p = Process.Start(startInfo);
        p.OutputDataReceived += (x, y) => cmdOut += y.Data;
        p.BeginOutputReadLine();
        p.WaitForExit();
        return cmdOut;
    }

    /////////////////////////////////////////////////// EXECUTE A LIST COMMAND
    private static List<string> ExecuteCommand(List<string> cmdOut, ProcessStartInfo startInfo)
    {
        Process p = Process.Start(startInfo);
        p.OutputDataReceived += (x, y) => cmdOut.Add(y.Data);
        p.BeginOutputReadLine();
        p.WaitForExit();
        return cmdOut;
    }

    private static SecureString pw()
    {

        SecureString ss = new SecureString();

        ss.AppendChar('p');
        ss.AppendChar('a');
        ss.AppendChar('s');
        ss.AppendChar('s');
        ss.AppendChar('w');
        ss.AppendChar('o');
        ss.AppendChar('r');
        ss.AppendChar('d');
        ss.AppendChar('1');
        ss.AppendChar('2');
        ss.AppendChar('3');

        return ss;
    }
}

Any feedback on the design at all is much appreciated. There is also an issue, where if CMD is run through Process.startInfo with admin details, the CMD window is not hidden no matter which options I choose. I can't believe that there is no workaround available for this!

Answer 1

Auto-Implemented Properties

If you're using any C# 3.0 or higher, you can use an Auto-Implemented Property, rather than manually declaring private property variables and the getters and setters. If you use Auto-Implemented Properties, then this chunk of code:

#region Fields
private string _username;
private string _pcname;
private int? _id; // the ? allows it to be nullable ... to make checking easier
private string _server;
#endregion

#region Properties
public string UserName
{
    get { return _username; }
    set { _username = value; }
}
public string PCName
{
    get { return _pcname; }
    set { _pcname = value; }
}
public int? ID
{
    get { return _id; }
    set { _id = value; }
} 
public string ServerName
{
    get { return _server; }
    set { _server = value; }
}
#endregion

Can become the following much shorter and much cleaner looking piece of code:

public string UserName { get; set; }
public string PCName { get; set; }
public int? ID { get; set; }
public string ServerName { get; set; }

You can also remove the #region Properties as well since there's no real need to be able to expand and minimize four lines of code.

---

Nitpicks

You have a lot of unnecessary comments in your code. The best example I can find was probably this one:

//////////////////////////////////////////////////////////// Reset

There are quite a few things wrong with this:

• The function it resides above is itself named "Reset".
• It contains an excessive amount of forward slashes, distracting whoever may be trying to read the code.

There are many more comments in your code like this one, and I'm sure you'll be able to pick them out and improve upon them.

If you really want to have a useful comment, I'd highly recommend using an XML Documentation Comment. A typical XML Documentation Comment looks something like this:

/// <summary>
/// Describe your property, function or class here.
/// </summary>
/// <remarks>Make a remark about your property, function or class here, if needed.</remark>
/// <param name="parameterName">Describe your parameter(s) here and below, if needed.</param>

I've also noticed that you've used string literals in some places for path strings, but in other places you just write a normal string with escaped backslashes, like you did here:

private static string AppDataCitrix = AppData + "\\Citrix\\SelfService\\";

Whether you use string literals for paths or choose not to, just remember the important rule of be-consistent.

When I was first looking at your code, I came across these two lines:

Session oldsession = _currentSession;
Session newsession = oldsession.Reset();

There is no need for the oldsession variable in this scenario. It's just extraneous code. You could write these two lines as one:

Session newSession = _currentSession.Reset();