From 7dcf5c5a09f68391e9fe2e79875a5812cee08f76 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 27 Feb 2026 22:22:53 +0100 Subject: [PATCH] refactor: remove accountURI field --- challenge/dns01/dns_challenge.go | 6 +-- .../dnspersist01/dns_persist_challenge.go | 53 +++++++++---------- .../dns_persist_challenge_test.go | 30 ++++++----- 3 files changed, 45 insertions(+), 44 deletions(-) diff --git a/challenge/dns01/dns_challenge.go b/challenge/dns01/dns_challenge.go index 9c6252a8c..5d15e527a 100644 --- a/challenge/dns01/dns_challenge.go +++ b/challenge/dns01/dns_challenge.go @@ -121,12 +121,12 @@ func (c *Challenge) Solve(ctx context.Context, authz acme.Authorization) error { time.Sleep(interval) err = wait.For("propagation", timeout, interval, func() (bool, error) { - stop, errP := c.preCheck.call(ctx, domain, info.EffectiveFQDN, info.Value) - if !stop || errP != nil { + stop, callErr := c.preCheck.call(ctx, domain, info.EffectiveFQDN, info.Value) + if !stop || callErr != nil { log.Info("dns01: waiting for record propagation.", log.DomainAttr(domain)) } - return stop, errP + return stop, callErr }) if err != nil { return err diff --git a/challenge/dnspersist01/dns_persist_challenge.go b/challenge/dnspersist01/dns_persist_challenge.go index 580383d9b..5892562f9 100644 --- a/challenge/dnspersist01/dns_persist_challenge.go +++ b/challenge/dnspersist01/dns_persist_challenge.go @@ -28,9 +28,6 @@ type Challenge struct { provider challenge.PersistentProvider preCheck preCheck - // only for testing purposes - accountURI string - userSuppliedIssuerDomainName string persistUntil time.Time } @@ -62,6 +59,11 @@ func (c *Challenge) Solve(ctx context.Context, authz acme.Authorization) error { return errors.New("dnspersist01: empty identifier") } + accountURI := c.core.GetKid() + if accountURI == "" { + return errors.New("dnspersist01: ACME account URI cannot be empty") + } + log.Info("dnspersist01: trying to solve the challenge.", log.DomainAttr(domain)) chlng, err := challenge.FindChallenge(challenge.DNSPersist01, authz) @@ -81,19 +83,19 @@ func (c *Challenge) Solve(ctx context.Context, authz acme.Authorization) error { return fmt.Errorf("dnspersist01: %w", err) } - issuerDomainName, err := c.selectIssuerDomainName(chlng.IssuerDomainNames, result.Records, authz.Wildcard) + issuerDomainName, err := c.selectIssuerDomainName(chlng.IssuerDomainNames, result.Records, accountURI, authz.Wildcard) if err != nil { return fmt.Errorf("dnspersist01: %w", err) } matcher := func(records []TXTRecord) bool { - return c.hasMatchingRecord(records, issuerDomainName, authz.Wildcard) + return c.hasMatchingRecord(records, issuerDomainName, accountURI, authz.Wildcard) } if !matcher(result.Records) { var info ChallengeInfo - info, err = GetChallengeInfo(authz, issuerDomainName, c.getAccountURI(), c.persistUntil) + info, err = GetChallengeInfo(authz, issuerDomainName, accountURI, c.persistUntil) if err != nil { return err } @@ -106,25 +108,29 @@ func (c *Challenge) Solve(ctx context.Context, authz acme.Authorization) error { log.Info("dnspersist01: found existing matching TXT record for %s, no need to create a new one", log.DomainAttr(fqdn)) } + err = c.waitForPropagation(ctx, domain, fqdn, matcher) + if err != nil { + return err + } + + return c.validate(ctx, c.core, domain, chlng) +} + +func (c *Challenge) waitForPropagation(ctx context.Context, domain, fqdn string, matcher RecordMatcher) error { timeout, interval := c.provider.Timeout() log.Info("dnspersist01: waiting for record propagation.", log.DomainAttr(domain)) time.Sleep(interval) - err = wait.For("propagation", timeout, interval, func() (bool, error) { - ok, callErr := c.preCheck.call(ctx, domain, fqdn, matcher) - if !ok || callErr != nil { + return wait.For("propagation", timeout, interval, func() (bool, error) { + stop, callErr := c.preCheck.call(ctx, domain, fqdn, matcher) + if !stop || callErr != nil { log.Info("dnspersist01: waiting for record propagation.", log.DomainAttr(domain)) } - return ok, callErr + return stop, callErr }) - if err != nil { - return err - } - - return c.validate(ctx, c.core, domain, chlng) } // selectIssuerDomainName selects the issuer-domain-name to use for a dns-persist-01 challenge. @@ -134,7 +140,7 @@ func (c *Challenge) Solve(ctx context.Context, authz acme.Authorization) error { // the first issuer-domain-name with a matching TXT record is selected. // If no issuer-domain-name has a matching TXT record, // a deterministic default issuer-domain-name is selected using lexicographic ordering. -func (c *Challenge) selectIssuerDomainName(challIssuers []string, records []TXTRecord, wildcard bool) (string, error) { +func (c *Challenge) selectIssuerDomainName(challIssuers []string, records []TXTRecord, accountURI string, wildcard bool) (string, error) { if len(challIssuers) == 0 { return "", errors.New("issuer-domain-names missing from the challenge") } @@ -151,7 +157,7 @@ func (c *Challenge) selectIssuerDomainName(challIssuers []string, records []TXTR } for _, issuerDomainName := range sortedIssuers { - if c.hasMatchingRecord(records, issuerDomainName, wildcard) { + if c.hasMatchingRecord(records, issuerDomainName, accountURI, wildcard) { return issuerDomainName, nil } } @@ -159,10 +165,10 @@ func (c *Challenge) selectIssuerDomainName(challIssuers []string, records []TXTR return sortedIssuers[0], nil } -func (c *Challenge) hasMatchingRecord(records []TXTRecord, issuerDomainName string, wildcard bool) bool { +func (c *Challenge) hasMatchingRecord(records []TXTRecord, issuerDomainName, accountURI string, wildcard bool) bool { iv := IssueValue{ IssuerDomainName: issuerDomainName, - AccountURI: c.getAccountURI(), + AccountURI: accountURI, PersistUntil: c.persistUntil, } @@ -173,6 +179,7 @@ func (c *Challenge) hasMatchingRecord(records []TXTRecord, issuerDomainName stri return slices.ContainsFunc(records, func(record TXTRecord) bool { parsed, err := parseIssueValue(record.Value) if err != nil { + log.Debug("dnspersist01: failed to parse TXT record value", log.ErrorAttr(err)) return false } @@ -180,14 +187,6 @@ func (c *Challenge) hasMatchingRecord(records []TXTRecord, issuerDomainName stri }) } -func (c *Challenge) getAccountURI() string { - if c.accountURI != "" { - return c.accountURI - } - - return c.core.GetKid() -} - // ChallengeInfo contains the information used to create a dns-persist-01 TXT record. type ChallengeInfo struct { // FQDN is the full-qualified challenge domain (i.e. `_validation-persist.[domain].`). diff --git a/challenge/dnspersist01/dns_persist_challenge_test.go b/challenge/dnspersist01/dns_persist_challenge_test.go index 08735edb1..2e1991fab 100644 --- a/challenge/dnspersist01/dns_persist_challenge_test.go +++ b/challenge/dnspersist01/dns_persist_challenge_test.go @@ -176,6 +176,8 @@ func TestGetChallengeInfo(t *testing.T) { } func TestChallenge_selectIssuerDomainName(t *testing.T) { + accountURI := "https://authority.example/acct/123" + testCases := []struct { desc string issuers []string @@ -196,7 +198,7 @@ func TestChallenge_selectIssuerDomainName(t *testing.T) { "ca.example", "backup.example", }, records: []TXTRecord{ - {Value: mustChallengeValue(t, "ca.example", "https://authority.example/acct/123", false, time.Time{})}, + {Value: mustChallengeValue(t, "ca.example", accountURI, false, time.Time{})}, }, expectIssuerDomainName: "ca.example", }, @@ -206,7 +208,7 @@ func TestChallenge_selectIssuerDomainName(t *testing.T) { "ca.example", "backup.example", }, records: []TXTRecord{ - {Value: mustChallengeValue(t, "ca.example", "https://authority.example/acct/123", false, time.Time{})}, + {Value: mustChallengeValue(t, "ca.example", accountURI, false, time.Time{})}, }, overrideIssuerDomainName: "backup.example", expectIssuerDomainName: "backup.example", @@ -222,11 +224,10 @@ func TestChallenge_selectIssuerDomainName(t *testing.T) { for _, test := range testCases { t.Run(test.desc, func(t *testing.T) { chlg := &Challenge{ - accountURI: "https://authority.example/acct/123", userSuppliedIssuerDomainName: test.overrideIssuerDomainName, } - issuer, err := chlg.selectIssuerDomainName(test.issuers, test.records, test.wildcard) + issuer, err := chlg.selectIssuerDomainName(test.issuers, test.records, accountURI, test.wildcard) if test.expectErr { require.Error(t, err) return @@ -242,6 +243,8 @@ func TestChallenge_hasMatchingRecord(t *testing.T) { expiredPersistUntil := time.Unix(1700000000, 0).UTC() futurePersistUntil := time.Unix(4102444800, 0).UTC() + accountURI := "acc" + testCases := []struct { desc string records []TXTRecord @@ -252,13 +255,13 @@ func TestChallenge_hasMatchingRecord(t *testing.T) { }{ { desc: "match basic", - records: []TXTRecord{{Value: mustChallengeValue(t, "ca.example", "acc", false, time.Time{})}}, + records: []TXTRecord{{Value: mustChallengeValue(t, "ca.example", accountURI, false, time.Time{})}}, issuer: "ca.example", assert: assert.True, }, { desc: "issuer mismatch", - records: []TXTRecord{{Value: mustChallengeValue(t, "other.example", "acc", false, time.Time{})}}, + records: []TXTRecord{{Value: mustChallengeValue(t, "other.example", accountURI, false, time.Time{})}}, issuer: "ca.example", assert: assert.False, }, @@ -270,21 +273,21 @@ func TestChallenge_hasMatchingRecord(t *testing.T) { }, { desc: "wildcard requires policy", - records: []TXTRecord{{Value: mustChallengeValue(t, "ca.example", "acc", false, time.Time{})}}, + records: []TXTRecord{{Value: mustChallengeValue(t, "ca.example", accountURI, false, time.Time{})}}, issuer: "ca.example", wildcard: true, assert: assert.False, }, { desc: "wildcard match", - records: []TXTRecord{{Value: mustChallengeValue(t, "ca.example", "acc", true, time.Time{})}}, + records: []TXTRecord{{Value: mustChallengeValue(t, "ca.example", accountURI, true, time.Time{})}}, issuer: "ca.example", wildcard: true, assert: assert.True, }, { desc: "policy wildcard allowed for non-wildcard", - records: []TXTRecord{{Value: mustChallengeValue(t, "ca.example", "acc", true, time.Time{})}}, + records: []TXTRecord{{Value: mustChallengeValue(t, "ca.example", accountURI, true, time.Time{})}}, issuer: "ca.example", wildcard: false, assert: assert.True, @@ -314,13 +317,13 @@ func TestChallenge_hasMatchingRecord(t *testing.T) { }, { desc: "persistUntil present without requirement is not a match", - records: []TXTRecord{{Value: mustChallengeValue(t, "ca.example", "acc", false, expiredPersistUntil)}}, + records: []TXTRecord{{Value: mustChallengeValue(t, "ca.example", accountURI, false, expiredPersistUntil)}}, issuer: "ca.example", assert: assert.False, }, { desc: "future persistUntil without requirement is not a match", - records: []TXTRecord{{Value: mustChallengeValue(t, "ca.example", "acc", false, futurePersistUntil)}}, + records: []TXTRecord{{Value: mustChallengeValue(t, "ca.example", accountURI, false, futurePersistUntil)}}, issuer: "ca.example", assert: assert.False, }, @@ -333,7 +336,7 @@ func TestChallenge_hasMatchingRecord(t *testing.T) { }, { desc: "required persistUntil matches even when expired", - records: []TXTRecord{{Value: mustChallengeValue(t, "ca.example", "acc", false, expiredPersistUntil)}}, + records: []TXTRecord{{Value: mustChallengeValue(t, "ca.example", accountURI, false, expiredPersistUntil)}}, issuer: "ca.example", requiredPersistUTC: expiredPersistUntil, assert: assert.True, @@ -357,11 +360,10 @@ func TestChallenge_hasMatchingRecord(t *testing.T) { for _, test := range testCases { t.Run(test.desc, func(t *testing.T) { chlg := &Challenge{ - accountURI: "acc", persistUntil: test.requiredPersistUTC, } - match := chlg.hasMatchingRecord(test.records, test.issuer, test.wildcard) + match := chlg.hasMatchingRecord(test.records, test.issuer, accountURI, test.wildcard) test.assert(t, match) })