diff --git a/api/openapi-spec/openapi.yaml b/api/openapi-spec/openapi.yaml index 4142b55..82b4ba7 100644 --- a/api/openapi-spec/openapi.yaml +++ b/api/openapi-spec/openapi.yaml @@ -121,6 +121,12 @@ paths: type: integer example: 4 default: -1 + - name: show + in: query + description: set to 'public' (the only possible value other than omitting this parameter) to list only groups available for joining. Mandatory for ordinary non-admin users. May be set for admins to essentially treat them like a normal user looking for a public group to join. Note that public groups may be disabled in the configuration, in which case this request will always return an empty list. + schema: + type: string + example: public responses: '200': description: successful operation @@ -372,6 +378,12 @@ paths: application/json: schema: $ref: '#/components/schemas/Error' + '502': + description: The attendee service failed to respond when asked for the user's registrations. + content: + application/json: + schema: + $ref: '#/components/schemas/Error' security: - BearerAuth: [] - ApiKeyAuth: [] @@ -442,6 +454,12 @@ paths: application/json: schema: $ref: '#/components/schemas/Error' + '502': + description: The attendee service failed to respond when asked for the user's registrations. + content: + application/json: + schema: + $ref: '#/components/schemas/Error' security: - BearerAuth: [] - ApiKeyAuth: [] @@ -497,6 +515,12 @@ paths: application/json: schema: $ref: '#/components/schemas/Error' + '502': + description: The attendee service failed to respond when asked for the user's registrations. + content: + application/json: + schema: + $ref: '#/components/schemas/Error' security: - BearerAuth: [] - ApiKeyAuth: [] @@ -634,6 +658,12 @@ paths: application/json: schema: $ref: '#/components/schemas/Error' + '502': + description: The attendee service failed to respond when asked for the user's registrations. + content: + application/json: + schema: + $ref: '#/components/schemas/Error' security: - BearerAuth: [] - ApiKeyAuth: [] diff --git a/internal/controller/v1/groupsctl/groups_get.go b/internal/controller/v1/groupsctl/groups_get.go index 23235df..f9e8ff6 100644 --- a/internal/controller/v1/groupsctl/groups_get.go +++ b/internal/controller/v1/groupsctl/groups_get.go @@ -15,13 +15,14 @@ import ( ) type ListGroupsRequest struct { - MemberIDs []int64 - MinSize uint - MaxSize int + MemberIDs []int64 + MinSize uint + MaxSize int + PublicOnly bool } func (h *Controller) ListGroups(ctx context.Context, req *ListGroupsRequest, w http.ResponseWriter) (*modelsv1.GroupList, error) { - groups, err := h.svc.FindGroups(ctx, req.MinSize, req.MaxSize, req.MemberIDs) + groups, err := h.svc.FindGroups(ctx, req.MinSize, req.MaxSize, req.MemberIDs, req.PublicOnly) if err != nil { return nil, err } @@ -67,6 +68,14 @@ func (h *Controller) ListGroupsRequest(r *http.Request, w http.ResponseWriter) ( req.MaxSize = -1 } + if show := query.Get("show"); show != "" { + if show == "public" { + req.PublicOnly = true + } else { + return nil, common.NewBadRequest(ctx, common.RequestParseFailed, common.Details("show parameter should be 'public' or missing")) + } + } + return &req, nil } diff --git a/internal/service/groups/groups.go b/internal/service/groups/groups.go index 16995ff..a13f585 100644 --- a/internal/service/groups/groups.go +++ b/internal/service/groups/groups.go @@ -30,7 +30,7 @@ func (g *groupService) FindMyGroup(ctx context.Context) (*modelsv1.Group, error) return nil, err } - groups, err := g.findGroupsFullAccess(ctx, 0, -1, []int64{attendee.ID}) + groups, err := g.findGroupsFullAccess(ctx, 0, -1, []int64{attendee.ID}, false) if err != nil { return nil, err } @@ -62,18 +62,26 @@ func (g *groupService) FindMyGroup(ctx context.Context) (*modelsv1.Group, error) // Normal users: can only see groups visible to them. If public groups are enabled in configuration, // this means all groups that are public and from which the user wasn't banned. Not all fields // will be filled in the results to protect the privacy of group members. -func (g *groupService) FindGroups(ctx context.Context, minSize uint, maxSize int, memberIDs []int64) ([]*modelsv1.Group, error) { +func (g *groupService) FindGroups(ctx context.Context, minSize uint, maxSize int, memberIDs []int64, public bool) ([]*modelsv1.Group, error) { validator, err := rbac.NewValidator(ctx) if err != nil { aulogging.ErrorErrf(ctx, err, "Could not retrieve RBAC validator from context. [error]: %v", err) return make([]*modelsv1.Group, 0), errCouldNotGetValidator(ctx) } - if validator.IsAdmin() || validator.IsAPITokenCall() { - return g.findGroupsFullAccess(ctx, minSize, maxSize, memberIDs) - } else if validator.IsUser() { + permAdmin := (validator.IsAdmin() && !public) || validator.IsAPITokenCall() + permUser := validator.IsUser() || (validator.IsAdmin() && public) + + if permAdmin { + return g.findGroupsFullAccess(ctx, minSize, maxSize, memberIDs, public) + } else if permUser { result := make([]*modelsv1.Group, 0) + // normal users must set the public flag + if !public { + return result, common.NewForbidden(ctx, common.AuthForbidden, common.Details("regular users cannot list private groups, must set show=public")) + } + // ensure attending registration attendee, err := g.loggedInUserValidRegistration(ctx) if err != nil { @@ -81,7 +89,7 @@ func (g *groupService) FindGroups(ctx context.Context, minSize uint, maxSize int } // normal users cannot specify memberIDs to filter for - ignore if set - unchecked, err := g.findGroupsFullAccess(ctx, minSize, maxSize, nil) + unchecked, err := g.findGroupsFullAccess(ctx, minSize, maxSize, nil, true) if err != nil { return result, err } @@ -102,7 +110,7 @@ func (g *groupService) FindGroups(ctx context.Context, minSize uint, maxSize int // 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) { +func (g *groupService) findGroupsFullAccess(ctx context.Context, minSize uint, maxSize int, memberIDs []int64, publicOnly bool) ([]*modelsv1.Group, error) { result := make([]*modelsv1.Group, 0) groupIDs, err := g.DB.FindGroups(ctx, minSize, maxSize, memberIDs) @@ -124,7 +132,13 @@ func (g *groupService) findGroupsFullAccess(ctx context.Context, minSize uint, m } } - result = append(result, group) + if publicOnly { + if groupHasFlag(group, "public") { + result = append(result, group) + } + } else { + result = append(result, group) + } } return result, nil @@ -351,19 +365,22 @@ func (g *groupService) UpdateGroup(ctx context.Context, group *modelsv1.Group) e // TODO check that new group size not too small (counting invitations and members) + // do not touch fields that we do not wish to change, like createdAt or referenced members + dbGroup.Name = group.Name + dbGroup.Flags = fmt.Sprintf(",%s,", strings.Join(group.Flags, ",")) + dbGroup.Comments = common.Deref(group.Comments) + dbGroup.MaximumSize = group.MaximumSize + if dbGroup.Owner != group.Owner { err := g.canChangeGroupOwner(ctx, group) if err != nil { return err } - } - // do not touch fields that we do not wish to change, like createdAt or referenced members - dbGroup.Name = group.Name - dbGroup.Flags = fmt.Sprintf(",%s,", strings.Join(group.Flags, ",")) - dbGroup.Comments = common.Deref(group.Comments) - dbGroup.MaximumSize = group.MaximumSize - dbGroup.Owner = group.Owner + _ = g.sendInfoMails(ctx, "", "group-new-owner", dbGroup, group.Owner, "") + + dbGroup.Owner = group.Owner + } return g.DB.UpdateGroup(ctx, dbGroup) } diff --git a/internal/service/groups/interface.go b/internal/service/groups/interface.go index aaaf8a6..8d3254f 100644 --- a/internal/service/groups/interface.go +++ b/internal/service/groups/interface.go @@ -22,7 +22,7 @@ type Service interface { // The same link will also be included in the email sent to the invited attendee. AddMemberToGroup(ctx context.Context, req *AddGroupMemberParams) (string, error) RemoveMemberFromGroup(ctx context.Context, req *RemoveGroupMemberParams) error - FindGroups(ctx context.Context, minSize uint, maxSize int, memberIDs []int64) ([]*modelsv1.Group, error) + FindGroups(ctx context.Context, minSize uint, maxSize int, memberIDs []int64, public bool) ([]*modelsv1.Group, error) FindMyGroup(ctx context.Context) (*modelsv1.Group, error) } diff --git a/internal/service/groups/members.go b/internal/service/groups/members.go index a53c4db..e38853f 100644 --- a/internal/service/groups/members.go +++ b/internal/service/groups/members.go @@ -87,7 +87,7 @@ func (g *groupService) AddMemberToGroup(ctx context.Context, req *AddGroupMember return "", errGroupWrite(ctx, err.Error()) } - informOwnerTemplate = "group-member-request" + informOwnerTemplate = "group-member-applied" } else if grp.Owner == loggedInAttendee.ID { // owner trying to invite another attendee - check nickname matches @@ -235,7 +235,7 @@ func (g *groupService) RemoveMemberFromGroup(ctx context.Context, req *RemoveGro adjustBan = true if gm.IsInvite { aulogging.Infof(ctx, "declined join request - group %s badge %d by owner %s", req.GroupID, req.BadgeNumber, common.GetSubject(ctx)) - informMemberTemplate = "group-request-declined" // your request to join was declined + informMemberTemplate = "group-application-declined" // your request to join was declined } else { aulogging.Infof(ctx, "kick from group - group %s badge %d by owner %s", req.GroupID, req.BadgeNumber, common.GetSubject(ctx)) informMemberTemplate = "group-member-kicked" @@ -243,7 +243,7 @@ func (g *groupService) RemoveMemberFromGroup(ctx context.Context, req *RemoveGro } else if gm.ID == loggedInAttendee.ID { if gm.IsInvite { aulogging.Infof(ctx, "declined group invitation - group %s badge %d by self", req.GroupID, req.BadgeNumber) - informOwnerTemplate = "group-request-declined" // your request to join was declined + informOwnerTemplate = "group-invitation-declined" // your request to join was declined } else { aulogging.Infof(ctx, "left group - group %s badge %d by self", req.GroupID, req.BadgeNumber) informOwnerTemplate = "group-member-left" // member left @@ -344,6 +344,7 @@ func (g *groupService) sendInfoMails(ctx context.Context, informOwnerTemplate st Variables: map[string]string{ "nickname": member.Nickname, "groupname": grp.Name, + "owner": owner.Nickname, }, } if inviteCode != "" { diff --git a/test/acceptance/groups_list_test.go b/test/acceptance/groups_list_test.go index 31a3f30..0ec9911 100644 --- a/test/acceptance/groups_list_test.go +++ b/test/acceptance/groups_list_test.go @@ -103,6 +103,44 @@ func TestGroupsList_AdminSuccess_Filtered(t *testing.T) { tstEqualResponseBodies(t, expected, actual) } +func TestGroupsList_AdminSuccess_Public(t *testing.T) { + tstSetup(tstDefaultConfigFileRoomGroups) + defer tstShutdown() + + docs.Given("Given two registered attendees with an active registration who are in a group each") + id1 := setupExistingGroup(t, "kittens", true, "101") + _ = setupExistingGroup(t, "puppies", false, "202") + + docs.When("When an admin with an active registration requests to list only public groups") + token := tstValidAdminToken(t) + attMock.SetupRegistered("1234567890", 84, attendeeservice.StatusApproved, "Panther", "panther@example.com") + response := tstPerformGet("/api/rest/v1/groups?show=public", token) + + docs.Then("Then the request is successful and the response includes only the public groups") + actual := modelsv1.GroupList{} + tstRequireSuccessResponse(t, response, http.StatusOK, &actual) + expected := modelsv1.GroupList{ + Groups: []*modelsv1.Group{ + { + ID: id1, + Name: "kittens", + Flags: []string{"public"}, + Comments: nil, // masked + MaximumSize: 6, + Owner: 42, + Members: []modelsv1.Member{ + { + ID: 0, // masked + Nickname: "", // masked + }, + }, + Invites: nil, + }, + }, + } + tstEqualResponseBodies(t, expected, actual) +} + func TestGroupsList_UserSuccess_Public(t *testing.T) { tstSetup(tstDefaultConfigFileRoomGroups) defer tstShutdown() @@ -113,7 +151,7 @@ func TestGroupsList_UserSuccess_Public(t *testing.T) { docs.When("When the attendee not in the group requests to list groups") token := tstValidUserToken(t, 202) - response := tstPerformGet("/api/rest/v1/groups", token) + response := tstPerformGet("/api/rest/v1/groups?show=public", token) docs.Then("Then the request is successful and the response includes only public information about public groups") actual := modelsv1.GroupList{} @@ -138,7 +176,7 @@ func TestGroupsList_UserSuccess_Public(t *testing.T) { tstEqualResponseBodies(t, expected, actual) } -func TestGroupsList_UserSuccess_NonPublic(t *testing.T) { +func TestGroupsList_UserSuccess_NotFindingPrivate(t *testing.T) { tstSetup(tstDefaultConfigFileRoomGroups) defer tstShutdown() @@ -148,7 +186,7 @@ func TestGroupsList_UserSuccess_NonPublic(t *testing.T) { docs.When("When the attendee not in the group requests to list groups") token := tstValidUserToken(t, 202) - response := tstPerformGet("/api/rest/v1/groups", token) + response := tstPerformGet("/api/rest/v1/groups?show=public", token) docs.Then("Then the request is successful but the response does not include the group") actual := modelsv1.GroupList{} @@ -159,6 +197,22 @@ func TestGroupsList_UserSuccess_NonPublic(t *testing.T) { tstEqualResponseBodies(t, expected, actual) } +func TestGroupsList_UserDeny_NonPublic(t *testing.T) { + tstSetup(tstDefaultConfigFileRoomGroups) + defer tstShutdown() + + docs.Given("Given a private group and a registered attendee who is not in the group") + _ = setupExistingGroup(t, "puppies", false, "101") + _ = registerSubject("202") + + docs.When("When the attendee not in the group requests to list groups") + token := tstValidUserToken(t, 202) + response := tstPerformGet("/api/rest/v1/groups", token) + + docs.Then("Then the request is denied and the error is as expected") + tstRequireErrorResponse(t, response, http.StatusForbidden, "auth.forbidden", "regular users cannot list private groups, must set show=public") +} + func TestGroupsList_AnonymousDeny(t *testing.T) { tstSetup(tstDefaultConfigFileRoomGroups) defer tstShutdown() @@ -181,7 +235,7 @@ func TestGroupsList_UserNoReg(t *testing.T) { token := tstValidUserToken(t, 101) docs.When("When they try to list groups") - response := tstPerformGet("/api/rest/v1/groups", token) + response := tstPerformGet("/api/rest/v1/groups?show=public", token) docs.Then("Then the request fails with the expected error") tstRequireErrorResponse(t, response, http.StatusForbidden, "attendee.notfound", "you do not have a valid registration") @@ -196,7 +250,7 @@ func TestGroupsList_UserNonAttendingReg(t *testing.T) { token := tstValidUserToken(t, 101) docs.When("When they try to list groups") - response := tstPerformGet("/api/rest/v1/groups", token) + response := tstPerformGet("/api/rest/v1/groups?show=public", token) docs.Then("Then the request fails with the expected error") tstRequireErrorResponse(t, response, http.StatusForbidden, "attendee.status.not.attending", "registration is not in attending status") diff --git a/test/acceptance/groups_member_add_test.go b/test/acceptance/groups_member_add_test.go index e15bb21..0377655 100644 --- a/test/acceptance/groups_member_add_test.go +++ b/test/acceptance/groups_member_add_test.go @@ -33,7 +33,7 @@ func TestGroupsAddMember_OwnerFirstSuccess(t *testing.T) { docs.Then("And the expected mail has been sent to the invited attendee") tstRequireMailRequests(t, - tstGroupMailToMember("group-invited", "kittens", "1234567890", response.location)) + tstGroupMailToMember("group-invited", "kittens", "1234567890", "101", response.location)) docs.Then("And the personalized join link can be used by the invited attendee") joinResponse := tstPerformPostNoBody(response.location, tstValidUserToken(t, 1234567890)) @@ -65,7 +65,7 @@ func TestGroupsAddMember_AttendeeFirstSuccess(t *testing.T) { docs.Then("And the expected mail is sent to the owner to inform them about the application") tstRequireMailRequests(t, - tstGroupMailToOwner("group-member-request", "kittens", "101", "1234567890")) + tstGroupMailToOwner("group-member-applied", "kittens", "101", "1234567890")) docs.Then("And the owner can accept the application") acceptResponse := tstPerformPostNoBody(response.location, tstValidUserToken(t, 101)) @@ -73,7 +73,7 @@ func TestGroupsAddMember_AttendeeFirstSuccess(t *testing.T) { docs.Then("And the expected mail is then sent to the invited attendee") tstRequireMailRequests(t, - tstGroupMailToMember("group-application-accepted", "kittens", "1234567890", "")) + tstGroupMailToMember("group-application-accepted", "kittens", "1234567890", "101", "")) } func TestGroupsAddMember_AdminForceSuccess(t *testing.T) { diff --git a/test/acceptance/groups_member_remove_test.go b/test/acceptance/groups_member_remove_test.go index 00ee79e..1a31b6b 100644 --- a/test/acceptance/groups_member_remove_test.go +++ b/test/acceptance/groups_member_remove_test.go @@ -31,7 +31,7 @@ func TestGroupsRemoveMember_OwnerSuccess(t *testing.T) { docs.Then("And the expected mail has been sent to the removed attendee") tstRequireMailRequests(t, - tstGroupMailToMember("group-member-kicked", "kittens", "202", "")) + tstGroupMailToMember("group-member-kicked", "kittens", "202", "101", "")) } func TestGroupsRemoveMember_AttendeeSelfSuccess(t *testing.T) { @@ -78,7 +78,7 @@ func TestGroupsRemoveMember_AdminSuccess(t *testing.T) { docs.Then("And the expected emails have been sent") tstRequireMailRequests(t, tstGroupMailToOwner("group-member-removed", "kittens", "101", "202"), - tstGroupMailToMember("group-member-kicked", "kittens", "202", ""), + tstGroupMailToMember("group-member-kicked", "kittens", "202", "101", ""), ) } @@ -104,7 +104,7 @@ func TestGroupsRemoveMember_AttendeeSelfInviteDeclineSuccess(t *testing.T) { docs.Then("And the expected mail is sent to the owner to inform them") tstRequireMailRequests(t, - tstGroupMailToOwner("group-request-declined", "kittens", "101", "202")) + tstGroupMailToOwner("group-invitation-declined", "kittens", "101", "202")) } func TestGroupsRemoveMember_ThirdPartyDeny(t *testing.T) { diff --git a/test/acceptance/groups_update_test.go b/test/acceptance/groups_update_test.go index 7713653..bf1fd52 100644 --- a/test/acceptance/groups_update_test.go +++ b/test/acceptance/groups_update_test.go @@ -178,3 +178,5 @@ func TestGroupsUpdate_NotFound(t *testing.T) { } // TODO change leads to duplicate group name + +// TODO owner change (incl. email check) diff --git a/test/acceptance/testcase_attendees_test.go b/test/acceptance/testcase_attendees_test.go index 694da2c..c765b02 100644 --- a/test/acceptance/testcase_attendees_test.go +++ b/test/acceptance/testcase_attendees_test.go @@ -223,8 +223,9 @@ func tstGroupMailToOwner(cid string, groupName string, target string, object str } } -func tstGroupMailToMember(cid string, groupName string, target string, url string) mailservice.MailSendDto { +func tstGroupMailToMember(cid string, groupName string, target string, owner string, url string) mailservice.MailSendDto { _, targetNick, targetEmail := tstInfosBySubject(target) + _, ownerNick, _ := tstInfosBySubject(owner) result := mailservice.MailSendDto{ CommonID: cid, @@ -233,6 +234,7 @@ func tstGroupMailToMember(cid string, groupName string, target string, url strin Variables: map[string]string{ "nickname": targetNick, "groupname": groupName, + "owner": ownerNick, }, } if url != "" {