Allow setting custom reason prompt for Access Requests

api/proto/teleport/legacy/types/types.proto

@@ -2828,6 +2828,18 @@ message PromotedAccessList {
   string Title = 2 [(gogoproto.jsontag) = "title"];
 }
 
+// AccessRequestDryRunEnrichment contains the extra info added in a response to a dry run request.
+message AccessRequestDryRunEnrichment {
+  // ReasonMode specifies the reason mode for this Access Request as defined in
+  // [AccessRequestConditionsReason].reason.
+  string ReasonMode = 1 [
+    (gogoproto.jsontag) = "reason_mode,omitempty",
+    (gogoproto.casttype) = "RequestReasonMode"
+  ];
+  // ReasonPrompts is a sorted and deduplicated list of reason prompts for this Access Request.
+  repeated string ReasonPrompts = 2 [(gogoproto.jsontag) = "reason_prompt,omitempty"];
+}
+
 // AccessReview is a review to be applied to an Access Request.
 message AccessReview {
   // Author is the teleport username of the review author.
@@ -3049,6 +3061,9 @@ message AccessRequestSpecV3 {
     (gogoproto.nullable) = true,
     (gogoproto.jsontag) = "expiry,omitempty"
   ];
+
+  // DryRunEnrichment contains the extra info added in response to a dry run request.
+  AccessRequestDryRunEnrichment DryRunEnrichment = 23 [(gogoproto.jsontag) = "dry_run_enrichment,omitempty"];
 }
 
 enum AccessRequestScope {

api/types/access_request.go

@@ -131,6 +131,10 @@ type AccessRequest interface {
 	GetDryRun() bool
 	// SetDryRun sets the dry run flag on the request.
 	SetDryRun(bool)
+	// GetDryRunEnrichment gets the dry run enrichment data.
+	GetDryRunEnrichment() *AccessRequestDryRunEnrichment
+	// SetDryRunEnrichment sets the dry run enrichment data.
+	SetDryRunEnrichment(*AccessRequestDryRunEnrichment)
 	// Copy returns a copy of the access request resource.
 	Copy() AccessRequest
 }
@@ -514,6 +518,16 @@ func (r *AccessRequestV3) SetDryRun(dryRun bool) {
 	r.Spec.DryRun = dryRun
 }
 
+// GetDryRunEnrichment gets the dry run enrichment data.
+func (r *AccessRequestV3) GetDryRunEnrichment() *AccessRequestDryRunEnrichment {
+	return r.Spec.DryRunEnrichment
+}
+
+// SetDryRunEnrichment sets the dry run enrichment data.
+func (r *AccessRequestV3) SetDryRunEnrichment(enrichment *AccessRequestDryRunEnrichment) {
+	r.Spec.DryRunEnrichment = enrichment
+}
+
 // Copy returns a copy of the access request resource.
 func (r *AccessRequestV3) Copy() AccessRequest {
 	return utils.CloneProtoMsg(r)

lib/auth/auth.go

@@ -5302,8 +5302,7 @@ func (a *Server) CreateAccessRequestV2(ctx context.Context, req types.AccessRequ
 
 	req.SetCreationTime(now)
 
-	// Always perform variable expansion on creation only; this ensures the
-	// access request that is reviewed is the same that is approved.
+	// Always perform variable expansion on creation.
 	expandOpts := services.WithExpandVars(true)
 	if err := services.ValidateAccessRequestForUser(ctx, a.clock, a, req, identity, expandOpts); err != nil {
 		return nil, trace.Wrap(err)
@@ -5321,11 +5320,10 @@ func (a *Server) CreateAccessRequestV2(ctx context.Context, req types.AccessRequ
 	}
 
 	if req.GetDryRun() {
+		// NOTE: Some dry-run options are set in [services.ValidateAccessRequestForUser].
 		_, promotions := a.generateAccessRequestPromotions(ctx, req)
-		// update the request with additional reviewers if possible.
 		updateAccessRequestWithAdditionalReviewers(ctx, req, a.AccessLists, promotions)
-		// Made it this far with no errors, return before creating the request
-		// if this is a dry run.
+		// Return before creating the request if this is a dry run.
 		return req, nil
 	}
 

lib/auth/auth_test.go

@@ -4195,6 +4195,50 @@ func TestAccessRequestAuditLog(t *testing.T) {
 	require.Equal(t, "APPROVED", arc.RequestState)
 }
 
+func testCreateRole(t *testing.T, server *TestTLSServer, name string, setup func(*types.RoleSpecV6)) types.Role {
+	t.Helper()
+	ctx := context.Background()
+
+	spec := types.RoleSpecV6{
+		Allow: types.RoleConditions{
+			Request: &types.AccessRequestConditions{
+				Reason: &types.AccessRequestConditionsReason{},
+			},
+			ReviewRequests: &types.AccessReviewConditions{},
+		},
+		Deny: types.RoleConditions{
+			Request:        &types.AccessRequestConditions{},
+			ReviewRequests: &types.AccessReviewConditions{},
+		},
+	}
+	setup(&spec)
+
+	role, err := types.NewRole(name, spec)
+	require.NoError(t, err, "types.NewRole")
+
+	createdRole, err := server.AuthServer.AuthServer.UpsertRole(ctx, role)
+	require.NoError(t, err, "AuthServer.UpsertRole")
+
+	return createdRole
+}
+
+func testCreateUserWithRoles(t *testing.T, server *TestTLSServer, user string, roles ...string) (TestIdentity, *authclient.Client) {
+	t.Helper()
+	ctx := context.Background()
+
+	u, err := types.NewUser(user)
+	require.NoError(t, err, "types.NewUser")
+	u.SetRoles(roles)
+	_, err = server.AuthServer.AuthServer.UpsertUser(ctx, u)
+	require.NoError(t, err, "AuthServer.UpsertUser")
+
+	identity := TestUser(user)
+	client, err := server.NewClient(identity)
+	require.NoError(t, err, "server.NewClient")
+
+	return identity, client
+}
+
 func TestAccessRequestNotifications(t *testing.T) {
 	t.Parallel()
 	ctx := context.Background()
@@ -4213,69 +4257,36 @@ func TestAccessRequestNotifications(t *testing.T) {
 	requesterUsername := "requester"
 	requestRoleName := "requestRole"
 
-	reviewerRole, err := types.NewRole(reviewerUsername, types.RoleSpecV6{
-		Allow: types.RoleConditions{
-			Logins: []string{"user"},
-			ReviewRequests: &types.AccessReviewConditions{
-				Roles: []string{"requestRole"},
-			},
-		},
+	reviewerRole := testCreateRole(t, testTLSServer, reviewerUsername, func(spec *types.RoleSpecV6) {
+		spec.Allow.Logins = []string{"user"}
+		spec.Allow.ReviewRequests.Roles = []string{"requestRole"}
 	})
-	require.NoError(t, err)
 
-	requesterRole, err := types.NewRole(requesterUsername, types.RoleSpecV6{
-		Allow: types.RoleConditions{
-			Request: &types.AccessRequestConditions{
-				Roles: []string{requestRoleName},
-			},
-		},
+	requesterRole := testCreateRole(t, testTLSServer, requesterUsername, func(spec *types.RoleSpecV6) {
+		spec.Allow.Request.Roles = []string{requestRoleName}
 	})
-	require.NoError(t, err)
 
-	requestedRole, err := types.NewRole(requestRoleName, types.RoleSpecV6{
-		Allow: types.RoleConditions{
-			Request: &types.AccessRequestConditions{
-				Roles: []string{requestRoleName},
-			},
-		},
+	requestRole := testCreateRole(t, testTLSServer, requestRoleName, func(spec *types.RoleSpecV6) {
+		spec.Allow.Request.Roles = []string{requestRoleName}
 	})
-	require.NoError(t, err)
-	_, err = testTLSServer.AuthServer.AuthServer.UpsertRole(ctx, requestedRole)
-	require.NoError(t, err)
 
-	_, err = testTLSServer.AuthServer.AuthServer.UpsertRole(ctx, reviewerRole)
-	require.NoError(t, err)
-	reviewer, err := types.NewUser(reviewerUsername)
-	require.NoError(t, err)
-	reviewer.SetRoles([]string{reviewerUsername})
-	_, err = testTLSServer.AuthServer.AuthServer.UpsertUser(ctx, reviewer)
-	require.NoError(t, err)
+	reviewer, reviewerClient := testCreateUserWithRoles(t, testTLSServer, reviewerUsername, reviewerRole.GetName())
 
-	_, err = testTLSServer.AuthServer.AuthServer.UpsertRole(ctx, requesterRole)
-	require.NoError(t, err)
-	requester, err := types.NewUser(requesterUsername)
-	require.NoError(t, err)
-	requester.SetRoles([]string{requesterUsername})
-	_, err = testTLSServer.AuthServer.AuthServer.UpsertUser(ctx, requester)
-	require.NoError(t, err)
+	requester, _ := testCreateUserWithRoles(t, testTLSServer, requesterUsername, requesterRole.GetName())
 
-	accessRequest, err := types.NewAccessRequest(uuid.NewString(), requesterUsername, requestRoleName)
+	accessRequest, err := types.NewAccessRequest(uuid.NewString(), requester.GetUsername(), requestRole.GetName())
 	require.NoError(t, err)
-	req, err := testTLSServer.AuthServer.AuthServer.CreateAccessRequestV2(ctx, accessRequest, TestUser(requesterUsername).I.GetIdentity())
+	req, err := testTLSServer.AuthServer.AuthServer.CreateAccessRequestV2(ctx, accessRequest, reviewer.I.GetIdentity())
 	require.NoError(t, err)
 
 	// Verify that a global notification was created which matches for users who can review the requestRole.
 	globalNotifsResp, _, err := testTLSServer.AuthServer.AuthServer.Notifications.ListGlobalNotifications(ctx, 100, "")
 	require.NoError(t, err)
 	require.Len(t, globalNotifsResp, 1)
 	require.Equal(t, &types.AccessReviewConditions{
-		Roles: []string{requestRoleName},
+		Roles: []string{requestRole.GetName()},
 	}, globalNotifsResp[0].GetSpec().GetByPermissions().GetRoleConditions()[0].ReviewRequests)
 
-	reviewerIdentity := TestUser(reviewerUsername)
-	reviewerClient, err := testTLSServer.NewClient(reviewerIdentity)
-	require.NoError(t, err)
-
 	// Approve the request
 	_, err = reviewerClient.SubmitAccessReview(ctx, types.AccessReviewSubmission{
 		RequestID: req.GetName(),
@@ -4291,9 +4302,9 @@ func TestAccessRequestNotifications(t *testing.T) {
 	require.Contains(t, userNotifsResp[0].GetMetadata().GetLabels()[types.NotificationTitleLabel], "reviewer approved your access request")
 
 	// Create another access request.
-	accessRequest, err = types.NewAccessRequest(uuid.NewString(), requesterUsername, requestRoleName)
+	accessRequest, err = types.NewAccessRequest(uuid.NewString(), requester.GetUsername(), requestRole.GetName())
 	require.NoError(t, err)
-	req, err = testTLSServer.AuthServer.AuthServer.CreateAccessRequestV2(ctx, accessRequest, TestUser(requesterUsername).I.GetIdentity())
+	req, err = testTLSServer.AuthServer.AuthServer.CreateAccessRequestV2(ctx, accessRequest, TestUser(requester.GetUsername()).I.GetIdentity())
 	require.NoError(t, err)
 
 	// Deny the request.
@@ -4311,6 +4322,165 @@ func TestAccessRequestNotifications(t *testing.T) {
 	require.Contains(t, userNotifsResp[1].GetMetadata().GetLabels()[types.NotificationTitleLabel], "reviewer denied your access request")
 }
 
+func testNewAccessRequest(t *testing.T, user string, roles ...string) types.AccessRequest {
+	t.Helper()
+	r, err := types.NewAccessRequest(uuid.NewString(), user, roles...)
+	require.NoError(t, err, "types.NewAccessRequest")
+	return r
+}
+
+func TestAccessRequestDryRunEnrichment(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+
+	testAuthServer, err := NewTestAuthServer(TestAuthServerConfig{
+		Dir:   t.TempDir(),
+		Clock: clockwork.NewFakeClock(),
+	})
+	require.NoError(t, err)
+	testTLSServer, err := testAuthServer.NewTestTLSServer()
+	require.NoError(t, err)
+
+	someRole := testCreateRole(t, testTLSServer, "some-role", func(spec *types.RoleSpecV6) {})
+
+	someRoleRequesterRole := testCreateRole(t, testTLSServer, "some-role-requester", func(spec *types.RoleSpecV6) {
+		spec.Allow.Request.Roles = []string{someRole.GetName()}
+	})
+
+	someRoleRequesterRoleRequiringReason := testCreateRole(t, testTLSServer, "some-role-requester-requiring-reason", func(spec *types.RoleSpecV6) {
+		spec.Allow.Request.Roles = []string{someRole.GetName()}
+		spec.Allow.Request.Reason.Mode = types.RequestReasonModeRequired
+	})
+
+	globalPromptRole1 := testCreateRole(t, testTLSServer, "prompt-role-1", func(spec *types.RoleSpecV6) {
+		spec.Options.RequestPrompt = "test prompt #1"
+	})
+	globalPromptRole2 := testCreateRole(t, testTLSServer, "prompt-role-2", func(spec *types.RoleSpecV6) {
+		spec.Options.RequestPrompt = "test prompt #2"
+	})
+
+	t.Run("requesting-role-no-reason-required-no-prompts", func(t *testing.T) {
+		requester, requesterClient := testCreateUserWithRoles(t, testTLSServer, "requester",
+			someRoleRequesterRole.GetName(),
+		)
+
+		dryRunAccessRequest := testNewAccessRequest(t, requester.GetUsername(), someRole.GetName())
+		dryRunAccessRequest.SetDryRun(true)
+
+		resp, err := requesterClient.CreateAccessRequestV2(ctx, dryRunAccessRequest)
+		require.NoError(t, err)
+
+		require.NotNil(t, resp.GetDryRunEnrichment())
+		// check reason mode
+		require.Equal(t, types.RequestReasonModeOptional, resp.GetDryRunEnrichment().ReasonMode)
+		// check prompts
+		require.Empty(t, resp.GetDryRunEnrichment().ReasonPrompts)
+	})
+
+	t.Run("requesting-role-reason-required", func(t *testing.T) {
+		requester, requesterClient := testCreateUserWithRoles(t, testTLSServer, "requester",
+			someRoleRequesterRoleRequiringReason.GetName(),
+		)
+
+		dryRunAccessRequest := testNewAccessRequest(t, requester.GetUsername(), someRole.GetName())
+		dryRunAccessRequest.SetDryRun(true)
+
+		resp, err := requesterClient.CreateAccessRequestV2(ctx, dryRunAccessRequest)
+		require.NoError(t, err)
+
+		require.NotNil(t, resp.GetDryRunEnrichment())
+		// check reason mode
+		require.Equal(t, types.RequestReasonModeRequired, resp.GetDryRunEnrichment().ReasonMode)
+		// check prompts
+		require.Empty(t, resp.GetDryRunEnrichment().ReasonPrompts)
+	})
+
+	t.Run("requesting-role-multiple-prompts", func(t *testing.T) {
+		requester, requesterClient := testCreateUserWithRoles(t, testTLSServer, "requester",
+			someRoleRequesterRole.GetName(),
+			globalPromptRole1.GetName(),
+			globalPromptRole2.GetName(),
+		)
+
+		dryRunAccessRequest := testNewAccessRequest(t, requester.GetUsername(), someRole.GetName())
+		dryRunAccessRequest.SetDryRun(true)
+
+		resp, err := requesterClient.CreateAccessRequestV2(ctx, dryRunAccessRequest)
+		require.NoError(t, err)
+
+		require.NotNil(t, resp.GetDryRunEnrichment())
+		// check reason mode
+		require.Equal(t, types.RequestReasonModeOptional, resp.GetDryRunEnrichment().ReasonMode)
+		// check prompts
+		require.Len(t, resp.GetDryRunEnrichment().ReasonPrompts, 2)
+		require.Contains(t, resp.GetDryRunEnrichment().ReasonPrompts, globalPromptRole1.GetOptions().RequestPrompt)
+		require.Contains(t, resp.GetDryRunEnrichment().ReasonPrompts, globalPromptRole2.GetOptions().RequestPrompt)
+	})
+
+	t.Run("requesting-role-reason-required-and-multiple-prompts", func(t *testing.T) {
+		requester, requesterClient := testCreateUserWithRoles(t, testTLSServer, "requester",
+			someRoleRequesterRole.GetName(),
+			someRoleRequesterRoleRequiringReason.GetName(),
+			globalPromptRole1.GetName(),
+			globalPromptRole2.GetName(),
+		)
+
+		dryRunAccessRequest := testNewAccessRequest(t, requester.GetUsername(), someRole.GetName())
+		dryRunAccessRequest.SetDryRun(true)
+
+		resp, err := requesterClient.CreateAccessRequestV2(ctx, dryRunAccessRequest)
+		require.NoError(t, err)
+
+		require.NotNil(t, resp.GetDryRunEnrichment())
+		// check reason mode
+		require.Equal(t, types.RequestReasonModeRequired, resp.GetDryRunEnrichment().ReasonMode)
+		// check prompts
+		require.Len(t, resp.GetDryRunEnrichment().ReasonPrompts, 2)
+		require.Contains(t, resp.GetDryRunEnrichment().ReasonPrompts, globalPromptRole1.GetOptions().RequestPrompt)
+		require.Contains(t, resp.GetDryRunEnrichment().ReasonPrompts, globalPromptRole2.GetOptions().RequestPrompt)
+	})
+
+	t.Run("requesting-role-prompts-sorted-and-duplicated", func(t *testing.T) {
+		globalPromptRole1 := testCreateRole(t, testTLSServer, "prompt-role-1", func(spec *types.RoleSpecV6) {
+			spec.Options.RequestPrompt = "C test prompt"
+		})
+		globalPromptRole2 := testCreateRole(t, testTLSServer, "prompt-role-2", func(spec *types.RoleSpecV6) {
+			spec.Options.RequestPrompt = "A test prompt"
+		})
+		globalPromptRole3 := testCreateRole(t, testTLSServer, "prompt-role-3", func(spec *types.RoleSpecV6) {
+			spec.Options.RequestPrompt = "B test prompt"
+		})
+		globalPromptRole4 := testCreateRole(t, testTLSServer, "prompt-role-4", func(spec *types.RoleSpecV6) {
+			spec.Options.RequestPrompt = "B test prompt"
+		})
+		globalPromptRole5 := testCreateRole(t, testTLSServer, "prompt-role-5", func(spec *types.RoleSpecV6) {
+			spec.Options.RequestPrompt = "C test prompt"
+		})
+
+		requester, requesterClient := testCreateUserWithRoles(t, testTLSServer, "requester",
+			someRoleRequesterRole.GetName(),
+			globalPromptRole1.GetName(),
+			globalPromptRole2.GetName(),
+			globalPromptRole3.GetName(),
+			globalPromptRole4.GetName(),
+			globalPromptRole5.GetName(),
+		)
+
+		dryRunAccessRequest := testNewAccessRequest(t, requester.GetUsername(), someRole.GetName())
+		dryRunAccessRequest.SetDryRun(true)
+
+		resp, err := requesterClient.CreateAccessRequestV2(ctx, dryRunAccessRequest)
+		require.NoError(t, err)
+
+		require.NotNil(t, resp.GetDryRunEnrichment())
+		// check prompts
+		require.Len(t, resp.GetDryRunEnrichment().ReasonPrompts, 3)
+		require.Equal(t, "A test prompt", resp.GetDryRunEnrichment().ReasonPrompts[0])
+		require.Equal(t, "B test prompt", resp.GetDryRunEnrichment().ReasonPrompts[1])
+		require.Equal(t, "C test prompt", resp.GetDryRunEnrichment().ReasonPrompts[2])
+	})
+}
+
 func TestCleanupNotifications(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	t.Cleanup(cancel)