whoami: List public group memberships (take 2) #780
Conversation
nextstrain-bot
temporarily deployed
to
nextstrain-s-victorlin--i1jip8
victorlin
force-pushed
the
victorlin/whoami
branch
from
3a4dcc7
to
1cfd307
Compare
The reference was removed in "splash page redesign" (fab5217).
The two exported values are independent of user information, so remove 'user' from the filename.
This is a re-attempt at ab569ec, which was reverted in 6e61054. Previously, there was no way to identify which public groups a user has membership in on any nextstrain.org page. This change makes that possible on the whoami page. Notes on implementation: 1. The visibleGroups variable cannot be used here because it provides no way to distinguish a member of a public group with view-only permission and an external user that has the same view-only permission. Those can only be distinguished by inspecting the user's authorization roles, which is exposed through a new variable groupMemberships. 2. There is some overlap between visibleGroups and groupMemberships, but they serve different purposes. An alternative to bring the two together would be to update the group listing page to show group memberships and other public groups separately, but that is a bigger change.
membershipRoles is internal and not used by the frontend. Future-proofs against any unintentional leaking of new properties.
victorlin
force-pushed
the
victorlin/whoami
branch
from
1cfd307
to
bd882f9
Compare
| * the Group class. | ||
| */ | ||
| const groupMemberships = (user) => user?.groups | ||
| ?.map(name => new Group(name)) |
There was a problem hiding this comment.
This ended up breaking /whoami for me (noticed in production just now) because my Cognito user is a member of Cognito groups for which there are no corresponding Nextstrain groups. This is a historical artifact and likely only affects me (and maybe one or two other devs), but it'd still be better to be robust in the face of unexpected input.
I noticed the issue in the web, where /whoami 404s for me when JSON is requested, but reproduced it below with curl for an example:
$ nextstrain whoami
# Logged into https://nextstrain.org as…
username: trs
email: tsibley@fredhutch.org
groups:
- !flags/canary
- blab-private/editors
- blab/editors
- seattleflu/editors
- test/editors
- trs-test/owners
$ curl -s https://nextstrain.org/whoami -H 'Accept: application/json' -H @<(nextstrain authorization)
{"error":"unknown group: test"}There was a problem hiding this comment.
I'd previously said:
Longer term, it probably makes sense to convert
req.user.groupsto Group objects in the authn code rather than here. There can also be some additional error/edge-case handling (e.g. group names reported by Cognito that the server doesn't know about).
and I guess I shouldn't have said "Longer term"!
Description of proposed changes
See commit messages.
Related issue(s)
Checklist