From 85a228c0e5bbd57e70b68e1ecb821529c5ff708b Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Fri, 6 Dec 2024 16:18:03 +0000 Subject: [PATCH] [v16] AWS OIDC: add aws account id as label to AWS App (#49866) * 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 b3ce94c684697..f2094e25391af 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 059928010698d..7e7e6e3fec16d 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" @@ -988,7 +990,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 21d5526ef890b..b9655c8268aea 100644 --- a/lib/web/integrations_awsoidc_test.go +++ b/lib/web/integrations_awsoidc_test.go @@ -1063,7 +1063,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) @@ -1090,7 +1090,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, @@ -1099,7 +1100,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", @@ -1129,7 +1131,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 06a2e2a028bf4..80ea7b02a9363 100644 --- a/tool/tctl/common/resource_command_test.go +++ b/tool/tctl/common/resource_command_test.go @@ -1925,11 +1925,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 @@ -1973,7 +1977,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,