[v18] Okta: Don't allow creating user_group Access Request in RO

lib/auth/auth.go

@@ -5508,10 +5508,7 @@ func (a *Server) checkResourcesRequestable(ctx context.Context, resourceIDs []ty
 		return nil
 	}
 
-	err := okta.CheckResourcesRequestable(ctx, resourceIDs, okta.AccessPoint{
-		Plugins:              a.Plugins,
-		UnifiedResourceCache: a.UnifiedResourceCache,
-	})
+	err := okta.CheckResourcesRequestable(ctx, resourceIDs, a)
 	if errors.Is(err, okta.OktaResourceNotRequestableError) {
 		return trace.Wrap(err)
 	} else if err != nil {

lib/auth/auth_with_roles_test.go

@@ -4612,12 +4612,12 @@ func TestListResources_KindUserGroup(t *testing.T) {
 	}
 
 	// Add user groups.
-	testUg1 := createUserGroup(t, s, "c", map[string]string{"label": "value"})
-	testUg2 := createUserGroup(t, s, "a", map[string]string{"label": "value"})
-	testUg3 := createUserGroup(t, s, "b", map[string]string{"label": "value"})
+	testUg1 := createUserGroup(t, s.authServer, "c", map[string]string{"label": "value"})
+	testUg2 := createUserGroup(t, s.authServer, "a", map[string]string{"label": "value"})
+	testUg3 := createUserGroup(t, s.authServer, "b", map[string]string{"label": "value"})
 
 	// This user group should never should up because the user doesn't have group label access to it.
-	_ = createUserGroup(t, s, "d", map[string]string{"inaccessible": "value"})
+	_ = createUserGroup(t, s.authServer, "d", map[string]string{"inaccessible": "value"})
 
 	authContext, err = srv.Authorizer.Authorize(authz.ContextWithUser(ctx, TestUser(user.GetName()).I))
 	require.NoError(t, err)
@@ -4702,13 +4702,15 @@ func TestListResources_KindUserGroup(t *testing.T) {
 	})
 }
 
-func createUserGroup(t *testing.T, s *ServerWithRoles, name string, labels map[string]string) types.UserGroup {
+func createUserGroup(t *testing.T, s *Server, name string, labels map[string]string) types.UserGroup {
+	t.Helper()
+	ctx := t.Context()
 	userGroup, err := types.NewUserGroup(types.Metadata{
 		Name:   name,
 		Labels: labels,
 	}, types.UserGroupSpecV1{})
 	require.NoError(t, err)
-	err = s.CreateUserGroup(context.Background(), userGroup)
+	err = s.CreateUserGroup(ctx, userGroup)
 	require.NoError(t, err)
 	return userGroup
 }
@@ -5435,12 +5437,23 @@ func TestCreateAccessRequestV2_oktaReadOnly(t *testing.T) {
 	ctx := context.Background()
 	srv := newTestTLSServer(t)
 
-	// 1. Create Okta-originated app server in the backend.
+	// 1. Create Okta-originated app_server and user_group in the backend.
 
-	searchableOktaApp := newTestAppServerV3(t, srv.Auth(), "serachable-okta-app", map[string]string{
-		"name":            "serachable-okta-app",
-		types.OriginLabel: types.OriginOkta,
-	})
+	searchableOktaApp := createTestAppServerV3(t, srv.Auth(),
+		"serachable-okta-app",
+		map[string]string{
+			"name":            "serachable-okta-app",
+			types.OriginLabel: types.OriginOkta,
+		},
+	)
+
+	searchableUserGroup := createUserGroup(t, srv.Auth(),
+		"serachable-okta-group",
+		map[string]string{
+			"name":            "serachable-okta-group",
+			types.OriginLabel: types.OriginOkta,
+		},
+	)
 
 	// 2. Create a role allowing the Okta app (used for search_as_roles)
 
@@ -5449,6 +5462,9 @@ func TestCreateAccessRequestV2_oktaReadOnly(t *testing.T) {
 			AppLabels: types.Labels{
 				"name": {searchableOktaApp.GetName()},
 			},
+			GroupLabels: types.Labels{
+				"name": {searchableUserGroup.GetName()},
+			},
 		},
 	})
 	require.NoError(t, err)
