Merge pull request #21549 from etcd-io/cherry-pick-36/allow-non-admin-user-to-list-members

[release-3.6] etcdserver: allow non-admin to fetch member list and alarms

Author: ahrtr

server/etcdserver/api/v3rpc/auth.go

@@ -185,3 +185,19 @@ func (aa *AuthAdmin) isPermitted(ctx context.Context) error {
 
 	return aa.ag.AuthStore().IsAdminPermitted(authInfo)
 }
+
+func (aa *AuthAdmin) requireAuthInfo(ctx context.Context) error {
+	if !aa.ag.AuthStore().IsAuthEnabled() {
+		return nil
+	}
+
+	authInfo, err := aa.ag.AuthInfoFromCtx(ctx)
+	if err != nil {
+		return err
+	}
+
+	if authInfo == nil {
+		return auth.ErrUserEmpty
+	}
+	return nil
+}

server/etcdserver/api/v3rpc/maintenance.go

@@ -350,8 +350,15 @@ func (ams *authMaintenanceServer) HashKV(ctx context.Context, r *pb.HashKVReques
 }
 
 func (ams *authMaintenanceServer) Alarm(ctx context.Context, ar *pb.AlarmRequest) (*pb.AlarmResponse, error) {
-	if err := ams.isPermitted(ctx); err != nil {
-		return nil, togRPCError(err)
+	switch ar.GetAction() {
+	case pb.AlarmRequest_GET:
+		if err := ams.requireAuthInfo(ctx); err != nil {
+			return nil, togRPCError(err)
+		}
+	default:
+		if err := ams.isPermitted(ctx); err != nil {
+			return nil, togRPCError(err)
+		}
 	}
 	return ams.maintenanceServer.Alarm(ctx, ar)
 }

server/etcdserver/server.go

@@ -1677,7 +1677,7 @@ func (s *EtcdServer) MemberList(ctx context.Context, r *pb.MemberListRequest) ([
 		}
 	}
 
-	if err := s.checkMembershipOperationPermission(ctx); err != nil {
+	if err := s.requireAuthInfo(ctx); err != nil {
 		return nil, err
 	}
 	return s.cluster.Members(), nil

tests/common/auth_test.go

@@ -691,9 +691,14 @@ func TestAuthMemberList(t *testing.T) {
 		require.NoErrorf(t, setupAuth(cc, []authRole{testRole}, []authUser{rootUser, testUser}), "failed to enable auth")
 		rootAuthClient := testutils.MustClient(clus.Client(WithAuth(rootUserName, rootPassword)))
 		testUserAuthClient := testutils.MustClient(clus.Client(WithAuth(testUserName, testPassword)))
+		anonAuthClient := testutils.MustClient(clus.Client())
 
 		_, err := testUserAuthClient.MemberList(ctx, false)
-		require.ErrorContains(t, err, PermissionDenied)
+		require.NoError(t, err)
+
+		_, err = anonAuthClient.MemberList(ctx, false)
+		require.Error(t, err)
+		require.ErrorContains(t, err, "etcdserver: user name is empty")
 
 		_, err = rootAuthClient.MemberList(ctx, false)
 		require.NoError(t, err)
@@ -994,6 +999,7 @@ func TestAuthAlarm(t *testing.T) {
 		require.NoErrorf(t, setupAuth(cc, []authRole{testRole}, []authUser{rootUser, testUser}), "failed to enable auth")
 		rootAuthClient := testutils.MustClient(clus.Client(WithAuth(rootUserName, rootPassword)))
 		testUserAuthClient := testutils.MustClient(clus.Client(WithAuth(testUserName, testPassword)))
+		anonAuthClient := testutils.MustClient(clus.Client())
 
 		for i := 0; ; i++ {
 			err := rootAuthClient.Put(ctx,
@@ -1006,9 +1012,6 @@ func TestAuthAlarm(t *testing.T) {
 			break
 		}
 
-		_, err := testUserAuthClient.AlarmList(ctx)
-		require.ErrorContains(t, err, PermissionDenied)
-
 		memberID := uint64(0)
 
 		for i := 0; i < 10; i++ {
@@ -1023,6 +1026,12 @@ func TestAuthAlarm(t *testing.T) {
 		}
 		require.NotEqualf(t, uint64(0), memberID, "expect to find alarm with non-zero member ID")
 
+		_, err := anonAuthClient.AlarmList(ctx)
+		require.ErrorContains(t, err, "etcdserver: user name is empty")
+
+		_, err = testUserAuthClient.AlarmList(ctx)
+		require.NoError(t, err)
+
 		_, err = testUserAuthClient.AlarmDisarm(ctx, &clientv3.AlarmMember{
 			MemberID: memberID,
 			Alarm:    pb.AlarmType_NOSPACE,

tests/e2e/ctl_v3_auth_test.go

@@ -15,7 +15,6 @@
 package e2e
 
 import (
-	"context"
 	"fmt"
 	"os"
 	"strings"
@@ -296,26 +295,7 @@ func authTestEndpointHealth(cx ctlCtx) {
 	require.NoError(cx.t, ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "health", "", false}))
 
 	cx.user, cx.pass = "test-user", "pass"
-	func(cx ctlCtx) {
-		cmdArgs := append(cx.PrefixArgs(), "endpoint", "health")
-		lines := make([]expect.ExpectedResponse, cx.epc.Cfg.ClusterSize)
-		for i := range lines {
-			lines[i] = expect.ExpectedResponse{
-				Value: cx.epc.Procs[i].EndpointsGRPC()[0] + " is unhealthy: failed to commit proposal: Unable to fetch the alarm list",
-			}
-		}
-
-		proc, err := e2e.SpawnCmd(cmdArgs, cx.envMap)
-		require.NoErrorf(cx.t, err, "failed to spawn endpoint health command")
-		defer func() {
-			require.Errorf(cx.t, proc.Close(), "endpoint health command should reject all non-root users")
-		}()
-
-		for _, line := range lines {
-			_, lerr := proc.ExpectWithContext(context.TODO(), line)
-			require.NoErrorf(cx.t, lerr, "endpoint health should fail with permission denied error")
-		}
-	}(cx)
+	require.NoErrorf(cx.t, ctlV3EndpointHealth(cx), "endpointStatusTest ctlV3EndpointHealth error")
 
 	cmdArgs := append(cx.PrefixArgs(), "endpoint", "health", "--user=root:root", "--cluster")
 	proc, err := e2e.SpawnCmd(cmdArgs, cx.envMap)