From 45b62c5a34132c66d6687a0a6abaf1140304c02d Mon Sep 17 00:00:00 2001
From: Vitaliy Kharechko <vitaliy.kharechko@axilera.com>
Date: Fri, 1 Jul 2016 10:30:51 +0300
Subject: [PATCH] Clean up auth impl, add env variables for auth, and
 documentation

Change-Id: I0df391da0df6e6e12daa323e659de80116e9de8f
---
 broadview_lib/config/agentconnection.py | 34 ++++++----
 broadview_lib/config/broadviewconfig.py |  1 -
 doc/auth.md                             | 88 +++++++++++++++++++++++++
 3 files changed, 108 insertions(+), 15 deletions(-)
 create mode 100644 doc/auth.md

diff --git a/broadview_lib/config/agentconnection.py b/broadview_lib/config/agentconnection.py
index 11af766..f620904 100644
--- a/broadview_lib/config/agentconnection.py
+++ b/broadview_lib/config/agentconnection.py
@@ -23,6 +23,7 @@ from xml.etree import ElementTree
 import json
 from sidauth import SIDAuth
 from broadview_lib.config.broadviewconfig import BroadViewBSTSwitches
+import os
 
 try:
     from oslo_log import log as logging
@@ -106,6 +107,13 @@ class AgentConnection():
         if "auth" in conf:
             auth["auth"] = conf["auth"]
 
+    if auth["auth"] == None:
+        auth["auth"] = os.getenv("BV_AUTH")
+    if auth["username"] == None:
+        auth["username"] = os.getenv("BV_USERNAME")
+    if auth["password"] == None:
+        auth["password"] = os.getenv("BV_PASSWORD", "")
+
     return auth
 
   def __makeRequest(self, request):
@@ -160,29 +168,27 @@ class AgentConnection():
         try:
             auth_method = r.headers["WWW-Authenticate"]
         except:
-            auth_method = None
+            # RFC 2616 requires a WWW-Authenticate header in 401 responses. If
+            # we get here, it was missing. Check if there is configuration that
+            # declares an auth method and use that.
+            LOG.info("makeRequest: 401 but no WWW-Authenticate")
+            auth_method = conf["auth"]
         if auth_method:
             auth_method = auth_method.lower()
             if auth_method == "basic":
                 self._auth = requests.HTTPBasicAuth(conf["username"], conf["password"])
             elif auth_method == "digest":
-                self._auth[self.host] = requests.HTTPDigestAuth(conf["username"], conf["password"])
+                self._auth = requests.HTTPDigestAuth(conf["username"], conf["password"])
             elif auth_method == "sidauth":
-                self._auth[self.host] = SIDAuth(self.host, self.port, conf["username"], conf["password"])
+                self._auth = SIDAuth(self.host, self.port, conf["username"], conf["password"])
             else:
                 LOG.info("unknown auth {}".format(auth_method))
-                return
-        else:
-            # RFC 2616 requires a WWW-Authenticate header in 401 responses. If
-            # we get here, it was missing. Check if there is configuration that
-            # declares an auth method and use that.
-            LOG.info("makeRequest: 401 but no WWW-Authenticate")
-            if conf["auth"] and conf["auth"].lower() == "sidauth":
-                self._auth = SIDAuth(self.host, self.port, conf["username"], conf["password"])
+                # return the 401 here
+                return (r.status_code, json_data)
+        
+            # try again
 
-        # try again
-
-        r, json_data = self.__makeRequest(request)
+            r, json_data = self.__makeRequest(request)
 
     return (r.status_code, json_data)
 
diff --git a/broadview_lib/config/broadviewconfig.py b/broadview_lib/config/broadviewconfig.py
index 31a63f9..f90fd66 100644
--- a/broadview_lib/config/broadviewconfig.py
+++ b/broadview_lib/config/broadviewconfig.py
@@ -73,7 +73,6 @@ class TestBSTSwitches(unittest.TestCase):
             self.assertTrue("ip" in y)
             self.assertTrue("port" in y)
             self.assertTrue("description" in y)