@@ -5493,6 +5509,13 @@ func TestCreateAccessRequestV2_oktaReadOnly(t *testing.T) {
 				mustResourceID(srv.ClusterName(), types.KindAppServer, searchableOktaApp.GetName()),
 			},
 		),
+		// requesting user_group
+		mustAccessRequest(t, alice.GetName(), types.RequestState_PENDING, srv.Clock().Now(), srv.Clock().Now().Add(time.Hour),
+			[]string{}, // roles
+			[]types.ResourceID{
+				mustResourceID(srv.ClusterName(), types.KindUserGroup, searchableUserGroup.GetName()),
+			},
+		),
 	}
 
 	// 7. Run tests
@@ -5503,10 +5526,11 @@ func TestCreateAccessRequestV2_oktaReadOnly(t *testing.T) {
 		// heartbeats for the Okta apps haven't expired yet. This is an edge-case so the
 		// error is a bit confusing.
 		for _, accessRequest := range testAccessRequests {
+			msg := fmt.Sprintf("requested resources = %v", accessRequest.GetRequestedResourceIDs())
 			_, err := aliceClt.CreateAccessRequestV2(ctx, accessRequest)
-			require.Error(t, err)
-			require.True(t, trace.IsBadParameter(err))
-			require.ErrorContains(t, err, okta.OktaResourceNotRequestableError.Error())
+			require.Error(t, err, msg)
+			require.True(t, trace.IsBadParameter(err), msg)
+			require.ErrorContains(t, err, okta.OktaResourceNotRequestableError.Error(), msg)
 		}
 	})
 
@@ -5524,8 +5548,9 @@ func TestCreateAccessRequestV2_oktaReadOnly(t *testing.T) {
 		)
 
 		for _, accessRequest := range testAccessRequests {
+			msg := fmt.Sprintf("requested resources = %v", accessRequest.GetRequestedResourceIDs())
 			_, err := aliceClt.CreateAccessRequestV2(ctx, accessRequest)
-			require.NoError(t, err)
+			require.NoError(t, err, msg)
 		}
 	})
 
@@ -5543,10 +5568,11 @@ func TestCreateAccessRequestV2_oktaReadOnly(t *testing.T) {
 		)
 
 		for _, accessRequest := range testAccessRequests {
+			msg := fmt.Sprintf("requested resources = %v", accessRequest.GetRequestedResourceIDs())
 			_, err := aliceClt.CreateAccessRequestV2(ctx, accessRequest)
-			require.Error(t, err)
-			require.True(t, trace.IsBadParameter(err))
-			require.ErrorContains(t, err, okta.OktaResourceNotRequestableError.Error())
+			require.Error(t, err, msg)
+			require.True(t, trace.IsBadParameter(err), msg)
+			require.ErrorContains(t, err, okta.OktaResourceNotRequestableError.Error(), msg)
 		}
 	})
 }
