letsencrypt: Don't prompt if user is not there

This change fixes the scenario where you reload the config and it tries to obtain a cert from the ACME server, but no email address is found or terms have not been agreed to in-process. This is unfortunate but it should not stop the server from reloading, so we assume empty email address in this case.

caddy/letsencrypt/letsencrypt.go

@@ -116,11 +116,11 @@ func MarkQualified(configs []server.Config) {
 //
 // TODO: Right now by potentially prompting about ToS error, we assume this function is only
 // called at startup, but that is not always the case because it could be during a restart.
-func ObtainCerts(configs []server.Config, optPort string) error {
-	groupedConfigs := groupConfigsByEmail(configs)
+func ObtainCerts(configs []server.Config, altPort string) error {
+	groupedConfigs := groupConfigsByEmail(configs, altPort != "") // don't prompt user if server already running
 
 	for email, group := range groupedConfigs {
-		client, err := newClientPort(email, optPort)
+		client, err := newClientPort(email, altPort)
 		if err != nil {
 			return errors.New("error creating client: " + err.Error())
 		}
@@ -147,11 +147,11 @@ func ObtainCerts(configs []server.Config, optPort string) error {
 					// TODO: Double-check, will obtainErr ever be nil?
 					if tosErr, ok := obtainErr.(acme.TOSError); ok {
 						// Terms of Service agreement error; we can probably deal with this
-						if !Agreed && !promptedForAgreement {
+						if !Agreed && !promptedForAgreement && altPort == "" { // don't prompt if server is already running
 							Agreed = promptUserAgreement(tosErr.Detail, true) // TODO: Use latest URL
 							promptedForAgreement = true
 						}
-						if Agreed {
+						if Agreed || altPort != "" {
 							err := client.AgreeToTOS()
 							if err != nil {
 								return errors.New("error agreeing to updated terms: " + err.Error())
@@ -174,14 +174,15 @@ func ObtainCerts(configs []server.Config, optPort string) error {
 
 // groupConfigsByEmail groups configs by the email address to be used by its
 // ACME client. It only includes configs that are marked as fully managed.
-// This is the function that may prompt for an email address.
-func groupConfigsByEmail(configs []server.Config) map[string][]server.Config {
+// This is the function that may prompt for an email address, unless skipPrompt
+// is true, in which case it will assume an empty email address.
+func groupConfigsByEmail(configs []server.Config, skipPrompt bool) map[string][]server.Config {
 	initMap := make(map[string][]server.Config)
 	for _, cfg := range configs {
 		if !cfg.TLS.Managed {
 			continue
 		}
-		leEmail := getEmail(cfg)
+		leEmail := getEmail(cfg, skipPrompt)
 		initMap[leEmail] = append(initMap[leEmail], cfg)
 	}
 	return initMap
@@ -451,7 +452,7 @@ func Revoke(host string) error {
 		return errors.New("no certificate and key for " + host)
 	}
 
-	email := getEmail(server.Config{Host: host})
+	email := getEmail(server.Config{Host: host}, false)
 	if email == "" {
 		return errors.New("email is required to revoke")
 	}

caddy/letsencrypt/letsencrypt_test.go

@@ -280,7 +280,7 @@ func TestEnableTLS(t *testing.T) {
 }
 
 func TestGroupConfigsByEmail(t *testing.T) {
-	if groupConfigsByEmail([]server.Config{}) == nil {
+	if groupConfigsByEmail([]server.Config{}, false) == nil {
 		t.Errorf("With empty input, returned map was nil, but expected non-nil map")
 	}
 
@@ -292,9 +292,9 @@ func TestGroupConfigsByEmail(t *testing.T) {
 		server.Config{Host: "sub4.example.com", TLS: server.TLSConfig{LetsEncryptEmail: "", Managed: true}},
 		server.Config{Host: "sub5.example.com", TLS: server.TLSConfig{LetsEncryptEmail: ""}}, // not managed
 	}
-	DefaultEmail = "test@example.com" // bypass prompt during tests...
+	DefaultEmail = "test@example.com"
 
-	groups := groupConfigsByEmail(configs)
+	groups := groupConfigsByEmail(configs, true)
 
 	if groups == nil {
 		t.Fatalf("Returned map was nil, but expected values")

caddy/letsencrypt/user.go

@@ -114,8 +114,10 @@ func newUser(email string) (User, error) {
 // address from the user to use for TLS for cfg. If it
 // cannot get an email address, it returns empty string.
 // (It will warn the user of the consequences of an
-// empty email.)
-func getEmail(cfg server.Config) string {
+// empty email.) If skipPrompt is true, the user will
+// NOT be prompted and an empty email will be returned
+// instead.
+func getEmail(cfg server.Config, skipPrompt bool) string {
 	// First try the tls directive from the Caddyfile
 	leEmail := cfg.TLS.LetsEncryptEmail
 	if leEmail == "" {
@@ -132,15 +134,12 @@ func getEmail(cfg server.Config) string {
 					continue
 				}
 				if mostRecent == nil || dir.ModTime().After(mostRecent.ModTime()) {
-					mostRecent = dir
-				}
+					leEmail = dir.Name()
 				}
-			if mostRecent != nil {
-				leEmail = mostRecent.Name()
 			}
 		}
 	}
-	if leEmail == "" {
+	if leEmail == "" && !skipPrompt {
 		// Alas, we must bother the user and ask for an email address;
 		// if they proceed they also agree to the SA.
 		reader := bufio.NewReader(stdin)

caddy/letsencrypt/user_test.go

@@ -140,13 +140,13 @@ func TestGetEmail(t *testing.T) {
 			LetsEncryptEmail: "test1@foo.com",
 		},
 	}
-	actual := getEmail(config)
+	actual := getEmail(config, false)
 	if actual != "test1@foo.com" {
 		t.Errorf("Did not get correct email from config; expected '%s' but got '%s'", "test1@foo.com", actual)
 	}
 
 	// Test2: Use default email from flag (or user previously typing it)
-	actual = getEmail(server.Config{})
+	actual = getEmail(server.Config{}, false)
 	if actual != DefaultEmail {
 		t.Errorf("Did not get correct email from config; expected '%s' but got '%s'", DefaultEmail, actual)
 	}
@@ -158,7 +158,7 @@ func TestGetEmail(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Could not simulate user input, error: %v", err)
 	}
-	actual = getEmail(server.Config{})
+	actual = getEmail(server.Config{}, false)
 	if actual != "test3@foo.com" {
 		t.Errorf("Did not get correct email from user input prompt; expected '%s' but got '%s'", "test3@foo.com", actual)
 	}
@@ -189,8 +189,7 @@ func TestGetEmail(t *testing.T) {
 			t.Fatalf("Could not change user folder mod time for '%s': %v", eml, err)
 		}
 	}
-
-	actual = getEmail(server.Config{})
+	actual = getEmail(server.Config{}, false)
 	if actual != "test4-3@foo.com" {
 		t.Errorf("Did not get correct email from storage; expected '%s' but got '%s'", "test4-3@foo.com", actual)
 	}

caddy/restart.go

@@ -13,7 +13,6 @@ import (
 	"path"
 
 	"github.com/mholt/caddy/caddy/letsencrypt"
-	"github.com/mholt/caddy/server"
 )
 
 func init() {
@@ -137,26 +136,8 @@ func getCertsForNewCaddyfile(newCaddyfile Input) error {
 	// we must make sure port is set before we group by bind address
 	letsencrypt.EnableTLS(configs)
 
-	// we only need to issue certs for hosts where we already have an active listener
-	groupings, err := arrangeBindings(configs)
-	if err != nil {
-		return errors.New("arranging bindings: " + err.Error())
-	}
-	var configsToSetup []server.Config
-	serversMu.Lock()
-GroupLoop:
-	for _, group := range groupings {
-		for _, server := range servers {
-			if server.Addr == group.BindAddr.String() {
-				configsToSetup = append(configsToSetup, group.Configs...)
-				continue GroupLoop
-			}
-		}
-	}
-	serversMu.Unlock()
-
 	// place certs on the disk
-	err = letsencrypt.ObtainCerts(configsToSetup, letsencrypt.AlternatePort)
+	err = letsencrypt.ObtainCerts(configs, letsencrypt.AlternatePort)
 	if err != nil {
 		return errors.New("obtaining certs: " + err.Error())
 	}