Skip to content

Commit

Permalink
Reslove conflict
Browse files Browse the repository at this point in the history
  • Loading branch information
yzhao583 committed Aug 26, 2024
2 parents 4fe7a8a + a7ad26d commit 70467d0
Show file tree
Hide file tree
Showing 110 changed files with 2,883 additions and 1,429 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ jobs:
if: ${{ success() }}
run: npm run test:backend && npm run test:frontend:coverage
- name: Upload coverage reports to Codecov
uses: codecov/codecov-action@v4.0.1
uses: codecov/codecov-action@v4.5.0
with:
token: ${{ secrets.CODECOV_TOKEN }}
fail_ci_if_error: false
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ WORKDIR /usr/src/app

RUN mkdir /usr/src/app/logs && chmod 775 /usr/src/app/logs

USER default
USER 1001:0

COPY --chown=default:root --from=builder /usr/src/app/frontend/public /usr/src/app/frontend/public
COPY --chown=default:root --from=builder /usr/src/app/backend/package.json /usr/src/app/backend/package.json
Expand Down
6 changes: 3 additions & 3 deletions backend/src/routes/api/groups-config/groupsConfigUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
KubeFastifyInstance,
} from '../../../types';
import { getAllGroups, getGroupsCR, updateGroupsCR } from '../../../utils/groupsUtils';
import { getUserName } from '../../../utils/userUtils';
import { getUserInfo } from '../../../utils/userUtils';
import { isUserAdmin } from '../../../utils/adminUtils';

