Skip to content

Commit

Permalink
Get the kube user info only once.
Browse files Browse the repository at this point in the history
  • Loading branch information
pilhuhn committed Aug 15, 2024
1 parent 29bf709 commit 8a4b88e
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 60 deletions.
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));
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: 2 additions & 3 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 { getUserId, 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,8 +19,7 @@ export const status = async (

const { server } = currentCluster;

const userName = await getUserName(fastify, request);
const userID: string = await getUserId(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 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
18 changes: 9 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}) 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,11 @@ 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} interacted with a resource that was not secure.`,
);
}
};
Expand Down
44 changes: 12 additions & 32 deletions backend/src/utils/userUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export type OpenShiftUser = {
name: string;
uid: string;
resourceVersion: string;
annotations: {
annotations?: {
'toolchain.dev.openshift.com/sso-user-id': string;
};
};
Expand Down Expand Up @@ -89,47 +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;
} catch (e) {
if (DEV_MODE) {
if (isImpersonating()) {
return DEV_IMPERSONATE_USER ?? '';
}
return (currentUser.username || currentUser.name).split('/')[0];
}
fastify.log.error(`Failed to retrieve username: ${errorHandler(e)}`);
const error = createCustomError(
'Unauthorized',
`Failed to retrieve username: ${errorHandler(e)}`,
(isHttpError(e) && e.statusCode) || 500,
);
throw error;
}
};

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

try {
const userOauth = await getUser(fastify, request);
return userOauth.metadata.annotations?.['toolchain.dev.openshift.com/sso-user-id'];
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
8 changes: 5 additions & 3 deletions frontend/src/concepts/analyticsTracking/useTrackUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@ export const useTrackUser = (
return aId;
};

computeAnonymousUserId().then((val) => {
setAnonymousId(val);
});
if (!userID) {
computeAnonymousUserId().then((val) => {
setAnonymousId(val);
});
}
// compute anonymousId only once
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
Expand Down

0 comments on commit 8a4b88e

Please sign in to comment.