Island: Update API spec comments after discussion

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)

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?
+    # API Spec: Rename to AgentStopStatus or something
-    # REST feels okay too but probably rename it to AgentStopStatus or something.
     urls = ["/api/monkey-control/needs-to-stop/<int:monkey_guid>"]
 
     def get(self, monkey_guid: int):

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

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

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
 

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

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

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()

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?
+    # API Spec: Instead of POST, this could just be PATCH
-    # POST is not updating/creating a "resource" but setting a property of the Island.
-    # Perhaps /api/setMode and /api/getMode would be more appropriate.
     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)

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)

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/<string:host_os>"]
 
     # Used by monkey. can't secure.

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/<string:filename>"]
     """
     File download endpoint used by monkey to download user's PBA file

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
+    # API Spec: FileUpload -> PBAFileUpload. Change endpoint accordingly.
-    # accordingly.
     """
     File upload endpoint used to send/receive Custom PBA files
     """

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

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

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

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/<string:report_data>"]
 
     @jwt_required