From 17fd2e72822146e9a486bb4c08ae620af0946052 Mon Sep 17 00:00:00 2001
From: Yap Sok Ann <sokann@gmail.com>
Date: Fri, 31 Dec 2021 13:17:02 +0700
Subject: [PATCH] Disable auth gracefully without impacting existing watchers

This attempts to fix a special case of the problem described in #12385,
where trying to do `clientv3.Watch` with an expired token would result
in `ErrGRPCPermissionDenied`, due to the failing authorization check in
`isWatchPermitted`. Furthermore, the client can't auto recover, since
`shouldRefreshToken` rightly returns false for the permission denied
error.

In this case, we would like to have a runbook to dynamically disable
auth, without causing any disruption. Doing so would immediately expire
all existing tokens, which would then cause the behavior described
above. This means existing watchers would still work for a period of
time after disabling auth, until they have to reconnect, e.g. due to a
rolling restart of server nodes.

This commit adds a client-side fix and a server-side fix, either of
which is sufficient to get the added test case to pass. Note that it is
an e2e test case instead of an integration one, as the reconnect only
happens if the server node is stopped via SIGINT or SIGTERM.

A generic fix for the problem described in #12385 would be better, as
that shall also fix this special case. However, the fix would likely be
a lot more involved, as some untangling of authn/authz is required.
---
 client/v3/client.go           |  1 +
 server/auth/store.go          |  4 ++++
 tests/e2e/ctl_v3_auth_test.go | 45 +++++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+)

diff --git a/client/v3/client.go b/client/v3/client.go
index c39f00421..971fea607 100644
--- a/client/v3/client.go
+++ b/client/v3/client.go
@@ -263,6 +263,7 @@ func (c *Client) getToken(ctx context.Context) error {
 	resp, err := c.Auth.Authenticate(ctx, c.Username, c.Password)
 	if err != nil {
 		if err == rpctypes.ErrAuthNotEnabled {
+			c.authTokenBundle.UpdateAuthToken("")
 			return nil
 		}
 		return err
diff --git a/server/auth/store.go b/server/auth/store.go
index 4d0e8d6b2..408b235ba 100644
--- a/server/auth/store.go
+++ b/server/auth/store.go
@@ -1026,6 +1026,10 @@ func (as *authStore) AuthInfoFromTLS(ctx context.Context) (ai *AuthInfo) {
 }
 
 func (as *authStore) AuthInfoFromCtx(ctx context.Context) (*AuthInfo, error) {
+	if !as.IsAuthEnabled() {
+		return nil, nil
+	}
+
 	md, ok := metadata.FromIncomingContext(ctx)
 	if !ok {
 		return nil, nil
diff --git a/tests/e2e/ctl_v3_auth_test.go b/tests/e2e/ctl_v3_auth_test.go
index ed3ffc416..4d376f64c 100644
--- a/tests/e2e/ctl_v3_auth_test.go
+++ b/tests/e2e/ctl_v3_auth_test.go
@@ -30,6 +30,7 @@ func TestCtlV3AuthEnable(t *testing.T) {
 	testCtl(t, authEnableTest)
 }
 func TestCtlV3AuthDisable(t *testing.T)             { testCtl(t, authDisableTest) }
+func TestCtlV3AuthGracefulDisable(t *testing.T)     { testCtl(t, authGracefulDisableTest) }
 func TestCtlV3AuthStatus(t *testing.T)              { testCtl(t, authStatusTest) }
 func TestCtlV3AuthWriteKey(t *testing.T)            { testCtl(t, authCredWriteKeyTest) }
 func TestCtlV3AuthRoleUpdate(t *testing.T)          { testCtl(t, authRoleUpdateTest) }
@@ -142,6 +143,50 @@ func authDisableTest(cx ctlCtx) {
 	}
 }
 
+func authGracefulDisableTest(cx ctlCtx) {
+	if err := authEnable(cx); err != nil {
+		cx.t.Fatal(err)
+	}
+
+	cx.user, cx.pass = "root", "root"
+
+	donec := make(chan struct{})
+
+	go func() {
+		defer close(donec)
+
+		// sleep a bit to let the watcher connects while auth is still enabled
+		time.Sleep(1000 * time.Millisecond)
+
+		// now disable auth...
+		if err := ctlV3AuthDisable(cx); err != nil {
+			cx.t.Fatalf("authGracefulDisableTest ctlV3AuthDisable error (%v)", err)
+		}
+
+		// ...and restart the node
+		node0 := cx.epc.Procs[0]
+		node0.WithStopSignal(syscall.SIGINT)
+		if rerr := node0.Restart(); rerr != nil {
+			cx.t.Fatal(rerr)
+		}
+
+		// the watcher should still work after reconnecting
+		if perr := ctlV3Put(cx, "key", "value", ""); perr != nil {
+			cx.t.Errorf("authGracefulDisableTest ctlV3Put error (%v)", perr)
+		}
+	}()
+
+	err := ctlV3Watch(cx, []string{"key"}, kvExec{key: "key", val: "value"})
+
+	if err != nil {
+		if cx.dialTimeout > 0 && !isGRPCTimedout(err) {
+			cx.t.Errorf("authGracefulDisableTest ctlV3Watch error (%v)", err)
+		}
+	}
+
+	<-donec
+}
+
 func ctlV3AuthDisable(cx ctlCtx) error {
 	cmdArgs := append(cx.PrefixArgs(), "auth", "disable")
 	return e2e.SpawnWithExpectWithEnv(cmdArgs, cx.envMap, "Authentication Disabled")