From 979418a1ae13a31afc1c617762e3f43f3e849498 Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Fri, 6 Dec 2024 08:43:03 +0000 Subject: [PATCH] AWS OIDC: add aws account id as label to AWS App (#49693) * 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 39ebb250a9452..331df6ba78dd1 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" @@ -984,7 +986,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 200d68ef7d9a0..9b2660fe36e99 100644 --- a/lib/web/integrations_awsoidc_test.go +++ b/lib/web/integrations_awsoidc_test.go @@ -1067,7 +1067,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) @@ -1094,7 +1094,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, @@ -1103,7 +1104,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", @@ -1133,7 +1135,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 a3dd729d72477..b3e13e9bffb6d 100644 --- a/tool/tctl/common/resource_command_test.go +++ b/tool/tctl/common/resource_command_test.go @@ -1939,11 +1939,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 @@ -1987,7 +1991,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,