diff --git a/components/backend/internal/service/hackathon_service.go b/components/backend/internal/service/hackathon_service.go index 4445bc69..52e3b8cf 100644 --- a/components/backend/internal/service/hackathon_service.go +++ b/components/backend/internal/service/hackathon_service.go @@ -88,6 +88,15 @@ func (s *HackathonService) Create( return nil, status.Errorf(codes.Internal, "couldn't create hackathon in database") } + // set permission based on visibility + if visibility == enthackathon.VisibilityPublic { + _, err = s.enforcer.AllowPublicHackathonAccess(h.ID.String()) + if err != nil { + slog.Error("create hackathon", "err", err) + return nil, status.Error(codes.Internal, "couldn't set hackathon permission") + } + } + // Create default state (all capabilities disabled). _, err = s.dbClient.HackathonState.Create(). SetHackathonID(h.ID). @@ -600,6 +609,23 @@ func (s *HackathonService) Edit( return nil, status.Error(codes.Internal, "couldn't query updated hackathon") } + //nolint:nestif // Complexity is ok here. + if req.Visibility != nil { + if updated.Visibility == enthackathon.VisibilityPublic { + _, err = s.enforcer.AllowPublicHackathonAccess(h.ID.String()) + if err != nil { + slog.Error("edit hackathon", "err", err) + return nil, status.Error(codes.Internal, "couldn't change hackathon permission") + } + } else { + _, err = s.enforcer.RemovePublicHackathonAccess(h.ID.String()) + if err != nil { + slog.Error("edit hackathon", "err", err) + return nil, status.Error(codes.Internal, "couldn't change hackathon permission") + } + } + } + entry := hackathonEntryFromEnt(updated, time.Now()) entry.Creator = userEntryFromEnt(updated.Edges.Creator) entry.Modifier = userEntryFromEnt(updated.Edges.Modifier) @@ -1423,7 +1449,6 @@ func (s *HackathonService) ListQuestions( if err != nil { return nil, status.Errorf(codes.InvalidArgument, "invalid hackathon_id: %v", err) } - if err := s.enforcer.RequirePermission(ctx, id.String(), mw.Hackathon, mw.Read); err != nil { return nil, err } @@ -1496,7 +1521,10 @@ func (s *HackathonService) SubmitAnswers( ).Only(ctx) if err != nil { if ent.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "user is not a participant in this hackathon") + return nil, status.Errorf( + codes.PermissionDenied, + "user is not a participant in this hackathon", + ) } slog.Error("query participant", "err", err) return nil, status.Error(codes.Internal, "couldn't query participant") @@ -1562,18 +1590,23 @@ func (s *HackathonService) ListParticipantAnswers( entanswer.HasQuestionWith( entquestion.HasHackathonWith(enthackathon.IDEQ(hackID)), ), - ) + ).WithQuestion() if req.GetUserId() != "" { uidParsed, err := uuid.Parse(req.GetUserId()) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "invalid user_id: %v", err) } + + if uidParsed != user.ID && !hasWrite { + return nil, status.Error(codes.PermissionDenied, "permission denied") + } q = q.Where(entanswer.HasUserWith( entuser.ID(uidParsed), )) - } else if !hasWrite { - // No write access: filter by requester's own answers + } + if !hasWrite { + // only return own user if user does not have write access q = q.Where(entanswer.HasUserWith( entuser.ID(user.ID), )) diff --git a/components/backend/internal/service/hackathon_service_test.go b/components/backend/internal/service/hackathon_service_test.go index 3e1fb300..c267bf6b 100644 --- a/components/backend/internal/service/hackathon_service_test.go +++ b/components/backend/internal/service/hackathon_service_test.go @@ -2067,12 +2067,12 @@ var _ = Describe("HackathonService", func() { Expect(err).NotTo(HaveOccurred()) Expect(h.Edges.Owners).To(HaveLen(1)) // only creator remains - // Verify casbin role was revoked (owner can no longer read hackathon) + // Verify casbin role was revoked (owner can no longer write hackathon) ok, err := enf.CheckPermission( ownerToRemove.KeycloakID, createdHackathonID, middleware.Hackathon, - middleware.Read, + middleware.Write, ) Expect(err).NotTo(HaveOccurred()) Expect(ok).To(BeFalse()) @@ -2525,6 +2525,31 @@ var _ = Describe("HackathonService", func() { // --- ListQuestions --- Describe("ListQuestions", func() { + // Regression: this required `hackathon:read`, which a non-member does + // not hold. Join validates the mandatory answers and answering needs + // the questions, so a hackathon asking anything mandatory could not be + // joined by anyone at all. + It("serves a non-member, so signup is not deadlocked", func() { + _, err := dbClient.User.Create(). + SetKeycloakID("lq-outsider"). + SetUsername("lq-outsider"). + Save(context.Background()) + Expect(err).NotTo(HaveOccurred()) + + outsiderCtx := metadata.NewOutgoingContext( + context.Background(), + metadata.Pairs( + "authorization", + "Bearer "+testutils.CreateTestJWTToken("lq-outsider"), + ), + ) + resp, err := client.ListQuestions(outsiderCtx, &msgs.ListQuestionsRequest{ + HackathonId: hackathonID, + }) + Expect(err).NotTo(HaveOccurred()) + Expect(resp.GetQuestions()).NotTo(BeEmpty()) + }) + BeforeEach(func() { token := testutils.CreateTestJWTToken(testAdmin) ctx := metadata.NewOutgoingContext( @@ -2633,9 +2658,26 @@ var _ = Describe("HackathonService", func() { Expect(resp.GetQuestions()).To(BeEmpty()) }) - It("requires Read permission", func() { + It("requires Read permission on a private hackathon", func() { + // Narrowed from "requires Read" outright: a public hackathon's + // questions must be answerable before Join, and a non-member holds + // no read grant — so requiring one deadlocked signup. A private + // event still refuses an uninvited caller. + adminCtx := metadata.NewOutgoingContext( + context.Background(), + metadata.Pairs( + "authorization", + "Bearer "+testutils.CreateTestJWTToken(testAdmin), + ), + ) + priv, err := client.Create(adminCtx, &msgs.CreateRequest{ + Name: "QA Private Questions Hackathon", + Visibility: entities.Visibility_VISIBILITY_PRIVATE, + }) + Expect(err).NotTo(HaveOccurred()) + nonOwnerKeycloakID := "non-owner-list-q" - _, err := dbClient.User.Create(). + _, err = dbClient.User.Create(). SetKeycloakID(nonOwnerKeycloakID). SetUsername("non-owner-list-q-username"). Save(context.Background()) @@ -2648,7 +2690,7 @@ var _ = Describe("HackathonService", func() { ) _, err = client.ListQuestions(ctx, &msgs.ListQuestionsRequest{ - HackathonId: hackathonID, + HackathonId: priv.GetHackathonId(), }) Expect(err).To(HaveOccurred()) st := status.Convert(err) @@ -3190,10 +3232,10 @@ var _ = Describe("HackathonService", func() { Expect(err).NotTo(HaveOccurred()) // Grant owner role so they have Read permission - _, err = dbClient.Hackathon.Update(). - Where(enthackathon.IDEQ(uuid.MustParse(hackathonID))). - AddOwners(user). - Save(context.Background()) + _, err = client.AddOwner(ctx, &msgs.AddOwnerRequest{ + HackathonId: hackathonID, + UserId: user.ID.String(), + }) Expect(err).NotTo(HaveOccurred()) // Also create a participant record for the admin user so admin token works @@ -3380,19 +3422,6 @@ var _ = Describe("HackathonService", func() { metadata.Pairs("authorization", "Bearer "+token), ) - // Create participant for this user - user, err := dbClient.User.Query(). - Where(entuser.KeycloakIDEQ(nonOwnerKeycloakID)). - Only(context.Background()) - Expect(err).NotTo(HaveOccurred()) - - _, err = dbClient.Participant.Create(). - SetHackathonID(uuid.MustParse(hackathonID)). - SetUserID(user.ID). - SetIsWaiting(false). - Save(context.Background()) - Expect(err).NotTo(HaveOccurred()) - _, err = client.SubmitAnswers(ctx, &msgs.SubmitAnswersRequest{ HackathonId: hackathonID, Answers: []*entities.Answer{ @@ -3457,6 +3486,95 @@ var _ = Describe("HackathonService", func() { Expect(err).NotTo(HaveOccurred()) }) + // Regression: `hasWrite` was computed but only consulted on the + // "no user_id" path, so naming any user id skipped the check entirely + // and returned what that person wrote about themselves. + It("refuses a non-organizer naming someone else", func() { + _, err := dbClient.User.Create(). + SetKeycloakID("lpa-outsider"). + SetUsername("lpa-outsider"). + Save(context.Background()) + Expect(err).NotTo(HaveOccurred()) + + outsiderCtx := metadata.NewOutgoingContext( + context.Background(), + metadata.Pairs( + "authorization", + "Bearer "+testutils.CreateTestJWTToken("lpa-outsider"), + ), + ) + _, err = client.ListParticipantAnswers( + outsiderCtx, + &msgs.ListParticipantAnswersRequest{ + HackathonId: hackathonID, + UserId: testutils.StringPtr(participantUser.ID.String()), + }) + Expect(status.Convert(err).Code()).To(Equal(codes.PermissionDenied)) + }) + + // Regression: the mapper always emitted text_value, so a BOOL answer + // read back as text — and SubmitAnswers refuses a text answer to a + // bool question. Loading a form and saving it unchanged therefore + // failed on every bool question, which is what an edit does on save. + It("reads a bool answer back as a bool, so a round trip re-submits", func() { + adminCtx := metadata.NewOutgoingContext( + context.Background(), + metadata.Pairs( + "authorization", + "Bearer "+testutils.CreateTestJWTToken(testAdmin), + ), + ) + boolQ, err := client.CreateQuestion(adminCtx, &msgs.CreateQuestionRequest{ + HackathonId: hackathonID, + Key: "conduct", + Label: "I accept the Code of Conduct", + Type: entities.QuestionType_QUESTION_TYPE_BOOL, + Order: 2, + }) + Expect(err).NotTo(HaveOccurred()) + + _, err = dbClient.Answer.Create(). + SetQuestionID(uuid.MustParse(boolQ.GetQuestionId())). + SetUserID(participantUser.ID). + SetValue("true"). + Save(context.Background()) + Expect(err).NotTo(HaveOccurred()) + + // The fixture writes the Participant row directly, so unlike a real + // Join it grants no casbin Member role — and SubmitAnswers checks + // hackathon:read. + _, err = enf.AddRole("list-answers-user", middleware.Member, hackathonID) + Expect(err).NotTo(HaveOccurred()) + + participantCtx := metadata.NewOutgoingContext( + context.Background(), + metadata.Pairs( + "authorization", + "Bearer "+testutils.CreateTestJWTToken("list-answers-user"), + ), + ) + resp, err := client.ListParticipantAnswers( + participantCtx, + &msgs.ListParticipantAnswersRequest{HackathonId: hackathonID}) + Expect(err).NotTo(HaveOccurred()) + + var readBack *entities.Answer + for _, a := range resp.GetAnswers() { + if a.GetQuestionId() == boolQ.GetQuestionId() { + readBack = a + } + } + Expect(readBack).NotTo(BeNil()) + Expect(readBack.GetBoolValue()).To(BeTrue()) + + // Hand straight back what was read: this is the save an edit form + // performs, and it used to fail with InvalidArgument. + _, err = client.SubmitAnswers(participantCtx, &msgs.SubmitAnswersRequest{ + HackathonId: hackathonID, + Answers: resp.GetAnswers(), + }) + Expect(err).NotTo(HaveOccurred()) + }) It("returns answers for requester's own answers (no write access)", func() { token := testutils.CreateTestJWTToken("list-answers-user") ctx := metadata.NewOutgoingContext( diff --git a/components/backend/internal/service/mappers.go b/components/backend/internal/service/mappers.go index 78d4a4de..31ad08c9 100644 --- a/components/backend/internal/service/mappers.go +++ b/components/backend/internal/service/mappers.go @@ -530,11 +530,16 @@ func questionEntryFromEnt(q *ent.Question) *hackEnts.Question { } func answerEntryFromEnt(a *ent.Answer) *hackEnts.Answer { - return &hackEnts.Answer{ + entry := &hackEnts.Answer{ QuestionId: a.QuestionID.String(), ParticipantId: a.UserID.String(), Value: &hackEnts.Answer_TextValue{TextValue: a.Value}, } + if a.Edges.Question != nil && a.Edges.Question.DataType == entquestion.DataTypeBool { + entry.Value = &hackEnts.Answer_BoolValue{BoolValue: a.Value == "true"} + } + + return entry } // protoAnswerValueToDB extracts the string representation from a proto Answer.