Skip to content

Commit

Permalink
[v15] AWS OIDC: add aws account id as label to AWS App (#49865)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
marcoandredinis authored Dec 6, 2024
1 parent 5f57fde commit 9fb2d6a
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 10 deletions.
8 changes: 5 additions & 3 deletions api/types/appserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion api/types/appserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ func TestNewAppServerForAWSOIDCIntegration(t *testing.T) {
integratioName string
hostID string
publicAddr string
labels map[string]string
expectedApp *AppServerV3
errCheck require.ErrorAssertionFunc
}{
Expand All @@ -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,
Expand All @@ -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",
Expand All @@ -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)
}
Expand Down
13 changes: 12 additions & 1 deletion lib/web/integrations_awsoidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down
10 changes: 6 additions & 4 deletions lib/web/integrations_awsoidc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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,
Expand All @@ -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",
Expand Down Expand Up @@ -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)

Expand Down
6 changes: 5 additions & 1 deletion tool/tctl/common/resource_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 9fb2d6a

Please sign in to comment.