Golang-ci-lint: Fixes

15 files changed, 141 insertions, 93 deletions

diff --git a/cmd/cli/main.go b/cmd/cli/main.go
index fe6b40e..caeef1c 100644
--- a/cmd/cli/main.go
+++ b/cmd/cli/main.go
@@ -6,7 +6,7 @@ import (
 	"os/exec"
 	"strings"
 
-	"github.com/jwijenbergh/eduvpn-common"
+	eduvpn "github.com/jwijenbergh/eduvpn-common"
 	"github.com/jwijenbergh/eduvpn-common/internal/server"
 )
 
@@ -27,22 +27,10 @@ func openBrowser(url interface{}) {
 	}
 	fmt.Printf("OAuth: Initialized with AuthURL %s\n", urlString)
 	fmt.Println("OAuth: Opening browser with xdg-open...")
-	exec.Command("xdg-open", urlString).Start()
-}
-
-// Taken from internal/server.go as it's an internal API for now
-// These are used to parse the profile info
-type ServerProfile struct {
-	ID             string   `json:"profile_id"`
-	DisplayName    string   `json:"display_name"`
-	VPNProtoList   []string `json:"vpn_proto_list"`
-	DefaultGateway bool     `json:"default_gateway"`
-}
-type ServerProfileInfo struct {
-	Current string `json:"current_profile"`
-	Info    struct {
-		ProfileList []ServerProfile `json:"profile_list"`
-	} `json:"info"`
+	cmdErr := exec.Command("xdg-open", urlString).Start()
+	if cmdErr != nil {
+		fmt.Println("OAuth: Browser opened with xdg-open...")
+	}
 }
 
 // Ask for a profile in the command line
