Refactor: Do not log in internal packages

The reason behind this is that we then do not have to pass a lot to
each function. Logging inside internal packages is less useful as we
want to let them return errors and only log in the 'public' facing API
or let the client decide

8 files changed, 26 insertions, 69 deletions

diff --git a/internal/discovery/discovery.go b/internal/discovery/discovery.go
index b4163a8..24ed01c 100644
--- a/internal/discovery/discovery.go
+++ b/internal/discovery/discovery.go
@@ -4,19 +4,15 @@ import (
 	"encoding/json"
 	"fmt"
 
-	"github.com/jwijenbergh/eduvpn-common/internal/fsm"
 	"github.com/jwijenbergh/eduvpn-common/internal/http"
-	"github.com/jwijenbergh/eduvpn-common/internal/log"
-	"github.com/jwijenbergh/eduvpn-common/internal/types"
 	"github.com/jwijenbergh/eduvpn-common/internal/util"
+	"github.com/jwijenbergh/eduvpn-common/internal/types"
 	"github.com/jwijenbergh/eduvpn-common/internal/verify"
 )
 
 type Discovery struct {
 	Organizations types.DiscoveryOrganizations
 	Servers       types.DiscoveryServers
-	FSM           *fsm.FSM
-	Logger        *log.FileLogger
 }
 
 // Helper function that gets a disco json
@@ -59,11 +55,6 @@ func getDiscoFile(jsonFile string, previousVersion uint64, structure interface{}
 	return string(fileBody), nil
 }
 
-func (discovery *Discovery) Init(fsm *fsm.FSM, logger *log.FileLogger) {
-	discovery.FSM = fsm
-	discovery.Logger = logger
-}
-
 // FIXME: Implement based on
 // https://github.com/eduvpn/documentation/blob/v3/SERVER_DISCOVERY.md
 // - [IMPLEMENTED] on "first launch" when offering the search for "Institute Access" and "Organizations";
@@ -150,7 +141,6 @@ func (discovery *Discovery) DetermineServersUpdate() bool {
 	if now >= should_update_time {
 		return true
 	}
-	discovery.Logger.Log(log.LOG_INFO, "No update needed for servers, 1h is not passed yet")
 	return false
 }
 
diff --git a/internal/fsm/fsm.go b/internal/fsm/fsm.go
index c87fd4f..b822a72 100644
--- a/internal/fsm/fsm.go
+++ b/internal/fsm/fsm.go
@@ -8,7 +8,6 @@ import (
 	"path"
 	"sort"
 
-	"github.com/jwijenbergh/eduvpn-common/internal/log"
 	"github.com/jwijenbergh/eduvpn-common/internal/types"
 )
 
@@ -126,12 +125,11 @@ type FSM struct {
 	// Info to be passed from the parent state
 	Name          string
 	StateCallback func(string, string, string)
-	Logger        *log.FileLogger
 	Directory     string
 	Debug         bool
 }
 
