Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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 @@ -793,13 +793,55 @@ public static void fillField(WebElement field, String value) {
field.sendKeys(value);
}

/**
* Fills a field with a value and fires input/change events to support any JavaScript
* framework (React, Angular, Vue, Svelte, Web Components, plain HTML with JS listeners).
*
* <p>This method performs the same field filling as {@link #fillField(WebElement, String)},
* but additionally fires 'input' and 'change' events via JavaScript. This ensures
* compatibility with frameworks that use synthetic or virtual event systems and may not
* respond to native browser events triggered by Selenium's sendKeys().
*
* <p>Events are fired with both {@code bubbles: true} and {@code composed: true} so they
* propagate correctly through Shadow DOM boundaries (Web Components).
*
* @param field the form field element
* @param value the value to fill into the field
* @param driver the WebDriver instance to execute JavaScript
*/
public static void fillFieldWithEvents(WebElement field, String value, WebDriver driver) {
// Fill the field using native sendKeys
if (StringUtils.isNotEmpty(getAttribute(field, "value"))) {
field.clear();
}
field.sendKeys(value);

// Fire input and change events for framework compatibility.
// bubbles:true — event travels up the DOM tree (React, Angular, Vue, jQuery)
// composed:true — event crosses Shadow DOM boundaries (Web Components)
try {
if (driver instanceof JavascriptExecutor je) {
je.executeScript(
"var element = arguments[0];"
+ "var event = new Event('input', { bubbles: true, composed: true });"
+ "element.dispatchEvent(event);"
+ "event = new Event('change', { bubbles: true, composed: true });"
+ "element.dispatchEvent(event);",
field);
}
} catch (Exception e) {
// Silently ignore JavaScript execution errors - field was still filled
LOGGER.debug("Failed to fire input/change events for field: {}", e.getMessage());
}
}

