Add DCV lookup locations by himaschal · Pull Request #92 · digicert/domain-control-validation

himaschal · 2025-11-13T14:42:13Z

Add getLookupLocations() method to DcvManager and all validators to return potential validation targets.
Supports all DCV methods with full test coverage.

- Add getLookupLocations() method to DcvManager and all validators to return potential validation targets. - Supports all DCV methods with full test coverage.

Copilot

Pull Request Overview

This PR adds getLookupLocations() methods to DcvManager and all validator classes to return potential validation targets (URLs, DNS names, or email addresses) for different DCV methods. The implementation provides a unified interface through DcvManager that delegates to validator-specific methods based on the DCV method type.

Key Changes

Added public API method getLookupLocations() in DcvManager with support for optional custom filenames
Implemented validator-specific lookup methods: getFileLookupUrls(), getEmailLookupLocations(), getDnsLookupNames(), and getAcmeLookupUrls()
Added comprehensive unit and integration tests covering all DCV methods

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
library/src/main/java/com/digicert/validation/DcvManager.java	Added two `getLookupLocations()` methods that route to appropriate validators based on DCV method
library/src/main/java/com/digicert/validation/methods/file/FileValidator.java	Added `getFileLookupUrls()` methods returning HTTP/HTTPS URLs for file validation
library/src/main/java/com/digicert/validation/methods/email/EmailValidator.java	Added `getEmailLookupLocations()` method returning email addresses or DNS records based on email DCV method
library/src/main/java/com/digicert/validation/methods/dns/DnsValidator.java	Added `getDnsLookupNames()` method returning DNS names with domain hierarchy and prefixes
library/src/main/java/com/digicert/validation/methods/acme/AcmeValidator.java	Added `getAcmeLookupUrls()` method returning HTTP/HTTPS URLs for ACME challenges
library/src/test/java/com/digicert/validation/DcvManagerTest.java	Added 10 unit tests covering various DCV methods, edge cases, and error scenarios
library/src/test/java/com/digicert/validation/methods/file/FileValidatorTest.java	Added 8 unit tests for file lookup URLs including custom filenames and edge cases
library/src/test/java/com/digicert/validation/methods/email/EmailValidatorTest.java	Added 4 unit tests for email lookup locations covering all three email DCV methods
library/src/test/java/com/digicert/validation/methods/acme/AcmeValidatorTest.java	Added 4 unit tests for ACME lookup URLs with various domain configurations
example-app/src/test/java/com/digicert/validation/FileMethodIT.java	Enhanced integration tests with lookup location verification and added 3 new test methods
example-app/src/test/java/com/digicert/validation/FileTokenMethodIT.java	Added lookup location verification to existing happy path test
example-app/src/test/java/com/digicert/validation/EmailMethodIT.java	Added lookup location verification to existing email validation test
example-app/src/test/java/com/digicert/validation/DnsTokenValueMethodIT.java	Added lookup location verification to existing DNS token test
example-app/src/test/java/com/digicert/validation/DnsRandomValueMethodIT.java	Added lookup location verification to existing DNS random value test
example-app/src/test/java/com/digicert/validation/AcmeHttpMethodIT.java	Added comprehensive lookup location verification to ACME HTTP test
example-app/src/test/java/com/digicert/validation/AcmeDnsMethodIT.java	Added lookup location verification to existing ACME DNS test

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

library/src/main/java/com/digicert/validation/methods/dns/DnsValidator.java

library/src/main/java/com/digicert/validation/DcvManager.java

library/src/main/java/com/digicert/validation/methods/file/FileValidator.java

library/src/main/java/com/digicert/validation/methods/dns/DnsValidator.java

library/src/main/java/com/digicert/validation/methods/email/EmailValidator.java

library/src/main/java/com/digicert/validation/methods/dns/DnsValidator.java

library/src/main/java/com/digicert/validation/methods/email/EmailValidator.java