-            self.assertTrue("asics" in y)
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/doc/auth.md b/doc/auth.md
new file mode 100644
index 0000000..21f2179
--- /dev/null
+++ b/doc/auth.md
@@ -0,0 +1,88 @@
+# Authentication
+
+broadview-lib uses python "requests" module for issuing HTTP 1.1 POST
+requests to the agent. In some cases, authentication is required by the
+device. The requests module methods (e.g., post(), get()) support an
+auth keyword argument which can be used to specify an instance of an
+object that derives from requests.auth.AuthBase, and handle the tasks
+related to authentication. More information can
+be found here: http://docs.python-requests.org/en/master/user/authentication/
+
+In broadview_lib/agentconnection.py we've included support for three such
+objects:
+
+* requests.HTTPBasicAuth
+* requests.HTTPDigestAuth
+* SIDAuth
+
+HTTPBasicAuth and HTTPDigestAuth are documented on the Requests page linked
+to above, and are provided by the requests module. SIDAuth is supplied as a 
+class by broadview-lib in config/siduath.py to provide authentication to 
+switches which are running the ICOS NOS.
+
+## Specifying Authentication Settings
+
+To use one of the above authentication methods, you must identify the 
+method, and supply a username and password for the device that you are
+trying to connect to. There are two methods for doing this supported by
+broadview-lib:
+
+* environment variables
+* configuration in /etc/broadviewswitches.conf
+
+Note: if a HTTP 401 error is returned by the agent and WWW-Authenticate header 
+is supplied by the server (as required by RFC 2616) then the authentation 
+method specified in the WWW-Authenticate header is used. Otherwise, the method 
+is determined by looking at either the environment variable or the setting in 
+/etc/broadviewswitches.conf. 
+
+The settings file has priority over environment variables. In other words,
+if both specify a value, the settings file value is used, and the environment
+variable value will be ignored.
+
+The following sections detail each method.
+
+### Authentication Environment Variables
+
+Three environment variables can be used to specify authentication settings:
+
+* BV_AUTH - a string that specifies the authentication method. Supported 
+values include "basic", for HTTPBasicAuth, "digest", for HTTPDigestAuth,
+and "sidauth", for SIDAuth authentication. Use this variable only against
+devices that are not providing a WWW-Authenticate header.
+* BV_USERNAME - a string that contains the username used for authentication 
+* BV_PASSWORD - a string that contains the password used for authentication 
+
+BV_AUTH is ignored if WWW-Authenticate header is present in the HTTP 401
+response.
+
+Each environment variable will be ignored if a corresponding value is present
+in /etc/broadviewswitches.conf.
+
+### Authentication settings in /etc/broadviewswitches.conf
+
+The broadviewswitches.conf was originally designed to provide topology 
+information for the BroadView horizon dashboard in OpenStack. It was extended
+to support 3 additional settings per switch:
+
+* auth - a string that specifies the authentication method. Supported
+values include "basic", for HTTPBasicAuth, "digest", for HTTPDigestAuth,
+and "sidauth", for SIDAuth authentication. Use this variable only against
+devices that are not providing a WWW-Authenticate header.
+* username - a string that contains the username used for authentication
+* password - a string that contains the password used for authentication
+
+These settings will override the corresponding environment variables, as
+defined in the previous section. auth is overridden on a per-response
+basis if WWW-Authenticate is present in the HTTP 401 response headers.
+
+The following is an example /etc/broadviewswitches.conf file.
+
+    [topology]
+
+    bst_switches = [ { "ip": "10.14.244.119", "port": 80, \
+    "description" : "switch 1", "auth": "sidauth", \
+    "username": "admin", "password": ""}, \
+    { "ip": "10.27.31.1", "port": 80, "description" : "switch 2", \
+    "auth": "sidauth", "username": "admin", "password": "" } ] 
+