Skip to content

Commit 8303c6e

Browse files
authored
Add manifest for kube RBAC enablement (opendatahub-io#5076)
* feat: add manifest for kube RBAC enablement * Refactor notebook routing and authentication for OpenShift 4.19+ BYO OIDC support - Removed useRouteForNotebook hook and replaced with getRoutePathForWorkbench utility for generating notebook links. - Updated NotebookLogoutRedirect, SetupCurrentNotebook, NotebookActions, and StopAllServersButton components to use the new routing utility. - Simplified notebook link generation by eliminating unnecessary API calls and polling logic. - Enhanced JWT username extraction with multi-strategy fallback approach. - Added comprehensive migration guide for transitioning to OpenShift 4.19+ with BYO OIDC. - Removed deprecated APIs and features related to previous routing methods. - Added unit tests for JWT utility functions to ensure robust username extraction. * feat: change logout route * fix project issue * bugfix: fix some related issues * bugfix: workaround to get a stable kube-rbac-proxy image
1 parent ccca066 commit 8303c6e

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+1150
-509
lines changed
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
/* eslint-disable no-restricted-properties */
2+
// Disabling no-restricted-properties for this test file as we need Buffer.toString()
3+
// for base64url encoding to create test JWT tokens
4+
import { extractUsernameFromJWT, getUsernameFromToken } from '../utils/jwtUtils';
5+
import { FastifyRequest } from 'fastify';
6+
7+
// Helper function to create base64url encoded JWT tokens for testing
8+
const createTestToken = (payload: object): string => {
9+
const payloadString = JSON.stringify(payload);
10+
const base64 = Buffer.from(payloadString).toString('base64');
11+
const base64url = base64.replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '');
12+
return `header.${base64url}.signature`;
13+
};
14+
15+
describe('JWT Username Extraction', () => {
16+
describe('extractUsernameFromJWT', () => {
17+
it('should extract username from preferred_username claim', () => {
18+
const token = createTestToken({
19+
preferred_username: 'testuser',
20+
sub: 'some-uuid',
21+
});
22+
23+
const username = extractUsernameFromJWT(token);
24+
expect(username).toBe('testuser');
25+
});
26+
27+
it('should fallback to username claim', () => {
28+
const token = createTestToken({
29+
username: 'testuser',
30+
sub: 'some-uuid',
31+
});
32+
33+
const username = extractUsernameFromJWT(token);
34+
expect(username).toBe('testuser');
35+
});
36+
37+
it('should fallback to sub claim', () => {
38+
const token = createTestToken({
39+
sub: 'user@example.com',
40+
});
41+
42+
const username = extractUsernameFromJWT(token);
43+
expect(username).toBe('user@example.com');
44+
});
45+
46+
it('should fallback to email claim (before @)', () => {
47+
const token = createTestToken({
48+
email: 'testuser@example.com',
49+
});
50+
51+
const username = extractUsernameFromJWT(token);
52+
expect(username).toBe('testuser');
53+
});
54+
55+
it('should handle invalid tokens gracefully', () => {
56+
const username = extractUsernameFromJWT('invalid-token');
57+
expect(username).toBeNull();
58+
});
59+
60+
it('should handle malformed tokens gracefully', () => {
61+
const username = extractUsernameFromJWT('header.payload');
62+
expect(username).toBeNull();
63+
});
64+
65+
it('should handle invalid JSON in payload', () => {
66+
// Manually create an invalid token with non-JSON payload
67+
const invalidPayload = 'not-json';
68+
const base64 = Buffer.from(invalidPayload).toString('base64');
69+
const base64url = base64.replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '');
70+
const token = `header.${base64url}.signature`;
71+
72+
const username = extractUsernameFromJWT(token);
73+
expect(username).toBeNull();
74+
});
75+
76+
it('should return null if no username claims present', () => {
77+
const token = createTestToken({
78+
iss: 'https://issuer.example.com',
79+
aud: 'my-app',
80+
});
81+
82+
const username = extractUsernameFromJWT(token);
83+
expect(username).toBeNull();
84+
});
85+
});
86+
87+
describe('getUsernameFromToken', () => {
88+
it('should extract username from request headers', () => {
89+
const token = createTestToken({
90+
preferred_username: 'testuser',
91+
});
92+
93+
const mockRequest = {
94+
headers: {
95+
'x-forwarded-access-token': token,
96+
},
97+
} as unknown as FastifyRequest;
98+
99+
const username = getUsernameFromToken(mockRequest);
100+
expect(username).toBe('testuser');
101+
});
102+
103+
it('should return null if token header is missing', () => {
104+
const mockRequest = {
105+
headers: {},
106+
} as FastifyRequest;
107+
108+
const username = getUsernameFromToken(mockRequest);
109+
expect(username).toBeNull();
110+
});
111+
112+
it('should return null if token is invalid', () => {
113+
const mockRequest = {
114+
headers: {
115+
'x-forwarded-access-token': 'invalid-token',
116+
},
117+
} as unknown as FastifyRequest;
118+
119+
const username = getUsernameFromToken(mockRequest);
120+
expect(username).toBeNull();
121+
});
122+
});
123+
});