library/src/main/java/com/digicert/validation/methods/acme/AcmeValidator.java

alexmitchell12

Copilot has some good suggestions, like updating the hardcoded "_dnsAuth" to use the dnsDomainLabel. Once the copilot findings are addressed i think we will be in a good state

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 8 comments.

Comments suppressed due to low confidence (1)

example-app/src/test/java/com/digicert/validation/FileMethodIT.java:82

The test verifies that lookup locations contain "customfilename" without an extension (lines 70-75), but then writes the file with ".txt" extension as "customfilename.txt" (line 80) and validates using that name (line 82). This inconsistency means the test is not validating that the lookup locations actually match what's being used for validation. Either:

Pass "customfilename.txt" to getLookupLocations() on line 70
Or use "customfilename" (without .txt) in lines 80 and 82

This same issue likely exists in the DcvRequest construction on line 65, which uses "customfilename" but the actual validation expects ".txt" extension.

        List<String> lookupLocations = dcvManager.getLookupLocations(dcvRequest.domain(), DcvMethod.BR_3_2_2_4_18, "customfilename");
        String expectedHttpUrl = "http://" + dcvRequest.domain() + "/.well-known/pki-validation/customfilename";
        String expectedHttpsUrl = "https://" + dcvRequest.domain() + "/.well-known/pki-validation/customfilename";
        assertTrue(lookupLocations.contains(expectedHttpUrl), "Lookup locations should include HTTP validation URL with custom filename");
        assertTrue(lookupLocations.contains(expectedHttpsUrl), "Lookup locations should include HTTPS validation URL with custom filename");
        assertEquals(2, lookupLocations.size(), "Should return exactly 2 lookup locations for file validation");

        // Setup DNS record for domain
        pdnsClient.createLocalhostARecord(dcvRequest.domain());
        // Write random value to file
        FileUtils.writeFileAuthFileWithContent("customfilename.txt", createdDomain.getRandomValueDetails().getFirst().getRandomValue());

        exampleAppClient.validateDomain(createValidateRequest(createdDomain, "customfilename.txt"), createdDomain.getId());

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-03T18:43:31Z

library/src/main/java/com/digicert/validation/methods/dns/DnsValidator.java

+    public List<String> getDnsLookupNames(String domain) {
+        try {
+            // Get domain and its parents (e.g., for sub.example.com: [sub.example.com, example.com])
+            List<String> allowedFqdns = domainNameUtils.getDomainAndParents(domain);
+
+            List<String> lookupLocations = new ArrayList<>();
+            for (String fqdn : allowedFqdns) {
+                // Use the configured DNS domain label (defaults to "_dnsauth.")
+                // This matches the actual validation logic in DnsValidationHandler
+                lookupLocations.add("_dnsauth." + fqdn);  // Primary lookup location
+                lookupLocations.add(fqdn);  // Fallback lookup location
+            }
+            return lookupLocations;
+        } catch (Exception e) {
+            // Fallback to basic domain if there's an issue
+            return List.of("_dnsauth." + domain, domain);
+        }
+    }


The try-catch block catches all exceptions and falls back to generating lookup locations even for invalid domains. This silently produces malformed results (e.g., _dnsauth. for empty domains) instead of properly validating input or reporting errors. Consider:

Adding domain validation before processing, similar to other validator methods

Re-throwing exceptions or returning an empty list for invalid domains

At minimum, logging the caught exception to aid debugging

Copilot · 2025-12-03T18:43:32Z

library/src/test/java/com/digicert/validation/methods/acme/AcmeValidatorTest.java

+        assertNotNull(locations);
+        assertEquals(2, locations.size());
+
+        // Should handle empty domain gracefully
+        assertTrue(locations.contains("http:///.well-known/acme-challenge/{token}"));
+        assertTrue(locations.contains("https:///.well-known/acme-challenge/{token}"));


The test expects to generate malformed URLs like http:///.well-known/acme-challenge/{token} when given an empty domain. This creates invalid URLs that would fail in real usage. Consider either:

