Skip to content

Commit 5bc0e74

Browse files
jinapurapujinapurapu
authored
Omit detailedMessage from login errors (#3136)
1 parent f0d4ddd commit 5bc0e74

File tree

3 files changed

+46
-28
lines changed

3 files changed

+46
-28
lines changed

‎restapi/errors.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ var (
5353
ErrAvoidSelfAccountDelete = errors.New("logged in user cannot be deleted by itself")
5454
ErrAccessDenied = errors.New("access denied")
5555
ErrOauth2Provider = errors.New("unable to contact configured identity provider")
56+
ErrOauth2Login = errors.New("unable to login using configured identity provider")
5657
ErrNonUniqueAccessKey = errors.New("access key already in use")
5758
ErrRemoteTierExists = errors.New("specified remote tier already exists")
5859
ErrRemoteTierNotFound = errors.New("specified remote tier was not found")
@@ -84,10 +85,12 @@ type CodedAPIError struct {
8485
func ErrorWithContext(ctx context.Context, err ...interface{}) *CodedAPIError {
8586
errorCode := 500
8687
errorMessage := ErrDefault.Error()
88+
var detailedMessage string
8789
var err1 error
8890
var exists bool
8991
if len(err) > 0 {
9092
if err1, exists = err[0].(error); exists {
93+
detailedMessage = err1.Error()
9194
var lastError error
9295
if len(err) > 1 {
9396
if err2, lastExists := err[1].(error); lastExists {
@@ -105,6 +108,7 @@ func ErrorWithContext(ctx context.Context, err ...interface{}) *CodedAPIError {
105108
errorMessage = ErrNotFound.Error()
106109
}
107110
if errors.Is(err1, ErrInvalidLogin) {
111+
detailedMessage = ""
108112
errorCode = 401
109113
errorMessage = ErrInvalidLogin.Error()
110114
}
@@ -114,10 +118,12 @@ func ErrorWithContext(ctx context.Context, err ...interface{}) *CodedAPIError {
114118
}
115119
// If the last error is ErrInvalidLogin, this is a login failure
116120
if errors.Is(lastError, ErrInvalidLogin) {
121+
detailedMessage = ""
117122
errorCode = 401
118123
errorMessage = err1.Error()
119124
}
120125
if strings.Contains(err1.Error(), ErrLoginNotAllowed.Error()) {
126+
detailedMessage = ""
121127
errorCode = 400
122128
errorMessage = ErrLoginNotAllowed.Error()
123129
}
@@ -211,6 +217,7 @@ func ErrorWithContext(ctx context.Context, err ...interface{}) *CodedAPIError {
211217
errorMessage = ErrAccessDenied.Error()
212218
}
213219
if madmin.ToErrorResponse(err1).Code == "InvalidAccessKeyId" {
220+
214221
errorCode = 401
215222
errorMessage = ErrInvalidSession.Error()
216223
}
@@ -261,7 +268,7 @@ func ErrorWithContext(ctx context.Context, err ...interface{}) *CodedAPIError {
261268
}
262269
}
263270
}
264-
return &CodedAPIError{Code: errorCode, APIError: &models.APIError{Message: errorMessage, DetailedMessage: err1.Error()}}
271+
return &CodedAPIError{Code: errorCode, APIError: &models.APIError{Message: errorMessage, DetailedMessage: detailedMessage}}
265272
}
266273

267274
// Error receives an errors object and parse it against k8sErrors, returns the right errors code paired with a generic errors message

‎restapi/errors_test.go‎

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -44,37 +44,38 @@ func TestError(t *testing.T) {
4444
}
4545

4646
appErrors := map[string]expectedError{
47-
"ErrDefault": {code: 500, err: ErrDefault},
48-
"ErrInvalidLogin": {code: 401, err: ErrInvalidLogin},
49-
"ErrForbidden": {code: 403, err: ErrForbidden},
50-
"ErrFileTooLarge": {code: 413, err: ErrFileTooLarge},
51-
"ErrInvalidSession": {code: 401, err: ErrInvalidSession},
52-
"ErrNotFound": {code: 404, err: ErrNotFound},
53-
"ErrGroupAlreadyExists": {code: 400, err: ErrGroupAlreadyExists},
54-
"ErrInvalidErasureCodingValue": {code: 400, err: ErrInvalidErasureCodingValue},
55-
"ErrBucketBodyNotInRequest": {code: 400, err: ErrBucketBodyNotInRequest},
56-
"ErrBucketNameNotInRequest": {code: 400, err: ErrBucketNameNotInRequest},
57-
"ErrGroupBodyNotInRequest": {code: 400, err: ErrGroupBodyNotInRequest},
58-
"ErrGroupNameNotInRequest": {code: 400, err: ErrGroupNameNotInRequest},
59-
"ErrPolicyNameNotInRequest": {code: 400, err: ErrPolicyNameNotInRequest},
60-
"ErrPolicyBodyNotInRequest": {code: 400, err: ErrPolicyBodyNotInRequest},
61-
"ErrInvalidEncryptionAlgorithm": {code: 500, err: ErrInvalidEncryptionAlgorithm},
62-
"ErrSSENotConfigured": {code: 404, err: ErrSSENotConfigured},
63-
"ErrBucketLifeCycleNotConfigured": {code: 404, err: ErrBucketLifeCycleNotConfigured},
64-
"ErrChangePassword": {code: 403, err: ErrChangePassword},
65-
"ErrInvalidLicense": {code: 404, err: ErrInvalidLicense},
66-
"ErrLicenseNotFound": {code: 404, err: ErrLicenseNotFound},
67-
"ErrAvoidSelfAccountDelete": {code: 403, err: ErrAvoidSelfAccountDelete},
68-
"ErrAccessDenied": {code: 403, err: ErrAccessDenied},
47+
"ErrDefault": {code: 500, err: ErrDefault},
48+
49+
"ErrForbidden": {code: 403, err: ErrForbidden},
50+
"ErrFileTooLarge": {code: 413, err: ErrFileTooLarge},
51+
"ErrInvalidSession": {code: 401, err: ErrInvalidSession},
52+
"ErrNotFound": {code: 404, err: ErrNotFound},
53+
"ErrGroupAlreadyExists": {code: 400, err: ErrGroupAlreadyExists},
54+
"ErrInvalidErasureCodingValue": {code: 400, err: ErrInvalidErasureCodingValue},
55+
"ErrBucketBodyNotInRequest": {code: 400, err: ErrBucketBodyNotInRequest},
56+
"ErrBucketNameNotInRequest": {code: 400, err: ErrBucketNameNotInRequest},
57+
"ErrGroupBodyNotInRequest": {code: 400, err: ErrGroupBodyNotInRequest},
58+
"ErrGroupNameNotInRequest": {code: 400, err: ErrGroupNameNotInRequest},
59+
"ErrPolicyNameNotInRequest": {code: 400, err: ErrPolicyNameNotInRequest},
60+
"ErrPolicyBodyNotInRequest": {code: 400, err: ErrPolicyBodyNotInRequest},
61+
"ErrInvalidEncryptionAlgorithm": {code: 500, err: ErrInvalidEncryptionAlgorithm},
62+
"ErrSSENotConfigured": {code: 404, err: ErrSSENotConfigured},
63+
"ErrBucketLifeCycleNotConfigured": {code: 404, err: ErrBucketLifeCycleNotConfigured},
64+
"ErrChangePassword": {code: 403, err: ErrChangePassword},
65+
"ErrInvalidLicense": {code: 404, err: ErrInvalidLicense},
66+
"ErrLicenseNotFound": {code: 404, err: ErrLicenseNotFound},
67+
"ErrAvoidSelfAccountDelete": {code: 403, err: ErrAvoidSelfAccountDelete},
68+
6969
"ErrNonUniqueAccessKey": {code: 500, err: ErrNonUniqueAccessKey},
7070
"ErrRemoteTierExists": {code: 400, err: ErrRemoteTierExists},
7171
"ErrRemoteTierNotFound": {code: 400, err: ErrRemoteTierNotFound},
7272
"ErrRemoteTierUppercase": {code: 400, err: ErrRemoteTierUppercase},
7373
"ErrRemoteTierBucketNotFound": {code: 400, err: ErrRemoteTierBucketNotFound},
7474
"ErrRemoteInvalidCredentials": {code: 403, err: ErrRemoteInvalidCredentials},
75+
"ErrTooFewNodes": {code: 500, err: ErrTooFewNodes},
7576
"ErrUnableToGetTenantUsage": {code: 500, err: ErrUnableToGetTenantUsage},
7677
"ErrTooManyNodes": {code: 500, err: ErrTooManyNodes},
77-
"ErrTooFewNodes": {code: 500, err: ErrTooFewNodes},
78+
"ErrAccessDenied": {code: 403, err: ErrAccessDenied},
7879
"ErrTooFewAvailableNodes": {code: 500, err: ErrTooFewAvailableNodes},
7980
"ErrFewerThanFourNodes": {code: 500, err: ErrFewerThanFourNodes},
8081
"ErrUnableToGetTenantLogs": {code: 500, err: ErrUnableToGetTenantLogs},
@@ -96,6 +97,7 @@ func TestError(t *testing.T) {
9697
},
9798
})
9899
}
100+
99101
tests = append(tests,
100102
testError{
101103
name: "passing multiple errors but ErrInvalidLogin is last",
@@ -104,9 +106,21 @@ func TestError(t *testing.T) {
104106
},
105107
want: &CodedAPIError{
106108
Code: int(401),
107-
APIError: &models.APIError{Message: ErrDefault.Error(), DetailedMessage: ErrDefault.Error()},
109+
APIError: &models.APIError{Message: ErrDefault.Error(), DetailedMessage: ""},
110+
},
111+
})
112+
tests = append(tests,
113+
testError{
114+
name: "login error omits detailedMessage",
115+
args: args{
116+
err: []interface{}{ErrInvalidLogin},
117+
},
118+
want: &CodedAPIError{
119+
Code: int(401),
120+
APIError: &models.APIError{Message: ErrInvalidLogin.Error(), DetailedMessage: ""},
108121
},
109122
})
123+
110124
for _, tt := range tests {
111125
t.Run(tt.name, func(t *testing.T) {
112126
got := Error(tt.args.err...)

‎sso-integration/sso_test.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,4 @@ func TestBadLogin(t *testing.T) {
288288
log.Println(err)
289289
assert.Nil(err)
290290
}
291-
detailedMessage := result2.DetailedMessage
292-
fmt.Println(detailedMessage)
293-
assert.Equal("expected 'code' response type - got [], login not allowed", detailedMessage)
294291
}

0 commit comments

Comments
 (0)