Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle grpc authentication errors #5297

Merged
merged 20 commits into from
Sep 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/kubeapps-apis/buf.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"encoding/json"
"fmt"
"io"
"io/ioutil"
"net/http"
"net/url"
"strings"
Expand Down Expand Up @@ -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))
Expand Down
6 changes: 3 additions & 3 deletions cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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{
Expand All @@ -1015,7 +1015,7 @@ func TestGetAvailablePackageDetail(t *testing.T) {
},
charts: []*models.Chart{{Name: "foo"}},
expectedPackage: &corev1.AvailablePackageDetail{},
statusCode: codes.Unauthenticated,
statusCode: codes.PermissionDenied,
},
}

Expand Down
4 changes: 1 addition & 3 deletions dashboard/src/actions/auth.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
23 changes: 22 additions & 1 deletion dashboard/src/actions/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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()) {
Expand Down
7 changes: 4 additions & 3 deletions dashboard/src/actions/availablepackages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
IReceivePackagesActionPayload as IReceiveAvailablePackageSummariesActionPayload,
IStoreState,
} from "../shared/types";
import { handleErrorAction } from "./auth";

const { createAction } = deprecated;

Expand Down Expand Up @@ -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))));
}
};
}
Expand All @@ -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))));
}
};
}
Expand All @@ -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))));
}
};
}
6 changes: 3 additions & 3 deletions dashboard/src/actions/installedpackages.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 ".";

Expand Down Expand Up @@ -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",
),
},
Expand Down Expand Up @@ -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",
),
},
Expand Down
42 changes: 29 additions & 13 deletions dashboard/src/actions/installedpackages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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])),
),
);
}
};
}
Expand All @@ -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])),
),
);
}
}
};
Expand All @@ -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;
}
};
Expand All @@ -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 [];
}
};
Expand All @@ -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}`,
);
}
Expand Down Expand Up @@ -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;
}
};
Expand All @@ -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}`,
);
}
Expand All @@ -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;
}
};
Expand All @@ -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 {
Expand Down
10 changes: 3 additions & 7 deletions dashboard/src/actions/namespace.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
{
Expand All @@ -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 = [
Expand Down Expand Up @@ -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 = [
{
Expand Down