backend/src/routes/api/notebooks/utils.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,29 +8,23 @@ import {
88
generateNotebookNameFromUsername,
99
getNamespaces,
1010
getNotebook,
11-
getRoute,
1211
updateNotebook,
1312
} from '../../../utils/notebookUtils';
1413

1514
export const getNotebookStatus = async (
1615
fastify: KubeFastifyInstance,
1716
namespace: string,
1817
name: string,
19-
): Promise<{ notebook: Notebook; isRunning: boolean; podUID: string; notebookLink: string }> => {
18+
): Promise<{ notebook: Notebook; isRunning: boolean; podUID: string }> => {
2019
const notebook = await getNotebook(fastify, namespace, name);
2120
const hasStopAnnotation = !!notebook.metadata.annotations?.['kubeflow-resource-stopped'];
2221
const [isRunning, podUID] = hasStopAnnotation
2322
? [false, '']
2423
: await listNotebookStatus(fastify, namespace, name);
25-
const route = await getRoute(fastify, namespace, name).catch((e) => {
26-
fastify.log.warn(`Failed getting route ${notebook.metadata.name}: ${e.message}`);
27-
return undefined;
28-
});
2924
return {
3025
notebook,
3126
isRunning,
3227
podUID,
33-
notebookLink: route ? `https://${route.spec.host}/notebook/${namespace}/${name}` : '',
3428
};
3529
};
3630

backend/src/routes/api/route/index.ts

Lines changed: 0 additions & 17 deletions
This file was deleted.
Lines changed: 42 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -1,68 +1,21 @@
11
import { FastifyRequest } from 'fastify';
22
import { KubeFastifyInstance } from '../../../types';
33
import { getUserInfo } from '../../../utils/userUtils';
4-
import {
5-
getAdminUserList,
6-
getAllowedUserList,
7-
getClusterAdminUserList,
8-
isUserAdmin,
9-
KUBE_SAFE_PREFIX,
10-
} from '../../../utils/adminUtils';
4+
import { isUserAdmin, KUBE_SAFE_PREFIX } from '../../../utils/adminUtils';
115
import { getNotebooks } from '../../../utils/notebookUtils';
126

137
type AllowedUser = {
148
username: string;
159
privilege: 'Admin' | 'User';
1610
lastActivity: string;
1711
};
18-
type AllowedUserMap = { [username: string]: AllowedUser };
19-
type UserActivityMap = { [username: string]: string };
2012

