Skip to content
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

DAST rescan #236

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
93187be
dast-rescan
vishalhcl-5960 Oct 16, 2024
a6e7a87
Update DynamicAnalyzer.java
vishalhcl-5960 Oct 19, 2024
5ae512d
Update DynamicAnalyzer.java
vishalhcl-5960 Oct 21, 2024
aa98bd6
URL validation check for A360
vishalhcl-5960 Nov 5, 2024
a7ea86f
updated scanId validation method
vishalhcl-5960 Nov 8, 2024
4a405fa
UI scanId validation
vishalhcl-5960 Nov 11, 2024
4ce66b7
Code enhancement for better UI
vishalhcl-5960 Nov 19, 2024
54e4c81
indentation checks & bug fixes
vishalhcl-5960 Nov 22, 2024
372de05
updated the scanId validation method
vishalhcl-5960 Nov 26, 2024
b3797ba
A360 version check for URL validation
vishalhcl-5960 Nov 29, 2024
408208a
UI URL validation for A360 connection
vishalhcl-5960 Nov 29, 2024
16b7a85
A360 version check for URL validation
vishalhcl-5960 Dec 3, 2024
b686bdb
As per PR comments
vishalhcl-5960 Dec 4, 2024
193d928
Addressing PR comments
vishalhcl-5960 Dec 6, 2024
66a62fb
Moved the common validation method to the base scanner class
vishalhcl-5960 Dec 6, 2024
ffb79e6
UI validation for base scan field
vishalhcl-5960 Dec 10, 2024
63b9f39
As per comments
vishalhcl-5960 Dec 12, 2024
9d05f21
indentation checks
vishalhcl-5960 Dec 12, 2024
d75f3cc
ASA-10068
Vishal5960 Dec 14, 2024
c8bf28e
ASA-10067
vishalhcl-5960 Dec 15, 2024
2a1c29f
Merge branch 'DAST-rescan' of https://github.com/jenkinsci/appscan-pl…
vishalhcl-5960 Dec 15, 2024
c9b1522
Update DynamicAnalyzer.java
vishalhcl-5960 Dec 15, 2024
cb9ddc5
Updated the warning message statement
vishalhcl-5960 Dec 16, 2024
c4e1cbd
ASA-9275
vishalhcl-5960 Dec 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ private Map<String, String> getScanProperties(Run<?,?> build, TaskListener liste
Map<String, String> properties = m_scanner.getProperties(resolver);
properties.put(CoreConstants.SCANNER_TYPE, m_scanner.getType());
properties.put(CoreConstants.APP_ID, m_application);
properties.put(CoreConstants.SCAN_NAME, resolver == null ? m_name : Util.replaceMacro(m_name, resolver) + "_" + SystemUtil.getTimeStamp()); //$NON-NLS-1$
properties.put(CoreConstants.SCAN_NAME, (resolver == null ? m_name : Util.replaceMacro(m_name, resolver)) + "_" + SystemUtil.getTimeStamp()); //$NON-NLS-1$
properties.put(CoreConstants.EMAIL_NOTIFICATION, Boolean.toString(m_emailNotification));
properties.put(CoreConstants.PERSONAL_SCAN, Boolean.toString(m_personalScan));
properties.put("FullyAutomatic", Boolean.toString(!m_intervention));
Expand Down Expand Up @@ -338,14 +338,20 @@ private void validateGeneralSettings(boolean isAppScan360, Map<String, String> p
}

private void scanIdValidation(Map<String, String> properties, IProgress progress) throws JSONException, IOException {
IScanServiceProvider scanServiceProvider = new CloudScanServiceProvider(progress, m_authProvider);
JSONObject scanDetails = scanServiceProvider.getScanDetails(properties.get(CoreConstants.SCAN_ID));
JSONObject scanDetails = ServiceUtil.scanSpecificDetails(properties.get(CoreConstants.SCANNER_TYPE), properties.get(CoreConstants.SCAN_ID), m_authProvider);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scanIdValidation() method does not belong in the AppScanBuildStep class. This should be a part of the Scanner.validateSettings() and any scanner specific checks (e.g. the one specific to SAST) should be in that scanner's validateSettings() method.
Also, the name of the method should be an action, like validateScanId().

Copy link
Collaborator Author

@vishalhcl-5960 vishalhcl-5960 Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 out of 4 validations are of general form means that is applicable for all the scan types. We can have 1 remaining validation of SAST scan in the respective class but for that we have to again make the API call in that staticAnalyzer class to fetch the scan details.
Sure, I will rename the method to "validateScanId()".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is currently an abstract method on the Scanner class named validateSettings(). Each of the individual scanners override that method to perform their own validation. There is also a validateGeneralSettings() method in the AppScanBuildStep, as well as this scanIdValidation() method. Why not put all of the settings validation into a single place in the scanners? The Scanner base class would perform the general settings validation and each individual scanner could call super.validateSettings() and then perform it's own scanner-specific validation. As it is now, the validation is happening in multiple places.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay Matt, Moved the general validations from the AppScanBuildStep to the Scanner base class & will call that new method from the respective scanner class.

if(scanDetails == null) {
throw new AbortException(Messages.error_invalid_scan_id());
} else if (!scanDetails.get(CoreConstants.APP_ID).equals(properties.get(CoreConstants.APP_ID))) {
throw new AbortException(Messages.error_invalid_scan_id_application());
} else if (!scanDetails.get("Technology").equals(ServiceUtil.updatedScanType(properties.get(CoreConstants.SCANNER_TYPE)))) {
throw new AbortException(Messages.error_invalid_scan_id_scan_type());
} else {
String status = scanDetails.getJSONObject("LatestExecution").getString("Status");
if(!(status.equals("Ready") || status.equals("Paused") || status.equals("Failed"))) {
throw new AbortException(Messages.error_scan_id_validation_status());
} else if (!scanDetails.get("RescanAllowed").equals(true) && scanDetails.get("ParsedFromUploadedFile").equals(true)) {
throw new AbortException(Messages.error_scan_id_validation_rescan_allowed());
} else if (properties.get(CoreConstants.SCANNER_TYPE).equals(Scanner.STATIC_ANALYZER) && scanDetails.containsKey("GitRepoPlatform") && scanDetails.get("GitRepoPlatform")!=null) {
throw new AbortException(Messages.error_invalid_scan_id_git_repo());
} else if (!scanDetails.get(CoreConstants.APP_ID).equals(properties.get(CoreConstants.APP_ID))) {
throw new AbortException(Messages.error_invalid_scan_id_application());
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,18 @@

package com.hcl.appscan.jenkins.plugin.scanners;

import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.util.HashMap;
import java.util.Map;

import com.hcl.appscan.sdk.CoreConstants;
import com.hcl.appscan.sdk.app.CloudApplicationProvider;
import com.hcl.appscan.sdk.logging.IProgress;
import org.apache.wink.json4j.JSONArray;
import org.apache.wink.json4j.JSONException;
import org.apache.wink.json4j.JSONObject;
import org.jenkinsci.Symbol;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
Expand Down Expand Up @@ -41,6 +49,8 @@ public class DynamicAnalyzer extends Scanner {

private static final String DYNAMIC_ANALYZER = "Dynamic Analyzer"; //$NON-NLS-1$

private boolean m_incrementalScan;
private String m_executionId;
private String m_presenceId;
private String m_scanFile;
private String m_scanType;
Expand All @@ -50,15 +60,21 @@ public class DynamicAnalyzer extends Scanner {
private String m_loginUser;
private Secret m_loginPassword;
private String m_trafficFile;
private boolean m_rescanDast;
private String m_scanId;

@Deprecated
public DynamicAnalyzer(String target) {
this(target, false, EMPTY, EMPTY, EMPTY, EMPTY, EMPTY, EMPTY, EMPTY, EMPTY, EMPTY);
this(target, false, false, EMPTY, false, EMPTY, EMPTY, EMPTY, EMPTY, EMPTY, EMPTY, EMPTY, EMPTY, EMPTY, EMPTY);
}

@Deprecated
public DynamicAnalyzer(String target, boolean hasOptions, String presenceId, String scanFile, String scanType, String optimization, String extraField, String loginUser, String loginPassword, String trafficFile, String loginType) {
public DynamicAnalyzer(String target, boolean hasOptions, boolean rescanDast, String scanId, boolean incrementalScan, String executionId, String presenceId, String scanFile, String scanType, String optimization, String extraField, String loginUser, String loginPassword, String trafficFile, String loginType) {
super(target, hasOptions);
m_rescanDast = rescanDast;
m_scanId = scanId;
m_incrementalScan = incrementalScan;
m_executionId = incrementalScan ? executionId : EMPTY;
m_presenceId = presenceId;
m_scanFile = scanFile;
m_scanType = scanFile != null && !scanFile.equals(EMPTY) ? CUSTOM : scanType;
Expand All @@ -74,6 +90,8 @@ public DynamicAnalyzer(String target, boolean hasOptions, String presenceId, Str

public DynamicAnalyzer(String target, boolean hasOptions) {
super(target, hasOptions);
m_rescanDast = false;
m_scanId = EMPTY;
m_presenceId = EMPTY;
m_scanFile = EMPTY;
m_scanType = EMPTY;
Expand Down Expand Up @@ -103,6 +121,41 @@ public String getLoginPassword() {
return Secret.toString(m_loginPassword);
}

@DataBoundSetter
public void setRescanDast(boolean rescanDast) {
m_rescanDast = rescanDast;
}

public boolean getRescanDast() {
return m_rescanDast;
}

@DataBoundSetter
public void setScanId(String scanId) {
m_scanId = scanId;
}
public String getScanId() {
return m_scanId;
}

@DataBoundSetter
public void setIncrementalScan(boolean incrementalScan) {
m_incrementalScan = incrementalScan;
}

public boolean getIncrementalScan() {
return m_incrementalScan;
}

@DataBoundSetter
public void setExecutionId(String executionId) {
m_executionId = m_incrementalScan ? executionId : EMPTY;
}

public String getExecutionId() {
return m_executionId;
}

@DataBoundSetter
public void setPresenceId(String presenceId) {
m_presenceId = presenceId;
Expand Down Expand Up @@ -198,10 +251,23 @@ public void validateSettings(JenkinsAuthenticationProvider authProvider, Map<Str
if(!ServiceUtil.hasDastEntitlement(authProvider)) {
throw new AbortException(Messages.error_active_subscription_validation(getType()));
}
if (authProvider.isAppScan360() && properties.containsKey(Scanner.PRESENCE_ID)) {
throw new AbortException(Messages.error_presence_AppScan360());
if(getRescanDast()) {
if(!properties.containsKey(CoreConstants.SCAN_ID)) {
throw new AbortException(Messages.error_empty_scan_id());
} else if (m_incrementalScan && !properties.containsKey("IncrementalBaseJobId")) {
throw new AbortException(Messages.error_empty_execution_id());
}
}
if (!authProvider.isAppScan360() && !properties.containsKey(Scanner.PRESENCE_ID) && !ServiceUtil.isValidUrl(properties.get(TARGET), authProvider, authProvider.getProxy())) {
if (authProvider.isAppScan360()) {
if (properties.containsKey(Scanner.PRESENCE_ID)) {
throw new AbortException(Messages.error_presence_AppScan360());
} else if (ServiceUtil.getA360Version(authProvider).substring(0,5).compareTo("1.4.0") != -1) {
if (!ServiceUtil.isValidUrl(properties.get(TARGET), authProvider, authProvider.getProxy())) {
throw new AbortException(Messages.error_url_validation(properties.get(TARGET)));
}
}
}
if (!getRescanDast() && !authProvider.isAppScan360() && !properties.containsKey(Scanner.PRESENCE_ID) && !ServiceUtil.isValidUrl(properties.get(TARGET), authProvider, authProvider.getProxy())) {
throw new AbortException(Messages.error_url_validation(properties.get(TARGET)));
}
}
Expand Down Expand Up @@ -259,7 +325,13 @@ public Map<String, String> getProperties(VariableResolver<String> resolver) thro
if (!m_presenceId.equals(EMPTY)) {
properties.put(PRESENCE_ID, m_presenceId);
}

if(getRescanDast() && isNullOrEmpty(getScanId()) ){
properties.put(CoreConstants.SCAN_ID,getScanId());
if(m_incrementalScan && isNullOrEmpty(m_executionId)) {
properties.put("IncrementalBaseJobId", m_executionId);
properties.put("IsIncrementalRetest", "true");
}
}
return properties;
}

Expand All @@ -286,6 +358,22 @@ public String getDisplayName() {
return "Dynamic Analysis (DAST)";
}

public ListBoxModel doFillExecutionIdItems(@RelativePath("..") @QueryParameter String credentials, @AncestorInPath ItemGroup<?> context, @QueryParameter String scanId) throws JSONException {
IAuthenticationProvider authProvider = new JenkinsAuthenticationProvider(credentials, context);
JSONArray executionDetails = ServiceUtil.getExecutionDetails(scanId, authProvider);
ListBoxModel model = new ListBoxModel();

if(executionDetails != null) {
for(int i = 0; i < executionDetails.length(); i++) {
JSONObject value = executionDetails.getJSONObject(i);
ZonedDateTime zdt = ZonedDateTime.parse((String) value.get("CreatedAt")).withZoneSameInstant(ZoneId.systemDefault());
DateTimeFormatter formatter = DateTimeFormatter.ofPattern("MMMM dd, yyyy, hh:mm a, z");
model.add(zdt.format(formatter), (String) value.get("Id"));
}
}
return model;
}

public ListBoxModel doFillScanTypeItems() {
ListBoxModel model = new ListBoxModel();
model.add(Messages.option_staging(), STAGING);
Expand Down Expand Up @@ -335,17 +423,49 @@ public FormValidation doCheckScanFile(@QueryParameter String scanFile) {
return FormValidation.ok();
}

public FormValidation doCheckTarget(@QueryParameter String target,@RelativePath("..") @QueryParameter String credentials, @AncestorInPath ItemGroup<?> context, @QueryParameter String presenceId) {
public FormValidation doCheckTarget(@QueryParameter String target,@RelativePath("..") @QueryParameter String credentials, @AncestorInPath ItemGroup<?> context, @QueryParameter String presenceId, @QueryParameter boolean rescanDast) {
JenkinsAuthenticationProvider authProvider = new JenkinsAuthenticationProvider(credentials,context);
if(!ServiceUtil.hasDastEntitlement(authProvider)) {
return FormValidation.error(Messages.error_active_subscription_validation_ui());
}
if(!authProvider.isAppScan360() && presenceId != null && presenceId.equals(EMPTY) && !target.equals(EMPTY) && !ServiceUtil.isValidUrl(target, authProvider, authProvider.getProxy())) {
if(!rescanDast && !authProvider.isAppScan360() && presenceId != null && presenceId.equals(EMPTY) && !target.equals(EMPTY) && !ServiceUtil.isValidUrl(target, authProvider, authProvider.getProxy())) {
return FormValidation.error(Messages.error_url_validation_ui());
}
if (authProvider.isAppScan360() && (ServiceUtil.getA360Version(authProvider).substring(0,5).compareTo("1.4.0") != -1)) {
if (!target.equals(EMPTY) && !ServiceUtil.isValidUrl(target, authProvider, authProvider.getProxy())) {
return FormValidation.error(Messages.error_url_validation_ui());
}
}
if(rescanDast) {
return FormValidation.ok();
}
return FormValidation.validateRequired(target);
}

public FormValidation doCheckScanId(@QueryParameter String scanId, @RelativePath("..") @QueryParameter String application, @RelativePath("..") @QueryParameter String credentials, @AncestorInPath ItemGroup<?> context) throws JSONException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much of the same logic is repeated numerous times in each of the scanners doCheckScanId() method. Common logic should be placed in the base class and only scanner specific logic should be in the scanner classes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this method for doing the UI validations, we can't have the same validations in the abstract scanner class.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is that the case? We could add a protected static method to the Scanner base class that performed all of the common checks. That method would then be called in each of the subclasses doCheckScanId() method, along with any scanner specific checks that are needed for that technology.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the "DescriptorImpl" class of each scan-type extends the "ScanDescriptor" class. I will add a new method in common class which will handle all the common validations for scanId.
protected FormValidation scanIdValidation(JSONObject scanDetails, String application)

JenkinsAuthenticationProvider provider = new JenkinsAuthenticationProvider(credentials, context);
if(scanId!=null && !scanId.isEmpty()) {
JSONObject scanDetails = ServiceUtil.scanSpecificDetails(DYNAMIC_ANALYZER, scanId, provider);
if (scanDetails == null) {
return FormValidation.error(Messages.error_invalid_scan_id_ui());
} else {
String status = scanDetails.getJSONObject("LatestExecution").getString("Status");
if (!(status.equals("Ready") || status.equals("Paused") || status.equals("Failed"))) {
return FormValidation.error(Messages.error_scan_id_validation_status());
} else if (!scanDetails.get("RescanAllowed").equals(true) && scanDetails.get("ParsedFromUploadedFile").equals(true)) {
return FormValidation.error(Messages.error_invalid_scan_id_rescan_allowed_ui());
} else if (!scanDetails.get(CoreConstants.APP_ID).equals(application)) {
return FormValidation.error(Messages.error_invalid_scan_id_application_ui());
}
}
}
return FormValidation.validateRequired(scanId);
}

public FormValidation doCheckExecutionId(@QueryParameter String executionId) {
return FormValidation.validateRequired(executionId);
}

public FormValidation doCheckPresenceId(@RelativePath("..") @QueryParameter String credentials, @AncestorInPath ItemGroup<?> context, @QueryParameter String presenceId) {
JenkinsAuthenticationProvider authProvider = new JenkinsAuthenticationProvider(credentials,context);
if(authProvider.isAppScan360()){
Expand Down
17 changes: 0 additions & 17 deletions src/main/java/com/hcl/appscan/jenkins/plugin/scanners/Scanner.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,11 @@ public abstract class Scanner extends AbstractDescribableImpl<Scanner> implement

private String m_target;
private boolean m_hasOptions;
private boolean m_rescan;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the rescan logic pulled out of the base class and added to each individual subclass?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in the description of the PR, Earlier we had the rescan parameter in the scanner class which is common class across 3 scan type classes. With that structure, I am facing issues in handling the UI part that while re-configuring a job, "include-SCA" checkbox is not getting disabled even the rescan checkbox is checked. As this is a important use-case to be handled because of that i have to insert the rescan parameter in the respective classes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know the technical reason why it wasn't working with the rescan logic in the base class?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I keep the rescan in the base scanner class then I have to assign the same name of the rescan optional block across all the config files. We can't differentiate the rescan options if we use the same name of optional block as the Event listener reads the elements by "name" or "id".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I'm following the problem, but if we need a special value in the DASTScanner, why not keep the rescan option in the base Scanner class and add a new member variable only in the DASTScanner that gets it's value from the base class?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Matt, We want to have special value in the config file of dynamic scan. If we put the rescan in the base class then we have to use same value in all the respective config files.

private String m_scanId;

public Scanner(String target, boolean hasOptions) {
m_target = target;
m_hasOptions = hasOptions;
}

public Scanner(String target, boolean hasOptions, boolean rescan, String scanId) {
m_target = target;
m_hasOptions = hasOptions;
m_rescan = rescan;
m_scanId = scanId;
}

public boolean getHasOptions() {
return m_hasOptions;
Expand All @@ -45,14 +36,6 @@ public boolean getHasOptions() {
public String getTarget() {
return m_target;
}

public boolean isRescan() {
return m_rescan;
}

public String getScanId() {
return m_scanId;
}

public abstract Map<String, String> getProperties(VariableResolver<String> resolver) throws AbortException;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,38 @@
import hudson.model.ItemGroup;
import hudson.util.FormValidation;
import hudson.util.VariableResolver;
import org.apache.wink.json4j.JSONException;
import org.apache.wink.json4j.JSONObject;
import org.jenkinsci.Symbol;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.QueryParameter;

import java.util.HashMap;
import java.util.Map;

public class SoftwareCompositionAnalyzer extends Scanner {

private boolean m_rescan;
private String m_scanId;

@Deprecated
public SoftwareCompositionAnalyzer(String target){
super(target, false);
}

public SoftwareCompositionAnalyzer(String target, boolean rescan, String scanId){
super(target, false, rescan, scanId);
public SoftwareCompositionAnalyzer(String target, boolean rescan, String scanId) {
super(target, false);
m_rescan = rescan;
m_scanId = scanId;
}

@DataBoundConstructor
public SoftwareCompositionAnalyzer(String target, boolean hasOptions, boolean rescan, String scanId){
super(target, false, rescan, scanId);
public SoftwareCompositionAnalyzer(String target, boolean hasOptions){
super(target, hasOptions);
m_rescan = false;
m_scanId = EMPTY;
}


Expand All @@ -46,6 +56,23 @@ public String getType() {
return SOFTWARE_COMPOSITION_ANALYZER;
}

@DataBoundSetter
public void setRescan(boolean rescan) {
m_rescan = rescan;
}

public boolean getRescan() {
return m_rescan;
}

@DataBoundSetter
public void setScanId(String scanId) {
m_scanId = scanId;
}
public String getScanId() {
return m_scanId;
}

public void validateSettings(JenkinsAuthenticationProvider authProvider, Map<String, String> properties, IProgress progress) throws AbortException {
if(!ServiceUtil.hasScaEntitlement(authProvider)) {
throw new AbortException(Messages.error_active_subscription_validation(getType()));
Expand All @@ -59,7 +86,7 @@ public void validateSettings(JenkinsAuthenticationProvider authProvider, Map<Str
public Map<String, String> getProperties(VariableResolver<String> resolver) throws AbortException {
Map<String, String> properties = new HashMap<String, String>();
properties.put(TARGET, resolver == null ? getTarget() : resolvePath(getTarget(), resolver));
if(isRescan() && isNullOrEmpty(getScanId())) {
if(getRescan() && isNullOrEmpty(getScanId())) {
properties.put(CoreConstants.SCAN_ID,getScanId());
}
return properties;
Expand All @@ -74,10 +101,22 @@ public String getDisplayName() {
return "Software Composition Analysis (SCA)";
}

public FormValidation doCheckScanId(@QueryParameter String scanId, @RelativePath("..") @QueryParameter String application, @RelativePath("..") @QueryParameter String credentials, @AncestorInPath ItemGroup<?> context) {
public FormValidation doCheckScanId(@QueryParameter String scanId, @RelativePath("..") @QueryParameter String application, @RelativePath("..") @QueryParameter String credentials, @AncestorInPath ItemGroup<?> context) throws JSONException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much of the same logic is repeated numerous times in each of the scanners doCheckScanId() method. Common logic should be placed in the base class and only scanner specific logic should be in the scanner classes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this method for doing the UI validations, we can't have the same validations in the abstract scanner class.
Even for "target" field, though that is present in the scanner class but we are doing the FormValidation for that field in the respective classes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the "DescriptorImpl" class of each scan-type extends the "ScanDescriptor" class. I will add a new method in common class which will handle all the common validations for scanId.
protected FormValidation scanIdValidation(JSONObject scanDetails, String application)

JenkinsAuthenticationProvider provider = new JenkinsAuthenticationProvider(credentials, context);
if(scanId!=null && !scanId.isEmpty() && !ServiceUtil.isScanId(scanId,application,SOFTWARE_COMPOSITION_ANALYZER,provider)) {
return FormValidation.error(Messages.error_invalid_scan_id_ui());
if(scanId!=null && !scanId.isEmpty()) {
JSONObject scanDetails = ServiceUtil.scanSpecificDetails(SOFTWARE_COMPOSITION_ANALYZER, scanId, provider);
if(scanDetails == null) {
return FormValidation.error(Messages.error_invalid_scan_id_ui());
} else {
String status = scanDetails.getJSONObject("LatestExecution").getString("Status");
if (!(status.equals("Ready") || status.equals("Paused") || status.equals("Failed"))) {
return FormValidation.error(Messages.error_scan_id_validation_status());
} else if (!scanDetails.get("RescanAllowed").equals(true) && scanDetails.get("ParsedFromUploadedFile").equals(true)) {
return FormValidation.error(Messages.error_invalid_scan_id_rescan_allowed_ui());
} else if (!scanDetails.get(CoreConstants.APP_ID).equals(application)) {
return FormValidation.error(Messages.error_invalid_scan_id_application_ui());
}
}
}
return FormValidation.validateRequired(scanId);
}
Expand Down
Loading
Loading