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.

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

Repository should provide abstract firewall contract #199

Closed
schemar opened this issue Apr 23, 2019 · 3 comments · Fixed by #200
Closed

Repository should provide abstract firewall contract #199

schemar opened this issue Apr 23, 2019 · 3 comments · Fixed by #200
Assignees
Milestone

Comments

@schemar
Copy link
Contributor

schemar commented Apr 23, 2019

ℹ️ Look at the parent epic for most of the details: #193

Story

As a consumer of the package
I want to have a re-usable firewall available
Because I want to limit access to some of my rules

Scenarios

Given that I implement a rule
When I extend the firewall
Then my rule can check for tx.origin to be a registered worker with an organisation

Info

The info is in the epic #193

This ticket is only concerned with:

  • Providing the abstract firewall contract that provides the modifier.
  • Making sure the firewall can be turned off by the organization.
  • Testing.
@jasonklein
Copy link
Contributor

@schemar: is this ticket ripe for the backlog? You note the following in the epic:

Do we really want [a firewall-disabling function modified by onlyOrganization] or would we rather replace the firewalled rules with non-firewalled pendants?

@schemar
Copy link
Contributor Author

schemar commented Apr 23, 2019

It can always be changed later if it really has to be changed. Which is unlikely. So we can do it the way Ben instructed last week.

@schemar schemar self-assigned this Apr 24, 2019
@schemar
Copy link
Contributor Author

schemar commented Apr 24, 2019

I cannot validate with Slither any more, opened a report at trailofbits/eth-security-toolbox#8

schemar added a commit to schemar/openst-contracts that referenced this issue Apr 24, 2019
I added a FirewalledRule which can be extended in order to restrict
relaying of meta-transactions to organization workers.
The rule that is to be restricted must extend the FirewalledRule and add
the modifier `firewalled` to all relevant functions.

For testing I created two test contract files. One that uses the
modifier, and one that calls the one that uses the modifier. Required in
order to simulate rules calling on each other and checking for the
relayer.

Added a step to manually install npx inside docker until the docker
image is fixed. The problem is reported and confirmed at
trailofbits/eth-security-toolbox#8

Fixes OpenST#199
@schemar schemar added this to the Sprint 0 milestone May 2, 2019
@pgev pgev closed this as completed in #200 May 2, 2019
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 a pull request may close this issue.

2 participants