Skip to content

Conversation

@ihsaan-ullah
Copy link
Collaborator

@ihsaan-ullah ihsaan-ullah commented Jun 10, 2023

@ mention of reviewers

@Didayolo

A brief description of the purpose of the changes contained in this PR.

Now user email will not be leaked in these apis:

  1. https://www.codabench.org/api/competitions/
Screenshot 2023-06-11 at 1 12 35 AM

now only username is shown in collaborators

  1. https://www.codabench.org/api/participants/
Screenshot 2023-06-11 at 1 12 53 AM

Now email is removed from the participants

Issues this PR resolves

#885

Checklist

  • Code review by me
  • Hand tested by me
  • I'm proud of my work
  • Code review by reviewer
  • Hand tested by reviewer
  • CircleCi tests are passing
  • Ready to merge

@ihsaan-ullah
Copy link
Collaborator Author

The change in Competition participant may break this:
Screenshot 2023-06-11 at 1 27 39 AM

It needs more investigation

@ihsaan-ullah
Copy link
Collaborator Author

Now codabench/api.participants will show participants without email and in competition->participants tab emails will be shown

API: no email
Screenshot 2023-06-11 at 12 53 32 PM

Participant tab: with email
Screenshot 2023-06-11 at 12 53 21 PM

Now the issue can be closed

@dtuantran
Copy link
Contributor

I think we should remove also "id" and "status" of users. Exposing the least information to public is the general security rule.

@ihsaan-ullah
Copy link
Collaborator Author

Screenshot 2023-06-15 at 8 48 04 PM

@Didayolo
Copy link
Member

I rebased the branch to have the CircleCI test fix.

@ihsaan-ullah ihsaan-ullah mentioned this pull request Jun 20, 2023
@ihsaan-ullah
Copy link
Collaborator Author

@dtuantran

This is working fine. It shows all partiicpants from competitions where you are owner or collaborator
Screenshot 2023-06-21 at 12 03 46 PM

Adding a participant from this interface does not work because it needs a user object and we have hidden almost all the attributes
Screenshot 2023-06-21 at 12 17 16 PM

The issue solved in this PR is the data leakage i.e. email, status and id. If something else needs to be solved then a separate issue should be created about it

@dtuantran
Copy link
Contributor

Tested, it's good to be merged !

@Didayolo Didayolo merged commit e9f7ef4 into develop Jun 21, 2023
@Didayolo Didayolo deleted the leaked_users branch June 21, 2023 14:06
This was referenced Jun 21, 2023
Didayolo added a commit that referenced this pull request Jun 30, 2023
This was referenced Jun 30, 2023
@Didayolo
Copy link
Member

The change in Competition participant may break this:

You were right!

Didayolo added a commit that referenced this pull request Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants