diff --git a/lib/componentfeatures/utils.go b/lib/componentfeatures/utils.go index 8e20bd0e05b..6ba518dbe97 100644 --- a/lib/componentfeatures/utils.go +++ b/lib/componentfeatures/utils.go @@ -21,6 +21,9 @@ package componentfeatures import ( "context" "log/slog" + "slices" + + "github.com/coreos/go-semver/semver" componentfeaturesv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/componentfeatures/v1" "github.com/gravitational/teleport/api/types" @@ -172,7 +175,7 @@ func GetClusterAuthProxyServerFeatures(ctx context.Context, clt AuthProxyServers features = append(features, nil) } else { for _, srv := range allProxies { - features = append(features, srv.GetComponentFeatures()) + features = append(features, GetEffectiveServerFeatures(srv)) } } @@ -189,9 +192,43 @@ func GetClusterAuthProxyServerFeatures(ctx context.Context, clt AuthProxyServers features = append(features, nil) } else { for _, srv := range allAuthServers { - features = append(features, srv.GetComponentFeatures()) + features = append(features, GetEffectiveServerFeatures(srv)) } } return Intersect(features...) } + +// versionedComponent is implemented by any server type that advertises a +// version and a set of component features. +// +// TODO(kiosion): DELETE in 20.0.0 +type versionedComponent interface { + GetTeleportVersion() string + GetComponentFeatures() *componentfeaturesv1.ComponentFeatures +} + +// GetEffectiveServerFeatures computes a server's effective feature support. +// Servers between v18.6.4 and v18.7.5 advertise FeatureResourceConstraintsV1 +// but predate the constraint parsing implementation; for those versions, the +// advertisement should be stripped prior to intersection so clients don't show +// constraints UI flows. +// +// TODO(kiosion): DELETE in 20.0.0 +func GetEffectiveServerFeatures(component versionedComponent) *componentfeaturesv1.ComponentFeatures { + f := component.GetComponentFeatures() + if f == nil { + return f + } + ver, err := semver.NewVersion(component.GetTeleportVersion()) + if err != nil { + return f + } + if ver.LessThan(semver.Version{Major: 18, Minor: 7, Patch: 6}) { + features := slices.DeleteFunc(slices.Clone(f.GetFeatures()), func(id componentfeaturesv1.ComponentFeatureID) bool { + return id == FeatureResourceConstraintsV1.ToProto() + }) + return &componentfeaturesv1.ComponentFeatures{Features: features} + } + return f +} diff --git a/lib/componentfeatures/utils_test.go b/lib/componentfeatures/utils_test.go index 38c69bc5551..85e4483d64e 100644 --- a/lib/componentfeatures/utils_test.go +++ b/lib/componentfeatures/utils_test.go @@ -335,6 +335,37 @@ func TestGetClusterAuthProxyServerFeatures(t *testing.T) { } } +func TestGetEffectiveServerFeatures(t *testing.T) { + t.Parallel() + rcv1 := FeatureResourceConstraintsV1 + otherFeature := FeatureID(9999) + makeAppServer := func(t *testing.T, version string, features *componentfeaturesv1.ComponentFeatures) types.AppServer { + app, err := types.NewAppV3(types.Metadata{Name: "aws-app"}, types.AppSpecV3{URI: "https://console.aws.amazon.com", Cloud: "AWS"}) + require.NoError(t, err) + srv, err := types.NewAppServerV3FromApp(app, "localhost", "host-1") + require.NoError(t, err) + srv.Spec.Version = version + srv.SetComponentFeatures(features) + return srv + } + + t.Run("old app server has ResourceConstraintsV1 stripped", func(t *testing.T) { + srv := makeAppServer(t, "18.7.5", New(rcv1, otherFeature)) + result := GetEffectiveServerFeatures(srv) + require.ElementsMatch(t, New(otherFeature).GetFeatures(), result.GetFeatures()) + }) + t.Run("new app server keeps ResourceConstraintsV1", func(t *testing.T) { + srv := makeAppServer(t, "18.7.6", New(rcv1, otherFeature)) + result := GetEffectiveServerFeatures(srv) + require.ElementsMatch(t, New(rcv1, otherFeature).GetFeatures(), result.GetFeatures()) + }) + t.Run("nil features on old app server returns empty", func(t *testing.T) { + srv := makeAppServer(t, "18.6.4", nil) + result := GetEffectiveServerFeatures(srv) + require.Empty(t, result.GetFeatures()) + }) +} + type fakeAuthProxyServersLister struct { proxies []types.Server auths []types.Server diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 14efcff47dd..a2af66c045f 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -44,7 +44,6 @@ import ( "time" template "github.com/DataDog/datadog-agent/pkg/template/html" - "github.com/coreos/go-semver/semver" gogoproto "github.com/gogo/protobuf/proto" "github.com/google/safetext/shsprintf" "github.com/google/uuid" @@ -66,7 +65,6 @@ import ( "github.com/gravitational/teleport/api/client/webclient" "github.com/gravitational/teleport/api/constants" apidefaults "github.com/gravitational/teleport/api/defaults" - componentfeaturesv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/componentfeatures/v1" linuxdesktopv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/linuxdesktop/v1" mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" notificationsv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/notifications/v1" @@ -3567,7 +3565,7 @@ func (h *Handler) clusterUnifiedResourcesGet(w http.ResponseWriter, request *htt // Compute end-to-end feature support for this app: only features that are supported by the AppServer *and* // by all required cluster hops (Auth + Proxy), so clients can hide features that would fail somewhere // along the request path. - appComponentFeatures := appServerEffectiveFeatures(r, clusterAuthProxyServerFeatures) + appComponentFeatures := componentfeatures.Intersect(componentfeatures.GetEffectiveServerFeatures(r), clusterAuthProxyServerFeatures) app := ui.MakeApp(r.GetApp(), ui.MakeAppsConfig{ LocalClusterName: h.auth.clusterName, @@ -3617,26 +3615,6 @@ func (h *Handler) clusterUnifiedResourcesGet(w http.ResponseWriter, request *htt return resp, nil } -// appServerEffectiveFeatures computes the effective feature set for an app server, -// intersected with cluster-wide auth/proxy features. App servers between v18.6.4 and -// v18.7.2 advertise FeatureResourceConstraintsV1 but predate the constraint enforcement -// code; for those versions, the advertisement is stripped before intersection so the -// frontend doesn't show the constraints UI. -// -// TODO(kiosion): DELETE in 20.0.0 -func appServerEffectiveFeatures(appServer types.AppServer, clusterFeatures *componentfeaturesv1.ComponentFeatures) *componentfeaturesv1.ComponentFeatures { - appFeatures := appServer.GetComponentFeatures() - minVer, minVerErr := semver.NewVersion("18.7.3") - ver, verErr := semver.NewVersion(appServer.GetTeleportVersion()) - if verErr == nil && minVerErr == nil && ver.LessThan(*minVer) && appFeatures != nil { - features := slices.DeleteFunc(slices.Clone(appFeatures.GetFeatures()), func(f componentfeaturesv1.ComponentFeatureID) bool { - return f == componentfeatures.FeatureResourceConstraintsV1.ToProto() - }) - appFeatures = &componentfeaturesv1.ComponentFeatures{Features: features} - } - return componentfeatures.Intersect(appFeatures, clusterFeatures) -} - // clusterNodesGet returns a list of nodes for a given cluster site. func (h *Handler) clusterNodesGet(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, cluster reversetunnelclient.Cluster) (any, error) { // Get a client to the Auth Server with the logged in user's identity. The diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index ed0c8dfa951..b6d907f3ac6 100644 --- a/lib/web/apiserver_test.go +++ b/lib/web/apiserver_test.go @@ -1467,38 +1467,6 @@ func TestUnifiedResourcesGet_AppComponentFeatures(t *testing.T) { }, app.SupportedFeatureIDs) } -func Test_appServerEffectiveFeatures(t *testing.T) { - t.Parallel() - rcv1 := componentfeatures.FeatureResourceConstraintsV1 - otherFeature := componentfeatures.FeatureID(9999) - clusterFeatures := componentfeatures.New(rcv1, otherFeature) - makeAppServer := func(t *testing.T, version string, features *componentfeaturesv1.ComponentFeatures) types.AppServer { - app, err := types.NewAppV3(types.Metadata{Name: "aws-app"}, types.AppSpecV3{URI: "https://console.aws.amazon.com", Cloud: "AWS"}) - require.NoError(t, err) - srv, err := types.NewAppServerV3FromApp(app, "localhost", "host-1") - require.NoError(t, err) - srv.Spec.Version = version - srv.SetComponentFeatures(features) - return srv - } - - t.Run("old app server has ResourceConstraintsV1 stripped", func(t *testing.T) { - srv := makeAppServer(t, "18.7.0", componentfeatures.New(rcv1, otherFeature)) - result := appServerEffectiveFeatures(srv, clusterFeatures) - require.ElementsMatch(t, componentfeatures.New(otherFeature).GetFeatures(), result.GetFeatures()) - }) - t.Run("new app server keeps ResourceConstraintsV1", func(t *testing.T) { - srv := makeAppServer(t, "18.7.3", componentfeatures.New(rcv1, otherFeature)) - result := appServerEffectiveFeatures(srv, clusterFeatures) - require.ElementsMatch(t, componentfeatures.New(rcv1, otherFeature).GetFeatures(), result.GetFeatures()) - }) - t.Run("nil features on old app server returns empty", func(t *testing.T) { - srv := makeAppServer(t, "18.6.4", nil) - result := appServerEffectiveFeatures(srv, clusterFeatures) - require.Empty(t, result.GetFeatures()) - }) -} - func TestUnifiedResourcesGet(t *testing.T) { t.Parallel() env := newWebPack(t, 1) diff --git a/lib/web/ui/app.go b/lib/web/ui/app.go index 53d6a7c3ed5..6d36499bc2d 100644 --- a/lib/web/ui/app.go +++ b/lib/web/ui/app.go @@ -26,6 +26,7 @@ import ( componentfeaturesv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/componentfeatures/v1" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/componentfeatures" "github.com/gravitational/teleport/lib/ui" "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/lib/utils/aws" @@ -201,7 +202,12 @@ func MakeApp(app types.Application, c MakeAppsConfig) App { } if app.IsAWSConsole() && c.AWSRoles != nil { - visibleRoles := aws.FilterAWSRoles(c.AWSRoles.All.Elements(), app.GetAWSAccountID()) + var visibleRoles []aws.Role + if componentfeatures.InAllSets(componentfeatures.FeatureResourceConstraintsV1, c.SupportedFeatures) { + visibleRoles = aws.FilterAWSRoles(c.AWSRoles.All.Elements(), app.GetAWSAccountID()) + } else { + visibleRoles = aws.FilterAWSRoles(c.AWSRoles.Granted.Elements(), app.GetAWSAccountID()) + } resultApp.AWSRoles = slices.Map(visibleRoles, func(r aws.Role) aws.Role { r.RequiresRequest = !c.AWSRoles.Granted.Contains(r.ARN) return r diff --git a/lib/web/ui/app_test.go b/lib/web/ui/app_test.go index 58f6fa9858a..fd9654e2ab0 100644 --- a/lib/web/ui/app_test.go +++ b/lib/web/ui/app_test.go @@ -28,6 +28,7 @@ import ( "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/componentfeatures" "github.com/gravitational/teleport/lib/ui" + "github.com/gravitational/teleport/lib/utils/set" ) func newApp(t *testing.T, name, publicAddr, description string, labels map[string]string) types.Application { @@ -84,6 +85,54 @@ func TestMakeApp_SupportedFeatureIDs(t *testing.T) { }) } +func TestMakeApp_AWSRolesVisibility(t *testing.T) { + t.Parallel() + + app, err := types.NewAppV3( + types.Metadata{ + Name: "aws-console", + Labels: map[string]string{"aws_account_id": "123456789012"}, + }, + types.AppSpecV3{ + URI: "https://console.aws.amazon.com", + Cloud: types.CloudAWS, + }, + ) + require.NoError(t, err) + + grantedARN := "arn:aws:iam::123456789012:role/granted" + requestableARN := "arn:aws:iam::123456789012:role/requestable" + baseCfg := MakeAppsConfig{ + LocalClusterName: "root", + LocalProxyDNSName: "proxy.example.com", + AppClusterName: "root", + UserGroupLookup: map[string]types.UserGroup{}, + AWSRoles: &PrincipalSet{ + All: set.New(grantedARN, requestableARN), + Granted: set.New(grantedARN), + }, + } + + t.Run("with constraint support returns granted and requestable", func(t *testing.T) { + t.Parallel() + cfg := baseCfg + cfg.SupportedFeatures = componentfeatures.New(componentfeatures.FeatureResourceConstraintsV1) + + out := MakeApp(app, cfg) + require.Len(t, out.AWSRoles, 2) + }) + t.Run("without constraint support returns only granted", func(t *testing.T) { + t.Parallel() + cfg := baseCfg + cfg.SupportedFeatures = nil + + out := MakeApp(app, cfg) + require.Len(t, out.AWSRoles, 1) + require.Equal(t, grantedARN, out.AWSRoles[0].ARN) + require.False(t, out.AWSRoles[0].RequiresRequest) + }) +} + func TestMakeAppTypeFromSAMLApp(t *testing.T) { tests := []struct { name string diff --git a/web/packages/teleport/src/UnifiedResources/ResourceActionButton.tsx b/web/packages/teleport/src/UnifiedResources/ResourceActionButton.tsx index ba6234e9149..bafe3a111ca 100644 --- a/web/packages/teleport/src/UnifiedResources/ResourceActionButton.tsx +++ b/web/packages/teleport/src/UnifiedResources/ResourceActionButton.tsx @@ -182,7 +182,9 @@ const AppLaunch = ({ app }: AppLaunchProps) => { const isAwsIdentityCenterApp = subKind === AppSubKind.AwsIcAccount; if (awsConsole || isAwsIdentityCenterApp) { - let awsConsoleOrIdentityCenterRoles: AwsRole[] = awsRoles; + let awsConsoleOrIdentityCenterRoles: AwsRole[] = awsRoles.filter( + ps => !ps.requiresRequest + ); if (isAwsIdentityCenterApp) { awsConsoleOrIdentityCenterRoles = permissionSets.map( (ps): AwsRole => ({