From 257d005ca32c7a1062a21b6d1089423aa2a3399b Mon Sep 17 00:00:00 2001
From: Edoardo Spadolini <edoardo.spadolini@goteleport.com>
Date: Thu, 17 Mar 2022 17:18:16 +0100
Subject: [PATCH] Revert "Only allow access request deletion through static
 roles' permissions (#9540)" (#11220)

This reverts commit 8db6aa58833ca5af93f0cdcae56b4dd862c81dfe.
---
 lib/auth/auth_with_roles.go      | 29 +----------
 lib/auth/auth_with_roles_test.go | 88 --------------------------------
 2 files changed, 1 insertion(+), 116 deletions(-)

diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go
index f677c1b771f73..c3bdb11ab38a8 100644
--- a/lib/auth/auth_with_roles.go
+++ b/lib/auth/auth_with_roles.go
@@ -95,23 +95,6 @@ func (a *ServerWithRoles) withOptions(opts ...actionOption) actionConfig {
 	return cfg
 }
 
-func (a *ServerWithRoles) withStaticRoles() (actionConfig, error) {
-	user, err := a.authServer.GetUser(a.context.User.GetName(), false)
-	if err != nil {
-		return actionConfig{}, trace.Wrap(err)
-	}
-
-	checker, err := services.FetchRoles(user.GetRoles(), a.authServer, user.GetTraits())
-	if err != nil {
-		return actionConfig{}, trace.Wrap(err)
-	}
-
-	return actionConfig{context: Context{
-		User:    user,
-		Checker: checker,
-	}}, nil
-}
-
 func (c actionConfig) action(namespace, resource string, verbs ...string) error {
 	if len(verbs) == 0 {
 		return trace.BadParameter("no verbs provided for authorization check on resource %q", resource)
@@ -1665,17 +1648,7 @@ func (a *ServerWithRoles) getProxyPublicAddr() string {
 }
 
 func (a *ServerWithRoles) DeleteAccessRequest(ctx context.Context, name string) error {
-	cfg, err := a.withStaticRoles()
-	if err != nil {
-		return err
-	}
-	if err := cfg.action(apidefaults.Namespace, types.KindAccessRequest, types.VerbDelete); err != nil {
-		if trace.IsAccessDenied(err) {
-			if a.withOptions(quietAction(true)).action(apidefaults.Namespace, types.KindAccessRequest, types.VerbDelete) == nil {
-				// the user would've had permission with the roles granted by access requests
-				return trace.WrapWithMessage(err, "access request deletion through elevated roles is not allowed")
-			}
-		}
+	if err := a.action(apidefaults.Namespace, types.KindAccessRequest, types.VerbDelete); err != nil {
 		return trace.Wrap(err)
 	}
 	return a.authServer.DeleteAccessRequest(ctx, name)
diff --git a/lib/auth/auth_with_roles_test.go b/lib/auth/auth_with_roles_test.go
index 5855cf8dab570..ae6359152c9d1 100644
--- a/lib/auth/auth_with_roles_test.go
+++ b/lib/auth/auth_with_roles_test.go
@@ -1909,94 +1909,6 @@ func TestKindClusterConfig(t *testing.T) {
 	})
 }
 
-func TestNoElevatedAccessRequestDeletion(t *testing.T) {
-	t.Parallel()
-	ctx := context.Background()
-
-	srv, err := NewTestAuthServer(TestAuthServerConfig{Dir: t.TempDir()})
-	require.NoError(t, err)
-	t.Cleanup(func() { srv.Close() })
-
-	deleterRole, err := types.NewRoleV3("deleter", types.RoleSpecV5{
-		Allow: types.RoleConditions{
-			Rules: []types.Rule{{
-				Resources: []string{"access_request"},
-				Verbs:     []string{"delete"},
-			}},
-		},
-	})
-	require.NoError(t, err)
-	deleterUser, err := CreateUser(srv.AuthServer, "deletey", deleterRole)
-	require.NoError(t, err)
-
-	requesterRole, err := types.NewRoleV3("requester", types.RoleSpecV5{
-		Allow: types.RoleConditions{
-			Request: &types.AccessRequestConditions{
-				Roles: []string{deleterRole.GetName()},
-			},
-		},
-	})
-	require.NoError(t, err)
-	requesterUser, err := CreateUser(srv.AuthServer, "requesty", requesterRole)
-	require.NoError(t, err)
-
-	request, err := services.NewAccessRequest(requesterUser.GetName(), deleterRole.GetName())
-	require.NoError(t, err)
-	// the request must be for an allowed user/role combination or it will get rejected
-	err = srv.AuthServer.CreateAccessRequest(ctx, request)
-	require.NoError(t, err)
-
-	// requesty has used some other unspecified access request to get the
-	// deleter role in this identity
-	requesterAuthContext, err := srv.Authorizer.Authorize(context.WithValue(ctx,
-		ContextUser,
-		LocalUser{
-			Username: requesterUser.GetName(),
-			Identity: tlsca.Identity{
-				Username: requesterUser.GetName(),
-				Groups:   []string{requesterRole.GetName(), deleterRole.GetName()},
-				// a tlsca.Identity must have a nonempty Traits field or the
-				// roles will be reloaded from the backend during Authorize
-				Traits: map[string][]string{"nonempty": {}},
-			},
-		},
-	))
-	require.NoError(t, err)
-	requesterAuth := &ServerWithRoles{
-		authServer: srv.AuthServer,
-		sessions:   srv.SessionServer,
-		alog:       srv.AuditLog,
-		context:    *requesterAuthContext,
-	}
-
-	err = requesterAuth.DeleteAccessRequest(ctx, request.GetName())
-	require.True(t, trace.IsAccessDenied(err))
-	// matches the message in lib/auth/auth_with_roles.go:(*ServerWithRoles).DeleteAccessRequest()
-	require.Contains(t, err.Error(), "deletion through elevated roles")
-
-	deleterAuthContext, err := srv.Authorizer.Authorize(context.WithValue(ctx,
-		ContextUser,
-		LocalUser{
-			Username: deleterUser.GetName(),
-			Identity: tlsca.Identity{
-				Username: deleterUser.GetName(),
-				Groups:   []string{deleterRole.GetName()},
-				Traits:   map[string][]string{"nonempty": {}},
-			},
-		},
-	))
-	require.NoError(t, err)
-	deleterAuth := &ServerWithRoles{
-		authServer: srv.AuthServer,
-		sessions:   srv.SessionServer,
-		alog:       srv.AuditLog,
-		context:    *deleterAuthContext,
-	}
-
-	err = deleterAuth.DeleteAccessRequest(ctx, request.GetName())
-	require.NoError(t, err)
-}
-
 func TestGetAndList_KubeServices(t *testing.T) {
 	t.Parallel()
 	ctx := context.Background()