fix(HMS-3185): fail with 403 on failed assume role

AssumeRole is supposed to assume the role that client is giving us.
This fixes the error code when this operation fails.

It could have also been 401 as the role might be considered an input.
Given this is a permissions operation, we went with 403.

cmd/pbackend/main.go

@@ -27,26 +27,27 @@ func init() {
 }
 
 func main() {
-	if len(os.Args) < 2 {
-		usage()
-	}
-
-	switch os.Args[1] {
-	case "api":
-		api()
-	case "worker":
-		worker()
-	case "migrate":
-		migrate()
-	case "statuser":
-		statuser()
-	case "stats":
-		stats()
-	case "version":
-		ver()
-	default:
-		usage()
-	}
+	//if len(os.Args) < 2 {
+	//	usage()
+	//}
+
+	//switch os.Args[1] {
+	//case "api":
+	//	api()
+	//case "worker":
+	//	worker()
+	//case "migrate":
+	//	migrate()
+	//case "statuser":
+	//	statuser()
+	//case "stats":
+	//	stats()
+	//case "version":
+	//	ver()
+	//default:
+	//	usage()
+	//}
+	api()
 }
 
 func usage() {

internal/clients/http/ec2/ec2_client.go

@@ -140,7 +140,7 @@ func getStsAssumedCredentials(ctx context.Context, arn string, region string) (*
 	})
 	if err != nil {
 		logger.Warn().Err(err).Msg("Cannot assume role")
-		return nil, fmt.Errorf("cannot assume role %w", err)
+		return nil, fmt.Errorf("cannot assume role: %w", err)
 	}
 
 	return output.Credentials, nil

internal/clients/stubs/ec2_stub.go

@@ -3,12 +3,15 @@ package stubs
 import (
 	"context"
 	"fmt"
+	"net/http"
 
 	"github.com/RHEnVision/provisioning-backend/internal/clients"
-	"github.com/RHEnVision/provisioning-backend/internal/clients/http"
+	httpClients "github.com/RHEnVision/provisioning-backend/internal/clients/http"
 	"github.com/RHEnVision/provisioning-backend/internal/models"
 	"github.com/RHEnVision/provisioning-backend/internal/ptr"
 	"github.com/aws/aws-sdk-go-v2/service/ec2/types"
+	"github.com/aws/smithy-go"
+	smithyhttp "github.com/aws/smithy-go/transport/http"
 )
 
 type ec2CtxKeyType int
@@ -42,7 +45,27 @@ func newEC2ServiceClientStubWithRegion(ctx context.Context, region string) (clie
 	return nil, nil
 }
 
-func newEC2CustomerClientStubWithRegion(ctx context.Context, _ *clients.Authentication, _ string) (si clients.EC2, err error) {
+func newEC2CustomerClientStubWithRegion(ctx context.Context, auth *clients.Authentication, _ string) (clients.EC2, error) {
+	// For special case - testing invalid ARN errors
+	if auth.Payload == "arn:aws:iam::0000:role/test-invalid" {
+		err := &smithy.OperationError{
+			ServiceID:     "STS",
+			OperationName: "AssumeRole",
+			Err: &smithyhttp.ResponseError{
+				Response: &smithyhttp.Response{
+					Response: &http.Response{
+						StatusCode: 403,
+					},
+				},
+				Err: &smithy.GenericAPIError{
+					Code:    "AccessDenied",
+					Fault:   smithy.FaultServer,
+					Message: fmt.Sprintf("User: %s is not authorized to perform: sts:AssumeRole on resource: %s", "<serviceAccount>", auth.Payload),
+				},
+			},
+		}
+		return nil, fmt.Errorf("cannot assume role: %w", err)
+	}
 	return getEC2StubFromContext(ctx)
 }
 
@@ -85,7 +108,7 @@ func (mock *EC2ClientStub) GetPubkeyName(ctx context.Context, fingerprint string
 			return *key.KeyName, nil
 		}
 	}
