always generate request IDs server-side

api/client/client.go

@@ -1040,16 +1040,6 @@ func (c *Client) GetAccessRequests(ctx context.Context, filter types.AccessReque
 	return reqs, nil
 }
 
-// CreateAccessRequest registers a new access request with the auth server.
-func (c *Client) CreateAccessRequest(ctx context.Context, req types.AccessRequest) error {
-	r, ok := req.(*types.AccessRequestV3)
-	if !ok {
-		return trace.BadParameter("unexpected access request type %T", req)
-	}
-	_, err := c.grpc.CreateAccessRequest(ctx, r)
-	return trace.Wrap(err)
-}
-
 // CreateAccessRequestV2 registers a new access request with the auth server.
 func (c *Client) CreateAccessRequestV2(ctx context.Context, req types.AccessRequest) (types.AccessRequest, error) {
 	r, ok := req.(*types.AccessRequestV3)

integration/integration_test.go

@@ -6764,11 +6764,11 @@ func testSessionStartContainsAccessRequest(t *testing.T, suite *integrationTestS
 	req, err := services.NewAccessRequest(suite.Me.Username, requestedRole.GetMetadata().Name)
 	require.NoError(t, err)
 
-	accessRequestID := req.GetName()
-
-	err = authServer.CreateAccessRequest(ctx, req, tlsca.Identity{})
+	req, err = authServer.CreateAccessRequestV2(ctx, req, tlsca.Identity{})
 	require.NoError(t, err)
 
+	accessRequestID := req.GetName()
+
 	err = authServer.SetAccessRequestState(ctx, types.AccessRequestUpdate{
 		RequestID: accessRequestID,
 		State:     types.RequestState_APPROVED,

integrations/access/discord/discord_test.go

@@ -235,9 +235,9 @@ func (s *DiscordSuite) createAccessRequest() types.AccessRequest {
 	t.Helper()
 
 	req := s.newAccessRequest()
-	err := s.requestor().CreateAccessRequest(s.Context(), req)
+	out, err := s.requestor().CreateAccessRequestV2(s.Context(), req)
 	require.NoError(t, err)
-	return req
+	return out
 }
 
 func (s *DiscordSuite) checkPluginData(reqID string, cond func(common.GenericPluginData) bool) common.GenericPluginData {
@@ -626,7 +626,7 @@ func (s *DiscordSuite) TestRace() {
 			if err != nil {
 				return setRaceErr(trace.Wrap(err))
 			}
-			if err := s.requestor().CreateAccessRequest(ctx, req); err != nil {
+			if _, err := s.requestor().CreateAccessRequestV2(ctx, req); err != nil {
 				return setRaceErr(trace.Wrap(err))
 			}
 			return nil

integrations/access/jira/jira_test.go

@@ -249,9 +249,9 @@ func (s *JiraSuite) createAccessRequest() types.AccessRequest {
 	t.Helper()
 
 	req := s.newAccessRequest()
-	err := s.requestor().CreateAccessRequest(s.Context(), req)
+	out, err := s.requestor().CreateAccessRequestV2(s.Context(), req)
 	require.NoError(t, err)
-	return req
+	return out
 }
 
 func (s *JiraSuite) checkPluginData(reqID string, cond func(PluginData) bool) PluginData {
@@ -323,7 +323,8 @@ func (s *JiraSuite) TestIssueCreationWithRequestReason() {
 
 	req := s.newAccessRequest()
 	req.SetRequestReason("because of")
-	err := s.requestor().CreateAccessRequest(s.Context(), req)
+	var err error
+	req, err = s.requestor().CreateAccessRequestV2(s.Context(), req)
 	require.NoError(t, err)
 	s.checkPluginData(req.GetName(), func(data PluginData) bool {
 		return data.IssueID != ""
@@ -344,7 +345,8 @@ func (s *JiraSuite) TestIssueCreationWithLargeRequestReason() {
 
 	req := s.newAccessRequest()
 	req.SetRequestReason(strings.Repeat("a", jiraReasonLimit+10))
-	err := s.requestor().CreateAccessRequest(s.Context(), req)
+	var err error
+	req, err = s.requestor().CreateAccessRequestV2(s.Context(), req)
 	require.NoError(t, err)
 	s.checkPluginData(req.GetName(), func(data PluginData) bool {
 		return data.IssueID != ""
@@ -757,7 +759,8 @@ func (s *JiraSuite) TestRace() {
 			if err != nil {
 				return setRaceErr(trace.Wrap(err))
 			}
-			if err = s.requestor().CreateAccessRequest(s.Context(), req); err != nil {
+			_, err = s.requestor().CreateAccessRequestV2(s.Context(), req)
+			if err != nil {
 				return setRaceErr(trace.Wrap(err))
 			}
 			return nil

integrations/access/mattermost/mattermost_test.go

@@ -256,9 +256,9 @@ func (s *MattermostSuite) createAccessRequest(reviewers []User) types.AccessRequ
 	t.Helper()
 
 	req := s.newAccessRequest(reviewers)
-	err := s.requestor().CreateAccessRequest(s.Context(), req)
+	out, err := s.requestor().CreateAccessRequestV2(s.Context(), req)
 	require.NoError(s.T(), err)
-	return req
+	return out
 }
 
 func (s *MattermostSuite) checkPluginData(reqID string, cond func(common.GenericPluginData) bool) common.GenericPluginData {
@@ -650,7 +650,7 @@ func (s *MattermostSuite) TestRace() {
 				return setRaceErr(trace.Wrap(err))
 			}
 			req.SetSuggestedReviewers([]string{reviewer1.Email, reviewer2.Email})
-			if err := s.requestor().CreateAccessRequest(ctx, req); err != nil {
+			if _, err := s.requestor().CreateAccessRequestV2(ctx, req); err != nil {
 				return setRaceErr(trace.Wrap(err))
 			}
 			return nil

integrations/access/opsgenie/opsgenie_test.go

@@ -331,9 +331,9 @@ func (s *OpsgenieSuite) createAccessRequest() types.AccessRequest {
 	t.Helper()
 
 	req := s.newAccessRequest()
-	err := s.requestor().CreateAccessRequest(s.Context(), req)
+	out, err := s.requestor().CreateAccessRequestV2(s.Context(), req)
 	require.NoError(t, err)
-	return req
+	return out
 }
 
 func (s *OpsgenieSuite) checkPluginData(reqID string, cond func(PluginData) bool) PluginData {

integrations/access/pagerduty/pagerduty_test.go

@@ -340,9 +340,9 @@ func (s *PagerdutySuite) createAccessRequest() types.AccessRequest {
 	t.Helper()
 
 	req := s.newAccessRequest()
-	err := s.requestor().CreateAccessRequest(s.Context(), req)
+	out, err := s.requestor().CreateAccessRequestV2(s.Context(), req)
 	require.NoError(t, err)
-	return req
+	return out
 }
 
 func (s *PagerdutySuite) checkPluginData(reqID string, cond func(PluginData) bool) PluginData {
@@ -851,7 +851,8 @@ func (s *PagerdutySuite) TestRace() {
 			if err != nil {
 				return setRaceErr(trace.Wrap(err))
 			}
-			if err := s.clients[userName].CreateAccessRequest(ctx, req); err != nil {
+			req, err = s.clients[userName].CreateAccessRequestV2(ctx, req)
+			if err != nil {
 				return setRaceErr(trace.Wrap(err))
 			}
 			pendingRequests.Store(req.GetName(), struct{}{})

integrations/access/servicenow/servicenow_test.go

@@ -321,9 +321,9 @@ func (s *ServiceNowSuite) createAccessRequest() types.AccessRequest {
 	t.Helper()
 
 	req := s.newAccessRequest()
-	err := s.requestor().CreateAccessRequest(s.Context(), req)
+	out, err := s.requestor().CreateAccessRequestV2(s.Context(), req)
 	require.NoError(t, err)
-	return req
+	return out
 }
 
 func (s *ServiceNowSuite) checkPluginData(reqID string, cond func(PluginData) bool) PluginData {

integrations/access/slack/slack_test.go

@@ -249,9 +249,9 @@ func (s *SlackSuite) createAccessRequest(reviewers []User) types.AccessRequest {
 	t.Helper()
 
 	req := s.newAccessRequest(reviewers)
-	err := s.requestor().CreateAccessRequest(s.Context(), req)
+	out, err := s.requestor().CreateAccessRequestV2(s.Context(), req)
 	require.NoError(t, err)
-	return req
+	return out
 }
 
 func (s *SlackSuite) checkPluginData(reqID string, cond func(common.GenericPluginData) bool) common.GenericPluginData {
@@ -651,7 +651,7 @@ func (s *SlackSuite) TestRace() {
 				return setRaceErr(trace.Wrap(err))
 			}
 			req.SetSuggestedReviewers([]string{reviewer1.Profile.Email, reviewer2.Profile.Email})
-			if err := s.requestor().CreateAccessRequest(ctx, req); err != nil {
+			if _, err := s.requestor().CreateAccessRequestV2(ctx, req); err != nil {
 				return setRaceErr(trace.Wrap(err))
 			}
 			return nil

lib/ai/model/tools/tool.go

@@ -62,7 +62,7 @@ type AccessPoint interface {
 
 // AccessRequestClient abstracts away the access request client for testing purposes.
 type AccessRequestClient interface {
-	CreateAccessRequest(ctx context.Context, req types.AccessRequest) error
+	CreateAccessRequestV2(ctx context.Context, req types.AccessRequest) (types.AccessRequest, error)
 	GetAccessRequests(ctx context.Context, filter types.AccessRequestFilter) ([]types.AccessRequest, error)
 }
 

lib/auth/access_request_test.go

@@ -313,7 +313,7 @@ func testSingleAccessRequests(t *testing.T, testPack *accessRequestTestPack) {
 			require.NoError(t, err)
 
 			// send the request to the auth server
-			err = requesterClient.CreateAccessRequest(ctx, req)
+			req, err = requesterClient.CreateAccessRequestV2(ctx, req)
 			require.ErrorIs(t, err, tc.expectRequestError)
 			if tc.expectRequestError != nil {
 				return

lib/auth/auth.go

@@ -4087,10 +4087,6 @@ func (a *Server) DeleteNamespace(namespace string) error {
 	}
 	return a.Services.DeleteNamespace(namespace)
 }
-func (a *Server) CreateAccessRequest(ctx context.Context, req types.AccessRequest, identity tlsca.Identity) error {
-	_, err := a.CreateAccessRequestV2(ctx, req, identity)
-	return trace.Wrap(err)
-}
 
 func (a *Server) CreateAccessRequestV2(ctx context.Context, req types.AccessRequest, identity tlsca.Identity) (types.AccessRequest, error) {
 	now := a.clock.Now().UTC()

lib/auth/auth_with_roles.go

@@ -24,6 +24,7 @@ import (
 	"time"
 
 	"github.com/coreos/go-semver/semver"
+	"github.com/google/uuid"
 	"github.com/gravitational/roundtrip"
 	"github.com/gravitational/trace"
 	"github.com/sirupsen/logrus"
@@ -2649,18 +2650,17 @@ func (a *ServerWithRoles) GetAccessRequests(ctx context.Context, filter types.Ac
 	return filtered, nil
 }
 
-func (a *ServerWithRoles) CreateAccessRequest(ctx context.Context, req types.AccessRequest) error {
-	_, err := a.CreateAccessRequestV2(ctx, req)
-	return trace.Wrap(err)
-}
-
 func (a *ServerWithRoles) CreateAccessRequestV2(ctx context.Context, req types.AccessRequest) (types.AccessRequest, error) {
 	// An exception is made to allow users to create access *pending* requests for themselves.
 	if !req.GetState().IsPending() || a.currentUserAction(req.GetUser()) != nil {
 		if err := a.action(apidefaults.Namespace, types.KindAccessRequest, types.VerbCreate); err != nil {
 			return nil, trace.Wrap(err)
 		}
 	}
+
+	// ensure request ID is set server-side
+	req.SetName(uuid.NewString())
+
 	resp, err := a.authServer.CreateAccessRequestV2(ctx, req, a.context.Identity.GetIdentity())
 	return resp, trace.Wrap(err)
 }

lib/auth/auth_with_roles_test.go

@@ -6352,10 +6352,19 @@ func TestCreateAccessRequest(t *testing.T) {
 			client, err := srv.NewClient(TestUser(test.user))
 			require.NoError(t, err)
 
-			test.errAssertionFunc(t, client.CreateAccessRequest(ctx, test.accessRequest))
+			req, err := client.CreateAccessRequestV2(ctx, test.accessRequest)
+			test.errAssertionFunc(t, err)
+
+			if err != nil {
+				require.Nil(t, test.expected, "erroring test-cases should not assert expectations (this is a bug)")
+				return
+			}
+
+			// id should be regenerated server-side
+			require.NotEqual(t, test.accessRequest.GetName(), req.GetName())
 
 			accessRequests, err := srv.Auth().GetAccessRequests(ctx, types.AccessRequestFilter{
-				ID: test.accessRequest.GetName(),
+				ID: req.GetName(),
 			})
 			require.NoError(t, err)
 

lib/auth/grpcserver.go

@@ -779,8 +779,7 @@ func (g *GRPCServer) GetAccessRequestsV2(f *types.AccessRequestFilter, stream au
 }
 
 func (g *GRPCServer) CreateAccessRequest(ctx context.Context, req *types.AccessRequestV3) (*emptypb.Empty, error) {
-	_, err := g.CreateAccessRequestV2(ctx, req)
-	return &emptypb.Empty{}, trace.Wrap(err)
+	return nil, trace.NotImplemented("access request creation API has changed, please update your client to v14 or newer")
 }
 
 func (g *GRPCServer) CreateAccessRequestV2(ctx context.Context, req *types.AccessRequestV3) (*types.AccessRequestV3, error) {
@@ -796,10 +795,17 @@ func (g *GRPCServer) CreateAccessRequestV2(ctx context.Context, req *types.Acces
 		return nil, trace.Wrap(err)
 	}
 
-	if err := auth.ServerWithRoles.CreateAccessRequest(ctx, req); err != nil {
+	out, err := auth.ServerWithRoles.CreateAccessRequestV2(ctx, req)
+	if err != nil {
 		return nil, trace.Wrap(err)
 	}
-	return req, nil
+
+	r, ok := out.(*types.AccessRequestV3)
+	if !ok {
+		return nil, trace.Wrap(trace.BadParameter("unexpected access request type %T", r))
+	}
+
+	return r, nil
 }
 
 func (g *GRPCServer) DeleteAccessRequest(ctx context.Context, id *authpb.RequestID) (*emptypb.Empty, error) {

lib/auth/tls_test.go

@@ -1486,7 +1486,7 @@ func TestWebSessionMultiAccessRequests(t *testing.T) {
 	require.NoError(t, err)
 	roleReq.SetState(types.RequestState_APPROVED)
 	roleReq.SetAccessExpiry(clock.Now().Add(8 * time.Hour))
-	err = clt.CreateAccessRequest(ctx, roleReq)
+	roleReq, err = clt.CreateAccessRequestV2(ctx, roleReq)
 	require.NoError(t, err)
 
 	// Create remote cluster so create access request doesn't err due to non existent cluster
@@ -1499,7 +1499,7 @@ func TestWebSessionMultiAccessRequests(t *testing.T) {
 	resourceReq, err := services.NewAccessRequestWithResources(username, []string{resourceRequestRoleName}, resourceIDs)
 	require.NoError(t, err)
 	resourceReq.SetState(types.RequestState_APPROVED)
-	err = clt.CreateAccessRequest(ctx, resourceReq)
+	resourceReq, err = clt.CreateAccessRequestV2(ctx, resourceReq)
 	require.NoError(t, err)
 
 	// Create a web session and client for the user.
@@ -1694,7 +1694,7 @@ func TestWebSessionWithApprovedAccessRequestAndSwitchback(t *testing.T) {
 	accessReq.SetAccessExpiry(clock.Now().Add(time.Minute * 10))
 	accessReq.SetState(types.RequestState_APPROVED)
 
-	err = clt.CreateAccessRequest(ctx, accessReq)
+	accessReq, err = clt.CreateAccessRequestV2(ctx, accessReq)
 	require.NoError(t, err)
 
 	sess1, err := web.ExtendWebSession(ctx, WebSessionReq{
@@ -1898,7 +1898,7 @@ func TestExtendWebSessionWithMaxDuration(t *testing.T) {
 			err = accessReq.SetState(types.RequestState_APPROVED)
 			require.NoError(t, err)
 
-			err = adminClient.CreateAccessRequest(ctx, accessReq)
+			accessReq, err = adminClient.CreateAccessRequestV2(ctx, accessReq)
 			require.NoError(t, err)
 
 			sess1, err := userClient.ExtendWebSession(ctx, WebSessionReq{
@@ -2055,7 +2055,8 @@ func TestPluginData(t *testing.T) {
 	req, err := services.NewAccessRequest(user, role)
 	require.NoError(t, err)
 
-	require.NoError(t, userClient.CreateAccessRequest(ctx, req))
+	req, err = userClient.CreateAccessRequestV2(ctx, req)
+	require.NoError(t, err)
 
 	err = pluginClient.UpdatePluginData(ctx, types.PluginDataUpdateParams{
 		Kind:     types.KindAccessRequest,

lib/auth/usage_test.go

@@ -108,13 +108,13 @@ func TestAccessRequestLimit(t *testing.T) {
 	// Check July
 	req, err := types.NewAccessRequest(uuid.New().String(), "alice", "access")
 	require.NoError(t, err)
-	err = p.a.CreateAccessRequest(ctx, req, tlsca.Identity{})
+	_, err = p.a.CreateAccessRequestV2(ctx, req, tlsca.Identity{})
 	require.Error(t, err, "expected access request creation to fail due to the monthly limit")
 
 	// Check August
 	clock.Advance(31 * 24 * time.Hour)
 	req, err = types.NewAccessRequest(uuid.New().String(), "alice", "access")
 	require.NoError(t, err)
-	err = p.a.CreateAccessRequest(ctx, req, tlsca.Identity{})
+	_, err = p.a.CreateAccessRequestV2(ctx, req, tlsca.Identity{})
 	require.NoError(t, err)
 }

lib/cache/cache_test.go

@@ -404,7 +404,8 @@ func TestWatchers(t *testing.T) {
 	req, err := services.NewAccessRequest("alice", "dictator")
 	require.NoError(t, err)
 
-	require.NoError(t, p.dynamicAccessS.CreateAccessRequest(ctx, req))
+	req, err = p.dynamicAccessS.CreateAccessRequestV2(ctx, req)
+	require.NoError(t, err)
 
 	select {
 	case e := <-w.Events():
@@ -429,7 +430,8 @@ func TestWatchers(t *testing.T) {
 	require.NoError(t, err)
 
 	// create and then delete the non-matching request.
-	require.NoError(t, p.dynamicAccessS.CreateAccessRequest(ctx, req2))
+	req2, err = p.dynamicAccessS.CreateAccessRequestV2(ctx, req2)
+	require.NoError(t, err)
 	require.NoError(t, p.dynamicAccessS.DeleteAccessRequest(ctx, req2.GetName()))
 
 	// because our filter did not match the request, the create event should never