Merge pull request #7145 from JPinkney/secured-namespaces

[ODC-4388] Allow cluster admins to create terminals

frontend/packages/console-app/src/components/cloud-shell/CloudShellTerminal.tsx

@@ -8,11 +8,13 @@ import { withUserSettingsCompatibility, WithUserSettingsCompatibilityProps } fro
 import CloudshellExec from './CloudShellExec';
 import TerminalLoadingBox from './TerminalLoadingBox';
 import { TerminalInitData, initTerminal } from './cloud-shell-utils';
-import CloudShellSetup from './setup/CloudShellSetup';
 import useCloudShellWorkspace from './useCloudShellWorkspace';
 import { CLOUD_SHELL_NAMESPACE, CLOUD_SHELL_NAMESPACE_CONFIG_STORAGE_KEY } from './const';
 
 import './CloudShellTerminal.scss';
+import CloudShellAdminSetup from './setup/CloudShellAdminSetup';
+import CloudShellDeveloperSetup from './setup/CloudShellDeveloperSetup';
+import { useAccessReview2 } from '@console/internal/components/utils/rbac';
 
 type StateProps = {
   user: UserKind;
@@ -33,8 +35,12 @@ const CloudShellTerminal: React.FC<CloudShellTerminalProps &
 }) => {
   const [initData, setInitData] = React.useState<TerminalInitData>();
   const [initError, setInitError] = React.useState<string>();
-
-  const [workspace, loaded, loadError] = useCloudShellWorkspace(user, namespace);
+  const [isAdmin, isAdminCheckLoading] = useAccessReview2({
+    namespace: 'openshift-terminal',
+    verb: 'create',
+    resource: 'pods',
+  });
+  const [workspace, loaded, loadError] = useCloudShellWorkspace(user, isAdmin, namespace);
 
   const workspacePhase = workspace?.status?.phase;
   const workspaceName = workspace?.metadata?.name;
@@ -113,7 +119,7 @@ const CloudShellTerminal: React.FC<CloudShellTerminalProps &
   }
 
   // loading the workspace resource
-  if (!loaded) {
+  if (!loaded || isAdminCheckLoading) {
     return <TerminalLoadingBox message="" />;
   }
 
@@ -138,9 +144,19 @@ const CloudShellTerminal: React.FC<CloudShellTerminalProps &
     );
   }
 
