From 9fb2d6a78af8484875eb8e6a2021a6aa81164fd7 Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Fri, 6 Dec 2024 16:38:20 +0000 Subject: [PATCH] [v15] AWS OIDC: add aws account id as label to AWS App (#49865) * AWS OIDC: add aws account id as label to AWS App We were not setting any labels in the AWS App when using the Discover Flow for a given AWS OIDC integration. This is a bad practice because this means that users must have `app_labels: *:*` in order to access this particular app. This is not recommended because it grants access to every app. This PR changes this so that the account id can be used to gate access. * fix test --- api/types/appserver.go | 8 +++++--- api/types/appserver_test.go | 6 +++++- lib/web/integrations_awsoidc.go | 13 ++++++++++++- lib/web/integrations_awsoidc_test.go | 10 ++++++---- tool/tctl/common/resource_command_test.go | 6 +++++- 5 files changed, 33 insertions(+), 10 deletions(-) diff --git a/api/types/appserver.go b/api/types/appserver.go index 5f0506e0d75e2..6fe3b67b7a2f1 100644 --- a/api/types/appserver.go +++ b/api/types/appserver.go @@ -86,13 +86,15 @@ func NewAppServerV3FromApp(app *AppV3, hostname, hostID string) (*AppServerV3, e // NewAppServerForAWSOIDCIntegration creates a new AppServer that will be used to grant AWS App Access // using the AWSOIDC credentials. -func NewAppServerForAWSOIDCIntegration(integrationName, hostID, publicAddr string) (*AppServerV3, error) { +func NewAppServerForAWSOIDCIntegration(integrationName, hostID, publicAddr string, labels map[string]string) (*AppServerV3, error) { return NewAppServerV3(Metadata{ - Name: integrationName, + Name: integrationName, + Labels: labels, }, AppServerSpecV3{ HostID: hostID, App: &AppV3{Metadata: Metadata{ - Name: integrationName, + Name: integrationName, + Labels: labels, }, Spec: AppSpecV3{ URI: constants.AWSConsoleURL, Integration: integrationName, diff --git a/api/types/appserver_test.go b/api/types/appserver_test.go index 74a0ab661f7dd..25fa31f6d4d63 100644 --- a/api/types/appserver_test.go +++ b/api/types/appserver_test.go @@ -63,6 +63,7 @@ func TestNewAppServerForAWSOIDCIntegration(t *testing.T) { integratioName string hostID string publicAddr string + labels map[string]string expectedApp *AppServerV3 errCheck require.ErrorAssertionFunc }{ @@ -71,12 +72,14 @@ func TestNewAppServerForAWSOIDCIntegration(t *testing.T) { integratioName: "valid", hostID: "my-host-id", publicAddr: "valid.proxy.example.com", + labels: map[string]string{"account_id": "123456789012"}, expectedApp: &AppServerV3{ Kind: KindAppServer, Version: V3, Metadata: Metadata{ Name: "valid", Namespace: "default", + Labels: map[string]string{"account_id": "123456789012"}, }, Spec: AppServerSpecV3{ Version: api.Version, @@ -87,6 +90,7 @@ func TestNewAppServerForAWSOIDCIntegration(t *testing.T) { Metadata: Metadata{ Name: "valid", Namespace: "default", + Labels: map[string]string{"account_id": "123456789012"}, }, Spec: AppSpecV3{ URI: "https://console.aws.amazon.com", @@ -106,7 +110,7 @@ func TestNewAppServerForAWSOIDCIntegration(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - app, err := NewAppServerForAWSOIDCIntegration(tt.integratioName, tt.hostID, tt.publicAddr) + app, err := NewAppServerForAWSOIDCIntegration(tt.integratioName, tt.hostID, tt.publicAddr, tt.labels) if tt.errCheck != nil { tt.errCheck(t, err) } diff --git a/lib/web/integrations_awsoidc.go b/lib/web/integrations_awsoidc.go index 631643f1fd7d4..ca4983354517f 100644 --- a/lib/web/integrations_awsoidc.go +++ b/lib/web/integrations_awsoidc.go @@ -35,6 +35,7 @@ import ( "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/client" "github.com/gravitational/teleport/api/client/proto" + "github.com/gravitational/teleport/api/constants" apidefaults "github.com/gravitational/teleport/api/defaults" integrationv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/integration/v1" "github.com/gravitational/teleport/api/types" @@ -49,6 +50,7 @@ import ( "github.com/gravitational/teleport/lib/reversetunnelclient" "github.com/gravitational/teleport/lib/services" libutils "github.com/gravitational/teleport/lib/utils" + awsutils "github.com/gravitational/teleport/lib/utils/aws" "github.com/gravitational/teleport/lib/utils/oidc" "github.com/gravitational/teleport/lib/web/scripts/oneoff" "github.com/gravitational/teleport/lib/web/ui" @@ -960,7 +962,16 @@ func (h *Handler) awsOIDCCreateAWSAppAccess(w http.ResponseWriter, r *http.Reque publicAddr := libutils.DefaultAppPublicAddr(integrationName, h.PublicProxyAddr()) - appServer, err := types.NewAppServerForAWSOIDCIntegration(integrationName, h.cfg.HostUUID, publicAddr) + parsedRoleARN, err := awsutils.ParseRoleARN(ig.GetAWSOIDCIntegrationSpec().RoleARN) + if err != nil { + return nil, trace.Wrap(err) + } + + labels := map[string]string{ + constants.AWSAccountIDLabel: parsedRoleARN.AccountID, + } + + appServer, err := types.NewAppServerForAWSOIDCIntegration(integrationName, h.cfg.HostUUID, publicAddr, labels) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/web/integrations_awsoidc_test.go b/lib/web/integrations_awsoidc_test.go index 761d794951fa1..ab8a45d36ec6e 100644 --- a/lib/web/integrations_awsoidc_test.go +++ b/lib/web/integrations_awsoidc_test.go @@ -982,7 +982,7 @@ func TestAWSOIDCAppAccessAppServerCreationDeletion(t *testing.T) { myIntegration, err := types.NewIntegrationAWSOIDC(types.Metadata{ Name: "my-integration", }, &types.AWSOIDCIntegrationSpecV1{ - RoleARN: "some-arn-role", + RoleARN: "arn:aws:iam::123456789012:role/teleport", }) require.NoError(t, err) @@ -1009,7 +1009,8 @@ func TestAWSOIDCAppAccessAppServerCreationDeletion(t *testing.T) { Kind: types.KindAppServer, Version: types.V3, Metadata: types.Metadata{ - Name: "my-integration", + Name: "my-integration", + Labels: map[string]string{"aws_account_id": "123456789012"}, }, Spec: types.AppServerSpecV3{ Version: api.Version, @@ -1018,7 +1019,8 @@ func TestAWSOIDCAppAccessAppServerCreationDeletion(t *testing.T) { Kind: types.KindApp, Version: types.V3, Metadata: types.Metadata{ - Name: "my-integration", + Name: "my-integration", + Labels: map[string]string{"aws_account_id": "123456789012"}, }, Spec: types.AppSpecV3{ URI: "https://console.aws.amazon.com", @@ -1048,7 +1050,7 @@ func TestAWSOIDCAppAccessAppServerCreationDeletion(t *testing.T) { myIntegrationWithAccountID, err := types.NewIntegrationAWSOIDC(types.Metadata{ Name: "123456789012", }, &types.AWSOIDCIntegrationSpecV1{ - RoleARN: "some-arn-role", + RoleARN: "arn:aws:iam::123456789012:role/teleport", }) require.NoError(t, err) diff --git a/tool/tctl/common/resource_command_test.go b/tool/tctl/common/resource_command_test.go index 347d073e68b5e..1423b2a06b3c8 100644 --- a/tool/tctl/common/resource_command_test.go +++ b/tool/tctl/common/resource_command_test.go @@ -1908,11 +1908,15 @@ func testCreateAppServer(t *testing.T, clt *authclient.Client) { kind: app_server metadata: name: my-integration + labels: + account_id: "123456789012" spec: app: kind: app metadata: name: my-integration + labels: + account_id: "123456789012" spec: uri: https://console.aws.amazon.com integration: my-integration @@ -1956,7 +1960,7 @@ version: v3 appServers := mustDecodeJSON[[]*types.AppServerV3](t, buf) require.Len(t, appServers, 1) - expectedAppServer, err := types.NewAppServerForAWSOIDCIntegration("my-integration", "c6cfe5c2-653f-4e5d-a914-bfac5a7baf38", "integration.example.com") + expectedAppServer, err := types.NewAppServerForAWSOIDCIntegration("my-integration", "c6cfe5c2-653f-4e5d-a914-bfac5a7baf38", "integration.example.com", map[string]string{"account_id": "123456789012"}) require.NoError(t, err) require.Empty(t, cmp.Diff( expectedAppServer,