internal/ldap: Validate the domain and make SetDomain repeatable

internal/ldap/domain_test.go

@@ -0,0 +1,68 @@
+package ldap
+
+import (
+	"strings"
+	"testing"
+
+	ldap "github.com/ps78674/ldapserver"
+)
+
+func TestSetDomainValidation(t *testing.T) {
+	s := New(WithLogger(testLogger()))
+
+	for _, bad := range []string{"", "example..com", ".example.com", "example.com."} {
+		if err := s.SetDomain(bad); err == nil {
+			t.Errorf("SetDomain(%q) accepted an invalid domain", bad)
+		}
+	}
+
+	if err := s.SetDomain("example.com"); err != nil {
+		t.Fatalf("SetDomain(example.com): %s", err)
+	}
+	want := []string{"dc=netauth", "dc=example", "dc=com"}
+	if strings.Join(s.nc, ",") != strings.Join(want, ",") {
+		t.Errorf("naming context is %v, want %v", s.nc, want)
+	}
+}
+
+// Search routes depend on the naming context, so a search must not be
+// routed at all until a domain is configured.
+func TestSearchBeforeSetDomain(t *testing.T) {
+	s := New(WithLogger(testLogger()), WithNetAuth(&fakeClient{}))
+
+	w := &recordingWriter{}
+	s.ServeLDAP(w, mustDecode(t, berSearchRequest("ou=entities,dc=netauth")))
+
+	code := resultCode(t, w.responses[len(w.responses)-1])
+	if code == ldap.LDAPResultSuccess {
+		t.Error("a search was served without a configured domain")
+	}
+}
+
+// Reconfiguring the domain must replace the old routes rather than
+// adding to them, otherwise a renamed server keeps serving its
+// previous base DN forever.
+func TestSetDomainReplacesRoutes(t *testing.T) {
+	s := New(WithLogger(testLogger()), WithNetAuth(&fakeClient{}))
+
+	if err := s.SetDomain("example.com"); err != nil {
+		t.Fatal(err)
+	}
+	if err := s.SetDomain("other.org"); err != nil {
+		t.Fatal(err)
+	}
+
+	// The old base DN must no longer route to the entity handler.
+	old := &recordingWriter{}
+	s.ServeLDAP(old, mustDecode(t, berSearchRequest("ou=entities,dc=netauth,dc=example,dc=com")))
+	if code := resultCode(t, old.responses[len(old.responses)-1]); code != ldap.LDAPResultUnwillingToPerform {
+		t.Errorf("stale base DN returned %d, want %d (unwillingToPerform)", code, ldap.LDAPResultUnwillingToPerform)
+	}
+
+	// The new base DN must route.
+	fresh := &recordingWriter{}
+	s.ServeLDAP(fresh, mustDecode(t, berSearchRequest("ou=entities,dc=netauth,dc=other,dc=org")))
+	if code := resultCode(t, fresh.responses[len(fresh.responses)-1]); code == ldap.LDAPResultUnwillingToPerform {
+		t.Error("the current base DN does not route to the entity handler")
+	}
+}

internal/ldap/ldap.go

@@ -1,6 +1,7 @@
 package ldap
 
 import (
+	"errors"
 	"strings"
 
 	ldap "github.com/ps78674/ldapserver"
@@ -14,20 +15,47 @@ func New(opts ...Option) *server {
 	}
 	x.Server = ldap.NewServer()
 
-	x.routes = ldap.NewRouteMux()
-	x.routes.NotFound(x.guard(x.handleNotFound, failOperationError))
-	x.routes.Abandon(x.guard(x.handleAbandon, nil))
-	x.routes.Bind(x.guard(x.handleBind, failBindError))
+	// The underlying server only permits a single handler and aborts
+	// the process on a second registration, so a stable delegating
+	// handler is registered once and SetDomain swaps the routed mux
+	// underneath it.
+	x.Handle(x)
 
-	x.routes.Search(x.guard(x.handleSearchDSE, failSearchError)).
+	x.buildRoutes()
+
+	return x
+}
+
+// buildRoutes (re)creates the route mux. It is called by New and again
+// by SetDomain so that the naming-convention dependent routes always
+// reflect the current domain without ever leaving duplicates behind.
+func (s *server) buildRoutes() {
+	routes := ldap.NewRouteMux()
+	routes.NotFound(s.guard(s.handleNotFound, failOperationError))
+	routes.Abandon(s.guard(s.handleAbandon, nil))
+	routes.Bind(s.guard(s.handleBind, failBindError))
+
+	routes.Search(s.guard(s.handleSearchDSE, failSearchError)).
 		BaseDn("").
 		Scope(ldap.SearchRequestScopeBaseObject).
 		Filter("(objectclass=*)").
 		Label("Search - ROOT DSE")
 
-	x.Handle(x.routes)
+	if len(s.nc) > 0 {
+		base := strings.Join(s.nc, ",")
 
-	return x
+		routes.Search(s.guard(s.handleSearchEntities, failSearchError)).
+			BaseDn("ou=entities," + base).
+			Scope(ldap.SearchRequestHomeSubtree).
+			Label("Search - Entities")
+
+		routes.Search(s.guard(s.handleSearchGroups, failSearchError)).
+			BaseDn("ou=groups," + base).
+			Scope(ldap.SearchRequestHomeSubtree).
+			Label("Search - Groups")
+	}
+
+	s.routes = routes
 }
 
 // Serve serves a plaintext DSA on the provided bind string.
