From b65d7fda06976ca85132467d61bb3985ed8f3390 Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Fri, 22 Aug 2025 12:07:15 +0100 Subject: [PATCH] AWS Roles Anywhere: endpoint to validate name (#57887) * AWS Roles Anywhere: endpoint to validate name When the user is configuring the integration, one of the steps is to give a name to the integration. After giving the name, the user is asked to run a script. Only after, the integration is created on Teleport side. This can cause a bad UX if the user picks an integration name that is not valid. This endpoint is going to be called from the UI when the user enters the integration name. It ensures the integration does not exist yet, and that it is a valid name. * review pt1 --- lib/auth/integration/integrationv1/service.go | 10 +--- lib/integrations/awscommon/validations.go | 37 +++++++++++++ lib/web/apiserver.go | 1 + lib/web/integrations.go | 12 ++-- lib/web/integrations_awsoidc.go | 26 ++++----- lib/web/integrations_awsra.go | 39 ++++++++++++- lib/web/integrations_awsra_test.go | 55 +++++++++++++++++++ 7 files changed, 152 insertions(+), 28 deletions(-) create mode 100644 lib/integrations/awscommon/validations.go diff --git a/lib/auth/integration/integrationv1/service.go b/lib/auth/integration/integrationv1/service.go index ab1f49e628b..e2ee691cf00 100644 --- a/lib/auth/integration/integrationv1/service.go +++ b/lib/auth/integration/integrationv1/service.go @@ -28,7 +28,6 @@ import ( "github.com/jonboulle/clockwork" "golang.org/x/crypto/ssh" "google.golang.org/protobuf/types/known/emptypb" - "k8s.io/apimachinery/pkg/util/validation" "github.com/gravitational/teleport" integrationpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/integration/v1" @@ -38,6 +37,7 @@ import ( "github.com/gravitational/teleport/lib/authz" "github.com/gravitational/teleport/lib/cryptosuites" "github.com/gravitational/teleport/lib/events" + "github.com/gravitational/teleport/lib/integrations/awscommon" "github.com/gravitational/teleport/lib/integrations/awsra/createsession" "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/services" @@ -248,12 +248,8 @@ func (s *Service) CreateIntegration(ctx context.Context, req *integrationpb.Crea return nil, trace.Wrap(err) } case types.IntegrationSubKindAWSOIDC, types.IntegrationSubKindAWSRolesAnywhere: - // AWS OIDC and Roles Anywhere Integrations can be used as source of credentials to access AWS Web/CLI. - // For OIDC, this creates a new AppServer whose endpoint is ., which can fail if integrationName is not a valid DNS Label. - // For Roles Anywhere, this creates a AppServers for each Roles Anywhere Profile whose endpoint is -., which can fail if integrationName is not a valid DNS Label. - // Instead of failing when the integration is already created, it fails at creation time. - if errs := validation.IsDNS1035Label(req.GetIntegration().GetName()); len(errs) > 0 { - return nil, trace.BadParameter("integration name %q must be a lower case valid DNS subdomain so that it can be used to allow Web/CLI access", req.GetIntegration().GetName()) + if err := awscommon.ValidIntegratioName(req.Integration.GetName()); err != nil { + return nil, trace.Wrap(err) } if err := validateAWSRolesAnywhereProfileFilters(req.Integration); err != nil { diff --git a/lib/integrations/awscommon/validations.go b/lib/integrations/awscommon/validations.go new file mode 100644 index 00000000000..48bf398fabd --- /dev/null +++ b/lib/integrations/awscommon/validations.go @@ -0,0 +1,37 @@ +/* + * Teleport + * Copyright (C) 2025 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package awscommon + +import ( + "github.com/gravitational/trace" + "k8s.io/apimachinery/pkg/util/validation" +) + +// ValidIntegratioName validates the integration name. +func ValidIntegratioName(name string) error { + // AWS OIDC and Roles Anywhere Integrations can be used as source of credentials to access AWS Web/CLI. + // For OIDC, this creates a new AppServer whose endpoint is ., which can fail if integrationName is not a valid DNS Label. + // For Roles Anywhere, this creates a AppServers for each Roles Anywhere Profile whose endpoint is -., which can fail if integrationName is not a valid DNS Label. + // Instead of failing when the integration is already created, it fails at creation time. + if errs := validation.IsDNS1035Label(name); len(errs) > 0 { + return trace.BadParameter("integration name %q must be a lower case valid DNS subdomain so that it can be used to allow Web/CLI access", name) + } + + return nil +} diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 01d6e53cebd..27cd2ed025a 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -1073,6 +1073,7 @@ func (h *Handler) bindDefaultEndpoints() { // AWS IAM Roles Anywhere Integration Actions h.GET("/webapi/scripts/integrations/configure/awsra-trust-anchor.sh", h.WithLimiter(h.awsRolesAnywhereConfigureTrustAnchor)) + h.POST("/webapi/sites/:site/integrations/aws-ra/:name/validate", h.WithClusterAuth(h.validateAWSRolesAnywhereIntegration)) h.POST("/webapi/sites/:site/integrations/aws-ra/:name/ping", h.WithClusterAuth(h.awsRolesAnywherePing)) h.POST("/webapi/sites/:site/integrations/aws-ra/:name/listprofiles", h.WithClusterAuth(h.awsRolesAnywhereListProfiles)) diff --git a/lib/web/integrations.go b/lib/web/integrations.go index ecf5da78cd9..d96a0341ca1 100644 --- a/lib/web/integrations.go +++ b/lib/web/integrations.go @@ -149,7 +149,7 @@ func (h *Handler) integrationsCreate(w http.ResponseWriter, r *http.Request, p h func (h *Handler) integrationsUpdate(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, cluster reversetunnelclient.Cluster) (any, error) { integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } var req *ui.UpdateIntegrationRequest @@ -239,7 +239,7 @@ func (h *Handler) integrationsUpdate(w http.ResponseWriter, r *http.Request, p h func (h *Handler) integrationsDelete(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, cluster reversetunnelclient.Cluster) (any, error) { integrationName := p.ByName("name_or_subkind") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } clt, err := sctx.GetUserClient(r.Context(), cluster) @@ -262,7 +262,7 @@ func (h *Handler) integrationsDelete(w http.ResponseWriter, r *http.Request, p h func (h *Handler) integrationsGet(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, cluster reversetunnelclient.Cluster) (any, error) { integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } clt, err := sctx.GetUserClient(r.Context(), cluster) @@ -287,7 +287,7 @@ func (h *Handler) integrationsGet(w http.ResponseWriter, r *http.Request, p http func (h *Handler) integrationStats(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, cluster reversetunnelclient.Cluster) (any, error) { integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } clt, err := sctx.GetUserClient(r.Context(), cluster) @@ -490,7 +490,7 @@ func rulesWithIntegration(dc *discoveryconfig.DiscoveryConfig, matcherType strin func (h *Handler) integrationDiscoveryRules(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, cluster reversetunnelclient.Cluster) (any, error) { integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } values := r.URL.Query() @@ -673,7 +673,7 @@ func (h *Handler) integrationsMsTeamsAppZipGet(w http.ResponseWriter, r *http.Re func (h *Handler) integrationsExportCA(_ http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, cluster reversetunnelclient.Cluster) (any, error) { integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } clt, err := sctx.GetUserClient(r.Context(), cluster) diff --git a/lib/web/integrations_awsoidc.go b/lib/web/integrations_awsoidc.go index 5f07faf66e6..a565bc844d6 100644 --- a/lib/web/integrations_awsoidc.go +++ b/lib/web/integrations_awsoidc.go @@ -71,7 +71,7 @@ func (h *Handler) awsOIDCListDatabases(w http.ResponseWriter, r *http.Request, p integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } clt, err := sctx.GetUserClient(ctx, cluster) @@ -113,7 +113,7 @@ func (h *Handler) awsOIDCDeployService(w http.ResponseWriter, r *http.Request, p integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } clt, err := sctx.GetUserClient(ctx, cluster) @@ -206,7 +206,7 @@ func (h *Handler) awsOIDCDeployDatabaseServices(w http.ResponseWriter, r *http.R integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } clt, err := sctx.GetUserClient(ctx, cluster) @@ -281,7 +281,7 @@ func (h *Handler) awsOIDCListDeployedDatabaseService(w http.ResponseWriter, r *h integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } regions, err := regionsForListingDeployedDatabaseService(ctx, r, clt, clt.DiscoveryConfigClient()) @@ -777,7 +777,7 @@ func (h *Handler) awsOIDCEnrollEKSClusters(w http.ResponseWriter, r *http.Reques integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } versionGetter := &handlerVersionGetter{h} @@ -829,7 +829,7 @@ func (h *Handler) awsOIDCListEKSClusters(w http.ResponseWriter, r *http.Request, integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } clt, err := sctx.GetUserClient(ctx, cluster) @@ -863,7 +863,7 @@ func (h *Handler) awsOIDCListSecurityGroups(w http.ResponseWriter, r *http.Reque integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } clt, err := sctx.GetUserClient(ctx, cluster) @@ -951,7 +951,7 @@ func (h *Handler) awsOIDCRequiredDatabasesVPCS(w http.ResponseWriter, r *http.Re integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } clt, err := sctx.GetUserClient(ctx, cluster) @@ -1069,7 +1069,7 @@ func (h *Handler) awsOIDCCreateAWSAppAccess(w http.ResponseWriter, r *http.Reque integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } clt, err := sctx.GetUserClient(ctx, cluster) @@ -1156,7 +1156,7 @@ func (h *Handler) awsOIDCDeleteAWSAppAccess(w http.ResponseWriter, r *http.Reque integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } clt, err := sctx.GetUserClient(ctx, cluster) @@ -1390,7 +1390,7 @@ func (h *Handler) awsOIDCListSubnets(w http.ResponseWriter, r *http.Request, p h integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } clt, err := sctx.GetUserClient(ctx, cluster) @@ -1436,7 +1436,7 @@ func (h *Handler) awsOIDCListDatabaseVPCs(w http.ResponseWriter, r *http.Request integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } clt, err := sctx.GetUserClient(ctx, cluster) @@ -1558,7 +1558,7 @@ func (h *Handler) awsOIDCPing(w http.ResponseWriter, r *http.Request, p httprout integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } var req ui.AWSOIDCPingRequest diff --git a/lib/web/integrations_awsra.go b/lib/web/integrations_awsra.go index 85752ecdfbe..4365be46460 100644 --- a/lib/web/integrations_awsra.go +++ b/lib/web/integrations_awsra.go @@ -34,6 +34,7 @@ import ( "github.com/gravitational/teleport/api/utils/aws" "github.com/gravitational/teleport/lib/client" "github.com/gravitational/teleport/lib/httplib" + "github.com/gravitational/teleport/lib/integrations/awscommon" "github.com/gravitational/teleport/lib/reversetunnelclient" "github.com/gravitational/teleport/lib/web/scripts/oneoff" "github.com/gravitational/teleport/lib/web/ui" @@ -144,6 +145,40 @@ func (h *Handler) awsRolesAnywhereConfigureTrustAnchor(w http.ResponseWriter, r return nil, trace.Wrap(err) } +// validateAWSRolesAnywhereIntegration performs a validation for the AWS Roles Anywhere Integration name. +// This ensures the integration name is not yet being used and that it is a valid name. +func (h *Handler) validateAWSRolesAnywhereIntegration(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, cluster reversetunnelclient.Cluster) (any, error) { + ctx := r.Context() + + integrationName := p.ByName("name") + if integrationName == "" { + return nil, trace.BadParameter("integration name is required") + } + + // validate integration name. + if err := awscommon.ValidIntegratioName(integrationName); err != nil { + return nil, trace.Wrap(err) + } + + clt, err := sctx.GetUserClient(ctx, cluster) + if err != nil { + return nil, trace.Wrap(err) + } + + _, err = clt.GetIntegration(ctx, integrationName) + switch { + case err == nil: + return nil, trace.AlreadyExists("integration named %q already exists", integrationName) + + case trace.IsNotFound(err): + + default: + return nil, trace.Wrap(err) + } + + return OK(), nil +} + // awsRolesAnywherePing performs an health check for the integration. // It returns the caller identity and the number of AWS Roles Anywhere Profiles that are active. // If a trust anchor is provided in the body, it will be used to check the connection ignoring the integration. @@ -153,7 +188,7 @@ func (h *Handler) awsRolesAnywherePing(w http.ResponseWriter, r *http.Request, p integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } var req ui.AWSRolesAnywherePingRequest @@ -209,7 +244,7 @@ func (h *Handler) awsRolesAnywhereListProfiles(w http.ResponseWriter, r *http.Re integrationName := p.ByName("name") if integrationName == "" { - return nil, trace.BadParameter("an integration name is required") + return nil, trace.BadParameter("integration name is required") } var req ui.AWSRolesAnywhereListProfilesRequest diff --git a/lib/web/integrations_awsra_test.go b/lib/web/integrations_awsra_test.go index 872747aeb8f..c20da94423f 100644 --- a/lib/web/integrations_awsra_test.go +++ b/lib/web/integrations_awsra_test.go @@ -25,6 +25,9 @@ import ( "github.com/gravitational/trace" "github.com/stretchr/testify/require" + + "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/services" ) func TestBuildAWSRATrustAnchorConfigureScript(t *testing.T) { @@ -124,3 +127,55 @@ func TestBuildAWSRATrustAnchorConfigureScript(t *testing.T) { }) } } + +func TestValidateAWSRolesAnywhereIntegration(t *testing.T) { + t.Parallel() + wPack := newWebPack(t, 1 /* proxies */) + proxy := wPack.proxies[0] + authPack := proxy.authPack(t, "user", []types.Role{services.NewPresetEditorRole()}) + ctx := t.Context() + + existingIntegration, err := types.NewIntegrationAWSOIDC(types.Metadata{ + Name: "existing-integration", + }, &types.AWSOIDCIntegrationSpecV1{ + RoleARN: "arn:aws:iam::123456789012:role/valid-role", + }) + require.NoError(t, err) + + _, err = wPack.server.Auth().CreateIntegration(ctx, existingIntegration) + require.NoError(t, err) + + for _, tt := range []struct { + name string + integrationName string + errCheck require.ErrorAssertionFunc + }{ + { + name: "valid", + integrationName: "valid-integration", + errCheck: require.NoError, + }, + { + name: "invalid", + integrationName: "INVALID-", + errCheck: func(tt require.TestingT, err error, i ...interface{}) { + require.Error(tt, err) + require.ErrorContains(tt, err, "must be a lower case valid DNS subdomain") + }, + }, + { + name: "valid name but it already exists", + integrationName: "existing-integration", + errCheck: func(tt require.TestingT, err error, i ...interface{}) { + require.Error(tt, err) + require.ErrorContains(tt, err, "already exists") + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + endpoint := authPack.clt.Endpoint("webapi", "sites", wPack.server.ClusterName(), "integrations", "aws-ra", tt.integrationName, "validate") + _, err := authPack.clt.PostJSON(ctx, endpoint, nil) + tt.errCheck(t, err) + }) + } +}