Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions internal/api/handler_marketplace.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,16 @@ func (h *Handler) buildMarketplaceEC2Client(cfg aws.Config) marketplaceEC2Client
// credential resolver the purchase path uses. A role_arn account without a role
// ARN is accepted only when it is the host account (credentials.ErrNotHostAccount
// otherwise). Rows without a cloud account (reachable only by sell-any callers
// with unrestricted scope) keep the host's own credentials.
// with unrestricted scope) get the host's own credentials only when the row's
// AWS account id is confirmed as the host account; see
// loadMarketplaceHostAWSConfig.
func (h *Handler) loadMarketplaceAWSConfig(ctx context.Context, row *config.PurchaseHistoryRecord) (aws.Config, error) {
base, err := h.loadAWSConfigWithRegion(ctx, row.Region)
if err != nil {
return aws.Config{}, fmt.Errorf("failed to load AWS config: %w", err)
}
if row.CloudAccountID == nil || strings.TrimSpace(*row.CloudAccountID) == "" {
return base, nil
return loadMarketplaceHostAWSConfig(ctx, row, base)
}
account, err := h.config.GetCloudAccount(ctx, *row.CloudAccountID)
if err != nil {
Expand All @@ -94,6 +96,32 @@ func (h *Handler) loadMarketplaceAWSConfig(ctx context.Context, row *config.Purc
return cfg, nil
}

// marketplaceNoCloudAccountMessage is the tenant-visible refusal for a row with
// no cloud account whose AWS account is not the host account. Like
// credentials.ErrNotHostAccount it never carries the host account id.
const marketplaceNoCloudAccountMessage = "this purchase is not linked to a cloud account and its AWS account is not confirmed as the CUDly host account; it cannot be listed or canceled from here"

// loadMarketplaceHostAWSConfig handles a row with no cloud account: the
// ambient single-account deployment (purchase.SingleCloudAccountIDFromRecs
// returns nil) and legacy or account-deleted rows (cloud_account_id has no
// backfill and is ON DELETE SET NULL). The host's credentials can only act on
// the host account, so they are used only when the row's AWS account id
// passes the same host identity check the resolver applies to a role_arn
// account with no role ARN. Anything else, including an empty account id or a
// failed STS lookup, is refused before any EC2 client is built.
func loadMarketplaceHostAWSConfig(ctx context.Context, row *config.PurchaseHistoryRecord, base aws.Config) (aws.Config, error) {
accountID := strings.TrimSpace(row.AccountID)
if accountID == "" {
logging.Warnf("marketplace: purchase %s has neither a cloud account nor an AWS account id; refusing to sign with host credentials", row.PurchaseID)
return aws.Config{}, NewClientError(400, marketplaceNoCloudAccountMessage)
}
hostCheck := &config.CloudAccount{ID: "purchase " + row.PurchaseID, Provider: "aws", ExternalID: accountID}
if err := credentials.VerifyHostAccount(ctx, hostCheck, sts.NewFromConfig(base)); err != nil {
return aws.Config{}, NewClientError(400, marketplaceNoCloudAccountMessage)
}
return base, nil
}

// MarketplacePriceTier is the JSON-decodable shape accepted in the request body.
type MarketplacePriceTier struct {
// TermMonths is the remaining-months count this tier covers.
Expand Down
154 changes: 139 additions & 15 deletions internal/api/handler_marketplace_credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

awsprovider "github.com/LeanerCloud/cloud-commitments-go/providers/aws"
ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2"
"github.com/LeanerCloud/cloud-commitments-platform/internal/auth"
"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
"github.com/LeanerCloud/cloud-commitments-platform/internal/credentials"
"github.com/aws/aws-sdk-go-v2/aws"
Expand Down Expand Up @@ -313,26 +314,149 @@ func TestLoadMarketplaceAWSConfig_FailsClosed(t *testing.T) {
})
}

// Rows without a cloud account keep the host's ambient credentials (reachable
// only by sell-any callers); this documents the current behavior.
func TestLoadMarketplaceAWSConfig_NoCloudAccountKeepsHostCredentials(t *testing.T) {
empty := ""
for name, id := range map[string]*string{"nil": nil, "empty": &empty} {
// countingSTS records how many STS calls were made and answers them with the
// wrapped transport.
type countingSTS struct {
calls *int
next aws.HTTPClient
}

func (c countingSTS) Do(r *http.Request) (*http.Response, error) {
*c.calls++
return c.next.Do(r)
}

// failingSTS fails every STS call at the transport.
type failingSTS struct{}

func (failingSTS) Do(*http.Request) (*http.Response, error) {
return nil, errors.New("sts unreachable")
}

// noCloudAccountRow is a purchase row with no cloud account (ambient
// single-account deployment, legacy or account-deleted row) owned by the given
// AWS account.
func noCloudAccountRow(cloudAccountID *string, awsAccountID string) *config.PurchaseHistoryRecord {
row := standardRow()
row.CloudAccountID = cloudAccountID
row.AccountID = awsAccountID
return row
}

func hostOnlyHandler(transport aws.HTTPClient) *Handler {
// No GetCloudAccount expectation: a row without a cloud account must not
// look one up (the mock panics on an unexpected call).
h := &Handler{config: &MockConfigStore{}}
h.awsCfgOnce.Do(func() {
h.awsCfg = aws.Config{
Region: "us-east-1",
Credentials: awscreds.NewStaticCredentialsProvider(marketplaceHostKeyID, "host-secret", ""),
HTTPClient: transport,
}
})
return h
}

// A row with no cloud account gets the host's credentials only when its AWS
// account id is the host account (issue #261).
func TestLoadMarketplaceAWSConfig_NoCloudAccountHostRowUsesHostCredentials(t *testing.T) {
empty, blank := "", " "
for name, id := range map[string]*string{"nil": nil, "empty": &empty, "blank": &blank} {
t.Run(name, func(t *testing.T) {
h := &Handler{config: &MockConfigStore{}}
h.awsCfgOnce.Do(func() {
h.awsCfg = aws.Config{
Region: "us-east-1",
Credentials: awscreds.NewStaticCredentialsProvider(marketplaceHostKeyID, "host-secret", ""),
}
})
row := standardRow()
row.CloudAccountID = id
calls := 0
h := hostOnlyHandler(countingSTS{&calls, hostSTSTransport{}})

cfg, err := h.loadMarketplaceAWSConfig(context.Background(), row)
cfg, err := h.loadMarketplaceAWSConfig(context.Background(), noCloudAccountRow(id, marketplaceHostAccount))

require.NoError(t, err)
assert.Equal(t, marketplaceHostKeyID, signingKeyID(t, cfg))
assert.Equal(t, 1, calls, "the host identity must be checked before the host's credentials are used")
})
}
}

func TestLoadMarketplaceAWSConfig_NoCloudAccountRefusedUnlessHost(t *testing.T) {
cases := map[string]struct {
owner string
transport aws.HTTPClient
wantSTS bool
}{
"member account": {owner: "222222222222", transport: hostSTSTransport{}, wantSTS: true},
"empty account id": {owner: "", transport: hostSTSTransport{}, wantSTS: false},
"blank account id": {owner: " ", transport: hostSTSTransport{}, wantSTS: false},
"host identity fail": {owner: marketplaceHostAccount, transport: failingSTS{}, wantSTS: true},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
calls := 0
h := hostOnlyHandler(countingSTS{&calls, tc.transport})
h.awsCfg.Retryer = noRetry

cfg, err := h.loadMarketplaceAWSConfig(context.Background(), noCloudAccountRow(nil, tc.owner))

ce, ok := IsClientError(err)
require.True(t, ok, "expected a client error, got %v", err)
assert.Equal(t, 400, ce.code)
assert.Equal(t, marketplaceNoCloudAccountMessage, ce.message)
assert.NotContains(t, ce.message, marketplaceHostAccount)
assert.Nil(t, cfg.Credentials, "a refused row must not get the host's credentials")
assert.Equal(t, tc.wantSTS, calls > 0)
})
}
}

// End to end through both handlers: a no-cloud-account row owned by a member
// account never reaches EC2, and one owned by the host account is signed with
// the host's credentials.
func TestMarketplaceHandlers_NoCloudAccountRow(t *testing.T) {
run := map[string]func(*Handler) (any, error){
"list": func(h *Handler) (any, error) {
return h.marketplaceList(context.Background(), marketplaceReq(), validMarketplacePurchaseID)
},
"cancel": func(h *Handler) (any, error) {
return h.marketplaceCancel(context.Background(), marketplaceReq(), validMarketplacePurchaseID)
},
}
// A row with no cloud account is reachable only by an unrestricted
// sell-any caller; authorization itself is unchanged.
unrestricted := func(h *Handler) {
h.auth.(*MockAuthService).On("HasPermissionForConstraintsAPI", mock.Anything, "admin", auth.ActionSellAny, auth.ResourcePurchases,
[]auth.PermissionConstraints{{StrictScope: true, Regions: []string{"us-east-1"}}}).Return(true, nil)
}
for name, call := range run {
row := func(accountID string) *config.PurchaseHistoryRecord {
r := noCloudAccountRow(nil, accountID)
if name == "cancel" {
r.ListingState = config.ListingStateActive
r.ListingID = "ril-cancel"
}
return r
}
t.Run(name+"/member account refused", func(t *testing.T) {
h, built := marketplaceAccountHandler(t, nil, row("222222222222"))
unrestricted(h)
built.Region = "unset"

_, err := call(h)

ce, ok := IsClientError(err)
require.True(t, ok, "expected a client error, got %v", err)
assert.Equal(t, 400, ce.code)
assert.Equal(t, marketplaceNoCloudAccountMessage, ce.message)
assert.Equal(t, "unset", built.Region, "no EC2 client may be built for a refused row")
store := h.config.(*MockConfigStore)
store.AssertNotCalled(t, "GetCloudAccount", mock.Anything, mock.Anything)
})
t.Run(name+"/host account uses host credentials", func(t *testing.T) {
h, built := marketplaceAccountHandler(t, nil, row(marketplaceHostAccount))
unrestricted(h)

_, err := call(h)

require.NoError(t, err)
assert.Equal(t, marketplaceHostKeyID, signingKeyID(t, *built))
store := h.config.(*MockConfigStore)
store.AssertNotCalled(t, "GetCloudAccount", mock.Anything, mock.Anything)
})
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,9 @@ func TestMarketplacePermissionConstraintsHTTP(t *testing.T) {
row.PurchaseID, row.CloudAccountID, row.Provider, row.Service = uuid.NewString(), &account, "aws", "ec2"
switch tc.missing {
case "account":
row.CloudAccountID = nil
// A row with no cloud account is signed with the host's
// credentials only when it belongs to the host account (#261).
row.CloudAccountID, row.AccountID = nil, marketplaceHostAccount
case "provider":
row.Provider = " "
case "service":
Expand Down
Loading