@@ -5558,22 +5584,22 @@ func TestListUnifiedResources_search_as_roles_oktaReadOnly(t *testing.T) {
 
 	// 1. Create app resources
 
-	searchableGenericApp := newTestAppServerV3(t, srv.Auth(),
+	searchableGenericApp := createTestAppServerV3(t, srv.Auth(),
 		"test_generic_app",
 		map[string]string{
 			"find_me": "please",
 		},
 	)
 
-	searchableOktaApp := newTestAppServerV3(t, srv.Auth(),
+	searchableOktaApp := createTestAppServerV3(t, srv.Auth(),
 		"test_searchable_okta_app",
 		map[string]string{
 			"find_me":         "please",
 			types.OriginLabel: types.OriginOkta,
 		},
 	)
 
-	assignedOktaApp := newTestAppServerV3(t, srv.Auth(),
+	assignedOktaApp := createTestAppServerV3(t, srv.Auth(),
 		"test_assigned_okta_app",
 		map[string]string{
 			"owner":           "alice",
@@ -10266,9 +10292,9 @@ func TestValidateOracleJoinToken(t *testing.T) {
 	})
 }
 
-func newTestAppServerV3(t *testing.T, auth *Server, name string, labels map[string]string) *types.AppServerV3 {
+func createTestAppServerV3(t *testing.T, auth *Server, name string, labels map[string]string) *types.AppServerV3 {
 	t.Helper()
-	ctx := context.Background()
+	ctx := t.Context()
 
 	app, err := types.NewAppV3(
 		types.Metadata{

lib/auth/okta/auth.go

@@ -20,13 +20,14 @@ package okta
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/gravitational/trace"
 
+	"github.com/gravitational/teleport/api/client/proto"
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/lib/authz"
 	oktaplugin "github.com/gravitational/teleport/lib/okta/plugin"
-	"github.com/gravitational/teleport/lib/services"
 )
 
 // Okta-origin resources have some special access rules that are implemented in
@@ -99,7 +100,7 @@ func CheckAccess(authzCtx *authz.Context, existingResource types.ResourceWithLab
 
 // BidirectionalSyncEnabled checks if the bidirectional sync is enabled on the Okta plugin.  If the
 // Okta plugin does not exist the result is false.
-func BidirectionalSyncEnabled(ctx context.Context, plugins services.Plugins) (bool, error) {
+func BidirectionalSyncEnabled(ctx context.Context, plugins PluginGetter) (bool, error) {
 	plugin, err := oktaplugin.Get(ctx, plugins, false /* withSecrets */)
 	if trace.IsNotFound(err) {
 		return false, nil
@@ -109,12 +110,6 @@ func BidirectionalSyncEnabled(ctx context.Context, plugins services.Plugins) (bo
 	return plugin.Spec.GetOkta().GetSyncSettings().GetEnableBidirectionalSync(), nil
 }
 
-// AccessPoint provides services required by [CheckResourcesRequestable].
-type AccessPoint struct {
-	Plugins              services.Plugins
-	UnifiedResourceCache *services.UnifiedResourceCache
-}
-
 var (
 	// OktaResourceNotRequestableError means resources cannot be requested because Okta
 	// bidirectional sync is disabled.
@@ -130,8 +125,12 @@ var (
 //
 // If any resource is not requestable, [OktaResourceNotRequestableError] will be returned. Any
 // other error can be returned.
-func CheckResourcesRequestable(ctx context.Context, ids []types.ResourceID, ap AccessPoint) error {
-	bidirectionalSyncEnabled, err := BidirectionalSyncEnabled(ctx, ap.Plugins)
+func CheckResourcesRequestable(ctx context.Context, ids []types.ResourceID, auth AuthServer) error {
+	if len(ids) == 0 {
+		return nil
+	}
+
+	bidirectionalSyncEnabled, err := BidirectionalSyncEnabled(ctx, auth)
 	if err != nil {
 		return trace.Wrap(err, "getting bidirectional sync")
 	}
@@ -140,19 +139,30 @@ func CheckResourcesRequestable(ctx context.Context, ids []types.ResourceID, ap A
 	}
 
 	var denied []types.ResourceID
-	for app, err := range ap.UnifiedResourceCache.AppServers(ctx, services.UnifiedResourcesIterateParams{}) {
-		if err != nil {
-			return trace.Wrap(err, "iterating cached AppServers")
-		}
-		for _, id := range ids {
-			switch id.Kind {
-			case types.KindApp, types.KindAppServer:
+
+	for _, id := range ids {
+		switch id.Kind {
+		case types.KindAppServer, types.KindApp:
+			_, err := getOktaAppServer(ctx, auth, id.Name)
+			switch {
+			case trace.IsNotFound(err):
 				// ok
+			case err != nil:
+				return trace.Wrap(err, "getting app_server %q", id.Name)
 			default:
+				denied = append(denied, id)
 				continue
 			}
-			if app.GetName() == id.Name && app.Origin() == types.OriginOkta {
+		case types.KindUserGroup:
+			userGroup, err := auth.GetUserGroup(ctx, id.Name)
+			switch {
+			case trace.IsNotFound(err):
+				// ok
+			case err != nil:
+				return trace.Wrap(err, "getting user_group %q", id.Name)
+			case userGroup.Origin() == types.OriginOkta:
 				denied = append(denied, id)
+				continue
 			}
 		}
 	}
@@ -168,3 +178,29 @@ func CheckResourcesRequestable(ctx context.Context, ids []types.ResourceID, ap A
 
 	return trace.Wrap(OktaResourceNotRequestableError, "requested Okta resources: %s", resourcesStr)
 }
+
+func getOktaAppServer(ctx context.Context, auth AuthServer, name string) (types.AppServer, error) {
+	req := proto.ListResourcesRequest{
+		ResourceType: types.KindAppServer,
+		Limit:        1,
+		Labels: map[string]string{
+			types.OriginLabel: types.OriginOkta,
+		},
+		PredicateExpression: fmt.Sprintf(`name == %q`, name),
+	}
+
+	resp, err := auth.ListResources(ctx, req)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	if len(resp.Resources) != 1 {
+		return nil, trace.NotFound("found %d resources when getting Okta-originated app_server %q", len(resp.Resources), name)
+	}
+
+	appServer, ok := resp.Resources[0].(types.AppServer)
+	if !ok {
+		return nil, trace.BadParameter("expected %T, found %T", appServer, resp.Resources[0])
+	}
+	return appServer, nil
+}

lib/auth/okta/interfaces.go

@@ -0,0 +1,38 @@
+/*
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+package okta
+
+import (
+	"context"
+
+	"github.com/gravitational/teleport/api/client/proto"
+	"github.com/gravitational/teleport/api/types"
+)
+
+type PluginGetter interface {
+	GetPlugin(ctx context.Context, name string, withSecrets bool) (types.Plugin, error)
+}
+
+type AuthServer interface {
+	PluginGetter
+	// ListResources returns paginated resources depending on the resource type.
+	ListResources(ctx context.Context, req proto.ListResourcesRequest) (*types.ListResourcesResponse, error)
+	// GetUserGroup returns the specified user group resources.
+	GetUserGroup(ctx context.Context, name string) (types.UserGroup, error)
+}

lib/okta/plugin/interfaces.go

@@ -0,0 +1,29 @@
+/*
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+package oktaplugin
+
+import (
+	"context"
+
+	"github.com/gravitational/teleport/api/types"
+)
+
+type PluginGetter interface {
+	GetPlugin(ctx context.Context, name string, withSecrets bool) (types.Plugin, error)
+}

lib/okta/plugin/okta_plugin.go

@@ -24,11 +24,10 @@ import (
 	"github.com/gravitational/trace"
 
 	"github.com/gravitational/teleport/api/types"
-	"github.com/gravitational/teleport/lib/services"
 )
 
 // Get fetches the Okta plugin if it exists and does proper type assertions.
-func Get(ctx context.Context, plugins services.Plugins, withSecrets bool) (*types.PluginV1, error) {
+func Get(ctx context.Context, plugins PluginGetter, withSecrets bool) (*types.PluginV1, error) {
 	plugin, err := plugins.GetPlugin(ctx, types.PluginTypeOkta, withSecrets)
 	if err != nil {
 		return nil, trace.Wrap(err, "getting Okta plugin")