@@ -68,30 +96,42 @@ func (s *server) handleAbandon(w ldap.ResponseWriter, m *ldap.Message) {
 	}
 }
 
-func (s *server) SetDomain(domain string) {
-	nc := "dc=netauth,"
-	parts := strings.Split(domain, ".")
-	for i := range parts {
-		nc += "dc=" + parts[i] + ","
-	}
-	nc = strings.TrimSuffix(nc, ",")
-
-	s.nc = strings.Split(nc, ",")
-	for i := range s.nc {
-		s.nc[i] = strings.TrimSpace(s.nc[i])
+// SetDomain configures the naming context from a plain domain name
+// such as "example.com". NetAuth has no notion of a base DN, so it is
+// supplied here: the domain is split on dots, each label becomes a
+// dc component, and dc=netauth is prepended. It must be called after
+// New and is safe to call more than once.
+func (s *server) SetDomain(domain string) error {
+	if domain == "" {
+		return errors.New("a domain must be configured")
 	}
 
-	// Register routes that are dependent on the namingConvention
-	entitySearchDN := "ou=entities," + strings.Join(s.nc, ",")
-	s.routes.Search(s.guard(s.handleSearchEntities, failSearchError)).
-		BaseDn(entitySearchDN).
-		Scope(ldap.SearchRequestHomeSubtree).
-		Label("Search - Entities")
-
-	groupSearchDN := "ou=groups," + strings.Join(s.nc, ",")
-	s.routes.Search(s.guard(s.handleSearchGroups, failSearchError)).
-		BaseDn(groupSearchDN).
-		Scope(ldap.SearchRequestHomeSubtree).
-		Label("Search - Groups")
+	labels := strings.Split(domain, ".")
+	nc := []string{"dc=netauth"}
+	for _, l := range labels {
+		l = strings.TrimSpace(l)
+		if l == "" {
+			return errors.New("domain contains an empty label")
+		}
+		nc = append(nc, "dc="+l)
+	}
 
+	s.nc = nc
+	s.buildRoutes()
+	return nil
+}
+
+// ServeLDAP implements the underlying server's Handler interface. It
+// is registered once with the server and forwards to whatever route
+// mux is current, which lets SetDomain rebuild routing without
+// re-registering a handler (the server aborts on a second
+// registration).
+func (s *server) ServeLDAP(w ldap.ResponseWriter, r *ldap.Message) {
+	if s.routes == nil {
+		res := ldap.NewResponse(ldap.LDAPResultUnavailable)
+		res.SetDiagnosticMessage("server is not configured with a domain")
+		w.Write(res)
+		return
+	}
+	s.routes.ServeLDAP(w, r)
 }

main.go

@@ -72,7 +72,10 @@ func main() {
 		ldap.WithAnonBind(viper.GetBool("ldap.allow_anon")),
 	)
 
-	ls.SetDomain(viper.GetString("ldap.domain"))
+	if err := ls.SetDomain(viper.GetString("ldap.domain")); err != nil {
+		appLogger.Error("Error configuring the base DN", "error", err)
+		os.Exit(3)
+	}
 
 	if !viper.GetBool("ldap.tls") {
 		if !strings.HasPrefix(viper.GetString("ldap.bind"), "localhost") {