Require username

[annehaley]

Resolves #446

[cloudflare-workers-and-pages]

Deploying geodatalytics with    Cloudflare Pages

Latest commit:	04728cf
Status:	✅  Deploy successful!
Preview URL:	https://6ed4bfa7.geodatalytics.pages.dev
Branch Preview URL:	https://require-usernames.geodatalytics.pages.dev

View logs

[BryonLewis]

I have some comments/questions about this PR. Mostly notes that may be require a subsequent PR.

• Just a reminder that we either need to create some sort of migration or manually make sure that all existing users that have username based on the email have it swapped to something that isn't their email, or remove all existing users.
• When doing a GET /projects there is information about the owner including the username and is_superuser, should we prevent that information from being sent during this request? This information leaks through for non-authenticated users. This may be a bigger lift because I think you would have a response with the owner/collaborator/follower information removed for unauthenticated users because they never open the access control.
• I question if the serializer should be returning is_superuser for most of the requests that contain users like project owners and user lists? Is it being used in the client anywhere? I think it's only used on the /users/me request. So access control doesn't need to leak who is a super user.
• UserFactory/SuperUserFactory in the tests still use username = email which isn't true anymore after this. Technically it doesn't matter but it would probably be a better representation if username was used.

[annehaley]

• Just a reminder that we either need to create some sort of migration or manually make sure that all existing users that have username based on the email have it swapped to something that isn't their email, or remove all existing users.

I was planning on doing a manual update on the production instance via the admin console. We only have a handful of users anyway, and they're mostly Kitware folks.

• When doing a GET /projects there is information about the owner including the username and is_superuser, should we prevent that information from being sent during this request? This information leaks through for non-authenticated users. This may be a bigger lift because I think you would have a response with the owner/collaborator/follower information removed for unauthenticated users because they never open the access control.
• I question if the serializer should be returning is_superuser for most of the requests that contain users like project owners and user lists? Is it being used in the client anywhere? I think it's only used on the /users/me request. So access control doesn't need to leak who is a super user.

The only place we use is_superuser on the client side is for the computed permissions, so we only need that field for the response on the me endpoint. So I made a separate MeSerializer that includes the field and removed it from the standard UserSerializer: dbf029b

• UserFactory/SuperUserFactory in the tests still use username = email which isn't true anymore after this. Technically it doesn't matter but it would probably be a better representation if username was used.

Changed in 04728cf.

[BryonLewis]

I may have been unclear in the one bullet point. A Non authenticated user when getting projects will also get the usernames of the owner/collaborator/followers for that user through the GET /projects endpoint. I mixed that together with the is_superuser question. Should we be allowing non-authenticated users to see the usernames in the system?

[annehaley]

I think that our goal for usernames going forward is that they do not contain sensitive information, and they serve as display names. First and last names would probably be considered more sensitive. An unauthenticated user would have to make direct API calls to get user information anyway, since we don't expose the project management interface in unauthenticated mode.

[BryonLewis]

👍