-func (fsm *FSM) Init(name string, callback func(string, string, string), logger *log.FileLogger, directory string, debug bool) {
+func (fsm *FSM) Init(name string, callback func(string, string, string), directory string, debug bool) {
 	fsm.States = FSMStates{
 		DEREGISTERED:   FSMState{Transitions: []FSMTransition{{NO_SERVER, "Client registers"}}},
 		NO_SERVER:      FSMState{Transitions: []FSMTransition{{CHOSEN_SERVER, "User chooses a server"}, {SEARCH_SERVER, "The user is trying to choose a Server in the UI"}, {CONNECTED, "The user is already connected"}}},
@@ -150,7 +148,6 @@ func (fsm *FSM) Init(name string, callback func(string, string, string), logger
 	fsm.Current = DEREGISTERED
 	fsm.Name = name
 	fsm.StateCallback = callback
-	fsm.Logger = logger
 	fsm.Directory = directory
 	fsm.Debug = debug
 }
@@ -180,7 +177,6 @@ func (fsm *FSM) writeGraph() {
 	graphImgFile := fsm.getGraphFilename(".png")
 	f, err := os.Create(graphFile)
 	if err != nil {
-		fsm.Logger.Log(log.LOG_INFO, fmt.Sprintf("Failed to write debug fsm graph with error %v", err))
 		return
 	}
 
@@ -205,8 +201,6 @@ func (fsm *FSM) GoTransitionWithData(newState FSMStateID, data string, backgroun
 			fsm.writeGraph()
 		}
 
-		fsm.Logger.Log(log.LOG_INFO, fmt.Sprintf("State: %s -> State: %s with data %s\n", oldState.String(), newState.String(), data))
-
 		if background {
 			go fsm.StateCallback(oldState.String(), newState.String(), data)
 		} else {
diff --git a/internal/oauth/oauth.go b/internal/oauth/oauth.go
index d828e57..b1569e9 100644
--- a/internal/oauth/oauth.go
+++ b/internal/oauth/oauth.go
@@ -11,7 +11,6 @@ import (
 
 	"github.com/jwijenbergh/eduvpn-common/internal/fsm"
 	httpw "github.com/jwijenbergh/eduvpn-common/internal/http"
-	"github.com/jwijenbergh/eduvpn-common/internal/log"
 	"github.com/jwijenbergh/eduvpn-common/internal/types"
 	"github.com/jwijenbergh/eduvpn-common/internal/util"
 )
@@ -62,7 +61,6 @@ type OAuth struct {
 	Token                OAuthToken           `json:"token"`
 	BaseAuthorizationURL string               `json:"base_authorization_url"`
 	TokenURL             string               `json:"token_url"`
-	Logger               *log.FileLogger      `json:"-"`
 	FSM                  *fsm.FSM             `json:"-"`
 }
 
@@ -223,11 +221,10 @@ func (oauth *OAuth) Callback(w http.ResponseWriter, req *http.Request) {
 	}
 }
 
-func (oauth *OAuth) Init(baseAuthorizationURL string, tokenURL string, fsm *fsm.FSM, logger *log.FileLogger) {
+func (oauth *OAuth) Init(baseAuthorizationURL string, tokenURL string, fsm *fsm.FSM) {
 	oauth.BaseAuthorizationURL = baseAuthorizationURL
 	oauth.TokenURL = tokenURL
 	oauth.FSM = fsm
-	oauth.Logger = logger
 }
 
 // Starts the OAuth exchange for eduvpn.
@@ -312,7 +309,6 @@ func (oauth *OAuth) Login(name string, postprocessAuth func(string) string) erro
 func (oauth *OAuth) NeedsRelogin() bool {
 	// Access Token or Refresh Tokens empty, definitely needs a relogin
 	if oauth.Token.Access == "" || oauth.Token.Refresh == "" {
-		oauth.Logger.Log(log.LOG_INFO, "OAuth: Tokens are empty")
 		return true
 	}
 
@@ -321,14 +317,12 @@ func (oauth *OAuth) NeedsRelogin() bool {
 	// The tokens are not expired yet
 	// No relogin is needed
 	if !oauth.isTokensExpired() {
-		oauth.Logger.Log(log.LOG_INFO, "OAuth: Tokens are not expired, re-login not needed")
 		return false
 	}
 
 	refreshErr := oauth.getTokensWithRefresh()
 	// We have obtained new tokens with refresh
 	if refreshErr == nil {
-		oauth.Logger.Log(log.LOG_INFO, "OAuth: Tokens could be re-acquired using the refresh token, re-login not needed")
 		return false
 	}
 
diff --git a/internal/server/api.go b/internal/server/api.go
index c8c7180..a3d8e31 100644
--- a/internal/server/api.go
+++ b/internal/server/api.go
@@ -8,7 +8,6 @@ import (
 	"net/url"
 
 	httpw "github.com/jwijenbergh/eduvpn-common/internal/http"
-	"github.com/jwijenbergh/eduvpn-common/internal/log"
 	"github.com/jwijenbergh/eduvpn-common/internal/types"
 	"github.com/jwijenbergh/eduvpn-common/internal/util"
 )
@@ -70,17 +69,12 @@ func apiAuthorized(server Server, method string, endpoint string, opts *httpw.HT
 func apiAuthorizedRetry(server Server, method string, endpoint string, opts *httpw.HTTPOptionalParams) (http.Header, []byte, error) {
 	errorMessage := "failed authorized API retry"
 	header, body, bodyErr := apiAuthorized(server, method, endpoint, opts)
-	base, baseErr := server.GetBase()
 
-	if baseErr != nil {
-		return nil, nil, &types.WrappedErrorMessage{Message: errorMessage, Err: baseErr}
-	}
 	if bodyErr != nil {
 		var error *httpw.HTTPStatusError
 
 		// Only retry authorized if we get a HTTP 401
 		if errors.As(bodyErr, &error) && error.Status == 401 {
-			base.Logger.Log(log.LOG_INFO, fmt.Sprintf("API: Got HTTP error %v, retrying authorized", error))
 			// Tell the method that the token is expired
 			server.GetOAuth().Token.ExpiredTimestamp = util.GenerateTimeSeconds()
 			retryHeader, retryBody, retryErr := apiAuthorized(server, method, endpoint, opts)
diff --git a/internal/server/common.go b/internal/server/common.go
index da71488..5340b39 100644
--- a/internal/server/common.go
+++ b/internal/server/common.go
@@ -5,7 +5,6 @@ import (
 	"fmt"
 
 	"github.com/jwijenbergh/eduvpn-common/internal/fsm"
-	"github.com/jwijenbergh/eduvpn-common/internal/log"
 	"github.com/jwijenbergh/eduvpn-common/internal/oauth"
 	"github.com/jwijenbergh/eduvpn-common/internal/types"
 	"github.com/jwijenbergh/eduvpn-common/internal/util"
@@ -23,7 +22,6 @@ type ServerBase struct {
 	StartTime      int64             `json:"start_time"`
 	EndTime        int64             `json:"expire_time"`
 	Type           string            `json:"server_type"`
-	Logger         *log.FileLogger   `json:"-"`
 	FSM            *fsm.FSM          `json:"-"`
 }
 
@@ -208,7 +206,7 @@ func (servers *Servers) GetCurrentServerInfoJSON() (string, error) {
 	return string(bytes), nil
 }
 
-func (servers *Servers) addInstituteAndCustom(discoServer *types.DiscoveryServer, isCustom bool, fsm *fsm.FSM, logger *log.FileLogger) (Server, error) {
+func (servers *Servers) addInstituteAndCustom(discoServer *types.DiscoveryServer, isCustom bool, fsm *fsm.FSM) (Server, error) {
 	url := discoServer.BaseURL
 	errorMessage := fmt.Sprintf("failed adding institute access server: %s", url)
 	toAddServers := &servers.InstituteServers
@@ -232,7 +230,7 @@ func (servers *Servers) addInstituteAndCustom(discoServer *types.DiscoveryServer
 
 	// Set the current server
 	toAddServers.CurrentURL = url
-	instituteInitErr := server.init(url, discoServer.DisplayName, discoServer.Type, discoServer.SupportContact, fsm, logger)
+	instituteInitErr := server.init(url, discoServer.DisplayName, discoServer.Type, discoServer.SupportContact, fsm)
 	if instituteInitErr != nil {
 		return nil, &types.WrappedErrorMessage{Message: errorMessage, Err: instituteInitErr}
 	}
@@ -241,22 +239,22 @@ func (servers *Servers) addInstituteAndCustom(discoServer *types.DiscoveryServer
 	return server, nil
 }
 
-func (servers *Servers) AddInstituteAccessServer(instituteServer *types.DiscoveryServer, fsm *fsm.FSM, logger *log.FileLogger) (Server, error) {
-	return servers.addInstituteAndCustom(instituteServer, false, fsm, logger)
+func (servers *Servers) AddInstituteAccessServer(instituteServer *types.DiscoveryServer, fsm *fsm.FSM) (Server, error) {
+	return servers.addInstituteAndCustom(instituteServer, false, fsm)
 }
 
-func (servers *Servers) AddCustomServer(customServer *types.DiscoveryServer, fsm *fsm.FSM, logger *log.FileLogger) (Server, error) {
-	return servers.addInstituteAndCustom(customServer, true, fsm, logger)
+func (servers *Servers) AddCustomServer(customServer *types.DiscoveryServer, fsm *fsm.FSM) (Server, error) {
+	return servers.addInstituteAndCustom(customServer, true, fsm)
 }
 
 func (servers *Servers) GetSecureLocation() string {
 	return servers.SecureInternetHomeServer.CurrentLocation
 }
 
-func (servers *Servers) SetSecureLocation(chosenLocationServer *types.DiscoveryServer, fsm *fsm.FSM, logger *log.FileLogger) error {
+func (servers *Servers) SetSecureLocation(chosenLocationServer *types.DiscoveryServer, fsm *fsm.FSM) error {
 	errorMessage := "failed to set secure location"
 	// Make sure to add the current location
-	_, addLocationErr := servers.SecureInternetHomeServer.addLocation(chosenLocationServer, fsm, logger)
+	_, addLocationErr := servers.SecureInternetHomeServer.addLocation(chosenLocationServer, fsm)
 
 	if addLocationErr != nil {
 		return &types.WrappedErrorMessage{Message: errorMessage, Err: addLocationErr}
@@ -266,11 +264,11 @@ func (servers *Servers) SetSecureLocation(chosenLocationServer *types.DiscoveryS
 	return nil
 }
 
-func (servers *Servers) AddSecureInternet(secureOrg *types.DiscoveryOrganization, secureServer *types.DiscoveryServer, fsm *fsm.FSM, logger *log.FileLogger) (Server, error) {
+func (servers *Servers) AddSecureInternet(secureOrg *types.DiscoveryOrganization, secureServer *types.DiscoveryServer, fsm *fsm.FSM) (Server, error) {
 	errorMessage := "failed adding secure internet server"
 	// If we have specified an organization ID
 	// We also need to get an authorization template
-	initErr := servers.SecureInternetHomeServer.init(secureOrg, secureServer, fsm, logger)
+	initErr := servers.SecureInternetHomeServer.init(secureOrg, secureServer, fsm)
 
 	if initErr != nil {
 		return nil, &types.WrappedErrorMessage{Message: errorMessage, Err: initErr}
@@ -318,13 +316,7 @@ func Login(server Server) error {
 
 func EnsureTokens(server Server) error {
 	errorMessage := "failed ensuring server tokens"
-	base, baseErr := server.GetBase()
-
-	if baseErr != nil {
-		return &types.WrappedErrorMessage{Message: errorMessage, Err: baseErr}
-	}
 	if server.GetOAuth().NeedsRelogin() {
-		base.Logger.Log(log.LOG_INFO, "OAuth: Tokens are invalid, relogging in")
 		loginErr := Login(server)
 
 		if loginErr != nil {
@@ -513,7 +505,6 @@ func GetConfig(server Server, forceTCP bool) (string, string, error) {
 	if base.Profiles.Current != "" {
 		_, existsProfileErr := getCurrentProfile(server)
 		if existsProfileErr != nil {
-			base.Logger.Log(log.LOG_INFO, fmt.Sprintf("Profile %s no longer exists, resetting the profile", base.Profiles.Current))
 			base.Profiles.Current = ""
 		}
 	}
diff --git a/internal/server/instituteaccess.go b/internal/server/instituteaccess.go
index 9e712bd..1da2d1e 100644
--- a/internal/server/instituteaccess.go
+++ b/internal/server/instituteaccess.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 
 	"github.com/jwijenbergh/eduvpn-common/internal/fsm"
-	"github.com/jwijenbergh/eduvpn-common/internal/log"
 	"github.com/jwijenbergh/eduvpn-common/internal/oauth"
 	"github.com/jwijenbergh/eduvpn-common/internal/types"
 )
@@ -38,19 +37,18 @@ func (institute *InstituteAccessServer) GetBase() (*ServerBase, error) {
 	return &institute.Base, nil
 }
 
-func (institute *InstituteAccessServer) init(url string, displayName map[string]string, serverType string, supportContact []string, fsm *fsm.FSM, logger *log.FileLogger) error {
+func (institute *InstituteAccessServer) init(url string, displayName map[string]string, serverType string, supportContact []string, fsm *fsm.FSM) error {
 	errorMessage := fmt.Sprintf("failed initializing institute server %s", url)
 	institute.Base.URL = url
 	institute.Base.DisplayName = displayName
 	institute.Base.SupportContact = supportContact
 	institute.Base.FSM = fsm
-	institute.Base.Logger = logger
 	institute.Base.Type = serverType
 	endpoints, endpointsErr := APIGetEndpoints(url)
 	if endpointsErr != nil {
 		return &types.WrappedErrorMessage{Message: errorMessage, Err: endpointsErr}
 	}
-	institute.OAuth.Init(endpoints.API.V3.Authorization, endpoints.API.V3.Token, fsm, logger)
+	institute.OAuth.Init(endpoints.API.V3.Authorization, endpoints.API.V3.Token, fsm)
 	institute.Base.Endpoints = *endpoints
 	return nil
 }
diff --git a/internal/server/secureinternet.go b/internal/server/secureinternet.go
index 95c14a6..9ada8ae 100644
--- a/internal/server/secureinternet.go
+++ b/internal/server/secureinternet.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 
 	"github.com/jwijenbergh/eduvpn-common/internal/fsm"
-	"github.com/jwijenbergh/eduvpn-common/internal/log"
 	"github.com/jwijenbergh/eduvpn-common/internal/oauth"
 	"github.com/jwijenbergh/eduvpn-common/internal/types"
 	"github.com/jwijenbergh/eduvpn-common/internal/util"
@@ -53,7 +52,7 @@ func (servers *Servers) HasSecureLocation() bool {
 	return servers.SecureInternetHomeServer.CurrentLocation != ""
 }
 
-func (secure *SecureInternetHomeServer) addLocation(locationServer *types.DiscoveryServer, fsm *fsm.FSM, logger *log.FileLogger) (*ServerBase, error) {
+func (secure *SecureInternetHomeServer) addLocation(locationServer *types.DiscoveryServer, fsm *fsm.FSM) (*ServerBase, error) {
 	errorMessage := "failed adding a location"
 	// Initialize the base map if it is non-nil
 	if secure.BaseMap == nil {
@@ -77,9 +76,8 @@ func (secure *SecureInternetHomeServer) addLocation(locationServer *types.Discov
 		base.Endpoints = *endpoints
 	}
 
-	// Pass the fsm and logger
+	// Pass the fsm
 	base.FSM = fsm
-	base.Logger = logger
 
 	// Ensure it is in the map
 	secure.BaseMap[locationServer.CountryCode] = base
@@ -87,7 +85,7 @@ func (secure *SecureInternetHomeServer) addLocation(locationServer *types.Discov
 }
 
 // Initializes the home server and adds its own location
-func (secure *SecureInternetHomeServer) init(homeOrg *types.DiscoveryOrganization, homeLocation *types.DiscoveryServer, fsm *fsm.FSM, logger *log.FileLogger) error {
+func (secure *SecureInternetHomeServer) init(homeOrg *types.DiscoveryOrganization, homeLocation *types.DiscoveryServer, fsm *fsm.FSM) error {
 	errorMessage := "failed initializing secure internet home server"
 
 	if secure.HomeOrganizationID != homeOrg.OrgId {
@@ -102,14 +100,14 @@ func (secure *SecureInternetHomeServer) init(homeOrg *types.DiscoveryOrganizatio
 	// Make sure to set the authorization URL template
 	secure.AuthorizationTemplate = homeLocation.AuthenticationURLTemplate
 
-	base, baseErr := secure.addLocation(homeLocation, fsm, logger)
+	base, baseErr := secure.addLocation(homeLocation, fsm)
 
 	if baseErr != nil {
 		return &types.WrappedErrorMessage{Message: errorMessage, Err: baseErr}
 	}
 
 	// Make sure oauth contains our endpoints
-	secure.OAuth.Init(base.Endpoints.API.V3.Authorization, base.Endpoints.API.V3.Token, fsm, logger)
+	secure.OAuth.Init(base.Endpoints.API.V3.Authorization, base.Endpoints.API.V3.Token, fsm)
 	return nil
 }
 
diff --git a/state.go b/state.go
index 93cf217..2f7e785 100644
--- a/state.go
+++ b/state.go
@@ -59,15 +59,12 @@ func (state *VPNState) Register(name string, directory string, stateCallback fun
 	}
 
 	// Initialize the FSM
-	state.FSM.Init(name, stateCallback, &state.Logger, directory, debug)
+	state.FSM.Init(name, stateCallback, directory, debug)
 	state.Debug = debug
 
 	// Initialize the Config
 	state.Config.Init(name, directory)
 
-	// Initialize Discovery
-	state.Discovery.Init(&state.FSM, &state.Logger)
-
 	// Try to load the previous configuration
 	if state.Config.Load(&state) != nil {
 		// This error can be safely ignored, as when the config does not load, the struct will not be filled
@@ -153,7 +150,7 @@ func (state *VPNState) SetSecureLocation(countryCode string) error {
 		return &types.WrappedErrorMessage{Message: "failed asking secure location", Err: serverErr}
 	}
 
-	state.Servers.SetSecureLocation(server, &state.FSM, &state.Logger)
+	state.Servers.SetSecureLocation(server, &state.FSM)
 	return nil
 }
 
@@ -178,7 +175,7 @@ func (state *VPNState) addSecureInternetHomeServer(orgID string) (server.Server,
 	}
 
 	// Add the secure internet server
-	server, serverErr := state.Servers.AddSecureInternet(secureOrg, secureServer, &state.FSM, &state.Logger)
+	server, serverErr := state.Servers.AddSecureInternet(secureOrg, secureServer, &state.FSM)
 
 	if serverErr != nil {
 		return nil, &types.WrappedErrorMessage{Message: errorMessage, Err: serverErr}
@@ -221,7 +218,7 @@ func (state *VPNState) addInstituteServer(url string) (server.Server, error) {
 		return nil, &types.WrappedErrorMessage{Message: errorMessage, Err: discoErr}
 	}
 	// Add the secure internet server
-	server, serverErr := state.Servers.AddInstituteAccessServer(instituteServer, &state.FSM, &state.Logger)
+	server, serverErr := state.Servers.AddInstituteAccessServer(instituteServer, &state.FSM)
 
 	if serverErr != nil {
 		return nil, &types.WrappedErrorMessage{Message: errorMessage, Err: serverErr}
@@ -238,7 +235,7 @@ func (state *VPNState) addCustomServer(url string) (server.Server, error) {
 	customServer := &types.DiscoveryServer{BaseURL: url, DisplayName: map[string]string{"en": url}, Type: "custom_server"}
 
 	// A custom server is just an institute access server under the hood
-	server, serverErr := state.Servers.AddCustomServer(customServer, &state.FSM, &state.Logger)
+	server, serverErr := state.Servers.AddCustomServer(customServer, &state.FSM)
 
 	if serverErr != nil {
 		return nil, &types.WrappedErrorMessage{Message: errorMessage, Err: serverErr}
@@ -368,6 +365,7 @@ func (state *VPNState) SetDisconnected() error {
 	}
 
 	state.FSM.GoTransitionWithData(fsm.HAS_CONFIG, state.getServerInfoData(), false)
+
 	return nil
 }