public static void fillUserName(
AuthenticationDiagnostics diags,
WebDriver wd,
String username,
WebElement field,
int stepDelayInSecs) {
fillField(field, username);
fillFieldWithEvents(field, username, wd);
diags.recordStep(
wd,
Constant.messages.getString("authhelper.auth.method.diags.steps.username"),
Expand All @@ -813,7 +855,7 @@ public static void fillPassword(
String password,
WebElement field,
int stepDelayInSecs) {
fillField(field, password);
fillFieldWithEvents(field, password, wd);
diags.recordStep(
wd,
Constant.messages.getString("authhelper.auth.method.diags.steps.password"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.openqa.selenium.By;
import org.openqa.selenium.JavascriptExecutor;
import org.openqa.selenium.Keys;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebElement;
Expand Down Expand Up @@ -182,6 +183,31 @@ public void setOrder(int order) {
}

public WebElement execute(WebDriver wd, UsernamePasswordAuthenticationCredentials credentials) {
return execute(wd, credentials, -1);
}

/**
* Executes this step.
*
* @param wd the WebDriver instance.
* @param credentials the user credentials.
* @param totpCharIndex when &gt;= 0, only the character at this index of the TOTP code is
* sent to the field. Used for pages with individual single-character OTP input boxes.
* Pass -1 to send the full TOTP code (default behaviour for a single combined field).
* @return the element interacted with, or {@code null} for WAIT steps.
*/
public WebElement execute(
WebDriver wd,
UsernamePasswordAuthenticationCredentials credentials,
int totpCharIndex) {
return execute(wd, credentials, totpCharIndex, null);
}

public WebElement execute(
WebDriver wd,
UsernamePasswordAuthenticationCredentials credentials,
int totpCharIndex,
String precomputedTotpCode) {
if (getType() == Type.WAIT) {
try {
Thread.sleep(timeout);
Expand All @@ -204,27 +230,35 @@ public WebElement execute(WebDriver wd, UsernamePasswordAuthenticationCredential
break;

case CUSTOM_FIELD:
AuthUtils.fillField(element, value);
AuthUtils.fillFieldWithEvents(element, value, wd);
break;

case ESCAPE:
element.sendKeys(Keys.ESCAPE);
break;

case PASSWORD:
AuthUtils.fillField(element, credentials.getPassword());
AuthUtils.fillFieldWithEvents(element, credentials.getPassword(), wd);
break;

case RETURN:
element.sendKeys(Keys.RETURN);
break;

case TOTP_FIELD:
element.sendKeys(getTotpCode(credentials));
String totpCode =
(precomputedTotpCode != null)
? precomputedTotpCode
: getTotpCode(credentials).toString();
String totpValue =
(totpCharIndex >= 0 && totpCharIndex < totpCode.length())
? String.valueOf(totpCode.charAt(totpCharIndex))
: totpCode;
AuthUtils.fillFieldWithEvents(element, totpValue, wd);
break;

case USERNAME:
AuthUtils.fillField(element, credentials.getUsername());
AuthUtils.fillFieldWithEvents(element, credentials.getUsername(), wd);
break;

default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.zaproxy.addon.authhelper.AuthUtils;
import org.zaproxy.addon.authhelper.AuthenticationDiagnostics;
import org.zaproxy.addon.authhelper.internal.AuthenticationStep;
import org.zaproxy.addon.commonlib.internal.TotpSupport;
import org.zaproxy.zap.authentication.UsernamePasswordAuthenticationCredentials;
import org.zaproxy.zap.model.Context;

Expand Down Expand Up @@ -62,6 +63,19 @@ public Result authenticate(
boolean userAdded = false;
boolean pwdAdded = false;

// Count how many TOTP_FIELD steps exist to detect split single-character OTP boxes.
long totpFieldStepCount =
steps.stream()
.filter(AuthenticationStep::isEnabled)
.filter(s -> s.getType() == AuthenticationStep.Type.TOTP_FIELD)
.count();
boolean splitTotpFields = totpFieldStepCount > 1;
int totpCharIndex = 0;
// Pre-generate TOTP code once so all 6 character fields use the same code,
// avoiding clock-window drift if a 30s boundary crosses during step execution.
String precomputedTotpCode =
splitTotpFields ? TotpSupport.getCode(credentials) : null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be generated when needed not beforehand, the code will be outdated if the steps before the TOTP_FIELD step take more time than the defined period. This also fails to validate the required number of digits.

The change itself is also out of place, this breaks the step implementation isolation, better to provide a state object for the steps than add ad-hoc step specific logic here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed. Added AuthenticationContext — a state object created once per authentication attempt and passed to every step. The TOTP code is now generated lazily at the moment the first TOTP_FIELD step executes (not before all steps run), so it stays fresh even if preceding steps take time. All TOTP logic remains inside AuthenticationStep; DefaultAuthenticator has no step-specific knowledge.


Iterator<AuthenticationStep> it = steps.stream().sorted().iterator();
while (it.hasNext()) {
AuthenticationStep step = it.next();
Expand All @@ -73,7 +87,12 @@ public Result authenticate(
break;
}

WebElement element = step.execute(wd, credentials);
WebElement element;
if (step.getType() == AuthenticationStep.Type.TOTP_FIELD && splitTotpFields) {
element = step.execute(wd, credentials, totpCharIndex++, precomputedTotpCode);
} else {
element = step.execute(wd, credentials);
}
diags.recordStep(wd, step.getDescription(), element);

switch (step.getType()) {
Expand Down Expand Up @@ -144,7 +163,11 @@ public Result authenticate(
continue;
}

step.execute(wd, credentials);
if (step.getType() == AuthenticationStep.Type.TOTP_FIELD && splitTotpFields) {
step.execute(wd, credentials, totpCharIndex++, precomputedTotpCode);
} else {
step.execute(wd, credentials);
}
diags.recordStep(wd, step.getDescription());

AuthUtils.sleepMax(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,15 @@
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
Expand Down Expand Up @@ -65,6 +69,7 @@
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestTemplate;
import org.junit.jupiter.api.extension.RegisterExtension;
Expand All @@ -75,6 +80,7 @@
import org.junit.jupiter.params.provider.ValueSource;
import org.openqa.selenium.By;
import org.openqa.selenium.Dimension;
import org.openqa.selenium.JavascriptExecutor;
import org.openqa.selenium.OutputType;
import org.openqa.selenium.Point;
import org.openqa.selenium.Rectangle;
Expand Down Expand Up @@ -1541,6 +1547,71 @@ void shouldConfigureContext() throws Exception {
}
}

@Nested
class FillFieldWithEventsTest {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These tests are not testing anything (you can change the script and the tests still pass). Please, add proper tests in BrowserTest.


private interface JsWebDriver extends WebDriver, JavascriptExecutor {}

@Test
void shouldFillFieldWithValue() {
// Given
WebElement field = mock(WebElement.class);
given(field.getAttribute("value")).willReturn(null);
WebDriver driver = mock(WebDriver.class);

// When
AuthUtils.fillFieldWithEvents(field, "testValue", driver);

// Then
verify(field).sendKeys("testValue");
verify(field, never()).clear();
}

@Test
void shouldClearExistingValueBeforeFilling() {
// Given
WebElement field = mock(WebElement.class);
given(field.getAttribute("value")).willReturn("existing");
WebDriver driver = mock(WebDriver.class);

// When
AuthUtils.fillFieldWithEvents(field, "newValue", driver);

// Then
var ordered = inOrder(field);
ordered.verify(field).clear();
ordered.verify(field).sendKeys("newValue");
}

@Test
void shouldFireJsEventsWhenDriverIsJavascriptExecutor() {
// Given
WebElement field = mock(WebElement.class);
given(field.getAttribute("value")).willReturn(null);
JsWebDriver driver = mock(JsWebDriver.class);

// When
AuthUtils.fillFieldWithEvents(field, "testValue", driver);

// Then
verify(field).sendKeys("testValue");
verify(driver).executeScript(anyString(), eq(field));
}

@Test
void shouldNotFailWhenDriverIsNotJavascriptExecutor() {
// Given
WebElement field = mock(WebElement.class);
given(field.getAttribute("value")).willReturn(null);
WebDriver driver = mock(WebDriver.class);

// When / Then
assertDoesNotThrow(() -> AuthUtils.fillFieldWithEvents(field, "testValue", driver));
verify(field).sendKeys("testValue");
verifyNoInteractions(driver);
}
}

class TestWebElement implements WebElement {

private String tag;
Expand Down
Loading
Loading