Throwing an exception for empty/invalid domains

Returning an empty list for invalid inputs

Adding validation to reject empty domains

The same issue exists in the corresponding tests for File and Email validators.

Suggested change

assertNotNull(locations);

assertEquals(2, locations.size());

// Should handle empty domain gracefully

assertTrue(locations.contains("http:///.well-known/acme-challenge/{token}"));

assertTrue(locations.contains("https:///.well-known/acme-challenge/{token}"));

// Should handle empty domain gracefully by returning an empty list

assertNotNull(locations);

assertTrue(locations.isEmpty(), "Expected empty list for empty domain");

Copilot · 2025-12-03T18:43:32Z

library/src/test/java/com/digicert/validation/DcvManagerTest.java

+        // Test with empty domain - should not throw exception but may return empty or basic results
+        assertDoesNotThrow(() -> {
+            List<String> locations = dcvManager.getLookupLocations("", DcvMethod.BR_3_2_2_4_7);
+            assertNotNull(locations);
+        });


The test uses assertDoesNotThrow with an empty domain, expecting it to succeed. However, an empty domain will produce malformed URLs/locations (e.g., http:///.well-known/...). The implementation should validate input and either throw an exception or return an empty list for invalid domains. The current behavior silently produces invalid results that would fail in real usage.

Suggested change

// Test with empty domain - should not throw exception but may return empty or basic results

assertDoesNotThrow(() -> {

List<String> locations = dcvManager.getLookupLocations("", DcvMethod.BR_3_2_2_4_7);

assertNotNull(locations);

});

// Test with empty domain - should return an empty list

List<String> locations = dcvManager.getLookupLocations("", DcvMethod.BR_3_2_2_4_7);

assertNotNull(locations);

assertTrue(locations.isEmpty(), "Expected empty list for empty domain");

Copilot · 2025-12-03T18:43:32Z

library/src/main/java/com/digicert/validation/methods/email/EmailValidator.java

+        if (dcvMethod == DcvMethod.BR_3_2_2_4_4) {
+            // Constructed email addresses (BR_3_2_2_4_4)
+            lookupLocations.add("admin@" + domain);
+            lookupLocations.add("administrator@" + domain);
+            lookupLocations.add("webmaster@" + domain);
+            lookupLocations.add("hostmaster@" + domain);
+            lookupLocations.add("postmaster@" + domain);
+        }
+
+        if (dcvMethod == DcvMethod.BR_3_2_2_4_14) {
+            // DNS TXT lookup for email contact (BR_3_2_2_4_14)
+            lookupLocations.add("_validation-contactemail." + domain);
+        }
+
+        if (dcvMethod == DcvMethod.BR_3_2_2_4_13) {
+            // DNS CAA lookup for email contact (BR_3_2_2_4_13)
+            lookupLocations.add(domain); // CAA records are at the domain itself
+        }


The method uses multiple if statements instead of else if, which means if a DCV method matches multiple conditions (though unlikely in practice), it would add multiple sets of lookup locations. Consider using else if for mutual exclusivity, or use a switch statement similar to the findEmailGenerator method below for consistency and to prevent potential issues.

Suggested change

if (dcvMethod == DcvMethod.BR_3_2_2_4_4) {

// Constructed email addresses (BR_3_2_2_4_4)

lookupLocations.add("admin@" + domain);

lookupLocations.add("administrator@" + domain);

lookupLocations.add("webmaster@" + domain);

lookupLocations.add("hostmaster@" + domain);

lookupLocations.add("postmaster@" + domain);

}

if (dcvMethod == DcvMethod.BR_3_2_2_4_14) {

// DNS TXT lookup for email contact (BR_3_2_2_4_14)

lookupLocations.add("_validation-contactemail." + domain);

}

if (dcvMethod == DcvMethod.BR_3_2_2_4_13) {

// DNS CAA lookup for email contact (BR_3_2_2_4_13)

lookupLocations.add(domain); // CAA records are at the domain itself

}

switch (dcvMethod) {

case BR_3_2_2_4_4 -> {

// Constructed email addresses (BR_3_2_2_4_4)

lookupLocations.add("admin@" + domain);

lookupLocations.add("administrator@" + domain);

lookupLocations.add("webmaster@" + domain);

lookupLocations.add("hostmaster@" + domain);

lookupLocations.add("postmaster@" + domain);

}

case BR_3_2_2_4_14 -> {

// DNS TXT lookup for email contact (BR_3_2_2_4_14)

lookupLocations.add("_validation-contactemail." + domain);

}

case BR_3_2_2_4_13 -> {

// DNS CAA lookup for email contact (BR_3_2_2_4_13)

lookupLocations.add(domain); // CAA records are at the domain itself

}

}

Copilot · 2025-12-03T18:43:33Z

library/src/test/java/com/digicert/validation/DcvManagerTest.java

-                .withDcvConfiguration(dcvConfiguration)
-                .build();
-
+        // Use the dcvManager created in setUp() to verify it was built correctly


[nitpick] The comment "Use the dcvManager created in setUp() to verify it was built correctly" is slightly misleading. While it does verify the dcvManager was built correctly, the test is still titled testBuilderWithValidDcvConfiguration which implies it's specifically testing the builder pattern. Consider either removing the comment or renaming to make it clear this test now relies on the setUp method.

Suggested change

// Use the dcvManager created in setUp() to verify it was built correctly

Copilot · 2025-12-03T18:43:33Z

library/src/main/java/com/digicert/validation/methods/file/FileValidator.java

+
+    /**
+     * Returns the list of file URLs where random values could be placed for DCV.
+     * This includes both HTTP and HTTPS URLs for the standard file validation path.


The javadoc is missing @param and @return tags. For consistency with other methods in this class and better API documentation, consider adding:

/** * Returns the list of file URLs where random values could be placed for DCV. * This includes both HTTP and HTTPS URLs for the standard file validation path. * * @param domain the domain for which to generate lookup URLs * @return list of HTTP and HTTPS URLs where the validation file would be located */

Suggested change

* This includes both HTTP and HTTPS URLs for the standard file validation path.

* This includes both HTTP and HTTPS URLs for the standard file validation path.

*

* @param domain the domain for which to generate lookup URLs

* @return list of HTTP and HTTPS URLs where the validation file would be located

Copilot · 2025-12-03T18:43:33Z

library/src/test/java/com/digicert/validation/methods/file/FileValidatorTest.java

+        assertEquals(2, locations.size());
+
+        // Should handle empty domain gracefully
+        assertTrue(locations.contains("http:///.well-known/pki-validation/fileauth.txt"));
+        assertTrue(locations.contains("https:///.well-known/pki-validation/fileauth.txt"));


The test expects to generate malformed URLs like http:///.well-known/pki-validation/fileauth.txt when given an empty domain. This creates invalid URLs that would fail in real usage. Consider either:

Throwing an exception for empty/invalid domains

Returning an empty list for invalid inputs

Adding validation to reject empty domains

The same issue exists in the corresponding tests for Email, ACME, and DNS validators.

Suggested change

assertEquals(2, locations.size());

// Should handle empty domain gracefully

assertTrue(locations.contains("http:///.well-known/pki-validation/fileauth.txt"));

assertTrue(locations.contains("https:///.well-known/pki-validation/fileauth.txt"));

assertEquals(0, locations.size());

// Should handle empty domain gracefully by returning an empty set

Copilot · 2025-12-03T18:43:34Z

library/src/test/java/com/digicert/validation/methods/email/EmailValidatorTest.java

+        // Get locations for constructed email method
+        var emailLocations = emailValidator.getEmailLookupLocations("", DcvMethod.BR_3_2_2_4_4);
+        assertNotNull(emailLocations);
+        assertEquals(5, emailLocations.size());
+        assertTrue(emailLocations.contains("admin@"));
+        assertTrue(emailLocations.contains("administrator@"));
+        assertTrue(emailLocations.contains("webmaster@"));
+        assertTrue(emailLocations.contains("hostmaster@"));
+        assertTrue(emailLocations.contains("postmaster@"));
+
+        // Get locations for DNS TXT method
+        var dnsLocations = emailValidator.getEmailLookupLocations("", DcvMethod.BR_3_2_2_4_14);
+        assertNotNull(dnsLocations);
+        assertEquals(1, dnsLocations.size());
+        assertTrue(dnsLocations.contains("_validation-contactemail."));
+
+        // Get locations for DNS CAA method
+        var caaLocations = emailValidator.getEmailLookupLocations("", DcvMethod.BR_3_2_2_4_13);
+        assertNotNull(caaLocations);
+        assertEquals(1, caaLocations.size());
+        assertTrue(caaLocations.contains(""));


The test expects to generate malformed email addresses and DNS names like admin@ and _validation-contactemail. when given an empty domain. These are invalid and would fail in real usage. Consider either:

Throwing an exception for empty/invalid domains

Returning an empty list for invalid inputs

Adding validation to reject empty domains

The same pattern exists in File and ACME validator tests.

Suggested change

// Get locations for constructed email method

var emailLocations = emailValidator.getEmailLookupLocations("", DcvMethod.BR_3_2_2_4_4);

assertNotNull(emailLocations);

assertEquals(5, emailLocations.size());

assertTrue(emailLocations.contains("admin@"));

assertTrue(emailLocations.contains("administrator@"));

assertTrue(emailLocations.contains("webmaster@"));

assertTrue(emailLocations.contains("hostmaster@"));

assertTrue(emailLocations.contains("postmaster@"));

// Get locations for DNS TXT method

var dnsLocations = emailValidator.getEmailLookupLocations("", DcvMethod.BR_3_2_2_4_14);

assertNotNull(dnsLocations);

assertEquals(1, dnsLocations.size());

assertTrue(dnsLocations.contains("_validation-contactemail."));

// Get locations for DNS CAA method

var caaLocations = emailValidator.getEmailLookupLocations("", DcvMethod.BR_3_2_2_4_13);

assertNotNull(caaLocations);

assertEquals(1, caaLocations.size());

assertTrue(caaLocations.contains(""));

// For empty domain, should throw exception or return empty list

assertThrows(IllegalArgumentException.class, () -> {

emailValidator.getEmailLookupLocations("", DcvMethod.BR_3_2_2_4_4);

});

assertThrows(IllegalArgumentException.class, () -> {

emailValidator.getEmailLookupLocations("", DcvMethod.BR_3_2_2_4_14);

});

assertThrows(IllegalArgumentException.class, () -> {

emailValidator.getEmailLookupLocations("", DcvMethod.BR_3_2_2_4_13);

});

Add DCV lookup locations

14d2396

- Add getLookupLocations() method to DcvManager and all validators to return potential validation targets. - Supports all DCV methods with full test coverage.

himaschal requested a review from Copilot November 13, 2025 14:42

Copilot started reviewing on behalf of himaschal November 13, 2025 14:43 View session

Copilot finished reviewing on behalf of himaschal November 13, 2025 14:47

Copilot AI reviewed Nov 13, 2025

View reviewed changes

alexmitchell12 reviewed Dec 3, 2025

View reviewed changes

himaschal marked this pull request as ready for review December 3, 2025 15:18

himaschal requested a review from Copilot December 3, 2025 18:37

Copilot started reviewing on behalf of himaschal December 3, 2025 18:38 View session

Copilot finished reviewing on behalf of himaschal December 3, 2025 18:41

Copilot AI reviewed Dec 3, 2025

View reviewed changes

Conversation

himaschal commented Nov 13, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexmitchell12 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants