Skip to content

Commit 642f0d5

Browse files
committed
1 parent 88e840d commit 642f0d5

File tree

2 files changed

+104
-57
lines changed

2 files changed

+104
-57
lines changed

account.go

+9
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,15 @@ func (am *ACMEIssuer) mostRecentAccountEmail(ctx context.Context, caURL string)
415415
return getPrimaryContact(account), true
416416
}
417417

418+
func accountRegLockKey(acc acme.Account) string {
419+
key := "register_acme_account"
420+
if len(acc.Contact) == 0 {
421+
return key
422+
}
423+
key += "_" + getPrimaryContact(acc)
424+
return key
425+
}
426+
418427
// getPrimaryContact returns the first contact on the account (if any)
419428
// without the scheme. (I guess we assume an email address.)
420429
func getPrimaryContact(account acme.Account) string {

acmeclient.go

+95-57
Original file line numberDiff line numberDiff line change
@@ -50,16 +50,28 @@ func (iss *ACMEIssuer) newACMEClientWithAccount(ctx context.Context, useTestCA,
5050
return nil, err
5151
}
5252

53-
// look up or create the ACME account
54-
var account acme.Account
55-
if iss.AccountKeyPEM != "" {
56-
iss.Logger.Info("using configured ACME account")
57-
account, err = iss.GetAccount(ctx, []byte(iss.AccountKeyPEM))
58-
} else {
59-
account, err = iss.getAccount(ctx, client.Directory, iss.getEmail())
53+
// we try loading the account from storage before a potential
54+
// lock, and 0after obtaining the lock as well, to ensure we don't
55+
// repeat work done by another instance or goroutine
56+
getAccount := func() (acme.Account, error) {
57+
// look up or create the ACME account
58+
var account acme.Account
59+
if iss.AccountKeyPEM != "" {
60+
iss.Logger.Info("using configured ACME account")
61+
account, err = iss.GetAccount(ctx, []byte(iss.AccountKeyPEM))
62+
} else {
63+
account, err = iss.getAccount(ctx, client.Directory, iss.getEmail())
64+
}
65+
if err != nil {
66+
return acme.Account{}, fmt.Errorf("getting ACME account: %v", err)
67+
}
68+
return account, nil
6069
}
70+
71+
// first try getting the account
72+
account, err := getAccount()
6173
if err != nil {
62-
return nil, fmt.Errorf("getting ACME account: %v", err)
74+
return nil, err
6375
}
6476

6577
// register account if it is new
@@ -68,67 +80,93 @@ func (iss *ACMEIssuer) newACMEClientWithAccount(ctx context.Context, useTestCA,
6880
zap.Strings("contact", account.Contact),
6981
zap.String("location", account.Location))
7082

71-
if iss.NewAccountFunc != nil {
72-
// obtain lock here, since NewAccountFunc calls happen concurrently and they typically read and change the issuer
73-
iss.mu.Lock()
74-
account, err = iss.NewAccountFunc(ctx, iss, account)
75-
iss.mu.Unlock()
76-
if err != nil {
77-
return nil, fmt.Errorf("account pre-registration callback: %v", err)
83+
// synchronize this so the account is only created once
84+
acctLockKey := accountRegLockKey(account)
85+
err = iss.config.Storage.Lock(ctx, acctLockKey)
86+
if err != nil {
87+
return nil, fmt.Errorf("locking account registration: %v", err)
88+
}
89+
defer func() {
90+
if err := iss.config.Storage.Unlock(ctx, acctLockKey); err != nil {
91+
iss.Logger.Error("failed to unlock account registration lock", zap.Error(err))
7892
}
93+
}()
94+
95+
// if we're not the only one waiting for this account, then by this point it should already be registered and in storage; reload it
96+
account, err = getAccount()
97+
if err != nil {
98+
return nil, err
7999
}
80100

81-
// agree to terms
82-
if interactive {
83-
if !iss.isAgreed() {
84-
var termsURL string
85-
dir, err := client.GetDirectory(ctx)
101+
// if we are the only or first one waiting for this account, then proceed to register it while we have the lock
102+
if account.Status == "" {
103+
if iss.NewAccountFunc != nil {
104+
// obtain lock here, since NewAccountFunc calls happen concurrently and they typically read and change the issuer
105+
iss.mu.Lock()
106+
account, err = iss.NewAccountFunc(ctx, iss, account)
107+
iss.mu.Unlock()
86108
if err != nil {
87-
return nil, fmt.Errorf("getting directory: %w", err)
109+
return nil, fmt.Errorf("account pre-registration callback: %v", err)
88110
}
89-
if dir.Meta != nil {
90-
termsURL = dir.Meta.TermsOfService
91-
}
92-
if termsURL != "" {
93-
agreed := iss.askUserAgreement(termsURL)
94-
if !agreed {
95-
return nil, fmt.Errorf("user must agree to CA terms")
111+
}
112+
113+
// agree to terms
114+
if interactive {
115+
if !iss.isAgreed() {
116+
var termsURL string
117+
dir, err := client.GetDirectory(ctx)
118+
if err != nil {
119+
return nil, fmt.Errorf("getting directory: %w", err)
120+
}
121+
if dir.Meta != nil {
122+
termsURL = dir.Meta.TermsOfService
123+
}
124+
if termsURL != "" {
125+
agreed := iss.askUserAgreement(termsURL)
126+
if !agreed {
127+
return nil, fmt.Errorf("user must agree to CA terms")
128+
}
129+
iss.mu.Lock()
130+
iss.agreed = agreed
131+
iss.mu.Unlock()
96132
}
97-
iss.mu.Lock()
98-
iss.agreed = agreed
99-
iss.mu.Unlock()
100133
}
134+
} else {
135+
// can't prompt a user who isn't there; they should
136+
// have reviewed the terms beforehand
137+
iss.mu.Lock()
138+
iss.agreed = true
139+
iss.mu.Unlock()
101140
}
102-
} else {
103-
// can't prompt a user who isn't there; they should
104-
// have reviewed the terms beforehand
105-
iss.mu.Lock()
106-
iss.agreed = true
107-
iss.mu.Unlock()
108-
}
109-
account.TermsOfServiceAgreed = iss.isAgreed()
141+
account.TermsOfServiceAgreed = iss.isAgreed()
110142

111-
// associate account with external binding, if configured
112-
if iss.ExternalAccount != nil {
113-
err := account.SetExternalAccountBinding(ctx, client.Client, *iss.ExternalAccount)
114-
if err != nil {
115-
return nil, err
143+
// associate account with external binding, if configured
144+
if iss.ExternalAccount != nil {
145+
err := account.SetExternalAccountBinding(ctx, client.Client, *iss.ExternalAccount)
146+
if err != nil {
147+
return nil, err
148+
}
116149
}
117-
}
118150

119-
// create account
120-
account, err = client.NewAccount(ctx, account)
121-
if err != nil {
122-
return nil, fmt.Errorf("registering account %v with server: %w", account.Contact, err)
123-
}
124-
iss.Logger.Info("new ACME account registered",
125-
zap.Strings("contact", account.Contact),
126-
zap.String("status", account.Status))
151+
// create account
152+
account, err = client.NewAccount(ctx, account)
153+
if err != nil {
154+
return nil, fmt.Errorf("registering account %v with server: %w", account.Contact, err)
155+
}
156+
iss.Logger.Info("new ACME account registered",
157+
zap.Strings("contact", account.Contact),
158+
zap.String("status", account.Status))
127159

128-
// persist the account to storage
129-
err = iss.saveAccount(ctx, client.Directory, account)
130-
if err != nil {
131-
return nil, fmt.Errorf("could not save account %v: %v", account.Contact, err)
160+
// persist the account to storage
161+
err = iss.saveAccount(ctx, client.Directory, account)
162+
if err != nil {
163+
return nil, fmt.Errorf("could not save account %v: %v", account.Contact, err)
164+
}
165+
} else {
166+
iss.Logger.Info("account has already been registered; reloaded",
167+
zap.Strings("contact", account.Contact),
168+
zap.String("status", account.Status),
169+
zap.String("location", account.Location))
132170
}
133171
}
134172

0 commit comments

Comments
 (0)