From caede2e40b83403f7d830065cff4c1f91997b031 Mon Sep 17 00:00:00 2001
From: Jordan Wright <jmwright798@gmail.com>
Date: Thu, 16 Jan 2020 22:21:58 -0600
Subject: [PATCH] Refactoring Logging (#1722)

* Added ParseLevel to set log level (#1671)
* Moved logger config into the logger package for better decoupling. Added logging tests.

Co-authored-by: Amal Alkhamees <Amalkh5@users.noreply.github.com>
---
 config.json           |  3 ++-
 config/config.go      | 24 ++++++++++--------------
 gophish.go            |  2 +-
 logger/logger.go      | 26 ++++++++++++++++++++++----
 logger/logger_test.go | 26 ++++++++++++++++++++++++++
 5 files changed, 61 insertions(+), 20 deletions(-)
 create mode 100644 logger/logger_test.go

diff --git a/config.json b/config.json
index 9abaf1d1..d25df02e 100644
--- a/config.json
+++ b/config.json
@@ -16,6 +16,7 @@
 	"migrations_prefix": "db/db_",
 	"contact_address": "",
 	"logging": {
-		"filename": ""
+		"filename": "",
+		"level": ""
 	}
 }
\ No newline at end of file
diff --git a/config/config.go b/config/config.go
index ccb6d639..70d76e29 100644
--- a/config/config.go
+++ b/config/config.go
@@ -2,6 +2,7 @@ package config
 
 import (
 	"encoding/json"
+	log "github.com/gophish/gophish/logger"
 	"io/ioutil"
 )
 
@@ -21,22 +22,17 @@ type PhishServer struct {
 	KeyPath   string `json:"key_path"`
 }
 
-// LoggingConfig represents configuration details for Gophish logging.
-type LoggingConfig struct {
-	Filename string `json:"filename"`
-}
-
 // Config represents the configuration information.
 type Config struct {
-	AdminConf      AdminServer   `json:"admin_server"`
-	PhishConf      PhishServer   `json:"phish_server"`
-	DBName         string        `json:"db_name"`
-	DBPath         string        `json:"db_path"`
-	DBSSLCaPath    string        `json:"db_sslca_path"`
-	MigrationsPath string        `json:"migrations_prefix"`
-	TestFlag       bool          `json:"test_flag"`
-	ContactAddress string        `json:"contact_address"`
-	Logging        LoggingConfig `json:"logging"`
+	AdminConf      AdminServer `json:"admin_server"`
+	PhishConf      PhishServer `json:"phish_server"`
+	DBName         string      `json:"db_name"`
+	DBPath         string      `json:"db_path"`
+	DBSSLCaPath    string      `json:"db_sslca_path"`
+	MigrationsPath string      `json:"migrations_prefix"`
+	TestFlag       bool        `json:"test_flag"`
+	ContactAddress string      `json:"contact_address"`
+	Logging        *log.Config `json:"logging"`
 }
 
 // Version contains the current gophish version
diff --git a/gophish.go b/gophish.go
index e86393d7..c8505da8 100644
--- a/gophish.go
+++ b/gophish.go
@@ -69,7 +69,7 @@ func main() {
 	}
 	config.Version = string(version)
 
-	err = log.Setup(conf)
+	err = log.Setup(conf.Logging)
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/logger/logger.go b/logger/logger.go
index e697f5f3..94692889 100644
--- a/logger/logger.go
+++ b/logger/logger.go
@@ -1,10 +1,10 @@
 package logger
 
 import (
+	"errors"
 	"io"
 	"os"
 
-	"github.com/gophish/gophish/config"
 	"github.com/sirupsen/logrus"
 )
 
@@ -12,16 +12,34 @@ import (
 // It is exported here for use with gorm.
 var Logger *logrus.Logger
 
+// ErrInvalidLevel is returned when an invalid log level is given in the config
+var ErrInvalidLevel = errors.New("invalid log level")
+
+// Config represents configuration details for logging.
+type Config struct {
+	Filename string `json:"filename"`
+	Level    string `json:"level"`
+}
+
 func init() {
 	Logger = logrus.New()
 	Logger.Formatter = &logrus.TextFormatter{DisableColors: true}
 }
 
 // Setup configures the logger based on options in the config.json.
-func Setup(conf *config.Config) error {
-	Logger.SetLevel(logrus.InfoLevel)
+func Setup(config *Config) error {
+	var err error
+	// Set up logging level
+	level := logrus.InfoLevel
+	if config.Level != "" {
+		level, err = logrus.ParseLevel(config.Level)
+		if err != nil {
+			return err
+		}
+	}
+	Logger.SetLevel(level)
 	// Set up logging to a file if specified in the config
-	logFile := conf.Logging.Filename
+	logFile := config.Filename
 	if logFile != "" {
 		f, err := os.OpenFile(logFile, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644)
 		if err != nil {
diff --git a/logger/logger_test.go b/logger/logger_test.go
new file mode 100644
index 00000000..6f648b42
--- /dev/null
+++ b/logger/logger_test.go
@@ -0,0 +1,26 @@
+package logger
+
+import "testing"
+
+import "github.com/sirupsen/logrus"
+
+func TestLogLevel(t *testing.T) {
+	tests := map[string]logrus.Level{
+		"":      logrus.InfoLevel,
+		"debug": logrus.DebugLevel,
+		"info":  logrus.InfoLevel,
+		"error": logrus.ErrorLevel,
+		"fatal": logrus.FatalLevel,
+	}
+	config := &Config{}
+	for level, expected := range tests {
+		config.Level = level
+		err := Setup(config)
+		if err != nil {
+			t.Fatalf("error setting logging level %v", err)
+		}
+		if Logger.Level != expected {
+			t.Fatalf("invalid logging level. expected %v got %v", expected, Logger.Level)
+		}
+	}
+}