rewrite logging in console (#788)

- enhance logging throughout the codebase
- all packages at pkg/ should never log
  or perform log.Fatal() instead packages
  should return errors through functions.
- simplified various user, group mapping
  and removed redundant functions.
- deprecate older flags like --tls-certificate
  --tls-key and --tls-ca as we do not use
  them anymore, keep them for backward compatibility
  for some time.

pkg/auth/idp/oauth2/provider.go

@@ -22,7 +22,6 @@ import (
 	"encoding/base64"
 	"errors"
 	"fmt"
-	"log"
 	"net/http"
 	"net/url"
 	"strconv"
@@ -37,10 +36,6 @@ import (
 	xoauth2 "golang.org/x/oauth2"
 )
 
-var (
-	errGeneric = errors.New("an error occurred, please try again")
-)
-
 type Configuration interface {
 	Exchange(ctx context.Context, code string, opts ...xoauth2.AuthCodeOption) (*xoauth2.Token, error)
 	AuthCodeURL(state string, opts ...xoauth2.AuthCodeOption) string
@@ -168,8 +163,8 @@ type User struct {
 // VerifyIdentity will contact the configured IDP and validate the user identity based on the authorization code
 func (client *Provider) VerifyIdentity(ctx context.Context, code, state string) (*credentials.Credentials, error) {
 	// verify the provided state is valid (prevents CSRF attacks)
-	if !validateOauth2State(state) {
-		return nil, errGeneric
+	if err := validateOauth2State(state); err != nil {
+		return nil, err
 	}
 	getWebTokenExpiry := func() (*credentials.WebIdentityToken, error) {
 		oauth2Token, err := client.oauth2Config.Exchange(ctx, code)
@@ -210,29 +205,30 @@ func (client *Provider) VerifyIdentity(ctx context.Context, code, state string)
 // validateOauth2State validates the provided state was originated using the same
 // instance (or one configured using the same secrets) of Console, this is basically used to prevent CSRF attacks
 // https://security.stackexchange.com/questions/20187/oauth2-cross-site-request-forgery-and-state-parameter
-func validateOauth2State(state string) bool {
+func validateOauth2State(state string) error {
 	// state contains a base64 encoded string that may ends with "==", the browser encodes that to "%3D%3D"
 	// query unescape is need it before trying to decode the base64 string
 	encodedMessage, err := url.QueryUnescape(state)
 	if err != nil {
-		log.Println(err)
-		return false
+		return err
 	}
 	// decode the state parameter value
 	message, err := base64.StdEncoding.DecodeString(encodedMessage)
 	if err != nil {
-		log.Println(err)
-		return false
+		return err
 	}
 	s := strings.Split(string(message), ":")
 	// Validate that the decoded message has the right format "message:hmac"
 	if len(s) != 2 {
-		return false
+		return fmt.Errorf("invalid number of tokens, expected only 2, got %d instead", len(s))
 	}
 	// extract the state and hmac
 	incomingState, incomingHmac := s[0], s[1]
 	// validate that hmac(incomingState + pbkdf2(secret, salt)) == incomingHmac
-	return utils.ComputeHmac256(incomingState, derivedKey) == incomingHmac
+	if calculatedHmac := utils.ComputeHmac256(incomingState, derivedKey); calculatedHmac != incomingHmac {
+		return fmt.Errorf("oauth2 state is invalid, expected %s, got %s", calculatedHmac, incomingHmac)
+	}
+	return nil
 }
 
 // GetRandomStateWithHMAC computes message + hmac(message, pbkdf2(key, salt)) to be used as state during the oauth authorization

pkg/auth/operator.go

@@ -18,7 +18,6 @@ package auth
 
 import (
 	"context"
-	"log"
 
 	"github.com/minio/console/cluster"
 	"github.com/minio/minio-go/v7/pkg/credentials"
@@ -64,16 +63,12 @@ func (c *operatorClient) Authenticate(ctx context.Context) ([]byte, error) {
 	return c.client.RESTClient().Verb("GET").RequestURI("/api").DoRaw(ctx)
 }
 
-// isServiceAccountTokenValid will make an authenticated request against kubernetes api, if the
+// checkServiceAccountTokenValid will make an authenticated request against kubernetes api, if the
 // request success means the provided jwt its a valid service account token and the console user can use it for future
 // requests until it expires
-func isServiceAccountTokenValid(ctx context.Context, operatorClient OperatorClient) bool {
+func checkServiceAccountTokenValid(ctx context.Context, operatorClient OperatorClient) error {
 	_, err := operatorClient.Authenticate(ctx)
-	if err != nil {
-		log.Println(err)
-		return false
-	}
-	return true
+	return err
 }
 
 // GetConsoleCredentialsForOperator will validate the provided JWT (service account token) and return it in the form of credentials.Login
@@ -86,8 +81,8 @@ func GetConsoleCredentialsForOperator(jwt string) (*credentials.Credentials, err
 	opClient := &operatorClient{
 		client: opClientClientSet,
 	}
-	if isServiceAccountTokenValid(ctx, opClient) {
-		return credentials.New(operatorCredentialsProvider{serviceAccountJWT: jwt}), nil
+	if err = checkServiceAccountTokenValid(ctx, opClient); err != nil {
+		return nil, errInvalidCredentials
 	}
-	return nil, errInvalidCredentials
+	return credentials.New(operatorCredentialsProvider{serviceAccountJWT: jwt}), nil
 }

pkg/auth/operator_test.go

@@ -20,8 +20,7 @@ func (c *operatorClientTest) Authenticate(ctx context.Context) ([]byte, error) {
 	return operatorAuthenticateMock(ctx)
 }
 
-func Test_isServiceAccountTokenValid(t *testing.T) {
-
+func Test_checkServiceAccountTokenValid(t *testing.T) {
 	successResponse := func() {
 		operatorAuthenticateMock = func(ctx context.Context) ([]byte, error) {
 			return nil, nil
@@ -70,12 +69,17 @@ func Test_isServiceAccountTokenValid(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
+		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
 			if tt.args.mockFunction != nil {
 				tt.args.mockFunction()
 			}
-			if got := isServiceAccountTokenValid(tt.args.ctx, tt.args.operatorClient); got != tt.want {
-				t.Errorf("isServiceAccountTokenValid() = %v, want %v", got, tt.want)
+			got := checkServiceAccountTokenValid(tt.args.ctx, tt.args.operatorClient)
+			if got != nil && tt.want {
+				t.Errorf("checkServiceAccountTokenValid() = expected success but got %s", got)
+			}
+			if got == nil && !tt.want {
+				t.Error("checkServiceAccountTokenValid() = expected failure but got success")
 			}
 		})
 	}

pkg/auth/token.go

@@ -29,7 +29,6 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
-	"log"
 	"net/http"
 	"strings"
 	"time"
@@ -48,8 +47,6 @@ var (
 	ErrNoAuthToken  = errors.New("session token missing")
 	errTokenExpired = errors.New("session token has expired")
 	errReadingToken = errors.New("session token internal data is malformed")
-	errClaimsFormat = errors.New("encrypted session token claims not in the right format")
-	errorGeneric    = errors.New("an error has occurred")
 )
 
 // derivedKey is the key used to encrypt the session token claims, its derived using pbkdf on CONSOLE_PBKDF_PASSPHRASE with CONSOLE_PBKDF_SALT
@@ -90,7 +87,6 @@ func SessionTokenAuthenticate(token string) (*TokenClaims, error) {
 	claimTokens, err := decryptClaims(token)
 	if err != nil {
 		// we print decryption token error information for debugging purposes
-		log.Println(err)
 		// we return a generic error that doesn't give any information to attackers
 		return nil, errReadingToken
 	}
@@ -126,8 +122,7 @@ func encryptClaims(credentials *TokenClaims) (string, error) {
 	}
 	ciphertext, err := encrypt(payload, []byte{})
 	if err != nil {
-		log.Println(err)
-		return "", errorGeneric
+		return "", err
 	}
 	return base64.StdEncoding.EncodeToString(ciphertext), nil
 }
@@ -136,19 +131,15 @@ func encryptClaims(credentials *TokenClaims) (string, error) {
 func decryptClaims(ciphertext string) (*TokenClaims, error) {
 	decoded, err := base64.StdEncoding.DecodeString(ciphertext)
 	if err != nil {
-		log.Println(err)
-		return nil, errClaimsFormat
+		return nil, err
 	}
 	plaintext, err := decrypt(decoded, []byte{})
 	if err != nil {
-		log.Println(err)
-		return nil, errClaimsFormat
+		return nil, err
 	}
 	tokenClaims := &TokenClaims{}
-	err = json.Unmarshal(plaintext, tokenClaims)
-	if err != nil {
-		log.Println(err)
-		return nil, errClaimsFormat
+	if err = json.Unmarshal(plaintext, tokenClaims); err != nil {
+		return nil, err
 	}
 	return tokenClaims, nil
 }