Omit detailedMessage from login errors (minio#3136)

cesnietor · Jan 12, 2024 · 1e7d480 · 1e7d480
1 parent f5ab620
commit 1e7d480
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 28 deletions.
diff --git a/restapi/errors.go b/restapi/errors.go
@@ -53,6 +53,7 @@ var (
 	ErrAvoidSelfAccountDelete           = errors.New("logged in user cannot be deleted by itself")
 	ErrAccessDenied                     = errors.New("access denied")
 	ErrOauth2Provider                   = errors.New("unable to contact configured identity provider")
+	ErrOauth2Login                      = errors.New("unable to login using configured identity provider")
 	ErrNonUniqueAccessKey               = errors.New("access key already in use")
 	ErrRemoteTierExists                 = errors.New("specified remote tier already exists")
 	ErrRemoteTierNotFound               = errors.New("specified remote tier was not found")
@@ -84,10 +85,12 @@ type CodedAPIError struct {
 func ErrorWithContext(ctx context.Context, err ...interface{}) *CodedAPIError {
 	errorCode := 500
 	errorMessage := ErrDefault.Error()
+	var detailedMessage string
 	var err1 error
 	var exists bool
 	if len(err) > 0 {
 		if err1, exists = err[0].(error); exists {
+			detailedMessage = err1.Error()
 			var lastError error
 			if len(err) > 1 {
 				if err2, lastExists := err[1].(error); lastExists {
@@ -105,6 +108,7 @@ func ErrorWithContext(ctx context.Context, err ...interface{}) *CodedAPIError {
 				errorMessage = ErrNotFound.Error()
 			}
 			if errors.Is(err1, ErrInvalidLogin) {
+				detailedMessage = ""
 				errorCode = 401
 				errorMessage = ErrInvalidLogin.Error()
 			}
@@ -114,10 +118,12 @@ func ErrorWithContext(ctx context.Context, err ...interface{}) *CodedAPIError {
 			}
 			// If the last error is ErrInvalidLogin, this is a login failure
 			if errors.Is(lastError, ErrInvalidLogin) {
+				detailedMessage = ""
 				errorCode = 401
 				errorMessage = err1.Error()
 			}
 			if strings.Contains(err1.Error(), ErrLoginNotAllowed.Error()) {
+				detailedMessage = ""
 				errorCode = 400
 				errorMessage = ErrLoginNotAllowed.Error()
 			}
@@ -211,6 +217,7 @@ func ErrorWithContext(ctx context.Context, err ...interface{}) *CodedAPIError {
 				errorMessage = ErrAccessDenied.Error()
 			}
 			if madmin.ToErrorResponse(err1).Code == "InvalidAccessKeyId" {
+
 				errorCode = 401
 				errorMessage = ErrInvalidSession.Error()
 			}
@@ -261,7 +268,7 @@ func ErrorWithContext(ctx context.Context, err ...interface{}) *CodedAPIError {
 			}
 		}
 	}
-	return &CodedAPIError{Code: errorCode, APIError: &models.APIError{Message: errorMessage, DetailedMessage: err1.Error()}}
+	return &CodedAPIError{Code: errorCode, APIError: &models.APIError{Message: errorMessage, DetailedMessage: detailedMessage}}
 }
 
 // Error receives an errors object and parse it against k8sErrors, returns the right errors code paired with a generic errors message

diff --git a/restapi/errors_test.go b/restapi/errors_test.go
@@ -44,37 +44,38 @@ func TestError(t *testing.T) {
 	}
 
 	appErrors := map[string]expectedError{
-		"ErrDefault":                          {code: 500, err: ErrDefault},
-		"ErrInvalidLogin":                     {code: 401, err: ErrInvalidLogin},
-		"ErrForbidden":                        {code: 403, err: ErrForbidden},
-		"ErrFileTooLarge":                     {code: 413, err: ErrFileTooLarge},
-		"ErrInvalidSession":                   {code: 401, err: ErrInvalidSession},
-		"ErrNotFound":                         {code: 404, err: ErrNotFound},
-		"ErrGroupAlreadyExists":               {code: 400, err: ErrGroupAlreadyExists},
-		"ErrInvalidErasureCodingValue":        {code: 400, err: ErrInvalidErasureCodingValue},
-		"ErrBucketBodyNotInRequest":           {code: 400, err: ErrBucketBodyNotInRequest},
-		"ErrBucketNameNotInRequest":           {code: 400, err: ErrBucketNameNotInRequest},
-		"ErrGroupBodyNotInRequest":            {code: 400, err: ErrGroupBodyNotInRequest},
-		"ErrGroupNameNotInRequest":            {code: 400, err: ErrGroupNameNotInRequest},
-		"ErrPolicyNameNotInRequest":           {code: 400, err: ErrPolicyNameNotInRequest},
-		"ErrPolicyBodyNotInRequest":           {code: 400, err: ErrPolicyBodyNotInRequest},
-		"ErrInvalidEncryptionAlgorithm":       {code: 500, err: ErrInvalidEncryptionAlgorithm},
-		"ErrSSENotConfigured":                 {code: 404, err: ErrSSENotConfigured},
-		"ErrBucketLifeCycleNotConfigured":     {code: 404, err: ErrBucketLifeCycleNotConfigured},
-		"ErrChangePassword":                   {code: 403, err: ErrChangePassword},
-		"ErrInvalidLicense":                   {code: 404, err: ErrInvalidLicense},
-		"ErrLicenseNotFound":                  {code: 404, err: ErrLicenseNotFound},
-		"ErrAvoidSelfAccountDelete":           {code: 403, err: ErrAvoidSelfAccountDelete},
-		"ErrAccessDenied":                     {code: 403, err: ErrAccessDenied},
+		"ErrDefault": {code: 500, err: ErrDefault},
+
+		"ErrForbidden":                    {code: 403, err: ErrForbidden},
+		"ErrFileTooLarge":                 {code: 413, err: ErrFileTooLarge},
+		"ErrInvalidSession":               {code: 401, err: ErrInvalidSession},
+		"ErrNotFound":                     {code: 404, err: ErrNotFound},
+		"ErrGroupAlreadyExists":           {code: 400, err: ErrGroupAlreadyExists},
+		"ErrInvalidErasureCodingValue":    {code: 400, err: ErrInvalidErasureCodingValue},
+		"ErrBucketBodyNotInRequest":       {code: 400, err: ErrBucketBodyNotInRequest},
+		"ErrBucketNameNotInRequest":       {code: 400, err: ErrBucketNameNotInRequest},
+		"ErrGroupBodyNotInRequest":        {code: 400, err: ErrGroupBodyNotInRequest},
+		"ErrGroupNameNotInRequest":        {code: 400, err: ErrGroupNameNotInRequest},
+		"ErrPolicyNameNotInRequest":       {code: 400, err: ErrPolicyNameNotInRequest},
+		"ErrPolicyBodyNotInRequest":       {code: 400, err: ErrPolicyBodyNotInRequest},
+		"ErrInvalidEncryptionAlgorithm":   {code: 500, err: ErrInvalidEncryptionAlgorithm},
+		"ErrSSENotConfigured":             {code: 404, err: ErrSSENotConfigured},
+		"ErrBucketLifeCycleNotConfigured": {code: 404, err: ErrBucketLifeCycleNotConfigured},
+		"ErrChangePassword":               {code: 403, err: ErrChangePassword},
+		"ErrInvalidLicense":               {code: 404, err: ErrInvalidLicense},
+		"ErrLicenseNotFound":              {code: 404, err: ErrLicenseNotFound},
+		"ErrAvoidSelfAccountDelete":       {code: 403, err: ErrAvoidSelfAccountDelete},
+
 		"ErrNonUniqueAccessKey":               {code: 500, err: ErrNonUniqueAccessKey},
 		"ErrRemoteTierExists":                 {code: 400, err: ErrRemoteTierExists},
 		"ErrRemoteTierNotFound":               {code: 400, err: ErrRemoteTierNotFound},
 		"ErrRemoteTierUppercase":              {code: 400, err: ErrRemoteTierUppercase},
 		"ErrRemoteTierBucketNotFound":         {code: 400, err: ErrRemoteTierBucketNotFound},
 		"ErrRemoteInvalidCredentials":         {code: 403, err: ErrRemoteInvalidCredentials},
+		"ErrTooFewNodes":                      {code: 500, err: ErrTooFewNodes},
 		"ErrUnableToGetTenantUsage":           {code: 500, err: ErrUnableToGetTenantUsage},
 		"ErrTooManyNodes":                     {code: 500, err: ErrTooManyNodes},
-		"ErrTooFewNodes":                      {code: 500, err: ErrTooFewNodes},
+		"ErrAccessDenied":                     {code: 403, err: ErrAccessDenied},
 		"ErrTooFewAvailableNodes":             {code: 500, err: ErrTooFewAvailableNodes},
 		"ErrFewerThanFourNodes":               {code: 500, err: ErrFewerThanFourNodes},
 		"ErrUnableToGetTenantLogs":            {code: 500, err: ErrUnableToGetTenantLogs},
@@ -96,6 +97,7 @@ func TestError(t *testing.T) {
 			},
 		})
 	}
+
 	tests = append(tests,
 		testError{
 			name: "passing multiple errors but ErrInvalidLogin is last",
@@ -104,9 +106,21 @@ func TestError(t *testing.T) {
 			},
 			want: &CodedAPIError{
 				Code:     int(401),
-				APIError: &models.APIError{Message: ErrDefault.Error(), DetailedMessage: ErrDefault.Error()},
+				APIError: &models.APIError{Message: ErrDefault.Error(), DetailedMessage: ""},
+			},
+		})
+	tests = append(tests,
+		testError{
+			name: "login error omits detailedMessage",
+			args: args{
+				err: []interface{}{ErrInvalidLogin},
+			},
+			want: &CodedAPIError{
+				Code:     int(401),
+				APIError: &models.APIError{Message: ErrInvalidLogin.Error(), DetailedMessage: ""},
 			},
 		})
+
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got := Error(tt.args.err...)

diff --git a/sso-integration/sso_test.go b/sso-integration/sso_test.go
@@ -288,7 +288,4 @@ func TestBadLogin(t *testing.T) {
 		log.Println(err)
 		assert.Nil(err)
 	}
-	detailedMessage := result2.DetailedMessage
-	fmt.Println(detailedMessage)
-	assert.Equal("expected 'code' response type - got [], login not allowed", detailedMessage)
 }