Return the group name when listing a user's groups. (#1422)

* return the group name when listing a user's groups.

* switch to dropping groups that don't exist

* tweak test name and update comment

internal/jimmhttp/rebac_admin/identities.go

@@ -121,17 +121,27 @@ func (s *identitiesService) GetIdentityGroups(ctx context.Context, identityId st
 		Relation:     ofganames.MemberRelation.String(),
 		TargetObject: openfga.GroupType.String(),
 	}, int32(filter.Limit()), filter.Token()) // #nosec G115 accept integer conversion
-
 	if err != nil {
 		return nil, err
 	}
-	groups := make([]resources.Group, len(tuples))
-	for i, t := range tuples {
-		groups[i] = resources.Group{
-			Id:   &t.Target.ID,
-			Name: t.Target.ID,
+
+	groups := make([]resources.Group, 0, len(tuples))
+	for _, t := range tuples {
+		dbGroup, err := s.jimm.GetGroupByUUID(ctx, user, t.Target.ID)
+		if err != nil {
+			// Handle the case where the group was removed from the DB but a lingering OpenFGA tuple still exists.
+			// Don't return an error as that would prevent a user from viewing their groups, instead drop the group from the result.
+			if errors.ErrorCode(err) == errors.CodeNotFound {
+				continue
+			}
+			return nil, err
 		}
+		groups = append(groups, resources.Group{
+			Id:   &t.Target.ID,
+			Name: dbGroup.Name,
+		})
 	}
+
 	originalToken := filter.Token()
 	return &resources.PaginatedResponse[resources.Group]{
 		Data: groups,

internal/jimmhttp/rebac_admin/identities_integration_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/juju/names/v5"
 	gc "gopkg.in/check.v1"
 
+	"github.com/canonical/jimm/v3/internal/dbmodel"
 	"github.com/canonical/jimm/v3/internal/jimmhttp/rebac_admin"
 	"github.com/canonical/jimm/v3/internal/openfga"
 	ofganames "github.com/canonical/jimm/v3/internal/openfga/names"
@@ -103,14 +104,54 @@ func (s *identitiesSuite) TestIdentityGetGroups(c *gc.C) {
 		c.Assert(err, gc.IsNil)
 		token = *groups.Next.PageToken
 		for j := 0; j < len(groups.Data); j++ {
-			c.Assert(groups.Data[j].Name, gc.Equals, groupTags[i+j].Id())
+			c.Assert(groups.Data[j].Name, gc.Equals, fmt.Sprintf("group-test%d", i+j))
+			c.Assert(groupTags[j].Id(), gc.Matches, `\w*-\w*-\w*-\w*-\w*`)
 		}
 		if *groups.Next.PageToken == "" {
 			break
 		}
 	}
 }
 
+// TestGetIdentityGroupsWithDeletedDbGroup tests the behaviour
+// of GetIdentityGroups when a tuple lingers in OpenFGA but the group
+// has been removed from the database.
+func (s *identitiesSuite) TestGetIdentityGroupsWithDeletedDbGroup(c *gc.C) {
+	ctx := context.Background()
+	ctx = rebac_handlers.ContextWithIdentity(ctx, s.AdminUser)
+	identitySvc := rebac_admin.NewidentitiesService(s.JIMM)
+	username := s.AdminUser.Name
+
+	group1 := s.AddGroup(c, "group1")
+	group2 := s.AddGroup(c, "group2")
+
+	baseTuple := openfga.Tuple{
+		Object:   ofganames.ConvertTag(s.AdminUser.ResourceTag()),
+		Relation: ofganames.MemberRelation,
+	}
+	group1Access := baseTuple
+	group1Access.Target = ofganames.ConvertTag(group1)
+	group2Access := baseTuple
+	group2Access.Target = ofganames.ConvertTag(group2)
+
+	err := s.JIMM.OpenFGAClient.AddRelation(ctx, group1Access, group2Access)
+	c.Assert(err, gc.IsNil)
+
+	groups, err := identitySvc.GetIdentityGroups(ctx, username, &resources.GetIdentitiesItemGroupsParams{})
+	c.Assert(err, gc.IsNil)
+	c.Assert(groups.Data, gc.HasLen, 2)
+
+	groupToDelete := dbmodel.GroupEntry{Name: "group2"}
+	err = s.JIMM.Database.GetGroup(ctx, &groupToDelete)
+	c.Assert(err, gc.IsNil)
+	err = s.JIMM.Database.RemoveGroup(ctx, &groupToDelete)
+	c.Assert(err, gc.IsNil)
+
+	groups, err = identitySvc.GetIdentityGroups(ctx, username, &resources.GetIdentitiesItemGroupsParams{})
+	c.Assert(err, gc.IsNil)
+	c.Assert(groups.Data, gc.HasLen, 1)
+}
+
 // TestIdentityEntitlements tests the listing of entitlements for a specific identityId.
 // Setup: add controllers, models to a user and add the user to a group.
 func (s *identitiesSuite) TestIdentityEntitlements(c *gc.C) {

internal/jimmhttp/rebac_admin/identities_test.go

@@ -148,7 +148,7 @@ func TestGetIdentityGroups(t *testing.T) {
 	testTuple := openfga.Tuple{
 		Object:   &ofga.Entity{Kind: "user", ID: "foo"},
 		Relation: ofga.Relation("member"),
-		Target:   &ofga.Entity{Kind: "group", ID: "my-group"},
+		Target:   &ofga.Entity{Kind: "group", ID: "my-group-id"},
 	}
 	jimm := jimmtest.JIMM{
 		FetchIdentity_: func(ctx context.Context, username string) (*openfga.User, error) {
@@ -162,6 +162,11 @@ func TestGetIdentityGroups(t *testing.T) {
 				return []openfga.Tuple{testTuple}, "continuation-token", listTuplesErr
 			},
 		},
+		GroupService: mocks.GroupService{
+			GetGroupByUUID_: func(ctx context.Context, user *openfga.User, uuid string) (*dbmodel.GroupEntry, error) {
+				return &dbmodel.GroupEntry{Name: "fake-group-name"}, nil
+			},
+		},
 	}
 	user := openfga.User{}
 	ctx := context.Background()
@@ -176,6 +181,8 @@ func TestGetIdentityGroups(t *testing.T) {
 	c.Assert(err, qt.IsNil)
 	c.Assert(res, qt.IsNotNil)
 	c.Assert(res.Data, qt.HasLen, 1)
+	c.Assert(*res.Data[0].Id, qt.Equals, "my-group-id")
+	c.Assert(res.Data[0].Name, qt.Equals, "fake-group-name")
 	c.Assert(*res.Next.PageToken, qt.Equals, "continuation-token")
 
 	listTuplesErr = errors.New("foo")