Skip to content

Commit

Permalink
Change error displayed for invalid internal ILM credential to be indi…
Browse files Browse the repository at this point in the history 
…stinguishable between valid and invalid accessKeys

Cleanup

Cleanup

Gofumpt

Removed detailed errors

Debugging sso test

Remove detailedMessage from login related errors passed to Console

Replaced user_loin with master

Updated tests to reflect removal of detailedMessage for login errors

Removed detailedMessage check from SSO badLogin test

Replaced detailed messsage for LoginNotAllowed

Replaced detailed message for accessdenied error

removed commented lines
  • Loading branch information
Jillian Inapurapu authored and Jillian Inapurapu committed Dec 6, 2023
1 parent cb14cb9 commit 41bd4b5
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 27 deletions.
9 changes: 8 additions & 1 deletion restapi/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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 {
Expand All @@ -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()
}
Expand All @@ -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()
}
Expand Down Expand Up @@ -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()
}
Expand Down Expand Up @@ -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
Expand Down
69 changes: 46 additions & 23 deletions restapi/errors_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,28 +44,28 @@ 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},
Expand Down Expand Up @@ -96,6 +96,7 @@ func TestError(t *testing.T) {
},
})
}

tests = append(tests,
testError{
name: "passing multiple errors but ErrInvalidLogin is last",
Expand All @@ -104,7 +105,29 @@ 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: ""},
},
})
tests = append(tests,
testError{
name: "accessDenied error omits detailedMessage",
args: args{
err: []interface{}{ErrAccessDenied},
},
want: &CodedAPIError{
Code: int(403),
APIError: &models.APIError{Message: ErrAccessDenied.Error(), DetailedMessage: ""},
},
})
for _, tt := range tests {
Expand Down
3 changes: 0 additions & 3 deletions sso-integration/sso_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

0 comments on commit 41bd4b5

Please sign in to comment.