[grid] Dynamic Grid standalone support passing basic auth credential

[VietND96]

🔗 Related Issues

💥 What does this PR do?

Dynamic Grid in Docker deploy passing all env variables SE_* to browser containers.
When deploying Dynamic Grid in Standalone Docker, SE_ROUTER_USERNAME, SE_ROUTER_PASSWORD to enable Grid basic authen, then it also passes to the browser container to start a session (using container standalone). There is a step to verify server to start

10:54:02.827 INFO [DockerSessionFactory.apply] - Waiting for server to start (job: selenium-session-chrome-1770720840810-90139f98, url: http://10.1.2.193:4444/wd/hub)⁠)

Without basic auth, this step will never pass event container is up fully.
So, Node Docker will get credential from env var to HttpClient config.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

• Cleanup (formatting, renaming)
• Bug fix (backwards compatible)
• New feature (non-breaking change which adds functionality and tests!)
• Breaking change (fix or feature that would cause existing functionality to change)

[qodo-code-review]

PR Type

Bug fix, Enhancement

---

Description

• Add basic authentication support for Dynamic Grid Docker standalone mode

• Read SE_ROUTER_USERNAME and SE_ROUTER_PASSWORD environment variables

• Apply credentials to HttpClient configuration for container communication

• Add warning message when server returns 401 authentication error

---

File Walkthrough

Relevant files

Bug fix

DockerSessionFactory.java Add basic auth support for Docker container communication java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java Added import for UsernameAndPassword class Introduced applyBasicAuth() method to read SE_ROUTER_USERNAME and SE_ROUTER_PASSWORD environment variables and configure client authentication Applied basic auth configuration to HttpClient before waiting for server startup Enhanced waitForServerToStart() to detect and log 401 authentication errors with helpful guidance +20/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟡	🎫 #1234 🔴 Restore behavior where click() triggers JavaScript in a link's href (regression in Selenium 2.48.x vs 2.47.1) on Firefox 42. Add/adjust a test case to cover clicking a link with javascript: in the href so the regression is prevented.
🟡	🎫 #5678 🔴 Prevent or resolve ChromeDriver "ConnectFailure (Connection refused)" errors that occur on repeated instantiation on Ubuntu 16.04 with Chrome 65 / ChromeDriver 2.35 / Selenium 3.9.0. Provide a reproducible fix or mitigation for repeated ChromeDriver session creation failures, validated against the reported environment.
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: 401 handling loops: When /status returns 401, the code only logs a warning and continues waiting for a 200 response, which can cause repeated warnings and an eventual timeout instead of failing fast with a clear, actionable error. Referred Code if (401 == response.getStatus()) { LOG.warning( "Server requires basic authentication. " + "Set SE_ROUTER_USERNAME and SE_ROUTER_PASSWORD environment variables " + "to provide credentials."); } return 200 == response.getStatus(); }); Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Warning may spam: The new 401 warning is emitted inside a polling loop and may be logged repeatedly at runtime, potentially creating noisy logs and making monitoring harder depending on polling frequency and logging configuration. Referred Code if (401 == response.getStatus()) { LOG.warning( "Server requires basic authentication. " + "Set SE_ROUTER_USERNAME and SE_ROUTER_PASSWORD environment variables " + "to provide credentials."); } Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fail fast on auth requirement Throw a SessionNotCreatedException when a 401 status is received to provide immediate feedback instead of waiting for a timeout. java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [525-530] if (401 == response.getStatus()) { LOG.warning( "Server requires basic authentication. " + "Set SE_ROUTER_USERNAME and SE_ROUTER_PASSWORD environment variables " + "to provide credentials."); + throw new SessionNotCreatedException( + "Basic authentication required: please set SE_ROUTER_USERNAME and SE_ROUTER_PASSWORD"); } Apply / Chat Suggestion importance[1-10]: 8 __ Why: This suggestion significantly improves user experience by failing fast with a clear error message when authentication is required, instead of making the user wait for a long timeout.	Medium
	Allow using an empty password Remove the check for an empty password string to allow its use as a valid credential. This involves removing !routerPassword.isEmpty() from the condition. java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [538-543] if (routerUsername != null && !routerUsername.isEmpty() - && routerPassword != null - && !routerPassword.isEmpty()) { + && routerPassword != null) { return clientConfig.authenticateAs(new UsernameAndPassword(routerUsername, routerPassword)); } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that disallowing empty passwords might be too restrictive and proposes a simple change to support this valid use case, improving flexibility.	Low
