Skip to content

Conversation

@maverbiest
Copy link
Contributor

@maverbiest maverbiest commented Jan 13, 2026

resolves #5615

We want to show some information about groups to unauthenticated users, but we don't want to expose contact information or which users belong to the group (see #5615).

The backend functionality for this was already merged in #5647. This PR incorporates the new functionality in the frontend.

What I tested manually

Both locally and on the preview, I can now view group information on the website when I am not logged in. However, I don't see the contact email or the users that belong to a group. When I log in, this information is shown again (same as on origin/main).

Screenshot

Viewing group while logged in (same as before):
Screenshot 2026-01-13 at 10 07 02

Viewing group while not logged in (no email and users, link to login page included in table):
Screenshot 2026-01-14 at 15 10 18

PR Checklist

  • All necessary documentation has been adapted.
  • The implemented feature is covered by appropriate, automated tests.
  • Any manual testing that has been done is documented (i.e. what exactly was tested?)

🚀 Preview: https://group-page-unauth.loculus.org

@maverbiest maverbiest added the preview Triggers a deployment to argocd label Jan 13, 2026
@maverbiest maverbiest added preview Triggers a deployment to argocd and removed preview Triggers a deployment to argocd labels Jan 13, 2026
@maverbiest maverbiest marked this pull request as ready for review January 13, 2026 12:32
@claude
Copy link
Contributor

claude bot commented Jan 13, 2026

Claude finished @maverbiest's task in 2m 9s —— View job


Code Review Complete

This PR successfully implements public group information visibility for unauthenticated users. The implementation is solid overall, with good type safety and test coverage. I've identified several issues that should be addressed before merging.

Summary

Strengths:

Issues Found:

Security & Authorization:

  1. website/src/components/User/GroupPage.tsx:197 - Contact email visibility logic: Currently shows email to ANY authenticated user, not just group members. Verify this matches the intended behavior from Group Page: expose group information (without the emails) for all users even when not logged in #5615.
  2. website/src/pages/group/[groupId]/index.astro:32 - Defense-in-depth concern: Frontend filters sensitive data based on accessToken, but relies on backend to not send it. This is correct but worth documenting.

Logic Issues:
3. website/src/components/User/GroupPage.tsx:176-190 - Misleading login prompt: Authenticated non-members see "login to see full group details" even though they ARE logged in.
4. website/src/components/User/GroupPage.tsx:63 - Incomplete null handling for users field when authenticated.

Type Safety:
5. website/src/pages/group/[groupId]/index.astro:12 - Unsafe non-null assertion on session could cause runtime errors.

Testing:
6. website/src/components/User/GroupPage.spec.tsx:33 - Missing assertion that Users section is hidden for unauthenticated users.
7. website/src/components/User/GroupPage.spec.tsx:51 - Missing test case for authenticated non-member scenario (important edge case).

Documentation:
8. website/src/types/backend.ts:310 - users field should have JSDoc explaining when it's null vs populated.

Recommendations

Must Fix:

Should Fix:

Consider:

  • Clarify if contact email should be visible to all authenticated users or only group members (issue #1)
  • Add a comment explaining the security model around data filtering (issue Test dataset with versions #2)

Testing Notes

According to website/AGENTS.md, you should run:

CI=1 npm run test
npm run check-types
npm run format

The existing tests are well-structured and use proper mocking. The test setup in vitest.setup.ts properly initializes test data.

@anna-parker anna-parker self-requested a review January 14, 2026 10:08
Copy link
Member

@theosanderson theosanderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks!

@maverbiest maverbiest merged commit 0d6b0e0 into main Jan 14, 2026
40 checks passed
@maverbiest maverbiest deleted the group-page-unauth branch January 14, 2026 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preview Triggers a deployment to argocd

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Group Page: expose group information (without the emails) for all users even when not logged in

3 participants