From 160d704114c57d08785c95e59357f6bc2306d6a0 Mon Sep 17 00:00:00 2001
From: James Pace <jpace121@gmail.com>
Date: Sat, 20 Nov 2021 15:02:30 +0000
Subject: [PATCH] General cleanup.

---
 include/jwp-plugin/AuthList.hpp   |  1 +
 include/jwp-plugin/Authorizer.hpp |  3 +--
 include/jwp-plugin/jwp-plugin.h   |  3 +++
 src/AuthList.cpp                  |  9 +++++----
 src/Authorizer.cpp                | 10 +++++-----
 src/jwp-plugin.cpp                | 19 ++++++++-----------
 6 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/include/jwp-plugin/AuthList.hpp b/include/jwp-plugin/AuthList.hpp
index 6255070..9b8e396 100644
--- a/include/jwp-plugin/AuthList.hpp
+++ b/include/jwp-plugin/AuthList.hpp
@@ -15,6 +15,7 @@
 #include <forward_list>
 #include <string>
 
+// A list with easily checkable contents.
 class AuthList
 {
 public:
diff --git a/include/jwp-plugin/Authorizer.hpp b/include/jwp-plugin/Authorizer.hpp
index a5687a7..0e81f81 100644
--- a/include/jwp-plugin/Authorizer.hpp
+++ b/include/jwp-plugin/Authorizer.hpp
@@ -15,11 +15,10 @@
 
 #include <jwp-plugin/AuthList.hpp>
 
-#include <jwt-cpp/jwt.h>
-
 #include <optional>
 #include <string>
 
+// Autenticates a user using jwts.
 class Authorizer
 {
 public:
diff --git a/include/jwp-plugin/jwp-plugin.h b/include/jwp-plugin/jwp-plugin.h
index b56aa6f..6f1451a 100644
--- a/include/jwp-plugin/jwp-plugin.h
+++ b/include/jwp-plugin/jwp-plugin.h
@@ -12,6 +12,9 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// Mosquitto authentication plugin that using Authorizer to authorize
+// users using jwts.
+
 extern "C" {
     #include "mosquitto.h"
     #include "mosquitto_broker.h"
diff --git a/src/AuthList.cpp b/src/AuthList.cpp
index f22320d..2c76757 100644
--- a/src/AuthList.cpp
+++ b/src/AuthList.cpp
@@ -23,8 +23,7 @@ void AuthList::add(const std::string& username)
 {
     // Is the username already in the list?
     // If not add it.
-    const auto found = std::find(std::begin(_allowedUsernames), std::end(_allowedUsernames), username);
-    if(found == std::end(_allowedUsernames))
+    if(not confirm(username))
     {
         _allowedUsernames.emplace_front(username);
     }
@@ -32,8 +31,9 @@ void AuthList::add(const std::string& username)
 
 void AuthList::remove(const std::string& username)
 {
-    const auto found = std::find(std::begin(_allowedUsernames), std::end(_allowedUsernames), username);
-    if(found != std::end(_allowedUsernames))
+    // Is the user in the list?
+    // Is so, remove it,
+    if(confirm(username))
     {
         _allowedUsernames.remove(username);
     }
@@ -41,6 +41,7 @@ void AuthList::remove(const std::string& username)
 
 bool AuthList::confirm(const std::string& username)
 {
+    // Is the user in the list?
     const auto found = std::find(std::begin(_allowedUsernames), std::end(_allowedUsernames), username);
     if(found != std::end(_allowedUsernames))
     {
diff --git a/src/Authorizer.cpp b/src/Authorizer.cpp
index 22ec220..f2caa32 100644
--- a/src/Authorizer.cpp
+++ b/src/Authorizer.cpp
@@ -63,7 +63,7 @@ bool Authorizer::add(const std::string& token, const std::string& username)
     }
     catch(jwt::error::token_verification_exception& exception)
     {
-        std::cout << exception.what() << std::endl;
+        std::cerr << exception.what() << std::endl;
         return false;
     }
     auto claims = decoded_token.get_payload_claims();
@@ -71,19 +71,19 @@ bool Authorizer::add(const std::string& token, const std::string& username)
     // Check username matches.
     if(not claims.contains("upn"))
     {
-        std::cout << "Missing upn." << std::endl;
+        std::cerr << "Missing upn." << std::endl;
         return false;
     }
     if(claims["upn"].as_string() != username)
     {
-        std::cout << "Wrong username." << std::endl;
+        std::cerr << "Wrong username." << std::endl;
         return false;
     }
 
     // Check for mqtt-write claim value.
     if(not (claims.contains("mqtt-write") and claims.contains("mqtt-read")))
     {
-        std::cout << "Missing mqtt-write or mqtt-read." << std::endl;
+        std::cerr << "Missing mqtt-write or mqtt-read." << std::endl;
         return false;
     }
 
@@ -91,7 +91,7 @@ bool Authorizer::add(const std::string& token, const std::string& username)
     bool can_read = claims["mqtt-read"].as_bool();
     if(not (can_write or can_read))
     {
-        std::cout << "Can't write or can't read." << std::endl;
+        std::cerr << "Can't write or can't read." << std::endl;
         return false;
     }
 
diff --git a/src/jwp-plugin.cpp b/src/jwp-plugin.cpp
index f8f53bf..1fbd8ae 100644
--- a/src/jwp-plugin.cpp
+++ b/src/jwp-plugin.cpp
@@ -50,27 +50,24 @@ int mosquitto_plugin_init(mosquitto_plugin_id_t *identifier, void **userdata, st
         if(key == "public_key")
         {
             const auto key = Authorizer::read_key(std::string(options[index].value));
-            if(key)
-            {
-                public_key = *key;
-            }
-            else
+            if(not key or key->empty())
             {
+                mosquitto_log_printf(MOSQ_LOG_ERR, "Could not read public key.");
                 return MOSQ_ERR_INVAL;
             }
+            public_key = *key;
         }
         else if(key == "issuer")
         {
             issuer = std::string(options[index].value);
+            if(issuer.empty())
+            {
+                mosquitto_log_printf(MOSQ_LOG_ERR, "issuer not set.");
+                return MOSQ_ERR_INVAL;
+            }
         }
     }
 
-    if(public_key.empty() or issuer.empty())
-    {
-        mosquitto_log_printf(MOSQ_LOG_ERR, "public_key or issue not set.");
-        return MOSQ_ERR_INVAL;
-    }
-
     authorizer = std::make_unique<Authorizer>(public_key, issuer);
 
     // Register the callbacks.