High-level	Decouple node from router configuration Decouple the DockerSessionFactory from the Router's configuration by passing credentials via the constructor instead of reading SE_ROUTER_* environment variables directly. This improves modularity and maintainability. Examples: java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [535-545] private ClientConfig applyBasicAuth(ClientConfig clientConfig) { String routerUsername = System.getenv("SE_ROUTER_USERNAME"); String routerPassword = System.getenv("SE_ROUTER_PASSWORD"); if (routerUsername != null && !routerUsername.isEmpty() && routerPassword != null && !routerPassword.isEmpty()) { return clientConfig.authenticateAs(new UsernameAndPassword(routerUsername, routerPassword)); } return clientConfig; ... (clipped 1 lines) Solution Walkthrough: Before: class DockerSessionFactory implements SessionFactory { // constructor does not receive credentials public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sessionRequest) { // ... ClientConfig clientConfig = ClientConfig.defaultConfig().baseUrl(remoteAddress); clientConfig = applyBasicAuth(clientConfig); HttpClient client = clientFactory.createClient(clientConfig); // ... } private ClientConfig applyBasicAuth(ClientConfig clientConfig) { String routerUsername = System.getenv("SE_ROUTER_USERNAME"); String routerPassword = System.getenv("SE_ROUTER_PASSWORD"); if (routerUsername != null && routerPassword != null) { return clientConfig.authenticateAs(new UsernameAndPassword(routerUsername, routerPassword)); } return clientConfig; } } After: class DockerSessionFactory implements SessionFactory { private final Optional<UsernameAndPassword> credentials; public DockerSessionFactory(..., Optional<UsernameAndPassword> credentials) { // ... this.credentials = credentials; } public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sessionRequest) { // ... ClientConfig clientConfig = ClientConfig.defaultConfig().baseUrl(remoteAddress); credentials.ifPresent(creds -> clientConfig.authenticateAs(creds)); HttpClient client = clientFactory.createClient(clientConfig); // ... } // The applyBasicAuth method is removed. // Logic to read env vars is moved to the class that instantiates DockerSessionFactory. } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a design flaw, tight coupling, where the DockerSessionFactory directly reads router-specific environment variables, and proposes a superior architectural solution using dependency injection.	Medium
General	Handle partial credential settings Add a warning log if only one of SE_ROUTER_USERNAME or SE_ROUTER_PASSWORD is set, as both are required for basic authentication. java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [538-544] -if (routerUsername != null - && !routerUsername.isEmpty() - && routerPassword != null - && !routerPassword.isEmpty()) { +if ((routerUsername == null || routerUsername.isEmpty()) + ^ (routerPassword == null || routerPassword.isEmpty())) { + LOG.warning( + "Both SE_ROUTER_USERNAME and SE_ROUTER_PASSWORD must be set together for basic auth"); + return clientConfig; +} +if (routerUsername != null && routerPassword != null) { return clientConfig.authenticateAs(new UsernameAndPassword(routerUsername, routerPassword)); } return clientConfig; Apply / Chat Suggestion importance[1-10]: 7 __ Why: This suggestion improves usability by adding a warning for a likely misconfiguration where only one of the two required authentication variables is set, helping users to fix it.	Medium
	Trim credentials from env Trim leading/trailing whitespace from the SE_ROUTER_USERNAME and SE_ROUTER_PASSWORD environment variables to prevent potential authentication issues. java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [536-537] -String routerUsername = System.getenv("SE_ROUTER_USERNAME"); -String routerPassword = System.getenv("SE_ROUTER_PASSWORD"); +String routerUsername = Optional.ofNullable(System.getenv("SE_ROUTER_USERNAME")) + .map(String::trim) + .orElse(null); +String routerPassword = Optional.ofNullable(System.getenv("SE_ROUTER_PASSWORD")) + .map(String::trim) + .orElse(null); Apply / Chat Suggestion importance[1-10]: 5 __ Why: This is a good defensive programming practice that improves robustness by trimming whitespace from credentials, which can prevent common and hard-to-debug configuration errors.	Low
Learned best practice	Gate polling-loop warning to once Avoid logging the same warning on every poll iteration by gating it to fire once (e.g., with an AtomicBoolean) to reduce log spam and keep retries idempotent. java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [521-532] +AtomicBoolean warned = new AtomicBoolean(false); + wait.until( obj -> { HttpResponse response = client.execute(new HttpRequest(GET, "/status")); LOG.fine(string(response)); - if (401 == response.getStatus()) { + if (401 == response.getStatus() && warned.compareAndSet(false, true)) { LOG.warning( "Server requires basic authentication. " + "Set SE_ROUTER_USERNAME and SE_ROUTER_PASSWORD environment variables " + "to provide credentials."); } return 200 == response.getStatus(); }); [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Make automation noise-free by avoiding repeated log/side-effects inside polling loops; ensure warnings are emitted once and remain idempotent across retries.	Low
More

[Copilot]

Pull request overview

Adds Basic Auth support when the Docker Node waits for a spawned browser container (standalone image) to become ready, using the same SE_ROUTER_USERNAME / SE_ROUTER_PASSWORD environment variables that can enable Grid Basic Auth.

Changes:

• Apply Basic Auth credentials from SE_ROUTER_USERNAME / SE_ROUTER_PASSWORD to the HttpClient used to poll /status.
• Emit a warning when /status responds with HTTP 401 to hint at missing credentials.
• Introduce a small helper (applyBasicAuth) to encapsulate the env var lookup and ClientConfig authentication wiring.