-
Notifications
You must be signed in to change notification settings - Fork 807
Ransomware quickstart endpoint #1318
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
Conversation
👉 View analysis in DeepCode’s Dashboard | Configure the bot👉 The DeepCode service and API will be deprecated in August, 2021. Here is the information how to migrate. Thank you for using DeepCode 🙏 ❤️ !If you are using our plugins, you might be interested in their successors: Snyk's JetBrains plugin and Snyk's VS Code plugin. |
Codecov Report
@@ Coverage Diff @@
## develop #1318 +/- ##
===========================================
+ Coverage 30.87% 40.24% +9.37%
===========================================
Files 454 471 +17
Lines 13545 13820 +275
===========================================
+ Hits 4182 5562 +1380
+ Misses 9363 8258 -1105
Continue to review full report at Codecov.
|
monkey/tests/unit_tests/monkey_island/cc/resources/test_island_mode.py
Outdated
Show resolved
Hide resolved
@pytest.mark.usefixtures("uses_database") | ||
def test_island_mode_post__set_model(flask_client): | ||
flask_client.post( | ||
"/api/island-mode", data=json.dumps({"mode": "ransomware"}), follow_redirects=True | ||
) | ||
assert IslandMode.objects[0].mode == "ransomware" | ||
|
||
|
||
@pytest.mark.usefixtures("uses_database") | ||
def test_island_mode_post__set_invalid_model(flask_client): | ||
flask_client.post( | ||
"/api/island-mode", data=json.dumps({"mode": "bogus mode"}), follow_redirects=True | ||
) | ||
assert len(IslandMode.objects) == 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Theses tests unnecessarily couple the API to the storage mechanism. What's needed is a test that calls the API to set the mode, and then calls the API again to get the mode.
We can commit these for now, but when we add the GET endpoint to the API, these tests should be replaced.
63101b1
to
d135b4b
Compare
set_mode(mode) | ||
return make_response({}, 200) | ||
except ValueError: | ||
return make_response({}, 404) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
404 means that the URL was not found. 400 would be a better return code. Better yet, it should return 200 with an error in the response body. See configuation_import.py
for an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say changing this to 400 is more than enough for now (or maybe to 422) There's no point in the error message, since this is not gonna be displayed anywhere and 200 will not throw a console error, thus once we add more modes, if the mode is miss-spelled on the UI it will be harder to notice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like 422
8471be4
to
c56ca37
Compare
Flask automatically traps exceptions, returns a 500, and logs a stack trace. Since Flask will automatically return a 500, we don't need to duplicate the functionality. Since it prints a stack trace, it provides more useful information than catching it did.
What does this PR do?
Fixes #1241 Add API endpoint for setting the island mode.
Add any further explanations here.
PR Checklist
Was the CHANGELOG.md updated to reflect the changes?Was the documentation framework updated to reflect the changes?Testing Checklist
If applicable, add screenshots or log transcripts of the feature workingExplain Changes
Are the commit messages enough? If not, elaborate.