Merge pull request #92 from eurofurence/issue-33-visibilities

group visibilities cleaned up

api/openapi-spec/openapi.yaml

@@ -71,13 +71,28 @@ paths:
       description: |-
         Obtain a list of all or selected groups, including their members.       
         
+        **Rules for group visibility**
+        
         Admin or Api Key authorization: can see all groups.
         
         Normal users: can only see groups visible to them. If public groups are enabled in configuration,
         this means all groups that are public, not full, and from which the user wasn't banned. Not all fields
         will be filled.
         
         Note: both admins and normal users can always use the findMyGroup operation to get the group they are in.
+        
+        **Rules for field visibility**
+        
+        Admin or Api Key authorization: can see all fields.
+        
+        Group owner: can see all fields (but only for the one group they own).
+        
+        Group members: can see all other members of the group with full information, but no invites.
+        
+        Invited: can see only their invite record, but no other invites, and no group members. The group comment is
+        hidden.
+        
+        All others can see neither members nor invites. The group comment is hidden.
       operationId: listGroups
       parameters:
         - name: member_ids
@@ -224,12 +239,24 @@ paths:
         - groups
       summary: find my group
       description: |-
-        Obtain the group you are in. Must have a valid registration.
+        Obtain the group you are in. Must have a valid registration in attending status.
         
         This works even for admins, still gives them the group they are in.
+        Admins are treated no different from ordinary users here!
         
         Because the user identity is taken from the logged in user, this does not work for Api Key authorization.
         Use the /groups endpoint with member_id parameter instead.
+
+        **Rules for field visibility**
+        
+        Group owner: can see all fields.
+        
+        Group members: can see all other members of the group with full information, but no invites.
+        
+        Invited: can see only their invite record, but no other invites, and no group members. The group comment is
+        hidden.
+
+        all others will receive 401, 403, or 404 from this endpoint anyway.
       operationId: findMyGroup
       responses:
         '200':
@@ -275,7 +302,30 @@ paths:
       tags:
         - groups
       summary: obtain a group by uuid
-      description: Returns a single group. You must be a member of the group or an admin in order to have access.
+      description: |-
+        Returns a single group. You must be a member of the group or an admin in order to have access.
+        
+        **Rules for group visibility**
+        
+        Admin or Api Key authorization: can see all groups.
+        
+        Normal users: can only see groups visible to them. If public groups are enabled in configuration,
+        this means all groups that are public, not full, and from which the user wasn't banned. Not all fields
+        will be filled.
+        
+        Note: both admins and normal users can always use the findMyGroup operation to get the group they are in.
+        
+        **Rules for field visibility**
+        
+        Admin or Api Key authorization: can see all fields.
+        
+        Group owner: can see all fields (but only for the one group they own).
+        
+        Group members: can see all other members of the group with full information, but no invites.
+        
+        Invited: can see only their invite record, but no other invites, and no group members.
+        
+        All others can see neither members nor invites.
       operationId: getGroupById
       parameters:
         - name: uuid
@@ -833,13 +883,16 @@ paths:
         - rooms
       summary: find my room
       description: |-
-        Obtain the room you are in. Must have a valid registration.
+        Obtain the room you are in. Must have a valid registration in attending status.
         
         Visibility of this information depends on the "final" flag that is set on the room, so admins can start planning
         room assignments without them becoming immediately visible to users.
         
         This endpoint works even for admins, giving them the room they are in, as long as they have a valid registration.
         
+        Admins are treated no different from regular users for this endpoint. This is so even an admin gets the
+        same user experience as a non-admin regarding their own room assignment.
+        
         Because the user identity is taken from the logged in user, this does not work for Api Key authorization.
         Use the /rooms endpoint with member_id parameter instead.
       operationId: findMyRoom

internal/service/groups/groups.go

