Update dashboard's validateToken not to use k8s api server. (#4043)

absoludity · web-flow · commit e0f8d17591ce · 2022-01-11T09:36:11.000+11:00
* Update dashboard's validateToken not to use k8s api server.

Signed-off-by: Michael Nelson &lt;minelson@vmware.com&gt;

* Lint

Signed-off-by: Michael Nelson &lt;minelson@vmware.com&gt;

* Removed debug code.

Signed-off-by: Michael Nelson &lt;minelson@vmware.com&gt;

* Use `.toThrow` now that it works with async.

Signed-off-by: Michael Nelson &lt;minelson@vmware.com&gt;
diff --git a/cmd/kubeapps-apis/plugins/resources/v1alpha1/server.go b/cmd/kubeapps-apis/plugins/resources/v1alpha1/server.go
@@ -415,14 +415,18 @@ func resourceRefsEqual(r1, r2 *pkgsGRPCv1alpha1.ResourceRef) bool {
 }
 
 // errorByStatus generates a meaningful error message
+// TODO(minelson): Move to a shared helper for plugins interacting
+// with the k8s cluster.
 func errorByStatus(verb, resource, identifier string, err error) error {
 	if identifier == "" {
 		identifier = "all"
 	}
 	if errors.IsNotFound(err) {
 		return status.Errorf(codes.NotFound, "unable to %s the %s '%s' due to '%v'", verb, resource, identifier, err)
-	} else if errors.IsForbidden(err) || errors.IsUnauthorized(err) {
-		return status.Errorf(codes.PermissionDenied, "Unauthorized to %s the %s '%s' due to '%v'", verb, resource, identifier, err)
+	} else if errors.IsForbidden(err) {
+		return status.Errorf(codes.PermissionDenied, "Forbidden to %s the %s '%s' due to '%v'", verb, resource, identifier, err)
+	} else if errors.IsUnauthorized(err) {
+		return status.Errorf(codes.Unauthenticated, "Authorization required to %s the %s '%s' due to '%v'", verb, resource, identifier, err)
 	} else if errors.IsAlreadyExists(err) {
 		return status.Errorf(codes.AlreadyExists, "Cannot %s the %s '%s' due to '%v' as it already exists", verb, resource, identifier, err)
 	}
diff --git a/dashboard/src/components/LoginForm/LoginForm.test.tsx b/dashboard/src/components/LoginForm/LoginForm.test.tsx
@@ -2,7 +2,7 @@ import LoadingWrapper from "components/LoadingWrapper";
 import { Location } from "history";
 import { act } from "react-dom/test-utils";
 import { Redirect } from "react-router";
-import { defaultStore, mountWrapper } from "shared/specs/mountWrapper";
+import { defaultStore, getStore, mountWrapper } from "shared/specs/mountWrapper";
 import LoginForm from "./LoginForm";
 import OAuthLogin from "./OauthLogin";
 import TokenLogin from "./TokenLogin";
@@ -145,21 +145,33 @@ describe("oauth login form", () => {
     expect(wrapper.find("input#token").exists()).toBe(false);
   });
 
-  it("displays the oauth login if oauthLoginURI provided", () => {
-    const wrapper = mountWrapper(defaultStore, <LoginForm {...props} />);
+  it("displays the oauth login if authProxyEnabled", () => {
+    const state = {
+      ...defaultStore,
+      config: {
+        authProxyEnabled: true,
+      },
+    };
+    const wrapper = mountWrapper(getStore({ ...state }), <LoginForm {...props} />);
     expect(props.checkCookieAuthentication).toHaveBeenCalled();
     expect(wrapper.find(OAuthLogin)).toExist();
     expect(wrapper.find("a").findWhere(a => a.prop("href") === props.oauthLoginURI)).toExist();
   });
 
   it("doesn't render the login form if the cookie has not been checked yet", () => {
+    const state = {
+      ...defaultStore,
+      config: {
+        authProxyEnabled: true,
+      },
+    };
     const props2 = {
       ...props,
       checkCookieAuthentication: jest.fn().mockReturnValue({
         then: jest.fn(() => false),
       }),
     };
-    const wrapper = mountWrapper(defaultStore, <LoginForm {...props2} />);
+    const wrapper = mountWrapper(getStore({ ...state }), <LoginForm {...props2} />);
     expect(wrapper.find(LoadingWrapper)).toExist();
     expect(wrapper.find(OAuthLogin)).not.toExist();
   });
diff --git a/dashboard/src/components/LoginForm/LoginForm.tsx b/dashboard/src/components/LoginForm/LoginForm.tsx
@@ -7,6 +7,8 @@ import LoadingWrapper from "../../components/LoadingWrapper";
 import "./LoginForm.css";
 import OAuthLogin from "./OauthLogin";
 import TokenLogin from "./TokenLogin";
+import { useSelector } from "react-redux";
+import { IStoreState } from "shared/types";
 
 export interface ILoginFormProps {
   cluster: string;
@@ -25,14 +27,19 @@ function LoginForm(props: ILoginFormProps) {
   const intl = useIntl();
   const [token, setToken] = useState("");
   const [cookieChecked, setCookieChecked] = useState(false);
-  const { oauthLoginURI, checkCookieAuthentication } = props;
+  const { checkCookieAuthentication } = props;
+
+  const {
+    config: { authProxyEnabled },
+  } = useSelector((state: IStoreState) => state);
+
   useEffect(() => {
-    if (oauthLoginURI) {
+    if (authProxyEnabled) {
       checkCookieAuthentication(props.cluster).then(() => setCookieChecked(true));
     } else {
       setCookieChecked(true);
     }
-  }, [oauthLoginURI, checkCookieAuthentication, props.cluster]);
+  }, [authProxyEnabled, checkCookieAuthentication, props.cluster]);
 
   if (props.authenticating || !cookieChecked) {
     return (
diff --git a/dashboard/src/shared/Auth.test.ts b/dashboard/src/shared/Auth.test.ts
@@ -1,63 +1,84 @@
+import { grpc } from "@improbable-eng/grpc-web";
 import Axios, { AxiosResponse } from "axios";
+import { CheckNamespaceExistsRequest } from "gen/kubeappsapis/plugins/resources/v1alpha1/resources";
 import * as jwt from "jsonwebtoken";
 import { Auth } from "./Auth";
 import { SupportedThemes } from "./Config";
+import { KubeappsGrpcClient } from "./KubeappsGrpcClient";
 
 describe("Auth", () => {
+  // Create a real client, but we'll stub out the function we're interested in.
+  const client = new KubeappsGrpcClient().getResourcesServiceClientImpl();
+  let mockClientCheckNamespaceExists: jest.MockedFunction<typeof client.CheckNamespaceExists>;
+
   beforeEach(() => {
     Axios.get = jest.fn();
+    mockClientCheckNamespaceExists = jest
+      .fn()
+      .mockImplementation(() => Promise.resolve({ exists: true } as CheckNamespaceExistsRequest));
+    jest.spyOn(client, "CheckNamespaceExists").mockImplementation(mockClientCheckNamespaceExists);
+    jest.spyOn(Auth, "resourcesClient").mockImplementation(() => client);
   });
   afterEach(() => {
     jest.resetAllMocks();
   });
-  it("should get an URL with the given token", async () => {
-    const mock = jest.fn();
-    Axios.get = mock;
+
+  it("should return without error when the endpoint succeeds with the given token", async () => {
     await Auth.validateToken("othercluster", "foo");
-    expect(mock.mock.calls[0]).toEqual([
-      "api/clusters/othercluster/",
-      { headers: { Authorization: "Bearer foo" } },
-    ]);
+
+    expect(Auth.resourcesClient).toHaveBeenCalledWith("foo");
+    expect(mockClientCheckNamespaceExists).toHaveBeenCalledWith({
+      context: {
+        cluster: "othercluster",
+        namespace: "default",
+      },
+    });
+  });
+
+  it("should return without error when the endpoint returns PermissionDenied with the given token", async () => {
+    mockClientCheckNamespaceExists = jest
+      .fn()
+      .mockImplementation(() => Promise.reject({ code: grpc.Code.PermissionDenied }));
+    jest.spyOn(client, "CheckNamespaceExists").mockImplementation(mockClientCheckNamespaceExists);
+    await Auth.validateToken("othercluster", "foo");
+
+    expect(Auth.resourcesClient).toHaveBeenCalledWith("foo");
+    expect(mockClientCheckNamespaceExists).toHaveBeenCalledWith({
+      context: {
+        cluster: "othercluster",
+        namespace: "default",
+      },
+    });
   });
 
   describe("when there is an error", () => {
     [
       {
         name: "should throw an invalid token error for 401 responses",
         response: { status: 401, data: "ignored anyway" },
+        grpcCode: grpc.Code.Unauthenticated,
         expectedError: new Error("invalid token"),
       },
       {
         name: "should throw a standard error for a 404 response",
-        response: { status: 404, data: "Not found" },
-        expectedError: new Error("404: Not found"),
+        grpcCode: grpc.Code.NotFound,
+        expectedError: new Error("not found"),
       },
       {
         name: "should throw a standard error for a 500 response",
-        response: { status: 500, data: "Server exception" },
-        expectedError: new Error("500: Server exception"),
-      },
-      {
-        name: "should succeed for a 403 response",
-        response: { status: 403, data: "Not Allowed" },
-        expectedError: null,
+        grpcCode: grpc.Code.Internal,
+        expectedError: new Error("internal error"),
       },
     ].forEach(testCase => {
       it(testCase.name, async () => {
-        const mock = jest.fn(() => {
-          return Promise.reject({ response: testCase.response });
-        });
-        Axios.get = mock;
-        // TODO(absoludity): tried using `expect(fn()).rejects.toThrow()` but it seems we need
-        // to upgrade jest for `toThrow()` to work with async.
-        let err = null;
-        try {
-          await Auth.validateToken("default", "foo");
-        } catch (e: any) {
-          err = e;
-        } finally {
-          expect(err).toEqual(testCase.expectedError);
-        }
+        mockClientCheckNamespaceExists = jest
+          .fn()
+          .mockImplementation(() => Promise.reject({ code: testCase.grpcCode }));
+        jest
+          .spyOn(client, "CheckNamespaceExists")
+          .mockImplementation(mockClientCheckNamespaceExists);
+
+        await expect(Auth.validateToken("default", "foo")).rejects.toThrow(testCase.expectedError);
       });
     });
   });
diff --git a/dashboard/src/shared/Auth.ts b/dashboard/src/shared/Auth.ts
@@ -3,10 +3,15 @@ import * as jwt from "jsonwebtoken";
 import { get } from "lodash";
 import * as url from "shared/url";
 import { IConfig } from "./Config";
+import { KubeappsGrpcClient } from "./KubeappsGrpcClient";
+import { grpc } from "@improbable-eng/grpc-web";
 const AuthTokenKey = "kubeapps_auth_token";
 const AuthTokenOIDCKey = "kubeapps_auth_token_oidc";
 
 export class Auth {
+  public static resourcesClient = (token?: string) =>
+    new KubeappsGrpcClient().getResourcesServiceClientImpl(token);
+
   public static getAuthToken() {
     return localStorage.getItem(AuthTokenKey);
   }
@@ -60,21 +65,27 @@ export class Auth {
   // Throws an error if the token is invalid
   public static async validateToken(cluster: string, token: string) {
     try {
-      await Axios.get(url.api.k8s.base(cluster) + "/", {
-        headers: { Authorization: `Bearer ${token}` },
+      await this.resourcesClient(token).CheckNamespaceExists({
+        context: { cluster, namespace: "default" },
       });
     } catch (e: any) {
-      const res = e.response as AxiosResponse<any>;
-      if (res.status === 401) {
+      if (e.code === grpc.Code.Unauthenticated) {
         throw new Error("invalid token");
       }
-      // A 403 authorization error only occurs if the token resulted in
-      // successful authentication. We don't make any assumptions over RBAC
-      // for the root "/" nonResourceURL or other required authz permissions
-      // until operations on those resources are attempted (though we may
-      // want to revisit this in the future).
-      if (res.status !== 403) {
-        throw new Error(`${res.status}: ${res.data}`);
+      // https://kubernetes.io/docs/reference/access-authn-authz/authentication/#anonymous-requests
+      // Since we are always passing a token here, A 403 authorization error
+      // only occurs if the token resulted in successful authentication. We
+      // don't make any assumptions over RBAC for the requested namespace or
+      // other required authz permissions until operations on those resources
+      // are attempted (though we may want to revisit this in the future).
+      if (e.code !== grpc.Code.PermissionDenied) {
+        if (e.code === grpc.Code.NotFound) {
+          throw new Error("not found");
+        }
+        if (e.code === grpc.Code.Internal) {
+          throw new Error("internal error");
+        }
+        throw new Error(`${e.code}: ${e.message}`);
       }
     }
   }
@@ -117,6 +128,7 @@ export class Auth {
       // The only error response which can possibly mean we did authenticate is
       // a 403 from the k8s api server (ie. we got through to k8s api server
       // but RBAC doesn't authorize us).
+      // See note below about anonymous requests.
       if (response.status !== 403) {
         return false;
       }
diff --git a/dashboard/src/shared/KubeappsGrpcClient.test.ts b/dashboard/src/shared/KubeappsGrpcClient.test.ts
@@ -89,3 +89,45 @@ describe("kubeapps grpc core plugin service", () => {
   // TODO(agamez): try to also mock the messages ussing the new FakeTransportBuilder().withMessages([])
   // More details: https://github.com/kubeapps/kubeapps/issues/3165#issuecomment-882944035
 });
+
+describe("kubeapps grpc resources plugin service", () => {
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
+  const fakeAuthTransport = new FakeTransportBuilder()
+    .withHeaders(new grpc.Metadata({ authorization: "Bearer topsecret" }))
+    .build();
+
+  it("it set the metadata if using token auth", async () => {
+    const kubeappsGrpcClient = new KubeappsGrpcClient(fakeAuthTransport);
+    jest.spyOn(window.localStorage.__proto__, "getItem").mockReturnValue("topsecret");
+
+    const getClientMetadataMock = jest.spyOn(KubeappsGrpcClient.prototype, "getClientMetadata");
+    kubeappsGrpcClient.getResourcesServiceClientImpl();
+
+    const expectedMetadata = new grpc.Metadata({ authorization: "Bearer topsecret" });
+    expect(getClientMetadataMock.mock.results[0].value).toEqual(expectedMetadata);
+  });
+
+  it("it doesn't set the metadata if not using token auth", async () => {
+    const kubeappsGrpcClient = new KubeappsGrpcClient(fakeAuthTransport);
+    jest.spyOn(window.localStorage.__proto__, "getItem").mockReturnValue(null);
+    const getClientMetadataMock = jest.spyOn(KubeappsGrpcClient.prototype, "getClientMetadata");
+
+    kubeappsGrpcClient.getResourcesServiceClientImpl();
+
+    expect(getClientMetadataMock.mock.results[0].value).toBeUndefined();
+  });
+
+  it("it sets the metadata if passed an explicit token", async () => {
+    const kubeappsGrpcClient = new KubeappsGrpcClient(fakeAuthTransport);
+    jest.spyOn(window.localStorage.__proto__, "getItem").mockReturnValue(null);
+    const getClientMetadataMock = jest.spyOn(KubeappsGrpcClient.prototype, "getClientMetadata");
+
+    kubeappsGrpcClient.getResourcesServiceClientImpl("topsecret");
+
+    const expectedMetadata = new grpc.Metadata({ authorization: "Bearer topsecret" });
+    expect(getClientMetadataMock.mock.results[0].value).toEqual(expectedMetadata);
+  });
+});
diff --git a/dashboard/src/shared/KubeappsGrpcClient.ts b/dashboard/src/shared/KubeappsGrpcClient.ts
@@ -12,7 +12,6 @@ import { Auth } from "./Auth";
 import * as URL from "./url";
 
 export class KubeappsGrpcClient {
-  private grpcWebImpl!: GrpcWebImpl;
   private transport: grpc.TransportFactory;
 
   constructor(transport?: grpc.TransportFactory) {
@@ -21,22 +20,22 @@ export class KubeappsGrpcClient {
 
   // getClientMetadata, if using token authentication, creates grpc metadata
   // and the token in the 'authorization' field
-  public getClientMetadata() {
+  public getClientMetadata(token?: string) {
+    if (token) {
+      return new grpc.Metadata({ authorization: `Bearer ${token}` });
+    }
+
     return Auth.getAuthToken()
       ? new grpc.Metadata({ authorization: `Bearer ${Auth.getAuthToken()}` })
       : undefined;
   }
 
-  // getGrpcClient returns the already configured grpcWebImpl
-  // if uncreated, it is created and returned
-  public getGrpcClient = () => {
-    return (
-      this.grpcWebImpl ||
-      new GrpcWebImpl(URL.api.kubeappsapis, {
-        transport: this.transport,
-        metadata: this.getClientMetadata(),
-      })
-    );
+  // getGrpcClient returns a grpc client
+  public getGrpcClient = (token?: string) => {
+    return new GrpcWebImpl(URL.api.kubeappsapis, {
+      transport: this.transport,
+      metadata: this.getClientMetadata(token),
+    });
   };
 
   // Core APIs
@@ -49,8 +48,12 @@ export class KubeappsGrpcClient {
   }
 
   // Resources API
-  public getResourcesServiceClientImpl() {
-    return new ResourcesServiceClientImpl(this.getGrpcClient());
+  //
+  // The resources API client implementation takes an optional token
+  // only because it is used to validate token authentication before
+  // the token is stored.
+  public getResourcesServiceClientImpl(token?: string) {
+    return new ResourcesServiceClientImpl(this.getGrpcClient(token));
   }
 
   // Plugins (packages) APIs
@@ -68,4 +71,4 @@ export class KubeappsGrpcClient {
   }
 }
 
-export default new KubeappsGrpcClient();
+export default KubeappsGrpcClient;