@@ -120,7 +108,7 @@ func getConfig(state *eduvpn.VPNState, url string, serverType ServerTypes) (stri
 func printConfig(url string, serverType ServerTypes) {
 	state := &eduvpn.VPNState{}
 
-	state.Register(
+	registerErr := state.Register(
 		"org.eduvpn.app.linux",
 		"configs",
 		func(old eduvpn.FSMStateID, new eduvpn.FSMStateID, data interface{}) {
@@ -128,6 +116,10 @@ func printConfig(url string, serverType ServerTypes) {
 		},
 		true,
 	)
+	if registerErr != nil {
+		fmt.Printf("Register error: %v", registerErr)
+		return
+	}
 
 	defer state.Deregister()
 
diff --git a/exports/disco.go b/exports/disco.go
index bd5a663..56918ec 100644
--- a/exports/disco.go
+++ b/exports/disco.go
@@ -41,7 +41,7 @@ import "C"
 import (
 	"unsafe"
 
-	"github.com/jwijenbergh/eduvpn-common"
+	eduvpn "github.com/jwijenbergh/eduvpn-common"
 	"github.com/jwijenbergh/eduvpn-common/internal/types"
 )
 
diff --git a/exports/exports.go b/exports/exports.go
index 4f0b865..cd02bd1 100644
--- a/exports/exports.go
+++ b/exports/exports.go
@@ -14,11 +14,10 @@ import "C"
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"unsafe"
 
-	"github.com/jwijenbergh/eduvpn-common"
+	eduvpn "github.com/jwijenbergh/eduvpn-common"
 )
 
 var P_StateCallbacks map[string]C.PythonCB
@@ -79,7 +78,7 @@ func GetVPNState(name string) (*eduvpn.VPNState, error) {
 	state, exists := VPNStates[name]
 
 	if !exists || state == nil {
-		return nil, errors.New(fmt.Sprintf("State with name %s not found", name))
+		return nil, fmt.Errorf("State with name %s not found", name)
 	}
 
 	return state, nil
diff --git a/exports/servers.go b/exports/servers.go
index 2dfb14b..b2ae0ee 100644
--- a/exports/servers.go
+++ b/exports/servers.go
@@ -51,7 +51,7 @@ import "C"
 import (
 	"unsafe"
 
-	"github.com/jwijenbergh/eduvpn-common"
+	eduvpn "github.com/jwijenbergh/eduvpn-common"
 	"github.com/jwijenbergh/eduvpn-common/internal/server"
 )
 
diff --git a/fsm.go b/fsm.go
index 75a2d8c..b508e6c 100644
--- a/fsm.go
+++ b/fsm.go
@@ -222,12 +222,10 @@ type FSMWrongStateTransitionError struct {
 func (e FSMWrongStateTransitionError) CustomError() *types.WrappedErrorMessage {
 	return &types.WrappedErrorMessage{
 		Message: "Wrong FSM transition",
-		Err: errors.New(
-			fmt.Sprintf(
-				"wrong FSM state, got: %s, want: a state with a transition to: %s",
-				GetStateName(e.Got),
-				GetStateName(e.Want),
-			),
+		Err: fmt.Errorf(
+			"wrong FSM state, got: %s, want: a state with a transition to: %s",
+			GetStateName(e.Got),
+			GetStateName(e.Want),
 		),
 	}
 }
@@ -240,12 +238,10 @@ type FSMWrongStateError struct {
 func (e FSMWrongStateError) CustomError() *types.WrappedErrorMessage {
 	return &types.WrappedErrorMessage{
 		Message: "Wrong FSM State",
-		Err: errors.New(
-			fmt.Sprintf(
-				"wrong FSM state, got: %s, want: %s",
-				GetStateName(e.Got),
-				GetStateName(e.Want),
-			),
+		Err: fmt.Errorf(
+			"wrong FSM state, got: %s, want: %s",
+			GetStateName(e.Got),
+			GetStateName(e.Want),
 		),
 	}
 }
diff --git a/internal/discovery/discovery.go b/internal/discovery/discovery.go
index b3b438c..61b5557 100644
--- a/internal/discovery/discovery.go
+++ b/internal/discovery/discovery.go
@@ -156,10 +156,7 @@ func (discovery *Discovery) DetermineServersUpdate() bool {
 	// 1 hour from the last update
 	should_update_time := discovery.Servers.Timestamp.Add(1 * time.Hour)
 	now := util.GetCurrentTime()
-	if !now.Before(should_update_time) {
-		return true
-	}
-	return false
+	return !now.Before(should_update_time)
 }
 
 // Get the organization list
diff --git a/internal/fsm/fsm.go b/internal/fsm/fsm.go
index 292e09e..4fc647e 100644
--- a/internal/fsm/fsm.go
+++ b/internal/fsm/fsm.go
@@ -99,11 +99,13 @@ func (fsm *FSM) writeGraph() {
 		return
 	}
 
-	f.WriteString(graph)
+	_, writeErr := f.WriteString(graph)
 	f.Close()
-	cmd := exec.Command("mmdc", "-i", graphFile, "-o", graphImgFile, "--scale", "4")
-
-	cmd.Start()
+	if writeErr != nil {
+		cmd := exec.Command("mmdc", "-i", graphFile, "-o", graphImgFile, "--scale", "4")
+		// Generating is best effort
+		_ = cmd.Start()
+	}
 }
 
 func (fsm *FSM) GoBack() {
diff --git a/internal/oauth/oauth.go b/internal/oauth/oauth.go
index e99b715..d8acfad 100644
--- a/internal/oauth/oauth.go
+++ b/internal/oauth/oauth.go
@@ -200,9 +200,9 @@ func (oauth *OAuth) Callback(w http.ResponseWriter, req *http.Request) {
 	code, success := req.URL.Query()["code"]
 	// Shutdown after we're done
 	defer func() {
-        if oauth.Session.Server != nil {
-            go oauth.Session.Server.Shutdown(oauth.Session.Context)
-        }
+		if oauth.Session.Server != nil {
+			go oauth.Session.Server.Shutdown(oauth.Session.Context) //nolint:errcheck
+		}
 	}()
 	if !success {
 		oauth.Session.CallbackError = &types.WrappedErrorMessage{
@@ -310,9 +310,9 @@ func (oauth *OAuth) Cancel() {
 		Message: "cancelled OAuth",
 		Err:     &OAuthCancelledCallbackError{},
 	}
-    if oauth.Session.Server != nil {
-        oauth.Session.Server.Shutdown(oauth.Session.Context)
-    }
+	if oauth.Session.Server != nil {
+		oauth.Session.Server.Shutdown(oauth.Session.Context) //nolint:errcheck
+	}
 }
 
 func (oauth *OAuth) EnsureTokens() error {
@@ -351,7 +351,7 @@ func (oauth *OAuth) EnsureTokens() error {
 type OAuthCancelledCallbackError struct{}
 
 func (e *OAuthCancelledCallbackError) Error() string {
-	return fmt.Sprintf("client cancelled OAuth")
+	return "client cancelled OAuth"
 }
 
 type OAuthCallbackParameterError struct {
diff --git a/internal/server/api.go b/internal/server/api.go
index b06b25c..76e3c72 100644
--- a/internal/server/api.go
+++ b/internal/server/api.go
@@ -199,7 +199,6 @@ func APIConnectOpenVPN(server Server, profile_id string) (string, time.Time, err
 }
 
 // This needs no further return value as it's best effort
-// FIXME: doAuth should not be needed here
 func APIDisconnect(server Server) {
-	apiAuthorized(server, http.MethodPost, "/disconnect", &httpw.HTTPOptionalParams{Timeout: 1})
+	_, _, _ = apiAuthorized(server, http.MethodPost, "/disconnect", &httpw.HTTPOptionalParams{Timeout: 1})
 }
diff --git a/internal/server/common.go b/internal/server/common.go
index 9c941cf..f4f9a42 100644
--- a/internal/server/common.go
+++ b/internal/server/common.go
@@ -475,9 +475,7 @@ func (e *ServerGetCurrentProfileNotFoundError) Error() string {
 type ServerGetConfigForceTCPError struct{}
 
 func (e *ServerGetConfigForceTCPError) Error() string {
-	return fmt.Sprintf(
-		"failed to get config, force TCP is on but the server does not support OpenVPN",
-	)
+	return "failed to get config, force TCP is on but the server does not support OpenVPN"
 }
 
 type ServerEnsureServerEmptyURLError struct{}
diff --git a/internal/util/util.go b/internal/util/util.go
index e652779..834b8d4 100644
--- a/internal/util/util.go
+++ b/internal/util/util.go
@@ -86,8 +86,8 @@ func ReplaceWAYF(authTemplate string, authURL string, orgID string) string {
 
 // https://github.com/eduvpn/documentation/blob/dc4d53c47dd7a69e95d6650eec408e16eaa814a2/SERVER_DISCOVERY.md#language-matching
 func GetLanguageMatched(languageMap map[string]string, languageTag string) string {
-	// If no or empty map is given, return the empty string
-	if languageMap == nil || len(languageMap) == 0 {
+	// If no map is given, return the empty string
+	if len(languageMap) == 0 {
 		return ""
 	}
 	// Try to find the exact match
diff --git a/internal/util/util_test.go b/internal/util/util_test.go
index 31d01e7..77af603 100644
--- a/internal/util/util_test.go
+++ b/internal/util/util_test.go
@@ -48,7 +48,7 @@ func Test_MakeRandomByteSlice(t *testing.T) {
 		t.Fatalf("2, Got: %v, want: nil", randomErr)
 	}
 
-	if bytes.Compare(random2, random) == 0 {
+	if bytes.Equal(random2, random) {
 		t.Fatalf("Two random byteslices are the same: %v, %v", random2, random)
 	}
 }
@@ -56,7 +56,7 @@ func Test_MakeRandomByteSlice(t *testing.T) {
 func Test_GetCurrentTime(t *testing.T) {
 	time_now := GetCurrentTime()
 
-	time.Sleep(1)
+	time.Sleep(1 * time.Second)
 
 	time_after_1_second := GetCurrentTime()
 
diff --git a/internal/verify/verify_test.go b/internal/verify/verify_test.go
index 47b1dc2..1720c97 100644
--- a/internal/verify/verify_test.go
+++ b/internal/verify/verify_test.go
@@ -10,8 +10,6 @@ import (
 )
 
 func Test_verifyWithKeys(t *testing.T) {
-	var err error
-
 	var pk []string
 	{
 		file, err := os.Open("test_data/public.key")
@@ -328,9 +326,9 @@ func Test_verifyWithKeys(t *testing.T) {
 	// Cache file contents in map, mapping file names to contents
 	files := map[string][]byte{}
 	loadFile := func(name string) {
-		content, loaded := files[name]
+		_, loaded := files[name]
 		if !loaded {
-			content, err = ioutil.ReadFile("test_data/" + name)
+			content, err := ioutil.ReadFile("test_data/" + name)
 			if err != nil {
 				panic(err)
 			}
diff --git a/state.go b/state.go
index 9f7bd1b..f311469 100644
--- a/state.go
+++ b/state.go
@@ -122,12 +122,33 @@ func (state *VPNState) Deregister() {
 	state.Logger.Close()
 
 	// Save the config
-	state.Config.Save(&state)
+	saveErr := state.Config.Save(&state)
+	if saveErr != nil {
+		state.Logger.Info(
+			fmt.Sprintf(
+				"Failed saving configuration, error: %s",
+				GetErrorTraceback(saveErr),
+			),
+		)
+	}
 
 	// Empty out the state
 	*state = VPNState{}
 }
 
+// goBackInternal uses the public go back but logs an error if it happened
+func (state *VPNState) goBackInternal() {
+	goBackErr := state.GoBack()
+	if goBackErr != nil {
+		state.Logger.Info(
+			fmt.Sprintf(
+				"Failed going back, error: %s",
+				GetErrorTraceback(goBackErr),
+			),
+		)
+	}
+}
+
 // GoBack transitions the FSM back to the previous UI state, for now this is always the NO_SERVER state
 func (state *VPNState) GoBack() error {
 	errorMessage := "failed to go back"
@@ -141,12 +162,6 @@ func (state *VPNState) GoBack() error {
 
 	// FIXME: Abitrary back transitions don't work because we need the approriate data
 	state.FSM.GoTransitionWithData(STATE_NO_SERVER, state.Servers, false)
-	// state.FSM.GoBack()
-	return nil
-}
-
-func (state *VPNState) doAuth(authURL string) error {
-	state.FSM.GoTransitionWithData(STATE_OAUTH_STARTED, authURL, true)
 	return nil
 }
 
@@ -160,14 +175,14 @@ func (state *VPNState) ensureLogin(chosenServer server.Server) error {
 		state.FSM.GoTransitionWithData(STATE_OAUTH_STARTED, url, true)
 
 		if urlErr != nil {
-			state.GoBack()
+			state.goBackInternal()
 			return &types.WrappedErrorMessage{Message: errorMessage, Err: urlErr}
 		}
 
 		exchangeErr := server.OAuthExchange(chosenServer)
 
 		if exchangeErr != nil {
-			state.GoBack()
+			state.goBackInternal()
 			return &types.WrappedErrorMessage{Message: errorMessage, Err: exchangeErr}
 		}
 	}
@@ -219,7 +234,7 @@ func (state *VPNState) retryConfigAuth(
 				forceTCP,
 			)
 			if retryConfigErr != nil {
-				state.GoBack()
+				state.goBackInternal()
 				return "", "", &types.WrappedErrorMessage{
 					Message: errorMessage,
 					Err:     retryConfigErr,
@@ -227,7 +242,7 @@ func (state *VPNState) retryConfigAuth(
 			}
 			return retryConfig, retryConfigType, nil
 		}
-		state.GoBack()
+		state.goBackInternal()
 		return "", "", &types.WrappedErrorMessage{Message: errorMessage, Err: configErr}
 	}
 	return config, configType, nil
@@ -258,8 +273,17 @@ func (state *VPNState) getConfig(
 
 	// Signal the server display info
 	state.FSM.GoTransitionWithData(STATE_DISCONNECTED, currentServer, false)
+
 	// Save the config
-	state.Config.Save(&state)
+	saveErr := state.Config.Save(&state)
+	if saveErr != nil {
+		state.Logger.Info(
+			fmt.Sprintf(
+				"Failed saving configuration after getting a server: %s",
+				GetErrorTraceback(saveErr),
+			),
+		)
+	}
 
 	return config, configType, nil
 }
@@ -277,7 +301,7 @@ func (state *VPNState) SetSecureLocation(countryCode string) error {
 				GetErrorTraceback(serverErr),
 			),
 		)
-		state.GoBack()
+		state.goBackInternal()
 		return &types.WrappedErrorMessage{Message: errorMessage, Err: serverErr}
 	}
 
@@ -289,7 +313,7 @@ func (state *VPNState) SetSecureLocation(countryCode string) error {
 				GetErrorTraceback(serverErr),
 			),
 		)
-		state.GoBack()
+		state.goBackInternal()
 		return &types.WrappedErrorMessage{Message: errorMessage, Err: setLocationErr}
 	}
 	return nil
@@ -368,7 +392,15 @@ func (state *VPNState) RemoveSecureInternet() error {
 	state.Servers.RemoveSecureInternet()
 	state.FSM.GoTransitionWithData(STATE_NO_SERVER, state.Servers, false)
 	// Save the config
-	state.Config.Save(&state)
+	saveErr := state.Config.Save(&state)
+	if saveErr != nil {
+		state.Logger.Info(
+			fmt.Sprintf(
+				"Failed saving configuration after removing a secure internet server: %s",
+				GetErrorTraceback(saveErr),
+			),
+		)
+	}
 	return nil
 }
 
@@ -384,7 +416,15 @@ func (state *VPNState) RemoveInstituteAccess(url string) error {
 	state.Servers.RemoveInstituteAccess(url)
 	state.FSM.GoTransitionWithData(STATE_NO_SERVER, state.Servers, false)
 	// Save the config
-	state.Config.Save(&state)
+	saveErr := state.Config.Save(&state)
+	if saveErr != nil {
+		state.Logger.Info(
+			fmt.Sprintf(
+				"Failed saving configuration after removing an institute access server: %s",
+				GetErrorTraceback(saveErr),
+			),
+		)
+	}
 	return nil
 }
 
@@ -400,7 +440,15 @@ func (state *VPNState) RemoveCustomServer(url string) error {
 	state.Servers.RemoveCustomServer(url)
 	state.FSM.GoTransitionWithData(STATE_NO_SERVER, state.Servers, false)
 	// Save the config
-	state.Config.Save(&state)
+	saveErr := state.Config.Save(&state)
+	if saveErr != nil {
+		state.Logger.Info(
+			fmt.Sprintf(
+				"Failed saving configuration after removing a custom server: %s",
+				GetErrorTraceback(saveErr),
+			),
+		)
+	}
 	return nil
 }
 
@@ -422,7 +470,7 @@ func (state *VPNState) GetConfigSecureInternet(
 				GetErrorTraceback(serverErr),
 			),
 		)
-		state.GoBack()
+		state.goBackInternal()
 		return "", "", &types.WrappedErrorMessage{Message: errorMessage, Err: serverErr}
 	}
 
@@ -494,7 +542,7 @@ func (state *VPNState) GetConfigInstituteAccess(url string, forceTCP bool) (stri
 				GetErrorTraceback(serverErr),
 			),
 		)
-		state.GoBack()
+		state.goBackInternal()
 		return "", "", &types.WrappedErrorMessage{Message: errorMessage, Err: serverErr}
 	}
 
@@ -523,7 +571,7 @@ func (state *VPNState) GetConfigCustomServer(url string, forceTCP bool) (string,
 				GetErrorTraceback(serverErr),
 			),
 		)
-		state.GoBack()
+		state.goBackInternal()
 		return "", "", &types.WrappedErrorMessage{Message: errorMessage, Err: serverErr}
 	}
 
@@ -639,7 +687,7 @@ func (state *VPNState) SetProfileID(profileID string) error {
 				GetErrorTraceback(serverErr),
 			),
 		)
-		state.GoBack()
+		state.goBackInternal()
 		return &types.WrappedErrorMessage{Message: errorMessage, Err: serverErr}
 	}
 
@@ -648,7 +696,7 @@ func (state *VPNState) SetProfileID(profileID string) error {
 		state.Logger.Error(
 			fmt.Sprintf("Failed setting a profile ID, Err: %s", GetErrorTraceback(serverErr)),
 		)
-		state.GoBack()
+		state.goBackInternal()
 		return &types.WrappedErrorMessage{Message: errorMessage, Err: baseErr}
 	}
 	base.Profiles.Current = profileID
diff --git a/state_test.go b/state_test.go
index b87b1ef..3915759 100644
--- a/state_test.go
+++ b/state_test.go
@@ -56,7 +56,7 @@ func loginOAuthSelenium(t *testing.T, url string, state *VPNState) {
 			err,
 			errBuffer.String(),
 		)
-		state.CancelOAuth()
+		_ = state.CancelOAuth()
 	}
 }
 
@@ -82,7 +82,7 @@ func Test_server(t *testing.T) {
 	state := &VPNState{}
 	ensureLocalWellKnown()
 
-	state.Register(
+	registerErr := state.Register(
 		"org.eduvpn.app.linux",
 		"configstest",
 		func(old FSMStateID, new FSMStateID, data interface{}) {
@@ -90,9 +90,11 @@ func Test_server(t *testing.T) {
 		},
 		false,
 	)
+	if registerErr != nil {
+		t.Fatalf("Register error: %v", registerErr)
+	}
 
 	_, _, configErr := state.GetConfigCustomServer(serverURI, false)
-
 	if configErr != nil {
 		t.Fatalf("Connect error: %v", configErr)
 	}
@@ -107,7 +109,7 @@ func test_connect_oauth_parameter(
 	state := &VPNState{}
 	configDirectory := "test_oauth_parameters"
 
-	state.Register(
+	registerErr := state.Register(
 		"org.eduvpn.app.linux",
 		configDirectory,
 		func(oldState FSMStateID, newState FSMStateID, data interface{}) {
@@ -121,11 +123,19 @@ func test_connect_oauth_parameter(
 						fmt.Sprint(parameters),
 					)
 				}
-				go http.Get(url)
+				go func() {
+					_, getErr := http.Get(url)
+					if getErr != nil {
+						t.Logf("HTTP GET error: %v", getErr)
+					}
+				}()
 			}
 		},
 		false,
 	)
+	if registerErr != nil {
+		t.Fatalf("Register error: %v", registerErr)
+	}
 	_, _, configErr := state.GetConfigCustomServer(serverURI, false)
 
 	var wrappedErr *types.WrappedErrorMessage
@@ -186,7 +196,7 @@ func Test_token_expired(t *testing.T) {
 	// Get a vpn state
 	state := &VPNState{}
 
-	state.Register(
+	registerErr := state.Register(
 		"org.eduvpn.app.linux",
 		"configsexpired",
 		func(old FSMStateID, new FSMStateID, data interface{}) {
@@ -194,6 +204,9 @@ func Test_token_expired(t *testing.T) {
 		},
 		false,
 	)
+	if registerErr != nil {
+		t.Fatalf("Register error: %v", registerErr)
+	}
 
 	_, _, configErr := state.GetConfigCustomServer(serverURI, false)
 
@@ -239,7 +252,7 @@ func Test_token_invalid(t *testing.T) {
 
 	ensureLocalWellKnown()
 
-	state.Register(
+	registerErr := state.Register(
 		"org.eduvpn.app.linux",
 		"configsinvalid",
 		func(old FSMStateID, new FSMStateID, data interface{}) {
@@ -247,6 +260,9 @@ func Test_token_invalid(t *testing.T) {
 		},
 		false,
 	)
+	if registerErr != nil {
+		t.Fatalf("Register error: %v", registerErr)
+	}
 
 	_, _, configErr := state.GetConfigCustomServer(serverURI, false)
 
@@ -289,7 +305,7 @@ func Test_invalid_profile_corrected(t *testing.T) {
 
 	ensureLocalWellKnown()
 
-	state.Register(
+	registerErr := state.Register(
 		"org.eduvpn.app.linux",
 		"configscancelprofile",
 		func(old FSMStateID, new FSMStateID, data interface{}) {
@@ -297,6 +313,9 @@ func Test_invalid_profile_corrected(t *testing.T) {
 		},
 		false,
 	)
+	if registerErr != nil {
+		t.Fatalf("Register error: %v", registerErr)
+	}
 
 	_, _, configErr := state.GetConfigCustomServer(serverURI, false)