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.

Sign up for GitHub

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

authhelper: SAST (SonarLint) Fixes #6236

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

authhelper: SAST (SonarLint) Fixes #6236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions addOns/authhelper/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- Tweaked the auth report summary keys.
- Only check URLs and methods once for being good verification requests.
- Added API support to the browser based auth method proxy.
- Maintenance changes.

### Fixed
- Correctly read the API parameters when setting up Browser Based Authentication.
Expand Down
16 changes: 8 additions & 8 deletions addOns/authhelper/src/main/java/org/zaproxy/addon/authhelper/AuthDiagnosticCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,8 @@ protected JSONObject sanitiseJson(JSONObject jsonObject) {
JSONObject sanObj = new JSONObject();
for (Object key : jsonObject.keySet()) {
Object val = jsonObject.get(key);
if (val instanceof String) {
sanObj.put(key, getSanitizedToken((String) val));
if (val instanceof String valStr) {
sanObj.put(key, getSanitizedToken(valStr));
} else {
sanObj.put(key, val);
}
Expand All @@ -244,17 +244,17 @@ protected JSONObject sanitiseJson(JSONObject jsonObject) {
}

protected Object sanitiseJson(Object obj) {
if (obj instanceof JSONObject) {
return sanitiseJson((JSONObject) obj);
} else if (obj instanceof JSONArray) {
if (obj instanceof JSONObject jObj) {
return sanitiseJson(jObj);
} else if (obj instanceof JSONArray jArr) {
JSONArray sanArr = new JSONArray();
Object[] oa = ((JSONArray) obj).toArray();
Object[] oa = jArr.toArray();
for (int i = 0; i < oa.length; i++) {
sanArr.add(sanitiseJson(oa[i]));
}
return sanArr;
} else if (obj instanceof String) {
return getSanitizedToken((String) obj);
} else if (obj instanceof String objStr) {
return getSanitizedToken(objStr);

} else {
return obj;
Expand Down
13 changes: 6 additions & 7 deletions addOns/authhelper/src/main/java/org/zaproxy/addon/authhelper/AuthTestDialog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,10 @@ public AuthTestDialog(ExtensionAuthhelper ext, Frame owner) {
JButton copyButton =
new JButton(Constant.messages.getString("authhelper.auth.test.dialog.button.copy"));
copyButton.addActionListener(
l -> {
Toolkit.getDefaultToolkit()
.getSystemClipboard()
.setContents(new StringSelection(diagnosticField.getText()), null);
});
l ->
Toolkit.getDefaultToolkit()
.getSystemClipboard()
.setContents(new StringSelection(diagnosticField.getText()), null));

buttonPanel.add(new JLabel(), LayoutHelper.getGBC(0, 0, 1, 0.3D));
buttonPanel.add(copyButton, LayoutHelper.getGBC(1, 0, 1, 0.3D));
Expand Down Expand Up @@ -236,7 +235,7 @@ private JPanel getResultsPanel() {
Constant.messages.getString(
"authhelper.auth.test.dialog.results.verif")),
LayoutHelper.getGBC(0, y, 1, 1L, insets));
resultsPanel.add(verifIdLabel, LayoutHelper.getGBC(1, y++, 1, 1L, insets));
resultsPanel.add(verifIdLabel, LayoutHelper.getGBC(1, y, 1, 1L, insets));
resetResultsPanel();
}
return resultsPanel;
Expand Down Expand Up @@ -529,7 +528,7 @@ public JButton[] getExtraButtons() {
@Override
public void save() {
resetResultsPanel();
Thread t = new Thread(() -> authenticate(), "ZAP-auth-tester");
Thread t = new Thread(this::authenticate, "ZAP-auth-tester");
t.start();
// Save the values for next time
this.saveDetails();
Expand Down
62 changes: 27 additions & 35 deletions addOns/authhelper/src/main/java/org/zaproxy/addon/authhelper/AuthUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@
import org.parosproxy.paros.network.HttpHeader;
import org.parosproxy.paros.network.HttpHeaderField;
import org.parosproxy.paros.network.HttpMessage;
import org.parosproxy.paros.network.HttpResponseHeader;
import org.parosproxy.paros.network.HttpSender;
import org.parosproxy.paros.network.HttpStatusCode;
import org.parosproxy.paros.view.View;
Expand All @@ -83,7 +82,6 @@
import org.zaproxy.zap.extension.users.ExtensionUserManagement;
import org.zaproxy.zap.model.Context;
import org.zaproxy.zap.model.SessionStructure;
import org.zaproxy.zap.session.WebSession;
import org.zaproxy.zap.users.User;
import org.zaproxy.zap.utils.Pair;
import org.zaproxy.zap.utils.Stats;
Expand Down Expand Up @@ -471,7 +469,7 @@ private static boolean internalAuthenticateAsUser(
boolean pwdAdded = false;

Iterator<AuthenticationStep> it = steps.stream().sorted().iterator();
for (; it.hasNext(); ) {
while (it.hasNext()) {
AuthenticationStep step = it.next();
if (!step.isEnabled()) {
continue;
Expand Down Expand Up @@ -539,7 +537,7 @@ private static boolean internalAuthenticateAsUser(
sendReturn(diags, wd, pwdField);
}

for (; it.hasNext(); ) {
while (it.hasNext()) {
AuthenticationStep step = it.next();
if (!step.isEnabled()) {
continue;
Expand All @@ -557,18 +555,15 @@ private static boolean internalAuthenticateAsUser(
incStatsCounter(loginPageUrl, AUTH_BROWSER_PASSED_STATS);
AuthUtils.sleep(TimeUnit.SECONDS.toMillis(waitInSecs));

if (context != null) {
if (context.getAuthenticationMethod().getPollUrl() == null) {
// We failed to identify a suitable URL for polling.
// This can happen for more traditional apps - refresh the current one in case
// its a good option.
wd.get(wd.getCurrentUrl());
AuthUtils.sleep(TimeUnit.SECONDS.toMillis(1));
diags.recordStep(
wd,
Constant.messages.getString(
"authhelper.auth.method.diags.steps.refresh"));
}
if (context != null && context.getAuthenticationMethod().getPollUrl() == null) {
// We failed to identify a suitable URL for polling.
// This can happen for more traditional apps - refresh the current one in case
// its a good option.
wd.get(wd.getCurrentUrl());
AuthUtils.sleep(TimeUnit.SECONDS.toMillis(1));
diags.recordStep(
wd,
Constant.messages.getString("authhelper.auth.method.diags.steps.refresh"));
}
return true;
}
Expand Down Expand Up @@ -757,7 +752,7 @@ public static Map<String, SessionToken> getResponseSessionTokens(HttpMessage msg
} catch (JSONException e) {
LOGGER.debug(
"Unable to parse authentication response body from {} as JSON: {} ",
msg.getRequestHeader().getURI().toString(),
msg.getRequestHeader().getURI(),
responseData,
e);
}
Expand Down Expand Up @@ -787,7 +782,7 @@ public static List<Pair<String, String>> getHeaderTokens(
String hv =
header.getValue()
.replace(token.getValue(), "{%" + token.getToken() + "%}");
list.add(new Pair<String, String>(header.getName(), hv));
list.add(new Pair<>(header.getName(), hv));
}
}
if (incCookies) {
Expand Down Expand Up @@ -831,7 +826,7 @@ public static Map<String, SessionToken> getAllTokens(HttpMessage msg, boolean in
} catch (JSONException e) {
LOGGER.debug(
"Unable to parse authentication response body from {} as JSON: {}",
msg.getRequestHeader().getURI().toString(),
msg.getRequestHeader().getURI(),
responseData);
}
}
Expand Down Expand Up @@ -928,15 +923,15 @@ public static void extractJsonTokens(

private static void extractJsonTokens(
Object obj, String parent, Map<String, SessionToken> tokens) {
if (obj instanceof JSONObject) {
extractJsonTokens((JSONObject) obj, parent, tokens);
} else if (obj instanceof JSONArray) {
Object[] oa = ((JSONArray) obj).toArray();
if (obj instanceof JSONObject jsonObj) {
extractJsonTokens(jsonObj, parent, tokens);
} else if (obj instanceof JSONArray jsonArr) {
Object[] oa = (jsonArr.toArray());
for (int i = 0; i < oa.length; i++) {
extractJsonTokens(oa[i], parent + "[" + i + "]", tokens);
}
} else if (obj instanceof String) {
addToMap(tokens, new SessionToken(SessionToken.JSON_SOURCE, parent, (String) obj));
} else if (obj instanceof String str) {
addToMap(tokens, new SessionToken(SessionToken.JSON_SOURCE, parent, str));
}
}

Expand Down Expand Up @@ -1153,7 +1148,7 @@ public static boolean isRelevantToAuth(HttpMessage msg) {
* given URL.
*/
public static void checkLoginLinkVerification(
HttpSender authSender, User user, WebSession session, String loginUrl) {
HttpSender authSender, User user, String loginUrl) {
AuthCheckingStrategy verif =
user.getContext().getAuthenticationMethod().getAuthCheckingStrategy();
if (!AuthCheckingStrategy.AUTO_DETECT.equals(verif)) {
Expand All @@ -1165,13 +1160,13 @@ public static void checkLoginLinkVerification(
// Try to top level link first, if the page has the login form then its less likely
// to have a link to one
testUri.setPath("");
if (checkLoginLinkVerification(authSender, user, session, testUri)) {
if (checkLoginLinkVerification(authSender, user, testUri)) {
// The top level URL worked :)
return;
}
testUri = new URI(loginUrl, true);
}
checkLoginLinkVerification(authSender, user, session, testUri);
checkLoginLinkVerification(authSender, user, testUri);

} catch (Exception e) {
LOGGER.warn(
Expand All @@ -1183,7 +1178,7 @@ public static void checkLoginLinkVerification(
}

private static boolean checkLoginLinkVerification(
HttpSender authSender, User user, WebSession session, URI testUri) {
HttpSender authSender, User user, URI testUri) {
try {
// Send an unauthenticated req to the test site, manually following redirects as needed
HttpMessage msg = new HttpMessage(testUri);
Expand All @@ -1192,10 +1187,7 @@ private static boolean checkLoginLinkVerification(
historyProvider.addAuthMessageToHistory(msg);
int count = 0;
while (HttpStatusCode.isRedirection(msg.getResponseHeader().getStatusCode())) {
testUri =
new URI(
msg.getResponseHeader().getHeader(HttpResponseHeader.LOCATION),
true);
testUri = new URI(msg.getResponseHeader().getHeader(HttpHeader.LOCATION), true);
msg = new HttpMessage(testUri);
unauthSender.sendAndReceive(msg);
historyProvider.addAuthMessageToHistory(msg);
Expand Down Expand Up @@ -1237,13 +1229,13 @@ private static boolean checkLoginLinkVerification(
LOGGER.debug(
"Response to {} is no good as a login link verification req, an authenticated request also includes the link {}",
testUri,
link.toString());
link);
return false;
}
LOGGER.debug(
"Found good login link verification req {}, contains login link {}",
testUri,
link.toString());
link);

AuthenticationMethod authMethod = user.getContext().getAuthenticationMethod();
authMethod.setAuthCheckingStrategy(AuthCheckingStrategy.POLL_URL);
Expand Down
22 changes: 10 additions & 12 deletions ...uthhelper/src/main/java/org/zaproxy/addon/authhelper/AuthenticationDetectionScanRule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,25 +249,23 @@ private static HtmlParameter getMatchingParam(
}

void extractJsonStrings(JSON json, String parent, TreeSet<HtmlParameter> params) {
if (json instanceof JSONObject) {
JSONObject jsonObject = (JSONObject) json;
for (Object key : jsonObject.keySet()) {
Object obj = jsonObject.get(key);
if (obj instanceof JSONObject) {
extractJsonStrings(
(JSONObject) obj, normalisedKey(parent, (String) key), params);
} else if (obj instanceof String) {
if (json instanceof JSONObject jsonObj) {
for (Object key : jsonObj.keySet()) {
Object obj = jsonObj.get(key);
if (obj instanceof JSONObject jsonObj2) {
extractJsonStrings(jsonObj2, normalisedKey(parent, (String) key), params);
} else if (obj instanceof String objStr) {
params.add(
new HtmlParameter(
HtmlParameter.Type.form,
normalisedKey(parent, (String) key),
(String) obj));
objStr));
}
}
} else if (json instanceof JSONArray) {
Object[] oa = ((JSONArray) json).toArray();
} else if (json instanceof JSONArray jsonArr) {
Object[] oa = jsonArr.toArray();
for (int i = 0; i < oa.length; i++) {
extractJsonStrings((JSONArray) json, parent + "[" + i + "]", params);
extractJsonStrings(jsonArr, parent + "[" + i + "]", params);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions addOns/authhelper/src/main/java/org/zaproxy/addon/authhelper/AuthenticationDiagnostics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ public void recordStep(WebDriver wd, String description, WebElement element) {
createStep();
}

private <T> void processStorage(JavascriptExecutor je, DiagnosticBrowserStorageItem.Type type) {
private void processStorage(JavascriptExecutor je, DiagnosticBrowserStorageItem.Type type) {
@SuppressWarnings("unchecked")
List<Map<String, String>> storage =
(List<Map<String, String>>) je.executeScript(type.getScript());
Expand All @@ -232,7 +232,7 @@ private <T> void processStorage(JavascriptExecutor je, DiagnosticBrowserStorageI
.forEach(currentStep.getBrowserStorageItems()::add);
}

private DiagnosticWebElement createDiagnosticWebElement(
private static DiagnosticWebElement createDiagnosticWebElement(
WebDriver wd, List<WebElement> forms, WebElement element) {
if (element == null) {
return null;
Expand Down
2 changes: 1 addition & 1 deletion ...s/authhelper/src/main/java/org/zaproxy/addon/authhelper/AuthenticationRequestDetails.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public class AuthenticationRequestDetails {
public enum AuthDataType {
FORM,
JSON
};
}

private final URI uri;
private final HtmlParameter userParam;
Expand Down
2 changes: 1 addition & 1 deletion ...helper/src/main/java/org/zaproxy/addon/authhelper/AutoDetectAuthenticationMethodType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ public AuthMethodApiResponseRepresentation(Map<String, T> values) {
@Override
public JSON toJSON() {
JSONObject response = new JSONObject();
response.put(getName(), super.toJSON());
response.put(super.getName(), super.toJSON());
return response;
}
}
Expand Down
7 changes: 4 additions & 3 deletions ...per/src/main/java/org/zaproxy/addon/authhelper/AutoDetectSessionManagementMethodType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,10 @@ public SessionManagementMethod clone() {

@Override
public boolean equals(Object obj) {
if (obj == null) return false;
if (getClass() != obj.getClass()) return false;
return true;
if (obj == null) {
return false;
}
return getClass() == obj.getClass();
}

@Override
Expand Down
Loading