Handle grpc authentication errors (#5297)

* Handle auth error in grpc calls Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Rename resource clients consistently Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Aply suggested refactor Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Remove leftover Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Fix test Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Add missing mocked "catch" Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Rename grpc clients in tests Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Add full mapping of grpc codes to app errors Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Return PermissionDenied instead of unauthenticated Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Replace deprecated button in error boundary Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Fix filter by plugin Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Use the error obj instead of the grpc code Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Use new error types Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Fix test case Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Fix buf lint error Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> * Fix lint error Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
vmware-tanzu · Sep 7, 2022 · a9b90a8 · a9b90a8
1 parent cfeac1c
commit a9b90a8
Show file tree

Hide file tree

Showing 37 changed files with 477 additions and 248 deletions.
diff --git a/cmd/kubeapps-apis/buf.lock b/cmd/kubeapps-apis/buf.lock
@@ -4,8 +4,8 @@ deps:
   - remote: buf.build
     owner: googleapis
     repository: googleapis
-    commit: 80720a488c9a414bb8d4a9f811084989
+    commit: 62f35d8aed1149c291d606d958a7ce32
   - remote: buf.build
     owner: grpc-ecosystem
     repository: grpc-gateway
-    commit: 00116f302b12478b85deb33b734e026c
+    commit: bc28b723cd774c32b6fbc77621518765
diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/harbor_api_v2_repo_lister.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/harbor_api_v2_repo_lister.go
@@ -7,7 +7,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"net/http"
 	"net/url"
 	"strings"
@@ -118,7 +117,7 @@ func pingHarbor(ctx context.Context, ref orasregistryv2.Reference, cred orasregi
 	switch resp.StatusCode {
 	case http.StatusOK:
 		lr := io.LimitReader(resp.Body, 100)
-		if pong, err := ioutil.ReadAll(lr); err != nil {
+		if pong, err := io.ReadAll(lr); err != nil {
 			return "", err
 		} else if string(pong) != "Pong" {
 			return "", fmt.Errorf("unexpected response body: %s", string(pong))

diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/server.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/server.go
@@ -441,7 +441,7 @@ func (s *Server) hasAccessToNamespace(ctx context.Context, cluster, namespace st
 	}
 	if !res.Status.Allowed {
 		// If the user has not access, return a unauthenticated response, otherwise, continue
-		return status.Errorf(codes.Unauthenticated, "The current user has no access to the namespace %q", namespace)
+		return status.Errorf(codes.PermissionDenied, "The current user has no access to the namespace %q", namespace)
 	}
 	return nil
 }
@@ -690,7 +690,7 @@ func (s *Server) CreateInstalledPackage(ctx context.Context, request *corev1.Cre
 	}
 	ch, registrySecrets, err := s.fetchChartWithRegistrySecrets(ctx, chartDetails, typedClient)
 	if err != nil {
-		return nil, status.Errorf(codes.Unauthenticated, "Missing permissions %v", err)
+		return nil, status.Errorf(codes.PermissionDenied, "Missing permissions %v", err)
 	}
 
 	// Create an action config for the target namespace.
@@ -761,7 +761,7 @@ func (s *Server) UpdateInstalledPackage(ctx context.Context, request *corev1.Upd
 	}
 	ch, registrySecrets, err := s.fetchChartWithRegistrySecrets(ctx, chartDetails, typedClient)
 	if err != nil {
-		return nil, status.Errorf(codes.Unauthenticated, "Missing permissions %v", err)
+		return nil, status.Errorf(codes.PermissionDenied, "Missing permissions %v", err)
 	}
 
 	// Create an action config for the installed pkg context.

diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/server_test.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/server_test.go
@@ -554,15 +554,15 @@ func TestGetAvailablePackageSummaries(t *testing.T) {
 			statusCode:             codes.Internal,
 		},
 		{
-			name:       "it returns an unauthenticated status if the user doesn't have permissions",
+			name:       "it returns a permissionDenied status if the user doesn't have permissions",
 			authorized: false,
 			request: &corev1.GetAvailablePackageSummariesRequest{
 				Context: &corev1.Context{
 					Namespace: "my-ns",
 				},
 			},
 			charts:     []*models.Chart{{Name: "foo"}},
-			statusCode: codes.Unauthenticated,
+			statusCode: codes.PermissionDenied,
 		},
 		{
 			name:       "it returns only the requested page of results and includes the next page token",
@@ -1005,7 +1005,7 @@ func TestGetAvailablePackageDetail(t *testing.T) {
 			statusCode:      codes.Internal,
 		},
 		{
-			name:       "it returns an unauthenticated status if the user doesn't have permissions",
+			name:       "it returns a permissionDenied status if the user doesn't have permissions",
 			authorized: false,
 			request: &corev1.GetAvailablePackageDetailRequest{
 				AvailablePackageRef: &corev1.AvailablePackageReference{
@@ -1015,7 +1015,7 @@ func TestGetAvailablePackageDetail(t *testing.T) {
 			},
 			charts:          []*models.Chart{{Name: "foo"}},
 			expectedPackage: &corev1.AvailablePackageDetail{},
-			statusCode:      codes.Unauthenticated,
+			statusCode:      codes.PermissionDenied,
 		},
 	}
 

diff --git a/dashboard/src/actions/auth.test.tsx b/dashboard/src/actions/auth.test.tsx
@@ -30,9 +30,7 @@ beforeEach(() => {
   Auth.isAuthenticatedWithCookie = jest.fn().mockReturnValue("token");
   Auth.setAuthToken = jest.fn();
   Auth.unsetAuthToken = jest.fn();
-  Namespace.list = jest.fn(async () => {
-    return { namespaceNames: [] };
-  });
+  Namespace.list = jest.fn(async () => []);
   jest.spyOn(NS, "unsetStoredNamespace");
 
   store = mockStore({

diff --git a/dashboard/src/actions/auth.ts b/dashboard/src/actions/auth.ts
@@ -4,7 +4,7 @@
 import { ThunkAction } from "redux-thunk";
 import { Auth } from "shared/Auth";
 import * as Namespace from "shared/Namespace";
-import { IStoreState } from "shared/types";
+import { IStoreState, UnauthorizedNetworkError } from "shared/types";
 import { ActionType, deprecated } from "typesafe-actions";
 import { clearClusters, NamespaceAction } from "./namespace";
 
@@ -71,6 +71,27 @@ export function logout(): ThunkAction<
   };
 }
 
+export function logoutByAuthenticationError(): ThunkAction<
+  Promise<void>,
+  IStoreState,
+  null,
+  AuthAction | NamespaceAction
+> {
+  return async dispatch => {
+    dispatch(logout());
+    dispatch(authenticationError("Unauthorized"));
+    dispatch(expireSession());
+  };
+}
+
+export function handleErrorAction(error: any, action?: ActionType<any>) {
+  if (error.constructor === UnauthorizedNetworkError) {
+    return logoutByAuthenticationError();
+  } else if (action) {
+    return action;
+  }
+}
+
 export function expireSession(): ThunkAction<Promise<void>, IStoreState, null, AuthAction> {
   return async dispatch => {
     if (Auth.usingOIDCToken()) {

diff --git a/dashboard/src/actions/availablepackages.ts b/dashboard/src/actions/availablepackages.ts
@@ -14,6 +14,7 @@ import {
   IReceivePackagesActionPayload as IReceiveAvailablePackageSummariesActionPayload,
   IStoreState,
 } from "../shared/types";
+import { handleErrorAction } from "./auth";
 
 const { createAction } = deprecated;
 
@@ -126,7 +127,7 @@ export function fetchAvailablePackageSummaries(
       );
       dispatch(receiveAvailablePackageSummaries({ response, paginationToken }));
     } catch (e: any) {
-      dispatch(createErrorPackage(new FetchError(e.message)));
+      dispatch(handleErrorAction(e, createErrorPackage(new FetchError(e.message))));
     }
   };
 }
@@ -140,7 +141,7 @@ export function fetchAvailablePackageVersions(
       const response = await PackagesService.getAvailablePackageVersions(availablePackageReference);
       dispatch(receiveSelectedAvailablePackageVersions(response));
     } catch (e: any) {
-      dispatch(createErrorPackage(new FetchError(e.message)));
+      dispatch(handleErrorAction(e, createErrorPackage(new FetchError(e.message))));
     }
   };
 }
@@ -162,7 +163,7 @@ export function fetchAndSelectAvailablePackageDetail(
         dispatch(createErrorPackage(new FetchError("could not find package version")));
       }
     } catch (e: any) {
-      dispatch(createErrorPackage(new FetchError(e.message)));
+      dispatch(handleErrorAction(e, createErrorPackage(new FetchError(e.message))));
     }
   };
 }
diff --git a/dashboard/src/actions/installedpackages.test.tsx b/dashboard/src/actions/installedpackages.test.tsx
@@ -15,7 +15,7 @@ import {
 import { Plugin } from "gen/kubeappsapis/core/plugins/v1alpha1/plugins";
 import { InstalledPackage } from "shared/InstalledPackage";
 import { getStore, initialState } from "shared/specs/mountWrapper";
-import { IStoreState, PluginNames, UnprocessableEntity, UpgradeError } from "shared/types";
+import { IStoreState, PluginNames, UnprocessableEntityError, UpgradeError } from "shared/types";
 import { getType } from "typesafe-actions";
 import actions from ".";
 
@@ -234,7 +234,7 @@ describe("deploy package", () => {
       { type: getType(actions.installedpackages.requestInstallPackage) },
       {
         type: getType(actions.installedpackages.errorInstalledPackage),
-        payload: new UnprocessableEntity(
+        payload: new UnprocessableEntityError(
           "The given values don't match the required format. The following errors were found:\n  - /foo: must be string",
         ),
       },
@@ -313,7 +313,7 @@ describe("updateInstalledPackage", () => {
       { type: getType(actions.installedpackages.requestUpdateInstalledPackage) },
       {
         type: getType(actions.installedpackages.errorInstalledPackage),
-        payload: new UnprocessableEntity(
+        payload: new UnprocessableEntityError(
           "The given values don't match the required format. The following errors were found:\n  - /foo: must be string",
         ),
       },

diff --git a/dashboard/src/actions/installedpackages.ts b/dashboard/src/actions/installedpackages.ts
@@ -21,13 +21,14 @@ import {
   FetchWarning,
   IStoreState,
   RollbackError,
-  UnprocessableEntity,
+  UnprocessableEntityError,
   UpgradeError,
 } from "shared/types";
 import { getPluginsSupportingRollback } from "shared/utils";
 import { ActionType, deprecated } from "typesafe-actions";
 import { InstalledPackage } from "../shared/InstalledPackage";
 import { validate } from "../shared/schema";
+import { handleErrorAction } from "./auth";
 
 const { createAction } = deprecated;
 
@@ -129,16 +130,24 @@ export function getInstalledPackage(
         availablePackageDetail = resp.availablePackageDetail;
       } catch (e: any) {
         dispatch(
-          errorInstalledPackage(
-            new FetchWarning(
-              "this package has missing information, some actions might not be available.",
+          handleErrorAction(
+            e,
+            errorInstalledPackage(
+              new FetchWarning(
+                "this package has missing information, some actions might not be available.",
+              ),
             ),
           ),
         );
       }
       dispatch(selectInstalledPackage(installedPackageDetail!, availablePackageDetail));
     } catch (e: any) {
-      dispatch(errorInstalledPackage(new FetchError("Unable to get installed package", [e])));
+      dispatch(
+        handleErrorAction(
+          e,
+          errorInstalledPackage(new FetchError("Unable to get installed package", [e])),
+        ),
+      );
     }
   };
 }
@@ -162,7 +171,12 @@ export function getInstalledPkgStatus(
         );
         dispatch(receiveInstalledPackageStatus(installedPackageDetail!.status!));
       } catch (e: any) {
-        dispatch(errorInstalledPackage(new FetchError("Unable to refresh installed package", [e])));
+        dispatch(
+          handleErrorAction(
+            e,
+            errorInstalledPackage(new FetchError("Unable to refresh installed package", [e])),
+          ),
+        );
       }
     }
   };
@@ -178,7 +192,7 @@ export function deleteInstalledPackage(
       dispatch(receiveDeleteInstalledPackage());
       return true;
     } catch (e: any) {
-      dispatch(errorInstalledPackage(new DeleteError(e.message)));
+      dispatch(handleErrorAction(e, errorInstalledPackage(new DeleteError(e.message))));
       return false;
     }
   };
@@ -199,7 +213,9 @@ export function fetchInstalledPackages(
       dispatch(receiveInstalledPackageList(installedPackageSummaries));
       return installedPackageSummaries;
     } catch (e: any) {
-      dispatch(errorInstalledPackage(new FetchError("Unable to list apps", [e])));
+      dispatch(
+        handleErrorAction(e, errorInstalledPackage(new FetchError("Unable to list apps", [e]))),
+      );
       return [];
     }
   };
@@ -223,7 +239,7 @@ export function installPackage(
           const errorText =
             validation.errors &&
             validation.errors.map(e => `  - ${e.instancePath}: ${e.message}`).join("\n");
-          throw new UnprocessableEntity(
+          throw new UnprocessableEntityError(
             `The given values don't match the required format. The following errors were found:\n${errorText}`,
           );
         }
@@ -251,7 +267,7 @@ export function installPackage(
         return false;
       }
     } catch (e: any) {
-      dispatch(errorInstalledPackage(new CreateError(e.message)));
+      dispatch(handleErrorAction(e, errorInstalledPackage(new CreateError(e.message))));
       return false;
     }
   };
@@ -272,7 +288,7 @@ export function updateInstalledPackage(
           const errorText =
             validation.errors &&
             validation.errors.map(e => `  - ${e.instancePath}: ${e.message}`).join("\n");
-          throw new UnprocessableEntity(
+          throw new UnprocessableEntityError(
             `The given values don't match the required format. The following errors were found:\n${errorText}`,
           );
         }
@@ -294,7 +310,7 @@ export function updateInstalledPackage(
         return false;
       }
     } catch (e: any) {
-      dispatch(errorInstalledPackage(new UpgradeError(e.message)));
+      dispatch(handleErrorAction(e, errorInstalledPackage(new UpgradeError(e.message))));
       return false;
     }
   };
@@ -317,7 +333,7 @@ export function rollbackInstalledPackage(
         dispatch(getInstalledPackage(installedPackageRef));
         return true;
       } catch (e: any) {
-        dispatch(errorInstalledPackage(new RollbackError(e.message)));
+        dispatch(handleErrorAction(e, errorInstalledPackage(new RollbackError(e.message))));
         return false;
       }
     } else {

diff --git a/dashboard/src/actions/namespace.test.tsx b/dashboard/src/actions/namespace.test.tsx
@@ -71,9 +71,7 @@ actionTestCases.forEach(tc => {
 describe("fetchNamespaces", () => {
   it("dispatches the list of namespace names if no error", async () => {
     Namespace.list = jest.fn().mockImplementationOnce(() => {
-      return {
-        namespaceNames: ["overlook-hotel", "room-217"],
-      };
+      return ["overlook-hotel", "room-217"];
     });
     const expectedActions = [
       {
@@ -88,7 +86,7 @@ describe("fetchNamespaces", () => {
 
   it("dispatches errorNamespace if the request returns no 'namespaces'", async () => {
     Namespace.list = jest.fn().mockImplementationOnce(() => {
-      return {};
+      return [];
     });
     const err = new Error("The current account does not have access to any namespaces");
     const expectedActions = [
@@ -122,9 +120,7 @@ describe("createNamespace", () => {
   it("dispatches the new namespace and re-fetch namespaces", async () => {
     Namespace.create = jest.fn();
     Namespace.list = jest.fn().mockImplementationOnce(() => {
-      return {
-        namespaceNames: ["overlook-hotel", "room-217"],
-      };
+      return ["overlook-hotel", "room-217"];
     });
     const expectedActions = [
       {