@@ -21,16 +21,16 @@ import (
 
 // FindMyGroup finds the group containing the currently logged in attendee.
 //
-// This even works for admins.
+// This even works for admins, who are treated exactly as if they were a regular user.
 //
-// Uses the attendee service to look up the badge number.
+// Uses the attendee service to look up the badge number and the registration status.
 func (g *groupService) FindMyGroup(ctx context.Context) (*modelsv1.Group, error) {
-	attendee, err := g.loggedInUserValidRegistrationBadgeNo(ctx)
+	attendee, err := g.loggedInUserValidRegistration(ctx)
 	if err != nil {
 		return nil, err
 	}
 
-	groups, err := g.findGroupsLowlevel(ctx, 0, -1, []int64{attendee.ID})
+	groups, err := g.findGroupsFullAccess(ctx, 0, -1, []int64{attendee.ID})
 	if err != nil {
 		return nil, err
 	}
@@ -42,7 +42,14 @@ func (g *groupService) FindMyGroup(ctx context.Context) (*modelsv1.Group, error)
 		return nil, errInternal(ctx, "multiple group memberships found - this is a bug")
 	}
 
-	return groups[0], nil
+	filtered := g.filterGroupAndFieldVisibilityForAttendee(groups[0], attendee)
+	if filtered == nil {
+		// this should never happen because then the group would not have been found by findGroupsFullAccess.
+		aulogging.Warnf(ctx, "group %s found as containing attendee %d but got filtered - possible bug", groups[0].ID, attendee.ID)
+		return nil, errNoAccess(ctx)
+	}
+
+	return filtered, nil
 }
 
 // FindGroups finds groups by size (number of members) and member badge numbers.
