From dfe93feb0b84a4115f88879bea12298442b64b93 Mon Sep 17 00:00:00 2001 From: Shreya Malviya Date: Thu, 26 May 2022 14:02:42 +0530 Subject: [PATCH] Island: Update API spec comments after discussion --- monkey/monkey_island/cc/app.py | 6 +++--- .../cc/resources/agent_controls/stop_agent_check.py | 3 +-- monkey/monkey_island/cc/resources/attack/attack_report.py | 1 - monkey/monkey_island/cc/resources/configuration_export.py | 1 - monkey/monkey_island/cc/resources/configuration_import.py | 2 +- .../cc/resources/exploitations/manual_exploitation.py | 5 ----- .../cc/resources/exploitations/monkey_exploitation.py | 5 ----- monkey/monkey_island/cc/resources/island_configuration.py | 4 +++- monkey/monkey_island/cc/resources/island_mode.py | 7 +------ monkey/monkey_island/cc/resources/local_run.py | 6 ++++-- monkey/monkey_island/cc/resources/monkey_download.py | 1 - monkey/monkey_island/cc/resources/pba_file_download.py | 1 - monkey/monkey_island/cc/resources/pba_file_upload.py | 3 +-- monkey/monkey_island/cc/resources/ransomware_report.py | 1 - monkey/monkey_island/cc/resources/remote_run.py | 2 +- monkey/monkey_island/cc/resources/security_report.py | 1 - .../cc/resources/zero_trust/zero_trust_report.py | 1 - 17 files changed, 15 insertions(+), 35 deletions(-) diff --git a/monkey/monkey_island/cc/app.py b/monkey/monkey_island/cc/app.py index 015c2e4a0..576f25890 100644 --- a/monkey/monkey_island/cc/app.py +++ b/monkey/monkey_island/cc/app.py @@ -155,7 +155,7 @@ def init_api_resources(api: FlaskDIWrapper): api.add_resource(IslandConfiguration) api.add_resource(ConfigurationExport) api.add_resource(ConfigurationImport) - # Rename to /api/agent-binary, because information about agent runs + # API Spec: Rename to /api/agent-binary, because information about agent runs # and binary files are different resources api.add_resource(MonkeyDownload) api.add_resource(NetMap) @@ -175,14 +175,14 @@ def init_api_resources(api: FlaskDIWrapper): api.add_resource(Log) api.add_resource(IslandLog) - # These two should have the same url syntax + # API Spec: These two should have the same url syntax api.add_resource(PBAFileDownload) api.add_resource(FileUpload) api.add_resource(PropagationCredentials) # API Spec: Should use RPC convention api.add_resource(RemoteRun) - # Rename to /version-info + # API Spec: Rename to /version-info api.add_resource(VersionUpdate) # API Spec: Fix endpoint (see comment in StopAgentCheck) api.add_resource(StopAgentCheck) diff --git a/monkey/monkey_island/cc/resources/agent_controls/stop_agent_check.py b/monkey/monkey_island/cc/resources/agent_controls/stop_agent_check.py index b5585dce3..fd1fc213a 100644 --- a/monkey/monkey_island/cc/resources/agent_controls/stop_agent_check.py +++ b/monkey/monkey_island/cc/resources/agent_controls/stop_agent_check.py @@ -2,8 +2,7 @@ from monkey_island.cc.resources.AbstractResource import AbstractResource from monkey_island.cc.services.infection_lifecycle import should_agent_die class StopAgentCheck(AbstractResource): - # API Spec: Not a resource, checking is an action; RPC-style endpoint? - # REST feels okay too but probably rename it to AgentStopStatus or something. + # API Spec: Rename to AgentStopStatus or something urls = ["/api/monkey-control/needs-to-stop/"] def get(self, monkey_guid: int): diff --git a/monkey/monkey_island/cc/resources/attack/attack_report.py b/monkey/monkey_island/cc/resources/attack/attack_report.py index ef5fdfa5d..2dffa929d 100644 --- a/monkey/monkey_island/cc/resources/attack/attack_report.py +++ b/monkey/monkey_island/cc/resources/attack/attack_report.py @@ -7,7 +7,6 @@ from monkey_island.cc.services.attack.attack_schema import SCHEMA class AttackReport(AbstractResource): - # API Spec: This is an action and there's no "resource"; RPC-style endpoint? urls = ["/api/report/attack"] @jwt_required diff --git a/monkey/monkey_island/cc/resources/configuration_export.py b/monkey/monkey_island/cc/resources/configuration_export.py index 972f833f4..e10e5d1a3 100644 --- a/monkey/monkey_island/cc/resources/configuration_export.py +++ b/monkey/monkey_island/cc/resources/configuration_export.py @@ -9,7 +9,6 @@ from monkey_island.cc.services.config import ConfigService class ConfigurationExport(AbstractResource): - # API Spec: This is an action and there's no "resource"; RPC-style endpoint? urls = ["/api/configuration/export"] @jwt_required diff --git a/monkey/monkey_island/cc/resources/configuration_import.py b/monkey/monkey_island/cc/resources/configuration_import.py index 94aa50def..15ab3e41f 100644 --- a/monkey/monkey_island/cc/resources/configuration_import.py +++ b/monkey/monkey_island/cc/resources/configuration_import.py @@ -39,7 +39,7 @@ class ResponseContents: class ConfigurationImport(AbstractResource): - # API Spec: This is an action and there's no "resource"; RPC-style endpoint? + # API Spec: Should probably be merged with IslandConfiguration urls = ["/api/configuration/import"] SUCCESS = False diff --git a/monkey/monkey_island/cc/resources/exploitations/manual_exploitation.py b/monkey/monkey_island/cc/resources/exploitations/manual_exploitation.py index bbb345483..8006e31af 100644 --- a/monkey/monkey_island/cc/resources/exploitations/manual_exploitation.py +++ b/monkey/monkey_island/cc/resources/exploitations/manual_exploitation.py @@ -6,11 +6,6 @@ from monkey_island.cc.services.reporting.exploitations.manual_exploitation impor class ManualExploitation(AbstractResource): - # API Spec: RPC-style endpoint? Getting manual exploitations is an action and there's no - # "resource", however one could argue that at some point in time, we may want to access - # specific manual exploitation data based on, say, the monkey guid, at a later stage. Leave - # as is in that case, and consider modifying the logic to make this behave more like a - # "resource" and change the name. urls = ["/api/exploitations/manual"] @jwt_required diff --git a/monkey/monkey_island/cc/resources/exploitations/monkey_exploitation.py b/monkey/monkey_island/cc/resources/exploitations/monkey_exploitation.py index be5669c22..5cbf42a35 100644 --- a/monkey/monkey_island/cc/resources/exploitations/monkey_exploitation.py +++ b/monkey/monkey_island/cc/resources/exploitations/monkey_exploitation.py @@ -6,11 +6,6 @@ from monkey_island.cc.services.reporting.exploitations.monkey_exploitation impor class MonkeyExploitation(AbstractResource): - # API Spec: RPC-style endpoint? Getting manual exploitations is an action and there's no - # "resource", however one could argue that at some point in time, we may want to access - # specific monkey exploitation data based on, say, the monkey guid, at a later stage. Leave - # as is in that case, and consider modifying the logic to make this behave more like a - # "resource" and change the name. urls = ["/api/exploitations/monkey"] @jwt_required diff --git a/monkey/monkey_island/cc/resources/island_configuration.py b/monkey/monkey_island/cc/resources/island_configuration.py index e75d44e6f..1360d0c91 100644 --- a/monkey/monkey_island/cc/resources/island_configuration.py +++ b/monkey/monkey_island/cc/resources/island_configuration.py @@ -21,7 +21,8 @@ class IslandConfiguration(AbstractResource): @jwt_required def post(self): config_json = json.loads(request.data) - # API Spec: Resetting config is an action; separate RPC-style endpoint for this? + # API Spec: Makes more sense to have a PATCH request for this since the resource, + # i.e. the configuration, is being updated. if "reset" in config_json: ConfigService.reset_config() else: @@ -31,4 +32,5 @@ class IslandConfiguration(AbstractResource): # RESTfulness of a POST request is to return an identifier of the updated/newly created # resource. Since there's only one thing we're updating (and not multiple "resources"), # should this also be an RPC-style endpoint (/api/resetConfig and /api/updateConfig)? + # Simplest way it to just send a GET request after this to get the updated config. return self.get() diff --git a/monkey/monkey_island/cc/resources/island_mode.py b/monkey/monkey_island/cc/resources/island_mode.py index a77829972..6978f3b14 100644 --- a/monkey/monkey_island/cc/resources/island_mode.py +++ b/monkey/monkey_island/cc/resources/island_mode.py @@ -13,9 +13,7 @@ logger = logging.getLogger(__name__) class IslandMode(AbstractResource): - # API Spec: Should these be RPC-style endpoints? - # POST is not updating/creating a "resource" but setting a property of the Island. - # Perhaps /api/setMode and /api/getMode would be more appropriate. + # API Spec: Instead of POST, this could just be PATCH urls = ["/api/island-mode"] @jwt_required @@ -33,9 +31,6 @@ class IslandMode(AbstractResource): "Using default advanced configuration." ) - # API Spec: RESTful way is to return an identifier of the updated/newly created - # resource but it doesn't make sense here which brings me back to the comment on - # lines 16-18. return make_response({}, 200) except (AttributeError, json.decoder.JSONDecodeError): return make_response({}, 400) diff --git a/monkey/monkey_island/cc/resources/local_run.py b/monkey/monkey_island/cc/resources/local_run.py index 54b0ac0b5..4e918e6c7 100644 --- a/monkey/monkey_island/cc/resources/local_run.py +++ b/monkey/monkey_island/cc/resources/local_run.py @@ -10,10 +10,11 @@ from monkey_island.cc.services.run_local_monkey import LocalMonkeyRunService class LocalRun(AbstractResource): - # API Spec: This should be an RPC-style API i.e. two endpoints - - # "/api/getLocalMonkeyRunningStatus" and "/api/startLocalMonkey" urls = ["/api/local-monkey"] + # API Spec: Both of these methods should be separated to their own resources + + # API Spec: This should be a REST endpoint, /api/monkeys or something @jwt_required def get(self): island_monkey = NodeService.get_monkey_island_monkey() @@ -24,6 +25,7 @@ class LocalRun(AbstractResource): return jsonify(is_running=is_monkey_running) + # API Spec: This should be an RPC-style endpoint @jwt_required def post(self): body = json.loads(request.data) diff --git a/monkey/monkey_island/cc/resources/monkey_download.py b/monkey/monkey_island/cc/resources/monkey_download.py index 775252c22..9c19b70dc 100644 --- a/monkey/monkey_island/cc/resources/monkey_download.py +++ b/monkey/monkey_island/cc/resources/monkey_download.py @@ -20,7 +20,6 @@ class UnsupportedOSError(Exception): class MonkeyDownload(AbstractResource): - # API Spec: This is an action and there's no "resource"; RPC-style endpoint? urls = ["/api/agent/download/"] # Used by monkey. can't secure. diff --git a/monkey/monkey_island/cc/resources/pba_file_download.py b/monkey/monkey_island/cc/resources/pba_file_download.py index 4415f90eb..73658d0ad 100644 --- a/monkey/monkey_island/cc/resources/pba_file_download.py +++ b/monkey/monkey_island/cc/resources/pba_file_download.py @@ -9,7 +9,6 @@ logger = logging.getLogger(__file__) class PBAFileDownload(AbstractResource): - # API Spec: This is an action and there's no "resource"; RPC-style endpoint? urls = ["/api/pba/download/"] """ File download endpoint used by monkey to download user's PBA file diff --git a/monkey/monkey_island/cc/resources/pba_file_upload.py b/monkey/monkey_island/cc/resources/pba_file_upload.py index 45b72727e..a4ce2705a 100644 --- a/monkey/monkey_island/cc/resources/pba_file_upload.py +++ b/monkey/monkey_island/cc/resources/pba_file_upload.py @@ -18,8 +18,7 @@ WINDOWS_PBA_TYPE = "PBAwindows" class FileUpload(AbstractResource): - # API Spec: FileUpload -> CustomPBAFile or something to make it more RESTful. Change endpoint - # accordingly. + # API Spec: FileUpload -> PBAFileUpload. Change endpoint accordingly. """ File upload endpoint used to send/receive Custom PBA files """ diff --git a/monkey/monkey_island/cc/resources/ransomware_report.py b/monkey/monkey_island/cc/resources/ransomware_report.py index e8661640c..d229361ea 100644 --- a/monkey/monkey_island/cc/resources/ransomware_report.py +++ b/monkey/monkey_island/cc/resources/ransomware_report.py @@ -6,7 +6,6 @@ from monkey_island.cc.services.ransomware import ransomware_report class RansomwareReport(AbstractResource): - # API Spec: This is an action and there's no "resource"; RPC-style endpoint? urls = ["/api/report/ransomware"] @jwt_required diff --git a/monkey/monkey_island/cc/resources/remote_run.py b/monkey/monkey_island/cc/resources/remote_run.py index 74a6f8abc..c9b7a5d87 100644 --- a/monkey/monkey_island/cc/resources/remote_run.py +++ b/monkey/monkey_island/cc/resources/remote_run.py @@ -55,7 +55,7 @@ class RemoteRun(AbstractResource): if body.get("type") == "aws": results = self.run_aws_monkeys(body) # API Spec: POST should return identifier or updated/newly created resource, not some - # kind of data. That's more of a GET thing. + # kind of data. That's a GET thing. return RemoteRun._encode_results(results) # default action diff --git a/monkey/monkey_island/cc/resources/security_report.py b/monkey/monkey_island/cc/resources/security_report.py index 7e1784dd2..62b254931 100644 --- a/monkey/monkey_island/cc/resources/security_report.py +++ b/monkey/monkey_island/cc/resources/security_report.py @@ -4,7 +4,6 @@ from monkey_island.cc.services.reporting.report import ReportService class SecurityReport(AbstractResource): - # API Spec: This is an action and there's no "resource"; RPC-style endpoint? urls = ["/api/report/security"] @jwt_required diff --git a/monkey/monkey_island/cc/resources/zero_trust/zero_trust_report.py b/monkey/monkey_island/cc/resources/zero_trust/zero_trust_report.py index 2f0305150..359328114 100644 --- a/monkey/monkey_island/cc/resources/zero_trust/zero_trust_report.py +++ b/monkey/monkey_island/cc/resources/zero_trust/zero_trust_report.py @@ -17,7 +17,6 @@ REPORT_DATA_PRINCIPLES_STATUS = "principles" class ZeroTrustReport(AbstractResource): - # API Spec: This is an action and there's no "resource"; RPC-style endpoint? urls = ["/api/report/zero-trust/"] @jwt_required