const SYSTEM_AUTHENTICATED = 'system:authenticated';
Expand All @@ -34,8 +34,8 @@ export const updateGroupsConfig = async (
const { customObjectsApi } = fastify.kube;
const { namespace } = fastify.kube;

const username = await getUserName(fastify, request);
const isAdmin = await isUserAdmin(fastify, username, namespace);
const userInfo = await getUserInfo(fastify, request);
const isAdmin = await isUserAdmin(fastify, userInfo.userName, namespace);

if (!isAdmin) {
const error = createError(403, 'Error updating groups, user needs to be admin');
Expand Down
4 changes: 2 additions & 2 deletions backend/src/routes/api/notebooks/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { V1ContainerStatus, V1Pod, V1PodList } from '@kubernetes/client-node';
import { FastifyRequest } from 'fastify';
import { isHttpError } from '../../../utils';
import { KubeFastifyInstance, Notebook, NotebookData } from '../../../types';
import { getUserName } from '../../../utils/userUtils';
import { getUserInfo } from '../../../utils/userUtils';
import {
createNotebook,
generateNotebookNameFromUsername,
Expand Down Expand Up @@ -69,7 +69,7 @@ export const enableNotebook = async (
): Promise<Notebook> => {
const notebookData = request.body;
const { notebookNamespace } = getNamespaces(fastify);
const username = request.body.username || (await getUserName(fastify, request));
const username = request.body.username || (await getUserInfo(fastify, request)).userName;
const name = generateNotebookNameFromUsername(username);
const url = request.headers.origin;

Expand Down
5 changes: 3 additions & 2 deletions backend/src/routes/api/status/adminAllowedUsers.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { FastifyRequest } from 'fastify';
import { KubeFastifyInstance } from '../../../types';
import { getUserName } from '../../../utils/userUtils';
import { getUserInfo } from '../../../utils/userUtils';
import {
getAdminUserList,
getAllowedUserList,
Expand Down Expand Up @@ -66,7 +66,8 @@ export const getAllowedUsers = async (
request: FastifyRequest<{ Params: { namespace: string } }>,
): Promise<AllowedUser[]> => {
const { namespace } = request.params;
const currentUser = await getUserName(fastify, request);
const userInfo = await getUserInfo(fastify, request);
const currentUser = userInfo.userName;
const isAdmin = await isUserAdmin(fastify, currentUser, namespace);
if (!isAdmin) {
// Privileged call -- return nothing
Expand Down
5 changes: 3 additions & 2 deletions backend/src/routes/api/status/statusUtils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { FastifyRequest } from 'fastify';
import { KubeFastifyInstance, KubeStatus } from '../../../types';
import { getUserName } from '../../../utils/userUtils';
import { getUserInfo } from '../../../utils/userUtils';
import { createCustomError } from '../../../utils/requestUtils';
import { isUserAdmin, isUserAllowed } from '../../../utils/adminUtils';
import { isImpersonating } from '../../../devFlags';
Expand All @@ -19,7 +19,7 @@ export const status = async (

const { server } = currentCluster;

const userName = await getUserName(fastify, request);
const { userName, userID } = await getUserInfo(fastify, request);
const isAdmin = await isUserAdmin(fastify, userName, namespace);
const isAllowed = isAdmin ? true : await isUserAllowed(fastify, userName);

Expand All @@ -37,6 +37,7 @@ export const status = async (
currentUser,
namespace,
userName,
userID,
clusterID,
clusterBranding,
isAdmin,
Expand Down
1 change: 1 addition & 0 deletions backend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ export type KubeStatus = {
currentUser: User;
namespace: string;
userName: string | string[];
userID?: string;
clusterID: string;
clusterBranding: string;
isAdmin: boolean;
Expand Down
8 changes: 4 additions & 4 deletions backend/src/utils/fileUtils.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as fs from 'fs';
import { KubeFastifyInstance, OauthFastifyRequest } from '../types';
import { LOG_DIR } from './constants';
import { getUserName } from './userUtils';
import { getUserInfo } from './userUtils';
import { getNamespaces } from './notebookUtils';
import { isUserAdmin } from './adminUtils';

Expand All @@ -27,13 +27,13 @@ export const logRequestDetails = (
};

const writeLogAsync = async () => {
const username = await getUserName(fastify, request);
const userInfo = await getUserInfo(fastify, request);
const { dashboardNamespace } = getNamespaces(fastify);
const isAdmin = await isUserAdmin(fastify, username, dashboardNamespace);
const isAdmin = await isUserAdmin(fastify, userInfo.userName, dashboardNamespace);

writeAdminLog(fastify, {
...data,
user: username,
user: userInfo.userName,
isAdmin: isAdmin,
});
};
Expand Down
4 changes: 2 additions & 2 deletions backend/src/utils/notebookUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
TolerationOperator,
VolumeMount,
} from '../types';
import { getUserName, usernameTranslate } from './userUtils';
import { getUserInfo, usernameTranslate } from './userUtils';
import { createCustomError } from './requestUtils';
import {
PatchUtils,
Expand Down Expand Up @@ -389,7 +389,7 @@ export const stopNotebook = async (
Body: NotebookData;
}>,
): Promise<Notebook> => {
const username = request.body.username || (await getUserName(fastify, request));
const username = request.body.username || (await getUserInfo(fastify, request)).userName;
const name = generateNotebookNameFromUsername(username);
const { notebookNamespace } = getNamespaces(fastify);

Expand Down
80 changes: 0 additions & 80 deletions backend/src/utils/resourceUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import {
SubscriptionKind,
SubscriptionStatusData,
Template,
TemplateList,
TolerationEffect,
TolerationOperator,
} from '../types';
Expand Down Expand Up @@ -67,7 +66,6 @@ const DASHBOARD_CONFIG = {

const fetchDashboardCR = async (fastify: KubeFastifyInstance): Promise<DashboardConfig[]> => {
return fetchOrCreateDashboardCR(fastify)
.then((dashboardCR) => migrateTemplateDisablement(fastify, dashboardCR))
.then((dashboardCR) => migrateBiasTrustyForRHOAI(fastify, dashboardCR))
.then((dashboardCR) => [dashboardCR]);
};
Expand Down Expand Up @@ -862,84 +860,6 @@ export const cleanupDSPSuffix = async (fastify: KubeFastifyInstance): Promise<vo
});
};

/**
* @deprecated - Look to remove asap (see comments below)
* Migrate the template enablement from a current annotation in the Template Object to a list in ODHDashboardConfig
* We are migrating this from Dashboard v2.11.0, so we can remove this code when we no longer support that version.
*/
export const migrateTemplateDisablement = async (
fastify: KubeFastifyInstance,
dashboardConfig: DashboardConfig,
): Promise<DashboardConfig> => {
if (dashboardConfig.spec.templateDisablement) {
return dashboardConfig;
}

const namespace = fastify.kube.namespace;
return fastify.kube.customObjectsApi
.listNamespacedCustomObject(
'template.openshift.io',
'v1',
namespace,
'templates',
undefined,
undefined,
undefined,
'opendatahub.io/dashboard=true',
)
.then((response) => response.body as TemplateList)
.then((templateList) => {
const templatesDisabled = templateList.items
.filter(
(template) =>
template.metadata.annotations?.['opendatahub.io/template-enabled'] === 'false',
)
.map((template) => getServingRuntimeNameFromTemplate(template));
if (templatesDisabled.length > 0) {
const options = {
headers: { 'Content-type': PatchUtils.PATCH_FORMAT_JSON_PATCH },
};

return fastify.kube.customObjectsApi
.patchNamespacedCustomObject(
DASHBOARD_CONFIG.group,
DASHBOARD_CONFIG.version,
dashboardConfig.metadata.namespace,
DASHBOARD_CONFIG.plural,
dashboardConfig.metadata.name,
[
{
op: 'replace',
path: '/spec/templateDisablement',
value: templatesDisabled,
},
],
undefined,
undefined,
undefined,
options,
)
.then(() => {
return {
...dashboardConfig,
spec: {
...dashboardConfig.spec,
templateDisablement: templatesDisabled,
},
};
});
} else {
return dashboardConfig;
}
})
.catch((e) => {
fastify.log.error(
`Error migrating template disablement: ${e.response?.body?.message || e.message || e}`,
);
return dashboardConfig;
});
};

/**
* TODO: There should be a better way to go about this... but the namespace is unlikely to ever change
*/
Expand Down
19 changes: 10 additions & 9 deletions backend/src/utils/route-security.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { FastifyReply, FastifyRequest } from 'fastify';
import { getOpenshiftUser, getUserName, usernameTranslate } from './userUtils';
import { getOpenshiftUser, getUserInfo, usernameTranslate } from './userUtils';
import { createCustomError } from './requestUtils';
import { isUserAdmin } from './adminUtils';
import { getNamespaces } from './notebookUtils';
Expand All @@ -18,9 +18,9 @@ const testAdmin = async (
request: OauthFastifyRequest,
needsAdmin: boolean,
): Promise<boolean> => {
const username = await getUserName(fastify, request);
const userInfo = await getUserInfo(fastify, request);
const { dashboardNamespace } = getNamespaces(fastify);
const isAdmin = await isUserAdmin(fastify, username, dashboardNamespace);
const isAdmin = await isUserAdmin(fastify, userInfo.userName, dashboardNamespace);
if (isAdmin) {
// User is an admin, pass to caller that we can bypass some logic
return true;
Expand All @@ -29,7 +29,7 @@ const testAdmin = async (
if (needsAdmin) {
// Not an admin, route needs one -- reject
fastify.log.error(
`A Non-Admin User (${username}) made a request against an endpoint that requires an admin.`,
`A Non-Admin User (${userInfo.userName}) made a request against an endpoint that requires an admin.`,
);
throw createCustomError(
'Not Admin',
Expand Down Expand Up @@ -68,8 +68,8 @@ const requestSecurityGuard = async (
name?: string,
): Promise<void> => {
const { notebookNamespace, dashboardNamespace } = getNamespaces(fastify);
const username = await getUserName(fastify, request);
const translatedUsername = usernameTranslate(username);
const userInfo = await getUserInfo(fastify, request);
const translatedUsername = usernameTranslate(userInfo.userName);
const isReadRequest = request.method.toLowerCase() === 'get';

// Check to see if a request was made against one of our namespaces
Expand Down Expand Up @@ -208,11 +208,12 @@ const handleSecurityOnRouteData = async (
}

// Not getting a resource, mutating something that is not verify-able theirs -- log the user encase of malicious behaviour
const username = await getUserName(fastify, request);
const userInfo = await getUserInfo(fastify, request);
const { dashboardNamespace } = getNamespaces(fastify);
const isAdmin = await isUserAdmin(fastify, username, dashboardNamespace);
const isAdmin = await isUserAdmin(fastify, userInfo.userName, dashboardNamespace);
fastify.log.warn(
`${isAdmin ? 'Admin ' : ''}User ${username} interacted with a resource that was not secure.`,
`${isAdmin ? 'Admin ' : ''}User
${userInfo.userName} interacted with a resource that was not secure.`,
);
}
};
Expand Down
19 changes: 14 additions & 5 deletions backend/src/utils/userUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ export type OpenShiftUser = {
name: string;
uid: string;
resourceVersion: string;
annotations?: {
'toolchain.dev.openshift.com/sso-user-id': string;
};
};
fullName: string;
identities: string[];
Expand Down Expand Up @@ -86,21 +89,27 @@ export const getUser = async (
}
};

export const getUserName = async (
export const getUserInfo = async (
fastify: KubeFastifyInstance,
request: FastifyRequest,
): Promise<string> => {
): Promise<{ userName: string; userID: string }> => {
const { currentUser } = fastify.kube;

try {
const userOauth = await getUser(fastify, request);
return userOauth.metadata.name;
return {
userName: userOauth.metadata.name,
userID: userOauth.metadata.annotations?.['toolchain.dev.openshift.com/sso-user-id'],
};
} catch (e) {
if (DEV_MODE) {
if (isImpersonating()) {
return DEV_IMPERSONATE_USER ?? '';
return { userName: DEV_IMPERSONATE_USER ?? '', userID: undefined };
}
return (currentUser.username || currentUser.name).split('/')[0];
return {
userName: (currentUser.username || currentUser.name).split('/')[0],
userID: undefined,
};
}
fastify.log.error(`Failed to retrieve username: ${errorHandler(e)}`);
const error = createCustomError(
Expand Down
2 changes: 2 additions & 0 deletions frontend/.slignore.generated
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
*.svg
**/third_party/*
Loading

0 comments on commit 70467d0

Please sign in to comment.