From 09d35d3b4c6b3c19d0e4934210daf5a8009a4791 Mon Sep 17 00:00:00 2001
From: Harshavardhana <harsha@minio.io>
Date: Wed, 18 Mar 2020 17:25:45 -0700
Subject: [PATCH] fix: sts to return appropriate errors (#9161)

---
 cmd/api-errors.go               | 14 ++++++++++----
 cmd/signature-v4-parser.go      |  6 +++++-
 cmd/signature-v4-parser_test.go |  2 +-
 cmd/sts-errors.go               | 15 +++++++++------
 cmd/sts-handlers.go             |  6 ------
 5 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/cmd/api-errors.go b/cmd/api-errors.go
index 5982efe44..15a1b4203 100644
--- a/cmd/api-errors.go
+++ b/cmd/api-errors.go
@@ -122,7 +122,8 @@ const (
 	ErrMissingCredTag
 	ErrCredMalformed
 	ErrInvalidRegion
-	ErrInvalidService
+	ErrInvalidServiceS3
+	ErrInvalidServiceSTS
 	ErrInvalidRequestVersion
 	ErrMissingSignTag
 	ErrMissingSignHeadersTag
@@ -653,9 +654,14 @@ var errorCodes = errorCodeMap{
 	// FIXME: Should contain the invalid param set as seen in https://github.com/minio/minio/issues/2385.
 	// right Description:   "Error parsing the X-Amz-Credential parameter; incorrect service \"s4\". This endpoint belongs to \"s3\".".
 	// Need changes to make sure variable messages can be constructed.
-	ErrInvalidService: {
-		Code:           "AuthorizationQueryParametersError",
-		Description:    "Error parsing the X-Amz-Credential parameter; incorrect service. This endpoint belongs to \"s3\".",
+	ErrInvalidServiceS3: {
+		Code:           "AuthorizationParametersError",
+		Description:    "Error parsing the Credential/X-Amz-Credential parameter; incorrect service. This endpoint belongs to \"s3\".",
+		HTTPStatusCode: http.StatusBadRequest,
+	},
+	ErrInvalidServiceSTS: {
+		Code:           "AuthorizationParametersError",
+		Description:    "Error parsing the Credential parameter; incorrect service. This endpoint belongs to \"sts\".",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
 	// FIXME: Should contain the invalid param set as seen in https://github.com/minio/minio/issues/2385.
diff --git a/cmd/signature-v4-parser.go b/cmd/signature-v4-parser.go
index ad8df3981..31006f118 100644
--- a/cmd/signature-v4-parser.go
+++ b/cmd/signature-v4-parser.go
@@ -108,7 +108,11 @@ func parseCredentialHeader(credElement string, region string, stype serviceType)
 
 	}
 	if credElements[2] != string(stype) {
-		return ch, ErrInvalidService
+		switch stype {
+		case serviceSTS:
+			return ch, ErrInvalidServiceSTS
+		}
+		return ch, ErrInvalidServiceS3
 	}
 	cred.scope.service = credElements[2]
 	if credElements[3] != "aws4_request" {
diff --git a/cmd/signature-v4-parser_test.go b/cmd/signature-v4-parser_test.go
index 240e87544..13b1ee7f9 100644
--- a/cmd/signature-v4-parser_test.go
+++ b/cmd/signature-v4-parser_test.go
@@ -151,7 +151,7 @@ func TestParseCredentialHeader(t *testing.T) {
 				"ABCD",
 				"ABCD"),
 			expectedCredentials: credentialHeader{},
-			expectedErrCode:     ErrInvalidService,
+			expectedErrCode:     ErrInvalidServiceS3,
 		},
 		// Test Case - 7.
 		// Test case with invalid region.
diff --git a/cmd/sts-errors.go b/cmd/sts-errors.go
index 63c8e6365..cb6f299fe 100644
--- a/cmd/sts-errors.go
+++ b/cmd/sts-errors.go
@@ -29,6 +29,14 @@ import (
 // writeSTSErrorRespone writes error headers
 func writeSTSErrorResponse(ctx context.Context, w http.ResponseWriter, errCode STSErrorCode, errCtxt error) {
 	err := stsErrCodes.ToSTSErr(errCode)
+	if err.Code == "InternalError" {
+		aerr := getAPIError(APIErrorCode(errCode))
+		if aerr.Code != "InternalError" {
+			err.Code = aerr.Code
+			err.Description = aerr.Description
+			err.HTTPStatusCode = aerr.HTTPStatusCode
+		}
+	}
 	// Generate error response.
 	stsErrorResponse := STSErrorResponse{}
 	stsErrorResponse.Error.Code = err.Code
@@ -73,12 +81,12 @@ type STSErrorCode int
 // Error codes, non exhaustive list - http://docs.aws.amazon.com/STS/latest/APIReference/API_AssumeRoleWithSAML.html
 const (
 	ErrSTSNone STSErrorCode = iota
+	ErrSTSInvalidService
 	ErrSTSAccessDenied
 	ErrSTSMissingParameter
 	ErrSTSInvalidParameterValue
 	ErrSTSWebIdentityExpiredToken
 	ErrSTSClientGrantsExpiredToken
-	ErrSTSInvalidAccessKey
 	ErrSTSInvalidClientGrantsToken
 	ErrSTSMalformedPolicyDocument
 	ErrSTSNotInitialized
@@ -128,11 +136,6 @@ var stsErrCodes = stsErrorCodeMap{
 		Description:    "The client grants token that was passed could not be validated by MinIO.",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
-	ErrSTSInvalidAccessKey: {
-		Code:           "InvalidClientTokenId",
-		Description:    "The security token included in the request is invalid.",
-		HTTPStatusCode: http.StatusForbidden,
-	},
 	ErrSTSMalformedPolicyDocument: {
 		Code:           "MalformedPolicyDocument",
 		Description:    "The request was rejected because the policy document was malformed.",
diff --git a/cmd/sts-handlers.go b/cmd/sts-handlers.go
index 6049c2512..7d5557b5e 100644
--- a/cmd/sts-handlers.go
+++ b/cmd/sts-handlers.go
@@ -119,17 +119,11 @@ func checkAssumeRoleAuth(ctx context.Context, r *http.Request) (user auth.Creden
 	case authTypeSigned:
 		s3Err := isReqAuthenticated(ctx, r, globalServerRegion, serviceSTS)
 		if STSErrorCode(s3Err) != ErrSTSNone {
-			if s3Err == ErrInvalidAccessKeyID {
-				return user, ErrSTSInvalidAccessKey
-			}
 			return user, STSErrorCode(s3Err)
 		}
 		var owner bool
 		user, owner, s3Err = getReqAccessKeyV4(r, globalServerRegion, serviceSTS)
 		if STSErrorCode(s3Err) != ErrSTSNone {
-			if s3Err == ErrInvalidAccessKeyID {
-				return user, ErrSTSInvalidAccessKey
-			}
 			return user, STSErrorCode(s3Err)
 		}
 		// Root credentials are not allowed to use STS API