-	return "", http.ErrPubkeyNotFound
+	return "", httpClients.ErrPubkeyNotFound
 }
 
 func (mock *EC2ClientStub) DeleteSSHKey(ctx context.Context, handle string) error {

internal/clients/stubs/sources_stub.go

@@ -39,7 +39,27 @@ func AddSource(ctx context.Context, provider models.ProviderType) (*clients.Sour
 	if err != nil {
 		return nil, err
 	}
-	return stub.addSource(ctx, provider)
+	switch provider {
+	case models.ProviderTypeAWS:
+		return stub.addAuth(ctx, clients.NewAuthentication("arn:aws:iam::230214684733:role/Test", provider))
+	case models.ProviderTypeAzure:
+		return stub.addAuth(ctx, clients.NewAuthentication("4b9d213f-712f-4d17-a483-8a10bbe9df3a", provider))
+	case models.ProviderTypeGCP:
+		return stub.addAuth(ctx, clients.NewAuthentication("[email protected]", provider))
+	case models.ProviderTypeUnknown, models.ProviderTypeNoop:
+		// not implemented
+		return nil, ErrNotImplemented
+	}
+
+	return nil, ErrNotImplemented
+}
+
+func AddAuth(ctx context.Context, authentication *clients.Authentication) (*clients.Source, error) {
+	stub, err := getSourcesClientStub(ctx)
+	if err != nil {
+		return nil, err
+	}
+	return stub.addAuth(ctx, authentication)
 }
 
 func getSourcesClient(ctx context.Context) (clients.Sources, error) {
@@ -54,24 +74,13 @@ func getSourcesClientStub(ctx context.Context) (si *SourcesClientStub, err error
 	return si, err
 }
 
-func (stub *SourcesClientStub) addSource(ctx context.Context, provider models.ProviderType) (*clients.Source, error) {
+func (stub *SourcesClientStub) addAuth(ctx context.Context, authentication *clients.Authentication) (*clients.Source, error) {
 	id := strconv.Itoa(len(stub.sources) + 2) // starts at 2 as 1 is reserved - TODO migrate users of the implicit id = 1
 	source := &clients.Source{
 		ID:   id,
 		Name: "source-" + id,
 	}
-	switch provider {
-	case models.ProviderTypeAWS:
-		stub.auths[id] = clients.NewAuthentication("arn:aws:iam::230214684733:role/Test", provider)
-	case models.ProviderTypeAzure:
-		stub.auths[id] = clients.NewAuthentication("4b9d213f-712f-4d17-a483-8a10bbe9df3a", provider)
-	case models.ProviderTypeGCP:
-		stub.auths[id] = clients.NewAuthentication("[email protected]", provider)
-	case models.ProviderTypeUnknown, models.ProviderTypeNoop:
-		// not implemented
-		return nil, ErrNotImplemented
-	}
-
+	stub.auths[id] = authentication
 	stub.sources = append(stub.sources, source)
 	return source, nil
 }

internal/payloads/error_payload.go

@@ -8,8 +8,8 @@ import (
 	"strings"
 
 	"github.com/RHEnVision/provisioning-backend/internal/clients"
-
 	"github.com/RHEnVision/provisioning-backend/internal/usrerr"
+	"github.com/aws/smithy-go"
 
 	"github.com/RHEnVision/provisioning-backend/internal/logging"
 	"github.com/RHEnVision/provisioning-backend/internal/version"
@@ -194,6 +194,11 @@ func NewStatusError(ctx context.Context, message string, err error) *ResponseErr
 }
 
 func NewAWSError(ctx context.Context, message string, err error) *ResponseError {
+	var awsAPIErr *smithy.GenericAPIError
+	if errors.As(err, &awsAPIErr) && awsAPIErr.Code == "AccessDenied" {
+		message = fmt.Sprintf("AWS assume role failed: %s", message)
+		return NewResponseError(ctx, http.StatusForbidden, message, err)
+	}
 	message = fmt.Sprintf("AWS API error: %s", message)
 	return NewResponseError(ctx, http.StatusInternalServerError, message, err)
 }

internal/services/sources_service_test.go

@@ -112,52 +112,80 @@ func TestGetAzureSourceDetails(t *testing.T) {
 	})
 }
 
-func TestSourceUploadInfoFails(t *testing.T) {
-	t.Run("fails on AWS upload info for invalid id", func(t *testing.T) {
-		ctx := stubs.WithAccountDaoOne(context.Background())
-		ctx = identity.WithTenant(t, ctx)
-		ctx = clientStub.WithSourcesClient(ctx)
-		ctx = clientStub.WithEC2Client(ctx)
-
-		req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("/api/provisioning/sources/abcdef/upload_info"), nil)
-		require.NoError(t, err, "failed to create request")
-
-		rr := httptest.NewRecorder()
-		handler := http.HandlerFunc(GetSourceUploadInfo)
-		handler.ServeHTTP(rr, req)
-
-		require.Equal(t, http.StatusBadRequest, rr.Code, "Handler returned wrong status code")
+func TestGetSourceUploadInfo(t *testing.T) {
+	t.Run("validates source ID", func(t *testing.T) {
+		t.Run("fails on AWS upload info for invalid id", func(t *testing.T) {
+			ctx := stubs.WithAccountDaoOne(context.Background())
+			ctx = identity.WithTenant(t, ctx)
+			ctx = clientStub.WithSourcesClient(ctx)
+			ctx = clientStub.WithEC2Client(ctx)
+
+			req, err := http.NewRequestWithContext(ctx, "GET", "/api/provisioning/sources/abcdef/upload_info", nil)
+			require.NoError(t, err, "failed to create request")
+
+			rr := httptest.NewRecorder()
+			handler := http.HandlerFunc(GetSourceUploadInfo)
+			handler.ServeHTTP(rr, req)
+
+			require.Equal(t, http.StatusBadRequest, rr.Code, "Handler returned wrong status code")
+		})
+
+		t.Run("fails on Azure upload info for invalid id", func(t *testing.T) {
+			ctx := stubs.WithAccountDaoOne(context.Background())
+			ctx = identity.WithTenant(t, ctx)
+			ctx = clientStub.WithSourcesClient(ctx)
+			ctx = clientStub.WithAzureClient(ctx)
+
+			req, err := http.NewRequestWithContext(ctx, "GET", "/api/provisioning/sources/{434FA35}/upload_info", nil)
+			require.NoError(t, err, "failed to create request")
+
+			rr := httptest.NewRecorder()
+			handler := http.HandlerFunc(GetSourceUploadInfo)
+			handler.ServeHTTP(rr, req)
+
+			require.Equal(t, http.StatusBadRequest, rr.Code, "Handler returned wrong status code")
+		})
+
+		t.Run("fails on GCP upload info for invalid id", func(t *testing.T) {
+			ctx := stubs.WithAccountDaoOne(context.Background())
+			ctx = identity.WithTenant(t, ctx)
+			ctx = clientStub.WithSourcesClient(ctx)
+			ctx = clientStub.WithGCPCCustomerClient(ctx)
+
+			req, err := http.NewRequestWithContext(ctx, "GET", "/api/provisioning/sources/{21234}/upload_info", nil)
+			require.NoError(t, err, "failed to create request")
+
+			rr := httptest.NewRecorder()
+			handler := http.HandlerFunc(GetSourceUploadInfo)
+			handler.ServeHTTP(rr, req)
+
+			require.Equal(t, http.StatusBadRequest, rr.Code, "Handler returned wrong status code")
+		})
 	})
 
-	t.Run("fails on Azure upload info for invalid id", func(t *testing.T) {
-		ctx := stubs.WithAccountDaoOne(context.Background())
-		ctx = identity.WithTenant(t, ctx)
-		ctx = clientStub.WithSourcesClient(ctx)
-		ctx = clientStub.WithAzureClient(ctx)
-
-		req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("/api/provisioning/sources/{434FA35}/upload_info"), nil)
-		require.NoError(t, err, "failed to create request")
-
-		rr := httptest.NewRecorder()
-		handler := http.HandlerFunc(GetSourceUploadInfo)
-		handler.ServeHTTP(rr, req)
-
-		require.Equal(t, http.StatusBadRequest, rr.Code, "Handler returned wrong status code")
-	})
-
-	t.Run("fails on GCP upload info for invalid id", func(t *testing.T) {
+	t.Run("fails with 403 on AWS invalid permissions or role", func(t *testing.T) {
 		ctx := stubs.WithAccountDaoOne(context.Background())
 		ctx = identity.WithTenant(t, ctx)
 		ctx = clientStub.WithSourcesClient(ctx)
 		ctx = clientStub.WithGCPCCustomerClient(ctx)
+		source, err := clientStub.AddAuth(ctx, &clients.Authentication{ProviderType: models.ProviderTypeAWS, Payload: "arn:aws:iam::0000:role/test-invalid"})
+		require.NoError(t, err, "failed to add source")
 
-		req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("/api/provisioning/sources/{21234}/upload_info"), nil)
+		rctx := chi.NewRouteContext()
+		ctx = context.WithValue(ctx, chi.RouteCtxKey, rctx)
+		rctx.URLParams.Add("ID", source.ID)
+		req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("/api/provisioning/sources/%s/upload_info", source.ID), nil)
 		require.NoError(t, err, "failed to create request")
 
 		rr := httptest.NewRecorder()
 		handler := http.HandlerFunc(GetSourceUploadInfo)
 		handler.ServeHTTP(rr, req)
 
-		require.Equal(t, http.StatusBadRequest, rr.Code, "Handler returned wrong status code")
+		var response payloads.ResponseError
+		err = json.NewDecoder(rr.Body).Decode(&response)
+		require.NoError(t, err, "failed to decode response body")
+
+		require.Equal(t, http.StatusForbidden, rr.Code, "Handler returned wrong status code")
+		require.Equal(t, "AWS assume role failed: unable to get AWS upload info", response.Message, "Response message is incorrect")
 	})
 }

scripts/rest_examples/source-1-upload-info.http

@@ -1,4 +1,4 @@
 // @no-log
-GET http://{{hostname}}:{{port}}/{{prefix}}/sources/1/upload_info HTTP/1.1
+GET http://{{hostname}}:{{port}}/{{prefix}}/sources/515515/upload_info HTTP/1.1
 Content-Type: application/json
 X-Rh-Identity: {{identity}}