[ASA 9675] DAST rescan #176
Conversation
| @@ -123,9 +123,6 @@ public static void zipFileOrFolder(File fileToZip, File zipFile) throws IOExcept | |||
| } | |||
|
|
|||
| private static void zipFile(File fileToZip, String fileName, ZipOutputStream zipOut) throws IOException { | |||
| if (fileToZip.getName().startsWith(".")) { | |||
There was a problem hiding this comment.
What was the reason for the removal of this check?
There was a problem hiding this comment.
This change is being made to include the ".git" folder when creating a zip archive of the parent folder from a Git repository.
Task detail: https://jira02.hclpnp.com/browse/ASA-9830
| * @param provider The IAuthenticationProvider for authentication. | ||
| * @return String. | ||
| */ | ||
| public static String getA360Version(IAuthenticationProvider provider) { |
There was a problem hiding this comment.
There are a number of problems with this method:
- The javadoc comment is not accurate.
- The method name should just be getServiceVersion, not getA360Version.
- We shouldn't need a valid token to check the service version.
- We shouldn't need the authentication header.
- You should use "body.getString()" as opposed to casting.
- The comment in the catch block doesn't make sense.
There was a problem hiding this comment.
Sure, Made the suggested changes.
| * @param provider The IAuthenticationProvider for authentication. | ||
| * @return JSONArray. | ||
| */ | ||
| public static JSONArray getExecutionDetails(String scanId, IAuthenticationProvider provider) { |
There was a problem hiding this comment.
Why are there DAST specific checks here? The "IsValidForIncremental" property would only be true for DAST scans, but what if someone called this method for a SAST or SCA scan? This seems like a very specific method that is being added as a general utility method.
There was a problem hiding this comment.
This method is used to fetch the "executionIds" which helps to fill the base scan dropdown field of incremental scan. I am renaming this method to "getBaseScanDetails" for better understanding.
| * @param provider The IAuthenticationProvider for authentication. | ||
| * @return JSONObject. | ||
| */ | ||
| public static JSONObject scanSpecificDetails(String type, String scanId, IAuthenticationProvider provider) { |
There was a problem hiding this comment.
The name of this method should be an action, like "getScanDetails()".
There was a problem hiding this comment.
Sure, renaming the method to "getScanDetails()".
| * @param provider The IAuthenticationProvider for authentication. | ||
| * @return JSONObject. | ||
| */ | ||
| public static JSONObject getScanDetails(String type, String scanId, IAuthenticationProvider provider) { |
There was a problem hiding this comment.
What's the need for the addition of this method here when we already have the CloudScanServiceProvider that does the same thing?
There was a problem hiding this comment.
There is difference in the APIs. The method which is there in the CloudScanServiceProvider call the general API "GET /api/v4/Scans $filter=Id eq scanId" to fetch the scan details but new method of ServiceUtil calls the scan specific API "GET /api/v4/Sast/scanId ".
For scanId validation, We want to get the value of the "gitRepoPlatform" parameter which is not present in the respeonse of general API.
There was a problem hiding this comment.
Matt, Should we move this new method into CloudScanServiceProvider class as it is perform related function?
| * @param provider The IAuthenticationProvider for authentication. | ||
| * @return JSONArray. | ||
| */ | ||
| public static JSONArray getBaseScanDetails(String scanId, IAuthenticationProvider provider) { |
There was a problem hiding this comment.
This method would be better as a part of the IScanServiceProvider (i.e. CloudScanServiceProvider) as opposed to being added as a utility method here.
There was a problem hiding this comment.
Okay, I will move it to the cloudScanServiceProvider class.
| */ | ||
| public static boolean isScanId(String scanId, String applicationId, String type, IAuthenticationProvider provider) { | ||
| public static void updateScanData(Map<String, String> params, String scanId, IAuthenticationProvider provider, IProgress progress) { |
There was a problem hiding this comment.
Similar to the other comments, it's not clear why this is being added as a utility method, as opposed to the IScanServiceProvider that performs related functions.
There was a problem hiding this comment.
Okay, I will move it to the cloudScanServiceProvider class.
| return null; | ||
| } | ||
|
|
||
| public void updateScanData(Map<String, String> params, String scanId, IAuthenticationProvider provider, IProgress progress) { |
There was a problem hiding this comment.
Why does this method take an IAuthenticationProvider and an IProgress as parameters when there are already member variables of those types?
Implementation of rescan functionality for the dynamic scans.