Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Verify the sender of Rollcall create/open/close #1778

Merged
merged 3 commits into from
Apr 10, 2024
Merged

Verify the sender of Rollcall create/open/close #1778

merged 3 commits into from
Apr 10, 2024

Conversation

arnauds5
Copy link
Collaborator

@arnauds5 arnauds5 commented Mar 13, 2024 •
edited
Loading

closes #1777

There was no verification that Rollcall create/open/close were only accepted from an organizer.
If theses messages were send by anyone, the go backend was accepting them.

This PR add a verification for each of theses messages and deny them if it was not send by an organizer.
An helper function was made using the existing code of the election#setup verificaiton.

@arnauds5 arnauds5 added be1-go bug-fix Fixes a bug labels Mar 13, 2024
@arnauds5 arnauds5 self-assigned this Mar 13, 2024
@github-actions GitHub Actions
Copy link

Pull reviewers stats

Stats of the last 30 days for popstellar:

User Total reviews Time to review Total comments
MariemBaccari
🥇		 5
▀▀		 1d 15h 8m
 1
matteosz
🥈		 4
▀		 4d 19h 33m
▀		 11
▀▀▀▀▀
Tyratox
🥉		 3
▀		 6d 19h 13m
▀▀		 1
pierluca
 3
▀		 4d 7h 5m
▀		 1
quadcopterman
 3
▀		 1d 2h 12m
 0
4xiom5
 2
▀		 4d 20h 10m
▀		 0
sgueissa
 2
▀		 4d 21h 28m
▀		 8
▀▀▀
emonnin-epfl
 2
▀		 7h 27m
 1
arnauds5
 1
 6h 55m
 0
osm-alt
 1
 5h 52m
 0
ljankoschek
 1
 20h 9m
 0
K1li4nL
 1
 6d 20h 39m
▀▀		 0
jbsv
 1
 21h 49m
 0
Kaz-ookid
 1
 4d 10h 8m
▀		 0
⚡️ Pull request stats

@arnauds5 arnauds5 marked this pull request as ready for review March 14, 2024 12:31
@arnauds5 arnauds5 requested a review from a team as a code owner March 14, 2024 12:31
sgueissa
sgueissa previously approved these changes Mar 29, 2024
View reviewed changes
Copy link
Collaborator

@sgueissa sgueissa left a comment

Choose a reason for hiding this comment

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

Everything looks good for me

MariemBaccari
MariemBaccari reviewed Mar 30, 2024
View reviewed changes
be1-go/channel/lao/lao.go Outdated
func (c *Channel) checkIsFromOrganizer(msg message.Message) error {
senderBuf, err := base64.URLEncoding.DecodeString(msg.Sender)
if err != nil {
return xerrors.Errorf(keyDecodeError, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why this is built using xerrors.Errorf not as an amswer.NewErrorf like the other errors returned ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was because this function was created using some existing code.
After reading answer/error.go, I just found that there are some functions with the error code already inside,
such as answer.NewInvalidMessageFieldError and answer.NewAccessDeniedError.

MariemBaccari
MariemBaccari previously approved these changes Mar 30, 2024
View reviewed changes
Copy link
Contributor

@MariemBaccari MariemBaccari left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Good job ! The fix looks good to me. I just have a small question.

@arnauds5 arnauds5 dismissed stale reviews from MariemBaccari and sgueissa via 6dcaf08 March 30, 2024 20:58
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Mar 30, 2024

Quality Gate Passed Quality Gate passed for 'PoP - PoPCHA-Web-Client'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Mar 30, 2024

Quality Gate Passed Quality Gate passed for 'PoP - Be2-Scala'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Mar 30, 2024

Quality Gate Passed Quality Gate passed for 'PoP - Be1-Go'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
87.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Mar 30, 2024

Quality Gate Passed Quality Gate passed for 'PoP - Fe1-Web'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Mar 30, 2024

Quality Gate Passed Quality Gate passed for 'PoP - Fe2-Android'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@arnauds5 arnauds5 merged commit af80986 into master Apr 10, 2024
18 checks passed
@arnauds5 arnauds5 deleted the work-be1-arnauds5-check-if-organizer branch April 10, 2024 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MariemBaccari MariemBaccari MariemBaccari approved these changes

@sgueissa sgueissa sgueissa left review comments

@1florentin 1florentin Awaiting requested review from 1florentin

@emonnin-epfl emonnin-epfl Awaiting requested review from emonnin-epfl

Assignees

@arnauds5 arnauds5

Labels
be1-go bug-fix Fixes a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] No verification that only organizer can create/open/close rollcall
3 participants
@arnauds5 @sgueissa @MariemBaccari