Reduce cost of admin user check

CHANGELOG.md

@@ -20,6 +20,7 @@
 - [#7995](https://github.com/influxdata/influxdb/issues/7995): Update liner dependency to handle docker exec.
 - [#7811](https://github.com/influxdata/influxdb/issues/7811): Kill query not killing query
 - [#7457](https://github.com/influxdata/influxdb/issues/7457): KILL QUERY should work during all phases of a query
+- [#8155](https://github.com/influxdata/influxdb/pull/8155): Simplify admin user check.
 
 ## v1.2.2 [2017-03-14]
 

services/httpd/handler.go

@@ -72,8 +72,8 @@ type Handler struct {
 	MetaClient interface {
 		Database(name string) *meta.DatabaseInfo
 		Authenticate(username, password string) (ui *meta.UserInfo, err error)
-		Users() []meta.UserInfo
 		User(username string) (*meta.UserInfo, error)
+		AdminUserExists() bool
 	}
 
 	QueryAuthorizer interface {
@@ -964,20 +964,8 @@ func authenticate(inner func(http.ResponseWriter, *http.Request, *meta.UserInfo)
 		}
 		var user *meta.UserInfo
 
-		// Retrieve user list.
-		uis := h.MetaClient.Users()
-
-		// See if admin user exists.
-		adminExists := false
-		for i := range uis {
-			if uis[i].Admin {
-				adminExists = true
-				break
-			}
-		}
-
 		// TODO corylanou: never allow this in the future without users
-		if requireAuthentication && adminExists {
+		if requireAuthentication && h.MetaClient.AdminUserExists() {
 			creds, err := parseCredentials(r)
 			if err != nil {
 				atomic.AddInt64(&h.stats.AuthenticationFailures, 1)

services/httpd/handler_test.go

@@ -88,16 +88,7 @@ func TestHandler_Query_Auth(t *testing.T) {
 	h := NewHandler(true)
 
 	// Set mock meta client functions for the handler to use.
-	h.MetaClient.UsersFn = func() []meta.UserInfo {
-		return []meta.UserInfo{
-			{
-				Name:       "user1",
-				Hash:       "abcd",
-				Admin:      true,
-				Privileges: make(map[string]influxql.Privilege),
-			},
-		}
-	}
+	h.MetaClient.AdminUserExistsFn = func() bool { return true }
 
 	h.MetaClient.UserFn = func(username string) (*meta.UserInfo, error) {
 		if username != "user1" {
@@ -355,8 +346,10 @@ func TestHandler_Query_ErrAuthorize(t *testing.T) {
 	h.QueryAuthorizer.AuthorizeQueryFn = func(u *meta.UserInfo, q *influxql.Query, db string) error {
 		return errors.New("marker")
 	}
-	h.MetaClient.UsersFn = func() []meta.UserInfo {
-		return []meta.UserInfo{
+	h.MetaClient.AdminUserExistsFn = func() bool { return true }
+	h.MetaClient.AuthenticateFn = func(u, p string) (*meta.UserInfo, error) {
+
+		users := []meta.UserInfo{
 			{
 				Name:  "admin",
 				Hash:  "admin",
@@ -370,9 +363,8 @@ func TestHandler_Query_ErrAuthorize(t *testing.T) {
 				},
 			},
 		}
-	}
-	h.MetaClient.AuthenticateFn = func(u, p string) (*meta.UserInfo, error) {
-		for _, user := range h.MetaClient.Users() {
+
+		for _, user := range users {
 			if u == user.Name {
 				if p == user.Hash {
 					return &user, nil
@@ -637,11 +629,11 @@ func NewHandler(requireAuthentication bool) *Handler {
 
 // HandlerMetaStore is a mock implementation of Handler.MetaClient.
 type HandlerMetaStore struct {
-	PingFn         func(d time.Duration) error
-	DatabaseFn     func(name string) *meta.DatabaseInfo
-	AuthenticateFn func(username, password string) (ui *meta.UserInfo, err error)
-	UsersFn        func() []meta.UserInfo
-	UserFn         func(username string) (*meta.UserInfo, error)
+	PingFn            func(d time.Duration) error
+	DatabaseFn        func(name string) *meta.DatabaseInfo
+	AuthenticateFn    func(username, password string) (ui *meta.UserInfo, err error)
+	UserFn            func(username string) (*meta.UserInfo, error)
+	AdminUserExistsFn func() bool
 }
 
 func (s *HandlerMetaStore) Ping(b bool) error {
@@ -660,8 +652,8 @@ func (s *HandlerMetaStore) Authenticate(username, password string) (ui *meta.Use
 	return s.AuthenticateFn(username, password)
 }
 
-func (s *HandlerMetaStore) Users() []meta.UserInfo {
-	return s.UsersFn()
+func (s *HandlerMetaStore) AdminUserExists() bool {
+	return s.AdminUserExistsFn()
 }
 
 func (s *HandlerMetaStore) User(username string) (*meta.UserInfo, error) {

services/meta/client.go

@@ -547,13 +547,7 @@ func (c *Client) UserPrivilege(username, database string) (*influxql.Privilege,
 func (c *Client) AdminUserExists() bool {
 	c.mu.RLock()
 	defer c.mu.RUnlock()
-
-	for _, u := range c.cacheData.Users {
-		if u.Admin {
-			return true
-		}
-	}
-	return false
+	return c.cacheData.AdminUserExists()
 }
 
 // Authenticate returns a UserInfo if the username and password match an existing entry.

services/meta/data.go

@@ -42,6 +42,10 @@ type Data struct {
 	Databases []DatabaseInfo
 	Users     []UserInfo
 
+	// adminUserExists provides a constant time mechanism for determining
+	// if there is at least one admin user.
+	adminUserExists bool
+
 	MaxShardGroupID uint64
 	MaxShardID      uint64
 }
@@ -568,17 +572,29 @@ func (data *Data) CreateUser(name, hash string, admin bool) error {
 		Admin: admin,
 	})
 
+	// We know there is now at least one admin user.
+	if admin {
+		data.adminUserExists = true
+	}
+
 	return nil
 }
 
 // DropUser removes an existing user by name.
 func (data *Data) DropUser(name string) error {
 	for i := range data.Users {
 		if data.Users[i].Name == name {
+			wasAdmin := data.Users[i].Admin
 			data.Users = append(data.Users[:i], data.Users[i+1:]...)
+
+			// Maybe we dropped the only admin user?
+			if wasAdmin {
+				data.adminUserExists = data.hasAdminUser()
+			}
 			return nil
 		}
 	}
+
 	return ErrUserNotFound
 }
 
@@ -630,9 +646,17 @@ func (data *Data) SetAdminPrivilege(name string, admin bool) error {
 
 	ui.Admin = admin
 
+	// We could have promoted or revoked the only admin. Check if an admin
+	// user exists.
+	data.adminUserExists = data.hasAdminUser()
 	return nil
 }
 
+// AdminUserExists returns true if an admin user exists.
+func (data Data) AdminUserExists() bool {
+	return data.adminUserExists
+}
+
 // UserPrivileges gets the privileges for a user.
 func (data *Data) UserPrivileges(name string) (map[string]influxql.Privilege, error) {
 	ui := data.User(name)
@@ -714,6 +738,10 @@ func (data *Data) unmarshal(pb *internal.Data) {
 	for i, x := range pb.GetUsers() {
 		data.Users[i].unmarshal(x)
 	}
+
+	// Exhaustively determine if there is an admin user. The marshalled cache
+	// value may not be correct.
+	data.adminUserExists = data.hasAdminUser()
 }
 
 // MarshalBinary encodes the metadata to a binary format.
@@ -731,6 +759,17 @@ func (data *Data) UnmarshalBinary(buf []byte) error {
 	return nil
 }
 
+// hasAdminUser exhaustively checks for the presence of at least one admin
+// user.
+func (data *Data) hasAdminUser() bool {
+	for _, u := range data.Users {
+		if u.Admin {
+			return true
+		}
+	}
+	return false
+}
+
 // NodeInfo represents information about a single node in the cluster.
 type NodeInfo struct {
 	ID      uint64

services/meta/data_test.go

@@ -110,6 +110,84 @@ func Test_Data_CreateRetentionPolicy(t *testing.T) {
 	}
 }
 
+func TestData_AdminUserExists(t *testing.T) {
+	data := meta.Data{}
+
+	// No users means no admin.
+	if data.AdminUserExists() {
+		t.Fatal("no admin user should exist")
+	}
+
+	// Add a non-admin user.
+	if err := data.CreateUser("user1", "a", false); err != nil {
+		t.Fatal(err)
+	}
+	if got, exp := data.AdminUserExists(), false; got != exp {
+		t.Fatalf("got %v, expected %v", got, exp)
+	}
+
+	// Add an admin user.
+	if err := data.CreateUser("admin1", "a", true); err != nil {
+		t.Fatal(err)
+	}
+	if got, exp := data.AdminUserExists(), true; got != exp {
+		t.Fatalf("got %v, expected %v", got, exp)
+	}
+
+	// Remove the original user
+	if err := data.DropUser("user1"); err != nil {
+		t.Fatal(err)
+	}
+	if got, exp := data.AdminUserExists(), true; got != exp {
+		t.Fatalf("got %v, expected %v", got, exp)
+	}
+
+	// Add another admin
+	if err := data.CreateUser("admin2", "a", true); err != nil {
+		t.Fatal(err)
+	}
+	if got, exp := data.AdminUserExists(), true; got != exp {
+		t.Fatalf("got %v, expected %v", got, exp)
+	}
+
+	// Revoke privileges of the first admin
+	if err := data.SetAdminPrivilege("admin1", false); err != nil {
+		t.Fatal(err)
+	}
+	if got, exp := data.AdminUserExists(), true; got != exp {
+		t.Fatalf("got %v, expected %v", got, exp)
+	}
+
+	// Add user1 back.
+	if err := data.CreateUser("user1", "a", false); err != nil {
+		t.Fatal(err)
+	}
+	// Revoke remaining admin.
+	if err := data.SetAdminPrivilege("admin2", false); err != nil {
+		t.Fatal(err)
+	}
+	// No longer any admins
+	if got, exp := data.AdminUserExists(), false; got != exp {
+		t.Fatalf("got %v, expected %v", got, exp)
+	}
+
+	// Make user1 an admin
+	if err := data.SetAdminPrivilege("user1", true); err != nil {
+		t.Fatal(err)
+	}
+	if got, exp := data.AdminUserExists(), true; got != exp {
+		t.Fatalf("got %v, expected %v", got, exp)
+	}
+
+	// Drop user1...
+	if err := data.DropUser("user1"); err != nil {
+		t.Fatal(err)
+	}
+	if got, exp := data.AdminUserExists(), false; got != exp {
+		t.Fatalf("got %v, expected %v", got, exp)
+	}
+}
+
 func TestUserInfo_AuthorizeDatabase(t *testing.T) {
 	emptyUser := &meta.UserInfo{}
 	if !emptyUser.AuthorizeDatabase(influxql.NoPrivileges, "anydb") {