21-
const convertUserListToMap = (
22-
userList: string[],
23-
privilege: 'Admin' | 'User',
24-
activityMap: UserActivityMap,
25-
): AllowedUserMap =>
26-
userList.reduce<AllowedUserMap>((acc, rawUsername) => {
27-
let username = rawUsername;
28-
if (username.startsWith(KUBE_SAFE_PREFIX)) {
29-
// Users who start with this designation are non-k8s names
30-
username = username.slice(KUBE_SAFE_PREFIX.length);
31-
}
32-
33-
return {
34-
...acc,
35-
[username]: { username, privilege, lastActivity: activityMap[username] ?? null },
36-
};
37-
}, {});
38-
39-
const getUserActivityFromNotebook = async (
40-
fastify: KubeFastifyInstance,
41-
namespace: string,
42-
): Promise<UserActivityMap> => {
43-
const notebooks = await getNotebooks(fastify, namespace);
44-
45-
return notebooks.items
46-
.map<[string | undefined, string | undefined]>((notebook) => [
47-
notebook.metadata.annotations?.['opendatahub.io/username'],
48-
notebook.metadata.annotations?.['notebooks.kubeflow.org/last-activity'] ||
49-
notebook.metadata.annotations?.['kubeflow-resource-stopped'] ||
50-
'Now',
51-
])
52-
.filter(
53-
(arr): arr is [string, string] =>
54-
Array.isArray(arr) && typeof arr[0] === 'string' && typeof arr[1] === 'string',
55-
)
56-
.reduce<UserActivityMap>(
57-
(acc, [username, lastActivity]) => ({
58-
...acc,
59-
[username]: lastActivity,
60-
}),
61-
{},
62-
);
63-
};
64-
65-
/** @deprecated -- Jupyter Tile reliance; group and user fetching is incompatible with OpenShift Console growth */
13+
/**
14+
* Get list of users with active notebooks (Group API independent).
15+
* This works in both traditional OpenShift and BYO OIDC environments.
16+
*
17+
* @deprecated -- Jupyter Tile reliance; limited functionality in BYO OIDC
18+
*/
6619
export const getAllowedUsers = async (
6720
fastify: KubeFastifyInstance,
6821
request: FastifyRequest<{ Params: { namespace: string } }>,
@@ -71,43 +24,46 @@ export const getAllowedUsers = async (
7124
const userInfo = await getUserInfo(fastify, request);
7225
const currentUser = userInfo.userName;
7326
const isAdmin = await isUserAdmin(fastify, request);
27+
7428
if (!isAdmin) {
75-
// Privileged call -- return nothing
76-
fastify.log.warn(
77-
`A request for all allowed users & their status was made as a non Admin (by ${currentUser})`,
78-
);
29+
fastify.log.warn(`A request for all allowed users was made as a non Admin (by ${currentUser})`);
7930
return [];
8031
}
8132

82-
const activityMap = await getUserActivityFromNotebook(fastify, namespace);
33+
// Get users from notebook resources only (no Group API required)
34+
const notebooks = await getNotebooks(fastify, namespace);
35+
36+
const usersMap = new Map<string, AllowedUser>();
37+
38+
for (const notebook of notebooks.items) {
39+
let username = notebook.metadata.annotations?.['opendatahub.io/username'];
40+
if (!username) {
41+
continue;
42+
}
43+
44+
// Handle kube-safe prefix
45+
if (username.startsWith(KUBE_SAFE_PREFIX)) {
46+
const buffer = Buffer.from(username.slice(KUBE_SAFE_PREFIX.length), 'base64');
47+
username = String(buffer);
48+
}
8349

84-
const withNotebookUsers = Object.keys(activityMap);
85-
// TODO: Move away from this group listing design...
86-
const adminUsers = await getAdminUserList(fastify);
87-
const allowedUsers = await getAllowedUserList(fastify);
88-
// get cluster admins that have a notebook
89-
const clusterAdminUsers = (await getClusterAdminUserList(fastify)).filter((user) =>
90-
withNotebookUsers.includes(user),
91-
);
50+
const lastActivity =
51+
notebook.metadata.annotations?.['notebooks.kubeflow.org/last-activity'] ||
52+
notebook.metadata.annotations?.['kubeflow-resource-stopped'] ||
53+
'Now';
9254

93-
const usersWithNotebooksMap: AllowedUserMap = convertUserListToMap(
94-
withNotebookUsers,
95-
'User',
96-
activityMap,
97-
);
98-
const allowedUsersMap: AllowedUserMap = convertUserListToMap(allowedUsers, 'User', activityMap);
99-
const adminUsersMap: AllowedUserMap = convertUserListToMap(adminUsers, 'Admin', activityMap);
100-
const clusterAdminUsersMap: AllowedUserMap = convertUserListToMap(
101-
clusterAdminUsers,
102-
'Admin',
103-
activityMap,
104-
);
55+
// In BYO OIDC, we can't determine admin status from Groups
56+
// Default to 'User' unless explicitly marked
57+
const isNotebookOwnerAdmin = notebook.metadata.labels?.['opendatahub.io/user-type'] === 'admin';
58+
59+
const existing = usersMap.get(username);
60+
const privilege = existing?.privilege === 'Admin' || isNotebookOwnerAdmin ? 'Admin' : 'User';
61+
usersMap.set(username, {
62+
username,
63+
privilege,
64+
lastActivity,
65+
});
66+
}
10567

106-
const returnUsers: AllowedUserMap = {
107-
...usersWithNotebooksMap,
108-
...allowedUsersMap,
109-
...adminUsersMap,
110-
...clusterAdminUsersMap,
111-
};
112-
return Object.values(returnUsers);
68+
return Array.from(usersMap.values());
11369
};

backend/src/routes/api/status/statusUtils.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { getUserInfo } from '../../../utils/userUtils';
44
import { createCustomError } from '../../../utils/requestUtils';
55
import { isUserAdmin, isUserAllowed } from '../../../utils/adminUtils';
66
import { isImpersonating } from '../../../devFlags';
7+
import { getSegmentUserId } from '../../../utils/segmentUtils';
78

89
export const status = async (
910
fastify: KubeFastifyInstance,
@@ -19,10 +20,13 @@ export const status = async (
1920

2021
const { server } = currentCluster;
2122

22-
const { userName, userID } = await getUserInfo(fastify, request);
23+
const { userName, userID: devSandboxUserId } = await getUserInfo(fastify, request);
2324
const isAdmin = await isUserAdmin(fastify, request);
2425
const isAllowed = isAdmin ? true : await isUserAllowed(fastify, request);
2526

27+
// Get segment user ID with fallbacks
28+
const userID = devSandboxUserId || (await getSegmentUserId(fastify, request, userName));
29+
2630
if (!kubeContext && !kubeContext.trim()) {
2731
const error = createCustomError(
2832
'failed to get kube status',

0 commit comments

Comments
 (0)