@@ -63,28 +70,26 @@ func (g *groupService) FindGroups(ctx context.Context, minSize uint, maxSize int
 	}
 
 	if validator.IsAdmin() || validator.IsAPITokenCall() {
-		return g.findGroupsLowlevel(ctx, minSize, maxSize, memberIDs)
+		return g.findGroupsFullAccess(ctx, minSize, maxSize, memberIDs)
 	} else if validator.IsUser() {
 		result := make([]*modelsv1.Group, 0)
 
 		// ensure attending registration
-		attendee, err := g.loggedInUserValidRegistrationBadgeNo(ctx)
+		attendee, err := g.loggedInUserValidRegistration(ctx)
 		if err != nil {
 			return result, err
 		}
 
 		// normal users cannot specify memberIDs to filter for - ignore if set
-		unchecked, err := g.findGroupsLowlevel(ctx, minSize, maxSize, nil)
+		unchecked, err := g.findGroupsFullAccess(ctx, minSize, maxSize, nil)
 		if err != nil {
 			return result, err
 		}
 
-		// filter result list for visibility
-		// if not public, only show the group if user is in it
-		// if public, show the group but filter out member info
 		for _, group := range unchecked {
-			if groupContains(group, attendee.ID) || groupInvited(group, attendee.ID) || groupHasFlag(group, "public") {
-				result = append(result, publicInfo(group, attendee.ID))
+			filtered := g.filterGroupAndFieldVisibilityForAttendee(group, attendee)
+			if filtered != nil {
+				result = append(result, filtered)
 			}
 		}
 
@@ -94,7 +99,10 @@ func (g *groupService) FindGroups(ctx context.Context, minSize uint, maxSize int
 	}
 }
 
-func (g *groupService) findGroupsLowlevel(ctx context.Context, minSize uint, maxSize int, memberIDs []int64) ([]*modelsv1.Group, error) {
+// findGroupsFullAccess searches for groups without permission checks.
+//
+// It returns all matching groups unfiltered, mapped to the API model with all fields visible.
+func (g *groupService) findGroupsFullAccess(ctx context.Context, minSize uint, maxSize int, memberIDs []int64) ([]*modelsv1.Group, error) {
 	result := make([]*modelsv1.Group, 0)
 
 	groupIDs, err := g.DB.FindGroups(ctx, minSize, maxSize, memberIDs)
@@ -108,7 +116,7 @@ func (g *groupService) findGroupsLowlevel(ctx context.Context, minSize uint, max
 	}
 
 	for _, id := range groupIDs {
-		group, err := g.GetGroupByID(ctx, id)
+		group, err := g.getGroupByIDFullAccess(ctx, id)
 		if err != nil {
 			if !errors.Is(err, gorm.ErrRecordNotFound) {
 				aulogging.WarnErrf(ctx, err, "find groups failed to read group %s - maybe intermittent change: %s", id, err.Error())
@@ -122,7 +130,7 @@ func (g *groupService) findGroupsLowlevel(ctx context.Context, minSize uint, max
 	return result, nil
 }
 
-// GetGroupByID attempts to retrieve a group and its members from the database by a given ID.
+// GetGroupByID retrieves a group and its members and invites from the database by a given ID.
 func (g *groupService) GetGroupByID(ctx context.Context, groupID string) (*modelsv1.Group, error) {
 	validator, err := rbac.NewValidator(ctx)
 	if err != nil {
@@ -132,16 +140,34 @@ func (g *groupService) GetGroupByID(ctx context.Context, groupID string) (*model
 
 	if validator.IsAdmin() {
 		// admins are allowed access
+		return g.getGroupByIDFullAccess(ctx, groupID)
 	} else if validator.IsUser() {
 		// ensure attending registration
-		_, err := g.loggedInUserValidRegistrationBadgeNo(ctx)
+		attendee, err := g.loggedInUserValidRegistration(ctx)
+		if err != nil {
+			return nil, err
+		}
+
+		group, err := g.getGroupByIDFullAccess(ctx, groupID)
 		if err != nil {
 			return nil, err
 		}
+
+		filtered := g.filterGroupAndFieldVisibilityForAttendee(group, attendee)
+		if filtered == nil {
+			return nil, errNoAccess(ctx)
+		}
+
+		return filtered, nil
 	} else {
 		return nil, errNotAttending(ctx) // shouldn't ever happen, just in case
 	}
+}
 
+// getGroupByIDFullAccess retrieves a group and its members and invites from the database by a given ID.
+//
+// It does so without permission checks and returns all group fields unfiltered.
+func (g *groupService) getGroupByIDFullAccess(ctx context.Context, groupID string) (*modelsv1.Group, error) {
 	grp, err := g.DB.GetGroupByID(ctx, groupID)
 	if err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
@@ -154,8 +180,11 @@ func (g *groupService) GetGroupByID(ctx context.Context, groupID string) (*model
 	groupMembers, err := g.DB.GetGroupMembersByGroupID(ctx, groupID)
 	if err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
+			// this is an error because a group should always contain its owner, or should have been deleted
 			return nil, errGroupHasNoMembers(ctx)
 		}
+
+		return nil, errGroupRead(ctx, err.Error())
 	}
 
 	return &modelsv1.Group{
@@ -194,7 +223,7 @@ func (g *groupService) CreateGroup(ctx context.Context, group *modelsv1.GroupCre
 		}
 	}
 	if ownerID == 0 {
-		attendee, err := g.loggedInUserValidRegistrationBadgeNo(ctx)
+		attendee, err := g.loggedInUserValidRegistration(ctx)
 		if err != nil {
 			return "", err
 		}
@@ -299,7 +328,7 @@ func (g *groupService) UpdateGroup(ctx context.Context, group *modelsv1.Group) e
 			group.Owner = dbGroup.Owner
 		}
 	} else if validator.IsUser() {
-		attendee, err := g.loggedInUserValidRegistrationBadgeNo(ctx)
+		attendee, err := g.loggedInUserValidRegistration(ctx)
 		if err != nil {
 			return err
 		}
@@ -382,7 +411,7 @@ func (g *groupService) DeleteGroup(ctx context.Context, groupID string) error {
 	if validator.IsAdmin() || validator.IsAPITokenCall() {
 		// admins and api token are allowed to make changes to any group
 	} else if validator.IsUser() {
-		attendee, err := g.loggedInUserValidRegistrationBadgeNo(ctx)
+		attendee, err := g.loggedInUserValidRegistration(ctx)
 		if err != nil {
 			return err
 		}
@@ -497,6 +526,10 @@ func errNotAttending(ctx context.Context) error {
 	return common.NewForbidden(ctx, common.NotAttending, common.Details("access denied - you must have a valid registration in status approved, (partially) paid, checked in"))
 }
 
+func errNoAccess(ctx context.Context) error {
+	return common.NewForbidden(ctx, common.AuthForbidden, common.Details("access denied - you do not have access to this group"))
+}
+
 func errGroupRead(ctx context.Context, details string) error {
 	return common.NewInternalServerError(ctx, common.GroupReadError, common.Details(details))
 }

internal/service/groups/helpers.go

@@ -9,7 +9,68 @@ import (
 	"github.com/eurofurence/reg-room-service/internal/repository/downstreams/attendeeservice"
 )
 
-func (g *groupService) loggedInUserValidRegistrationBadgeNo(ctx context.Context) (attendeeservice.Attendee, error) {
+// filterGroupAndFieldVisibilityForAttendee takes a fully populated group model, and filters it using
+// the visibility rules for a regular attendee (non-admin).
+//
+// Warning: **may return nil** if the attendee is not allowed to see the group at all. In this case, the
+// calling function should return a proper error, or just omit the group from the result listing.
+func (g *groupService) filterGroupAndFieldVisibilityForAttendee(group *modelsv1.Group, attendee attendeeservice.Attendee) *modelsv1.Group {
+	if group == nil || attendee.ID <= 0 {
+		return nil
+	}
+
+	if group.Owner == attendee.ID {
+		// owner can see all group info
+		return group
+	} else if groupContains(group, attendee.ID) {
+		// group members can see all group info, but no invites
+		return &modelsv1.Group{
+			ID:          group.ID,
+			Name:        group.Name,
+			Flags:       group.Flags,
+			Comments:    group.Comments,
+			MaximumSize: group.MaximumSize,
+			Owner:       group.Owner,
+			Members:     group.Members,
+			Invites:     nil,
+		}
+	} else if groupInvited(group, attendee.ID) {
+		// group invitees can see masked members and only their own invite
+		return &modelsv1.Group{
+			ID:          group.ID,
+			Name:        group.Name,
+			Flags:       group.Flags,
+			Comments:    nil, // hide comment
+			MaximumSize: group.MaximumSize,
+			Owner:       group.Owner,
+			Members:     maskMembers(group.Members, attendee.ID),
+			Invites:     filterInvites(group.Invites, attendee.ID),
+		}
+	} else if groupHasFlag(group, "public") {
+		// non-members get even less information
+		return &modelsv1.Group{
+			ID:          group.ID,
+			Name:        group.Name,
+			Flags:       group.Flags,
+			Comments:    nil, // hide comment
+			MaximumSize: group.MaximumSize,
+			Owner:       group.Owner,
+			Members:     maskMembers(group.Members, attendee.ID),
+			Invites:     nil,
+		}
+	} else {
+		return nil
+	}
+}
+
+// loggedInUserValidRegistration obtains the attendee record for the currently logged-in user,
+// but only if they have a valid registration with attending status.
+//
+// It will return a suitable common.APIError if no registration or not in attending status
+// (or if attendee service fails to respond).
+//
+// Here, admins are treated exactly the same as normal users.
+func (g *groupService) loggedInUserValidRegistration(ctx context.Context) (attendeeservice.Attendee, error) {
 	myRegIDs, err := g.AttSrv.ListMyRegistrationIds(ctx)
 	if err != nil {
 		aulogging.WarnErrf(ctx, err, "failed to obtain registrations for currently logged in user: %s", err.Error())
@@ -66,23 +127,6 @@ func allowedFlags() []string {
 	return conf.Service.GroupFlags
 }
 
-func publicInfo(grp *modelsv1.Group, myID int64) *modelsv1.Group {
-	if grp == nil {
-		return nil
-	}
-
-	return &modelsv1.Group{
-		ID:          grp.ID,
-		Name:        grp.Name,
-		Flags:       grp.Flags,
-		Comments:    nil,
-		MaximumSize: grp.MaximumSize,
-		Owner:       0,
-		Members:     maskMembers(grp.Members, myID),
-		Invites:     filterInvites(grp.Invites, myID),
-	}
-}
-
 func maskMembers(members []modelsv1.Member, myID int64) []modelsv1.Member {
 	result := make([]modelsv1.Member, 0)
 	for _, member := range members {
@@ -102,7 +146,6 @@ func filterInvites(members []modelsv1.Member, myID int64) []modelsv1.Member {
 	for _, member := range members {
 		if member.ID == myID {
 			// can only see myself if invited
-			// TODO filter in result list of groups if banned instead
 			result = append(result, member)
 		}
 	}