diff --git a/components/backend/internal/service/submission_read_gate_test.go b/components/backend/internal/service/submission_read_gate_test.go new file mode 100644 index 00000000..944475ad --- /dev/null +++ b/components/backend/internal/service/submission_read_gate_test.go @@ -0,0 +1,105 @@ +//go:build test && unittest + +package service_test + +import ( + "context" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/timestamppb" + + entuser "github.com/swissdatasciencecenter/hackagon/components/backend/ent/user" + hackathonSvc "github.com/swissdatasciencecenter/hackagon/components/backend/internal/proto/hackathon" + ents "github.com/swissdatasciencecenter/hackagon/components/backend/internal/proto/hackathon/entities" + msgs "github.com/swissdatasciencecenter/hackagon/components/backend/internal/proto/hackathon/messages/hackathon_svc" + projectMsgs "github.com/swissdatasciencecenter/hackagon/components/backend/internal/proto/hackathon/messages/project_svc" + teamMsgs "github.com/swissdatasciencecenter/hackagon/components/backend/internal/proto/hackathon/messages/team_svc" + "github.com/swissdatasciencecenter/hackagon/components/backend/internal/testutils" +) + +// D5: a waitlisted registrant holds the Member role (granted at Join so they can +// propose) and Member carries hackathon-wide Submission:Read — so without the +// gate an unapproved registrant could read every team's work. The gate excludes +// callers who hold a waitlisted participant row; approval lifts it. +var _ = Describe("Submission read waitlist gate (D5)", func() { + ctxFor := func(kc string) context.Context { + return metadata.NewOutgoingContext( + context.Background(), + metadata.Pairs("authorization", "Bearer "+testutils.CreateTestJWTToken(kc)), + ) + } + + It("D5 refuses a waitlisted registrant, and approval lets them past", func() { + dbClient, conn, _ := testutils.CreateTestServer() + hackathonClient := hackathonSvc.NewHackathonServiceClient(conn) + projectClient := hackathonSvc.NewProjectServiceClient(conn) + teamClient := hackathonSvc.NewTeamServiceClient(conn) + + adminCtx := ctxFor(testutils.TestAdminKeycloakID) + now := time.Now() + h, err := hackathonClient.Create(adminCtx, &msgs.CreateRequest{ + Name: "Sub Read Gate", + Description: testutils.StringPtr("d"), + Visibility: ents.Visibility_VISIBILITY_PUBLIC, + StartsAt: timestamppb.New(now.Add(24 * time.Hour)), + EndsAt: timestamppb.New(now.Add(48 * time.Hour)), + }) + Expect(err).NotTo(HaveOccurred()) + hackathonID := h.GetHackathonId() + _, err = hackathonClient.EditSettings(adminCtx, &msgs.EditSettingsRequest{ + HackathonId: hackathonID, + RegistrationsEnabled: testutils.BoolPtr(true), + }) + Expect(err).NotTo(HaveOccurred()) + + // A team with a project. No submission is needed: the read gate runs + // before the submission query, so the code it returns is what is under + // test (PermissionDenied from the gate vs NotFound past it). + p, err := projectClient.Propose(adminCtx, &projectMsgs.ProposeRequest{ + HackathonId: hackathonID, + Title: "Gate Project", + Description: "desc", + }) + Expect(err).NotTo(HaveOccurred()) + t, err := teamClient.Create(adminCtx, &teamMsgs.CreateRequest{ + Name: "Gate Team", + ProjectId: p.GetProjectId(), + }) + Expect(err).NotTo(HaveOccurred()) + teamID := t.GetTeamId() + + // An outsider who joins (waitlisted) and is not on the team. + outsider := "d5-outsider" + _, err = dbClient.User.Create(). + SetKeycloakID(outsider).SetUsername(outsider).Save(context.Background()) + Expect(err).NotTo(HaveOccurred()) + outsiderCtx := ctxFor(outsider) + _, err = hackathonClient.Join(outsiderCtx, &msgs.JoinRequest{HackathonId: hackathonID}) + Expect(err).NotTo(HaveOccurred()) + + // Waitlisted → refused by the gate. + _, err = teamClient.GetSubmission(outsiderCtx, + &teamMsgs.GetSubmissionRequest{TeamId: teamID}) + Expect(status.Code(err)).To(Equal(codes.PermissionDenied)) + + // Approve → confirmed participant → past the gate; there is no submission, + // so NotFound. A different code is the proof the gate opened on approval. + outsiderUser, err := dbClient.User.Query(). + Where(entuser.KeycloakIDEQ(outsider)).Only(context.Background()) + Expect(err).NotTo(HaveOccurred()) + _, err = hackathonClient.ApproveParticipant(adminCtx, &msgs.ApproveParticipantRequest{ + HackathonId: hackathonID, + UserId: outsiderUser.ID.String(), + }) + Expect(err).NotTo(HaveOccurred()) + + _, err = teamClient.GetSubmission(outsiderCtx, + &teamMsgs.GetSubmissionRequest{TeamId: teamID}) + Expect(status.Code(err)).To(Equal(codes.NotFound)) + }) +}) diff --git a/components/backend/internal/service/team_service.go b/components/backend/internal/service/team_service.go index da66135f..0c44c282 100644 --- a/components/backend/internal/service/team_service.go +++ b/components/backend/internal/service/team_service.go @@ -9,6 +9,7 @@ import ( "github.com/swissdatasciencecenter/hackagon/components/backend/ent" enthackathon "github.com/swissdatasciencecenter/hackagon/components/backend/ent/hackathon" enthackathonforms "github.com/swissdatasciencecenter/hackagon/components/backend/ent/hackathonforms" + entparticipant "github.com/swissdatasciencecenter/hackagon/components/backend/ent/participant" entproject "github.com/swissdatasciencecenter/hackagon/components/backend/ent/project" entsubmission "github.com/swissdatasciencecenter/hackagon/components/backend/ent/submission" entteam "github.com/swissdatasciencecenter/hackagon/components/backend/ent/team" @@ -585,6 +586,52 @@ func (s *TeamService) CreateSubmission( return &msgs.CreateSubmissionResponse{Id: subm.ID.String()}, nil } +// authorizeSubmissionRead decides who may read a team's submissions. A member of +// THIS team reads their own team's, team-scoped. The hackathon-wide grant — which +// exists so participants can read every team's work to vote on it — is for +// CONFIRMED participants only (D5): a waitlisted registrant holds the Member role +// (granted at Join so they can propose) but has not been approved into the event, +// so without this an unapproved registrant could read every team's submissions. +// Organizers and admins reach the hackathon-wide grant via Owner/admin, not a +// participant row, so they are unaffected. +func (s *TeamService) authorizeSubmissionRead( + ctx context.Context, + hackathonID, teamID uuid.UUID, +) error { + if err := s.enforcer.RequirePermission( + ctx, hackathonID.String(), m.Submission, m.Read, m.WithTeam(teamID.String()), + ); err == nil { + return nil + } + if err := s.enforcer.RequirePermission( + ctx, hackathonID.String(), m.Submission, m.Read, + ); err != nil { + return err + } + uid, _, err := m.RequireSubject(ctx) + if err != nil { + return err + } + waitlisted, err := s.dbClient.Participant.Query(). + Where( + entparticipant.HasUserWith(entuser.KeycloakIDEQ(uid)), + entparticipant.HasHackathonWith(enthackathon.IDEQ(hackathonID)), + entparticipant.IsWaiting(true), + ). + Exist(ctx) + if err != nil { + slog.Error("query participant for submission read", "err", err) + + return status.Error(codes.Internal, "couldn't query database") + } + if waitlisted { + return status.Error(codes.PermissionDenied, + "your registration is still on the waiting list") + } + + return nil +} + func (s *TeamService) GetSubmission( ctx context.Context, req *msgs.GetSubmissionRequest, @@ -603,16 +650,10 @@ func (s *TeamService) GetSubmission( return nil, status.Error(codes.Internal, "team project or hackathon not found") } - hackathonID := t.Edges.Project.Edges.Hackathon.ID.String() - if err := s.enforcer.RequirePermission( - ctx, hackathonID, m.Submission, m.Read, - m.WithTeam(t.ID.String()), + if err := s.authorizeSubmissionRead( + ctx, t.Edges.Project.Edges.Hackathon.ID, t.ID, ); err != nil { - if err := s.enforcer.RequirePermission( - ctx, hackathonID, m.Submission, m.Read, - ); err != nil { - return nil, err - } + return nil, err } // Find the latest submission for this team (highest version). @@ -656,16 +697,10 @@ func (s *TeamService) ListSubmissions( return nil, status.Error(codes.Internal, "team project or hackathon not found") } - hackathonID := t.Edges.Project.Edges.Hackathon.ID.String() - if err := s.enforcer.RequirePermission( - ctx, hackathonID, m.Submission, m.Read, - m.WithTeam(t.ID.String()), + if err := s.authorizeSubmissionRead( + ctx, t.Edges.Project.Edges.Hackathon.ID, t.ID, ); err != nil { - if err := s.enforcer.RequirePermission( - ctx, hackathonID, m.Submission, m.Read, - ); err != nil { - return nil, err - } + return nil, err } submissions, err := s.dbClient.Submission.Query(). diff --git a/components/backend/internal/service/vote_service.go b/components/backend/internal/service/vote_service.go index 72237313..51a949f6 100644 --- a/components/backend/internal/service/vote_service.go +++ b/components/backend/internal/service/vote_service.go @@ -1253,7 +1253,13 @@ func (s *VoteService) ListVoteResults( // // requireCapability lets organisers through regardless, which is what makes // reviewing a tally before publishing it possible. - if _, _, err := m.RequireSubject(ctx); err != nil { + // + // RequireUser, not RequireSubject: the comment above says "any signed-in + // user", but RequireSubject admits the anonymous subject, so an unauthenticated + // caller could read placements the moment VIEW_RESULTS was open. Results are + // an in-event surface; there is no anonymous caller of ListVoteResults (only + // the authenticated /my/hackathon/[id]/voting route reads it). + if _, _, err := m.RequireUser(ctx); err != nil { return nil, err } categoryID, err := uuid.Parse(req.GetCategoryId())