diff --git a/lib/auth/auth.go b/lib/auth/auth.go index bc0d7002cc3ee..0b6690b56106e 100644 --- a/lib/auth/auth.go +++ b/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 { diff --git a/lib/auth/auth_with_roles_test.go b/lib/auth/auth_with_roles_test.go index 94a877551e2fb..fbbcb0dbb3493 100644 --- a/lib/auth/auth_with_roles_test.go +++ b/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,14 +5584,14 @@ 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", @@ -5573,7 +5599,7 @@ func TestListUnifiedResources_search_as_roles_oktaReadOnly(t *testing.T) { }, ) - 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{ diff --git a/lib/auth/okta/auth.go b/lib/auth/okta/auth.go index 558a992686662..c58a8f459d685 100644 --- a/lib/auth/okta/auth.go +++ b/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 +} diff --git a/lib/auth/okta/interfaces.go b/lib/auth/okta/interfaces.go new file mode 100644 index 0000000000000..421394253c39c --- /dev/null +++ b/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 . + */ + +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) +} diff --git a/lib/okta/plugin/interfaces.go b/lib/okta/plugin/interfaces.go new file mode 100644 index 0000000000000..2f8d7509ac50d --- /dev/null +++ b/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 . + */ + +package oktaplugin + +import ( + "context" + + "github.com/gravitational/teleport/api/types" +) + +type PluginGetter interface { + GetPlugin(ctx context.Context, name string, withSecrets bool) (types.Plugin, error) +} diff --git a/lib/okta/plugin/okta_plugin.go b/lib/okta/plugin/okta_plugin.go index 53d7d7f727012..084878e78d073 100644 --- a/lib/okta/plugin/okta_plugin.go +++ b/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")