+  if (isAdmin) {
+    return (
+      <CloudShellAdminSetup
+        onInitialize={(ns: string) => {
+          setNamespace(ns);
+        }}
+      />
+    );
+  }
+
   // show the form to let the user create a new workspace
   return (
-    <CloudShellSetup
+    <CloudShellDeveloperSetup
       onCancel={onCancel}
       onSubmit={(ns: string) => {
         setNamespace(ns);

frontend/packages/console-app/src/components/cloud-shell/__tests__/CloudShellTerminal.spec.tsx

@@ -3,9 +3,9 @@ import { shallow } from 'enzyme';
 import { StatusBox } from '@console/internal/components/utils/status-box';
 import { InternalCloudShellTerminal } from '../CloudShellTerminal';
 import TerminalLoadingBox from '../TerminalLoadingBox';
-import CloudShellSetup from '../setup/CloudShellSetup';
 import { user } from './cloud-shell-test-data';
 import useCloudShellWorkspace from '../useCloudShellWorkspace';
+import CloudShellDeveloperSetup from '../setup/CloudShellDeveloperSetup';
 
 jest.mock('../useCloudShellWorkspace', () => ({
   default: jest.fn(),
@@ -19,6 +19,10 @@ jest.mock('react-i18next', () => {
   };
 });
 
+jest.mock('@console/internal/components/utils/rbac', () => ({
+  useAccessReview2: () => [false, false],
+}));
+
 jest.mock('@console/shared/src/hooks/useUserSettingsCompatibility', () => {
   return {
     useUserSettingsCompatibility: () => ['', () => {}],
@@ -59,6 +63,6 @@ describe('CloudShellTerminal', () => {
         setUserSettingState={jest.fn()}
       />,
     );
-    expect(wrapper.find(CloudShellSetup)).toHaveLength(1);
+    expect(wrapper.find(CloudShellDeveloperSetup)).toHaveLength(1);
   });
 });

frontend/packages/console-app/src/components/cloud-shell/cloud-shell-utils.ts

@@ -32,6 +32,7 @@ export const CLOUD_SHELL_LABEL = 'console.openshift.io/terminal';
 export const CLOUD_SHELL_CREATOR_LABEL = 'controller.devfile.io/creator';
 export const CLOUD_SHELL_RESTRICTED_ANNOTATION = 'controller.devfile.io/restricted-access';
 export const CLOUD_SHELL_STOPPED_BY_ANNOTATION = 'controller.devfile.io/stopped-by';
+export const CLOUD_SHELL_PROTECTED_NAMESPACE = 'openshift-terminal';
 
 export const createCloudShellResourceName = () => `terminal-${getRandomChars(6)}`;
 

frontend/packages/console-app/src/components/cloud-shell/setup/CloudShellAdminSetup.tsx

@@ -0,0 +1,74 @@
+import * as React from 'react';
+import { connect } from 'react-redux';
+
+import { k8sCreate, k8sGet } from '@console/internal/module/k8s';
+import { WorkspaceModel } from '../../../models';
+import {
+  newCloudShellWorkSpace,
+  createCloudShellResourceName,
+  CLOUD_SHELL_PROTECTED_NAMESPACE,
+} from '../cloud-shell-utils';
+import { NamespaceModel } from '@console/internal/models';
+import TerminalLoadingBox from '../TerminalLoadingBox';
+import { LoadError } from '@console/internal/components/utils/status-box';
+import { useTranslation } from 'react-i18next';
+
+type Props = {
+  onInitialize: (namespace: string) => void;
+};
+
+const CloudShellAdminSetup: React.FunctionComponent<Props> = ({ onInitialize }) => {
+  const { t } = useTranslation();
+
+  const [initError, setInitError] = React.useState<string>();
+  React.useEffect(() => {
+    (async () => {
+      async function namespaceExists(): Promise<boolean> {
+        try {
+          await k8sGet(NamespaceModel, CLOUD_SHELL_PROTECTED_NAMESPACE);
+          return true;
+        } catch (error) {
+          if (error.json.code !== 404) {
+            setInitError(error);
+          }
+          return false;
+        }
+      }
+
+      try {
+        const protectedNamespaceExists = await namespaceExists();
+        if (!protectedNamespaceExists) {
+          await k8sCreate(NamespaceModel, {
+            metadata: {
+              name: CLOUD_SHELL_PROTECTED_NAMESPACE,
+            },
+          });
+        }
+        await k8sCreate(
+          WorkspaceModel,
+          newCloudShellWorkSpace(createCloudShellResourceName(), CLOUD_SHELL_PROTECTED_NAMESPACE),
+        );
+        onInitialize(CLOUD_SHELL_PROTECTED_NAMESPACE);
+      } catch (error) {
+        setInitError(error);
+      }
+    })();
+    // Don't include dependencies because if the CLOUD_SHELL_PROTECTED_NAMESPACE
+    // is not found a refresh will be triggered, creating an extra terminal
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, []);
+
+  if (initError) {
+    return (
+      <LoadError message={initError} label={t('cloudshell~OpenShift command line terminal')} />
+    );
+  }
+
+  return (
+    <div className="co-cloudshell-terminal__container">
+      <TerminalLoadingBox />
+    </div>
+  );
+};
+
+export default connect()(CloudShellAdminSetup);

frontend/packages/console-app/src/components/cloud-shell/setup/CloudShellSetup.tsx → frontend/packages/console-app/src/components/cloud-shell/setup/CloudShellDeveloperSetup.tsx

@@ -26,7 +26,7 @@ type Props = StateProps & {
   onCancel?: () => void;
 };
 
-const CloudShellSetup: React.FunctionComponent<Props> = ({
+const CloudShellDeveloperSetup: React.FunctionComponent<Props> = ({
   activeNamespace,
   onSubmit,
   onCancel,
@@ -80,4 +80,4 @@ const mapStateToProps = (state: RootState): StateProps => ({
   activeNamespace: state.UI.get('activeNamespace'),
 });
 
-export default connect<StateProps>(mapStateToProps)(CloudShellSetup);
+export default connect<StateProps>(mapStateToProps)(CloudShellDeveloperSetup);

frontend/packages/console-app/src/components/cloud-shell/useCloudShellWorkspace.ts

@@ -11,6 +11,7 @@ import {
   CloudShellResource,
   CLOUD_SHELL_RESTRICTED_ANNOTATION,
   startWorkspace,
+  CLOUD_SHELL_PROTECTED_NAMESPACE,
 } from './cloud-shell-utils';
 import { useAccessReview2 } from '@console/internal/components/utils';
 import { ProjectModel } from '@console/internal/models';
@@ -30,6 +31,7 @@ const findWorkspace = (data?: CloudShellResource[]): CloudShellResource | undefi
 
 const useCloudShellWorkspace = (
   user: UserKind,
+  isClusterAdmin: boolean,
   defaultNamespace: string = null,
 ): WatchK8sResult<CloudShellResource> => {
   const [namespace, setNamespace] = useSafetyFirst(defaultNamespace);
@@ -67,11 +69,14 @@ const useCloudShellWorkspace = (
       },
     };
 
-    if (!canListWorkspaces) {
+    if (isClusterAdmin) {
+      result.namespace = CLOUD_SHELL_PROTECTED_NAMESPACE;
+    } else if (!canListWorkspaces) {
       result.namespace = namespace;
     }
+
     return result;
-  }, [isKubeAdmin, uid, namespace, loadingAccessReview, canListWorkspaces]);
+  }, [loadingAccessReview, canListWorkspaces, namespace, isKubeAdmin, uid, isClusterAdmin]);
 
   // call k8s api to fetch workspace
   const [data, loaded, loadError] = useK8sWatchResource<CloudShellResource[]>(resource);

pkg/terminal/auth.go

@@ -7,10 +7,10 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-// checkUserPermissions checks if the terminal proxy is supported for a given user.
-// Returns true if we're willing to proxy the user's token, false otherwise. We don't
-// want to proxy highly privileged tokens to avoid privilege escalation issues.
-func (p *Proxy) checkUserPermissions(token string) (bool, error) {
+// isClusterAdmin does a subject access review to see if the user can create pods in openshift-terminal
+// if they can then they are considered a cluster admin
+// if they cannot they are not a cluster admin
+func (p *Proxy) isClusterAdmin(token string) (bool, error) {
 	client, err := p.createTypedClient(token)
 	if err != nil {
 		return false, err
@@ -19,7 +19,7 @@ func (p *Proxy) checkUserPermissions(token string) (bool, error) {
 	sar := &authv1.SelfSubjectAccessReview{
 		Spec: authv1.SelfSubjectAccessReviewSpec{
 			ResourceAttributes: &authv1.ResourceAttributes{
-				Namespace: "openshift-operators",
+				Namespace: "openshift-terminal",
 				Verb:      "create",
 				Resource:  "pods",
 			},
@@ -29,5 +29,5 @@ func (p *Proxy) checkUserPermissions(token string) (bool, error) {
 	if err != nil || res == nil {
 		return false, err
 	}
-	return !res.Status.Allowed, nil
+	return res.Status.Allowed, nil
 }

pkg/terminal/proxy.go

@@ -89,19 +89,20 @@ func (p *Proxy) HandleProxy(user *auth.User, w http.ResponseWriter, r *http.Requ
 		return
 	}
 
-	enabledForUser, err := p.checkUserPermissions(user.Token)
-	if err != nil {
-		http.Error(w, "Failed to check workspace operator state. Cause: "+err.Error(), http.StatusInternalServerError)
+	ok, namespace, workspaceName, path := stripTerminalAPIPrefix(r.URL.Path)
+	if !ok {
+		http.NotFound(w, r)
 		return
 	}
-	if !enabledForUser {
-		http.Error(w, "Terminal is disabled for cluster-admin users.", http.StatusForbidden)
+
+	isClusterAdmin, err := p.isClusterAdmin(user.Token)
+	if err != nil {
+		http.Error(w, "Failed to check the current users privileges. Cause: "+err.Error(), http.StatusInternalServerError)
 		return
 	}
-
-	ok, namespace, workspaceName, path := stripTerminalAPIPrefix(r.URL.Path)
-	if !ok {
-		http.NotFound(w, r)
+	// Cluster admin terminals must live in the openshift-terminal namespace to prevent privilege escalation
+	if isClusterAdmin && namespace != "openshift-terminal" {
+		http.Error(w, "cluster-admin users must create and use terminals in the openshift-terminal namespace", http.StatusForbidden)
 		return
 	}