Secure endpoints #3203
Secure endpoints #3203
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## develop #3203 +/- ##
===========================================
+ Coverage 73.02% 73.14% +0.11%
===========================================
Files 469 469
Lines 13566 13616 +50
===========================================
+ Hits 9907 9959 +52
+ Misses 3659 3657 -2 see 21 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
shreyamalviya
force-pushed
the
3078-secure-endpoints
branch
from
e1445f5
to
b841c28
Compare
mssalvatore
force-pushed
the
3078-secure-endpoints
branch
from
5ad0924
to
395a714
Compare
mssalvatore
force-pushed
the
3078-secure-endpoints
branch
2 times, most recently
from
9193972
to
40f15b1
Compare
There was a problem hiding this comment.
Note: We currently use this endpoint in the manual run commands to fetch the agent binaries. Requiring authentication would cause the manual run commands to fail
There was a problem hiding this comment.
Good catch, I'll remove it
| @@ -29,7 +32,8 @@ def get(self, collection=None): | |||
|
|
|||
| return propagation_credentials, HTTPStatus.OK | |||
|
|
|||
| # Used by Agent. Can't secure. | |||
| @auth_token_required | |||
| @roles_accepted(AccountRole.AGENT.name, AccountRole.ISLAND_INTERFACE.name) | |||
There was a problem hiding this comment.
Do agents actually need to put propagation credentials?
There was a problem hiding this comment.
I suppose they don't, since they send them via events.
cakekoa
force-pushed
the
3078-secure-endpoints
branch
from
f64d0e5
to
168cbd7
Compare
- adds a comment where it can't be done - changes @roles_required (requires caller to have all the specified roles) to @roles_accepted (requires caller to have one of the specified roles) where applicable - adds @include_auth_token to AgentOTPLogin resource
shreyamalviya
force-pushed
the
3078-secure-endpoints
branch
from
d93489d
to
2127563
Compare
monkey/infection_monkey/monkey.py
Outdated
| # if OTP_FLAG not in os.environ: | ||
| # return OTP("PLACEHOLDER_OTP") |
There was a problem hiding this comment.
Either we can leave this or remove it. I'm not sure this PR is the right place to remove it.
There was a problem hiding this comment.
Just a temporary commit for testing
| pass | ||
|
|
||
|
|
||
| class AgentRequests(IMonkeyIslandRequests): |
There was a problem hiding this comment.
Should we just use infection_monkey.island_api_client.IIslandAPIClient?
Pros:
- Less code in BB tests
- Adds a quick, ETE test for HTTPIslandAPIClient
Cons:
- Couples the tests to that component
There was a problem hiding this comment.
I don't like it. These tests shouldn't know anything about the internals. We don't do anything like this anywhere else in the BB tests either.
There were too many things called 'requests'.
shreyamalviya
force-pushed
the
3078-secure-endpoints
branch
2 times, most recently
from
fcc5b60
to
4bd5e5c
Compare
What does this PR do?
Fixes a part of #3078
What's left:
PR Checklist
Was the documentation framework updated to reflect the changes?Testing Checklist
Added relevant unit tests?If applicable, add screenshots or log transcripts of the feature working