Finalize the Download Command changes (#203)

* Refactor parameter parsing in Download.cs, add more checks

* Handle Download#unsecureAuth as boolean

* Parse Enums in a case-insensitive mode, propagate error correctly

* Add tests for the newly introduced functionality

* Update the configuration sample to reflect the recent changes

* Update the sample text according to the proposal from @nightman68

doc/xmlConfigFile.md

@@ -154,7 +154,7 @@ For servers requiring authentication some parameters must be specified depending
 * `basic`: Basic authentication, sub-parameters:
 	* `username=“UserName”`
 	* `password=“Passw0rd”`
-	* `unsecureAuth=“enabled”: default=“disabled"`
+	* `unsecureAuth=“true”: default=“false"`
 
 The parameter “unsecureAuth” is only effective when the transfer protocol is HTTP - unencrypted data transfer. This is a security vulnerability because the credentials are send in clear text! For a SSPI authentication this is not relevant because the authentication tokens are encrypted.
 
@@ -176,8 +176,8 @@ Examples:
               auth="basic" username="aUser" password="aPassw0rd" />
 
     <download from="http://example.com/some.dat" to="%BASE%\some.dat"
-              auth="basic" unsecureAuth=“enabled”
-              username="aUser" password=“aPassw0rd" />
+              auth="basic" unsecureAuth="true"
+              username="aUser" password="aPassw0rd" />
 ```
 
 This is another useful building block for developing a self-updating service.

examples/sample-allOptions.xml

@@ -251,18 +251,21 @@ SECTION: Environment setup
   -->
   <!--
   <download from="http://www.google.com/" to="%BASE%\index.html" />
-  <download from="http://www.nosuchhostexists.com/" to="%BASE%\dummy.html" />
+  
+  Download and fail the service startup on Error:
+  <download from="http://www.nosuchhostexists.com/" to="%BASE%\dummy.html" failOnError="true"/>
 
   An example for unsecure Basic authentication because the connection is not encrypted:
   <download from="http://example.com/some.dat" to="%BASE%\some.dat"
-            auth="basic" unsecureAuth=“enabled”
+            auth="basic" unsecureAuth=“true”
             username="aUser" password=“aPassw0rd" />
 
   Secure Basic authentication via HTTPS:
   <download from="https://example.com/some.dat" to="%BASE%\some.dat"
             auth="basic" username="aUser" password="aPassw0rd" />
 
-  Secure authentication when the target server and the client are members of domain:
+  Secure authentication when the target server and the client are members of the same domain or 
+  the server domain and the client domain belong to the same forest with a trust:
   <download from="https://example.com/some.dat" to="%BASE%\some.dat" auth="sspi" />
   -->
 
@@ -297,4 +300,4 @@ More info is available here: https://github.com/kohsuke/winsw/blob/master/doc/ex
 </extensions>
 -->
 
-</configuration>
+</configuration>

src/Core/WinSWCore/Download.cs

@@ -3,6 +3,7 @@
 using System.Net;
 using System.Text;
 using System.Xml;
+using winsw.Util;
 
 namespace winsw
 {
@@ -19,72 +20,64 @@ public enum AuthType { none = 0, sspi, basic }
         public readonly AuthType Auth = AuthType.none;
         public readonly string Username;
         public readonly string Password;
-        public readonly bool UnsecureAuth = false;
+        public readonly bool UnsecureAuth;
         public readonly bool FailOnError;
 
-        public Download(string from, string to, bool failOnError = false)
+        public string ShortId { get { return String.Format("(download from {0})", From); } }
+
+        public Download(string from, string to, bool failOnError = false, AuthType auth = AuthType.none, 
+            string username = null, string password = null, bool unsecureAuth = false)
         {
             From = from;
             To = to;
             FailOnError = failOnError;
+            Auth = auth;
+            Username = username;
+            Password = password;
+            UnsecureAuth = unsecureAuth;
         }
 
-        internal Download(XmlNode n)
+        /// <summary>
+        /// Constructs the download setting sfrom the XML entry
+        /// </summary>
+        /// <param name="n">XML element</param>
+        /// <exception cref="InvalidDataException">The required attribute is missing or the configuration is invalid</exception>
+        internal Download(XmlElement n)
         {
-            From = Environment.ExpandEnvironmentVariables(n.Attributes["from"].Value);
-            To = Environment.ExpandEnvironmentVariables(n.Attributes["to"].Value);
-
-            var failOnErrorNode = n.Attributes["failOnError"];
-            FailOnError = failOnErrorNode != null ? Boolean.Parse(failOnErrorNode.Value) : false;
-
-            string tmpStr = "";
-            try
-            {
-                tmpStr = Environment.ExpandEnvironmentVariables(n.Attributes["auth"].Value);
-            }
-            catch (Exception)
-            {
-            }
-            Auth = tmpStr != "" ? (AuthType)Enum.Parse(typeof(AuthType), tmpStr) : AuthType.none;
+            From = XmlHelper.SingleAttribute<String>(n, "from");
+            To = XmlHelper.SingleAttribute<String>(n, "to");
 
-            try
-            {
-                tmpStr = Environment.ExpandEnvironmentVariables(n.Attributes["username"].Value);
-            }
-            catch (Exception)
-            {
-            }
-            Username = tmpStr;
+            // All arguments below are optional
+            FailOnError = XmlHelper.SingleAttribute<bool>(n, "failOnError", false);
 
-            try
-            {
-                tmpStr = Environment.ExpandEnvironmentVariables(n.Attributes["password"].Value);
-            }
-            catch (Exception)
-            {
-            }
-            Password = tmpStr;
-
-            try
-            {
-                tmpStr = Environment.ExpandEnvironmentVariables(n.Attributes["unsecureAuth"].Value);
-            }
-            catch (Exception)
-            {
-            }
-            UnsecureAuth = tmpStr == "enabled" ? true : false;
+            Auth = XmlHelper.EnumAttribute<AuthType>(n, "auth", AuthType.none);
+            Username = XmlHelper.SingleAttribute<String>(n, "user", null);
+            Password = XmlHelper.SingleAttribute<String>(n, "password", null);
+            UnsecureAuth = XmlHelper.SingleAttribute<bool>(n, "unsecureAuth", false);
 
             if (Auth == AuthType.basic)
             {
-                if (From.StartsWith("http:") && UnsecureAuth == false)
+                // Allow it only for HTTPS or for UnsecureAuth 
+                if (!From.StartsWith("https:") && !UnsecureAuth)
+                {
+                    throw new InvalidDataException("Warning: you're sending your credentials in clear text to the server " + ShortId + 
+                                                   "If you really want this you must enable 'unsecureAuth' in the configuration");
+                }
+
+                // Also fail if there is no user/password
+                if (Username == null)
+                {
+                    throw new InvalidDataException("Basic Auth is enabled, but username is not specified " + ShortId);
+                }
+                if (Password == null)
                 {
-                    throw new Exception("Warning: you're sending your credentials in clear text to the server. If you really want this you must enable this in the configuration!");
+                    throw new InvalidDataException("Basic Auth is enabled, but password is not specified " + ShortId);
                 }
             }
         }
 
         // Source: http://stackoverflow.com/questions/2764577/forcing-basic-authentication-in-webrequest
-        public void SetBasicAuthHeader(WebRequest request, String username, String password)
+        private void SetBasicAuthHeader(WebRequest request, String username, String password)
         {
             string authInfo = username + ":" + password;
             authInfo = Convert.ToBase64String(Encoding.GetEncoding("ISO-8859-1").GetBytes(authInfo));
@@ -103,6 +96,10 @@ public void Perform()
 
             switch (Auth)
             {
+                case AuthType.none:
+                    // Do nothing
+                    break;
+
                 case AuthType.sspi:
                     req.UseDefaultCredentials = true;
                     req.PreAuthenticate = true;
@@ -112,6 +109,9 @@ public void Perform()
                 case AuthType.basic:
                     SetBasicAuthHeader(req, Username, Password);
                     break;
+
+                default:
+                    throw new WebException("Code defect. Unsupported authentication type: " + Auth);
             }
 
             WebResponse rsp = req.GetResponse();
@@ -123,15 +123,6 @@ public void Perform()
             File.Move(To + ".tmp", To);
         }
 
-        /// <summary>
-        /// Produces the XML configuuration entry.
-        /// </summary>
-        /// <returns>XML String for the configuration file</returns>
-        public String toXMLConfig()
-        {
-            return "<download from=\"" + From + "\" to=\"" + To + "\" failOnError=\"" + FailOnError + "\"/>";
-        }
-
         private static void CopyStream(Stream i, Stream o)
         {
             byte[] buf = new byte[8192];

src/Core/WinSWCore/ServiceDescriptor.cs

@@ -563,7 +563,11 @@ public List<Download> Downloads
                 List<Download> r = new List<Download>();
                 foreach (XmlNode n in xmlNodeList)
                 {
-                    r.Add(new Download(n));
+                    XmlElement el = n as XmlElement;
+                    if (el != null)
+                    {
+                        r.Add(new Download(el));
+                    }
                 }
                 return r;
             }

src/Core/WinSWCore/Util/XmlHelper.cs

@@ -71,5 +71,32 @@ public static TAttributeType SingleAttribute<TAttributeType>(XmlElement node, st
              var value = (TAttributeType)Convert.ChangeType(substitutedValue, typeof(TAttributeType));
              return value;
         }
+
+        /// <summary>
+        /// Retireves a single enum attribute
+        /// </summary>
+        /// <typeparam name="TAttributeType">Type of the enum</typeparam>
+        /// <param name="node">Parent node</param>
+        /// <param name="attributeName">Attribute name</param>
+        /// <param name="defaultValue">Default value</param>
+        /// <returns>Attribute value (or default)</returns>
+        /// <exception cref="InvalidDataException">Wrong enum value</exception>
+        public static TAttributeType EnumAttribute<TAttributeType>(XmlElement node, string attributeName, TAttributeType defaultValue)
+        {
+            if (!node.HasAttribute(attributeName)) return defaultValue;
+
+            string rawValue = node.GetAttribute(attributeName);
+            string substitutedValue = Environment.ExpandEnvironmentVariables(rawValue);
+            try
+            {
+                var value = Enum.Parse(typeof(TAttributeType), substitutedValue, true);
+                return (TAttributeType)value;
+            }
+            catch (Exception ex) // Most likely ArgumentException
+            {
+                throw new InvalidDataException("Cannot parse <" +  attributeName + "> Enum value from string '" + substitutedValue +
+                    "'. Enum type: " + typeof(TAttributeType), ex);
+            }
+        }
     }
 }