Skip to content

whitelist emails leaking handled#1526

Closed
ihsaan-ullah wants to merge 4 commits into
developfrom
whitelist_emails
Closed

whitelist emails leaking handled#1526
ihsaan-ullah wants to merge 4 commits into
developfrom
whitelist_emails

Conversation

@ihsaan-ullah

@ihsaan-ullah ihsaan-ullah commented Jul 13, 2024

Copy link
Copy Markdown
Collaborator

@ mention of reviewers

@Didayolo

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

  • Whitelist emails were leaked in API. Now it is handled in a way that only loggedin users and API calls from inside codabench can access it.
  • fixed a console error
  • removed console prints

Issues this PR resolves

A checklist for hand testing

  • before deploying this change access http://localhost/api/competitions/1/ and check that whitelist_emails is there
  • after deploying, you should not see whitelist_emails

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 ihsaan-ullah linked an issue Jul 13, 2024 that may be closed by this pull request
Terms
</div>
<div if={competition.files.length != 0} class="{active: _.get(competition.pages, 'length') === 0} item" data-tab="files">
<div if={competition.files && competition.files.length != 0} class="{active: _.get(competition.pages, 'length') === 0} item" data-tab="files">

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this was giving an error in the console. Fixed it by adding competition.files to the condition

@ihsaan-ullah

ihsaan-ullah commented Jul 16, 2024

Copy link
Copy Markdown
Collaborator Author

Problems

  1. Accessing the competition via API does not show whitelist emails but there is an update view that shows the emails
Screenshot 2024-07-13 at 12 19 12 PM

Comments

  1. The main problem is the way this API is designed. there is one endpoint /competitions/ and then everything is handled in it.
  2. To simplify things, we should have small readable and straightforward apis.
  3. We should also disable the django view of the apis so that we can control things (not sure if that is possible because the default views come with the django rest-framework)

Edit

We are going to get more similar issues. I suggest that we start redesiging the apis step by step and also update the documentation with it and add tests with each api

@ihsaan-ullah

Copy link
Copy Markdown
Collaborator Author

Benjamin ideas:

  • only admin should be able to get white-lists

Isabell ideas:

  • just create participants from whitelist emails and remove
  • rename whitelist to approved list
  • a problem when whitelist person is not a participants.

@ihsaan-ullah

Copy link
Copy Markdown
Collaborator Author

Solved in #1560

@ihsaan-ullah ihsaan-ullah deleted the whitelist_emails branch August 13, 2024 06:23
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.

1 participant