diff --git a/frontend/server/app.test.ts b/frontend/server/app.test.ts index 0e0319ebd57..f82817f43f6 100644 --- a/frontend/server/app.test.ts +++ b/frontend/server/app.test.ts @@ -321,6 +321,33 @@ describe('UIServer apis', () => { done(err); }); }); + + it('responds with 403 when authorization is rejected', done => { + // Set up a mock auth server that rejects authorization + const authPort = 3002; + const authServer = express() + .post('/apis/v1beta1/auth', (_, res) => { + res.status(401).send('Unauthorized'); + }) + .listen(authPort); + + app = new UIServer( + loadConfigs(argv, { + ENABLE_AUTHZ: 'true', + ML_PIPELINE_SERVICE_PORT: `${authPort}`, + ML_PIPELINE_SERVICE_HOST: 'localhost', + }), + ); + const authRequest = requests(app.start()); + + const spyError = jest.spyOn(console, 'error').mockImplementation(() => null); + authRequest + .get('/k8s/pod?podname=test-pod&podnamespace=test-ns') + .expect(403, 'Access denied to namespace', err => { + authServer.close(); + done(err); + }); + }); }); describe('/k8s/pod/events', () => { @@ -400,10 +427,73 @@ describe('UIServer apis', () => { done(err); }); }); + + it('responds with 403 when authorization is rejected', done => { + // Set up a mock auth server that rejects authorization + const authPort = 3003; + const authServer = express() + .post('/apis/v1beta1/auth', (_, res) => { + res.status(401).send('Unauthorized'); + }) + .listen(authPort); + + app = new UIServer( + loadConfigs(argv, { + ENABLE_AUTHZ: 'true', + ML_PIPELINE_SERVICE_PORT: `${authPort}`, + ML_PIPELINE_SERVICE_HOST: 'localhost', + }), + ); + const authRequest = requests(app.start()); + + const spyError = jest.spyOn(console, 'error').mockImplementation(() => null); + authRequest + .get('/k8s/pod/events?podname=test-pod&podnamespace=test-ns') + .expect(403, 'Access denied to namespace', err => { + authServer.close(); + done(err); + }); + }); }); - // TODO: Add integration tests for k8s helper related endpoints - // describe('/k8s/pod/logs', () => {}); + describe('/k8s/pod/logs', () => { + let request: requests.SuperTest; + beforeEach(() => { + app = new UIServer(loadConfigs(argv, {})); + request = requests(app.start()); + }); + + it('asks for podname if not provided', done => { + request.get('/k8s/pod/logs').expect(400, 'podname argument is required', done); + }); + + it('responds with 403 when authorization is rejected', done => { + // Set up a mock auth server that rejects authorization + const authPort = 3004; + const authServer = express() + .post('/apis/v1beta1/auth', (_, res) => { + res.status(401).send('Unauthorized'); + }) + .listen(authPort); + + app = new UIServer( + loadConfigs(argv, { + ENABLE_AUTHZ: 'true', + ML_PIPELINE_SERVICE_PORT: `${authPort}`, + ML_PIPELINE_SERVICE_HOST: 'localhost', + }), + ); + const authRequest = requests(app.start()); + + const spyError = jest.spyOn(console, 'error').mockImplementation(() => null); + authRequest + .get('/k8s/pod/logs?podname=test-pod&podnamespace=test-ns') + .expect(403, 'Access denied to namespace', err => { + authServer.close(); + done(err); + }); + }); + }); describe('/apis/v1beta1/', () => { let request: requests.SuperTest; diff --git a/frontend/server/app.ts b/frontend/server/app.ts index 08a960512eb..bd549df901d 100644 --- a/frontend/server/app.ts +++ b/frontend/server/app.ts @@ -27,7 +27,7 @@ import { import { getTensorboardHandlers } from './handlers/tensorboard'; import { getAuthorizeFn } from './helpers/auth'; import { getPodLogsHandler } from './handlers/pod-logs'; -import { podInfoHandler, podEventsHandler } from './handlers/pod-info'; +import { getPodInfoHandlers } from './handlers/pod-info'; import { getClusterNameHandler, getProjectIdHandler } from './handlers/gke-metadata'; import { getAllowCustomVisualizationsHandler } from './handlers/vis'; import { getIndexHTMLHandler } from './handlers/index-html'; @@ -199,7 +199,7 @@ function createUIServer(options: UIConfigs) { registerHandler( app.get, '/k8s/pod/logs', - getPodLogsHandler(options.argo, options.artifacts, options.pod.logContainerName), + getPodLogsHandler(options.argo, options.artifacts, options.pod.logContainerName, authorizeFn), ); } @@ -225,11 +225,12 @@ function createUIServer(options: UIConfigs) { registerHandler( app.get, '/k8s/pod/logs', - getPodLogsHandler(options.argo, options.artifacts, options.pod.logContainerName), + getPodLogsHandler(options.argo, options.artifacts, options.pod.logContainerName, authorizeFn), ); } /** Pod info */ + const { podInfoHandler, podEventsHandler } = getPodInfoHandlers(authorizeFn); registerHandler(app.get, '/k8s/pod', podInfoHandler); registerHandler(app.get, '/k8s/pod/events', podEventsHandler); diff --git a/frontend/server/handlers/pod-info.ts b/frontend/server/handlers/pod-info.ts index 96544eb3613..007f24d6cc5 100644 --- a/frontend/server/handlers/pod-info.ts +++ b/frontend/server/handlers/pod-info.ts @@ -14,53 +14,102 @@ import { Handler } from 'express'; import * as k8sHelper from '../k8s-helper'; +import { AuthorizeRequestResources, AuthorizeRequestVerb } from '../src/generated/apis/auth'; +import { AuthorizeFn } from '../helpers/auth'; /** - * podInfoHandler retrieves pod info and sends back as JSON format. + * Get pod info handlers. */ -export const podInfoHandler: Handler = async (req, res) => { - const { podname, podnamespace } = req.query; - if (!podname) { - // 422 status code "Unprocessable entity", refer to https://stackoverflow.com/a/42171674 - res.status(422).send('podname argument is required'); - return; - } - if (!podnamespace) { - res.status(422).send('podnamespace argument is required'); - return; - } - const podName = decodeURIComponent(podname as string); - const podNamespace = decodeURIComponent(podnamespace as string); +export function getPodInfoHandlers(authorizeFn: AuthorizeFn) { - const [pod, err] = await k8sHelper.getPod(podName, podNamespace); - if (err) { - const { message, additionalInfo } = err; - console.error(message, additionalInfo); - res.status(500).send(message); - return; - } - res.status(200).send(JSON.stringify(pod)); -}; + const podInfoHandler: Handler = async (req, res) => { + const { podname, podnamespace } = req.query; + if (!podname) { + // 422 status code "Unprocessable entity", refer to https://stackoverflow.com/a/42171674 + res.status(422).send('podname argument is required'); + return; + } + if (!podnamespace) { + res.status(422).send('podnamespace argument is required'); + return; + } -export const podEventsHandler: Handler = async (req, res) => { - const { podname, podnamespace } = req.query; - if (!podname) { - res.status(422).send('podname argument is required'); - return; - } - if (!podnamespace) { - res.status(422).send('podnamespace argument is required'); - return; - } - const podName = decodeURIComponent(podname as string); - const podNamespace = decodeURIComponent(podnamespace as string); + // Check access to namespace + try { + const authError = await authorizeFn( + { + verb: AuthorizeRequestVerb.GET, + resources: AuthorizeRequestResources.VIEWERS, + namespace: podnamespace as string, + }, + req, + ); + if (authError) { + res.status(403).send('Access denied to namespace'); + return; + } + } catch (error) { + console.error('Authorization error:', error); + res.status(500).send('Authorization check failed'); + return; + } - const [eventList, err] = await k8sHelper.listPodEvents(podName, podNamespace); - if (err) { - const { message, additionalInfo } = err; - console.error(message, additionalInfo); - res.status(500).send(message); - return; - } - res.status(200).send(JSON.stringify(eventList)); -}; + const podName = decodeURIComponent(podname as string); + const podNamespace = decodeURIComponent(podnamespace as string); + + const [pod, err] = await k8sHelper.getPod(podName, podNamespace); + if (err) { + const { message, additionalInfo } = err; + console.error(message, additionalInfo); + res.status(500).send(message); + return; + } + res.status(200).send(JSON.stringify(pod)); + }; + + const podEventsHandler: Handler = async (req, res) => { + const { podname, podnamespace } = req.query; + if (!podname) { + res.status(422).send('podname argument is required'); + return; + } + if (!podnamespace) { + res.status(422).send('podnamespace argument is required'); + return; + } + + // Check access to namespace + try { + const authError = await authorizeFn( + { + verb: AuthorizeRequestVerb.GET, + resources: AuthorizeRequestResources.VIEWERS, + namespace: podnamespace as string, + }, + req, + ); + if (authError) { + res.status(403).send('Access denied to namespace'); + return; + } + } catch (error) { + console.error('Authorization error:', error); + res.status(500).send('Authorization check failed'); + return; + } + + const podName = decodeURIComponent(podname as string); + const podNamespace = decodeURIComponent(podnamespace as string); + + const [eventList, err] = await k8sHelper.listPodEvents(podName, podNamespace); + if (err) { + const { message, additionalInfo } = err; + console.error(message, additionalInfo); + res.status(500).send(message); + return; + } + res.status(200).send(JSON.stringify(eventList)); + }; + + return { podInfoHandler, podEventsHandler }; +} diff --git a/frontend/server/handlers/pod-logs.ts b/frontend/server/handlers/pod-logs.ts index cf2c3633f57..ec11c5c0dd0 100644 --- a/frontend/server/handlers/pod-logs.ts +++ b/frontend/server/handlers/pod-logs.ts @@ -21,6 +21,8 @@ import { toGetPodLogsStream, } from '../workflow-helper'; import { ArgoConfigs, MinioConfigs, AWSConfigs } from '../configs'; +import { AuthorizeRequestResources, AuthorizeRequestVerb } from '../src/generated/apis/auth'; +import { AuthorizeFn } from '../helpers/auth'; /** * Returns a handler which attempts to retrieve the logs for the specific pod, @@ -30,6 +32,7 @@ import { ArgoConfigs, MinioConfigs, AWSConfigs } from '../configs'; * - retrieve log archive with the provided argo archive settings * @param argoOptions fallback options to retrieve log archive * @param artifactsOptions configs and credentials for the different artifact backend + * @param authorizeFn function to authorize namespace access */ export function getPodLogsHandler( argoOptions: ArgoConfigs, @@ -38,6 +41,7 @@ export function getPodLogsHandler( aws: AWSConfigs; }, podLogContainerName: string, + authorizeFn: AuthorizeFn, ): Handler { const { archiveLogs, @@ -82,6 +86,28 @@ export function getPodLogsHandler( // Note decodeURIComponent(undefined) === 'undefined', so I cannot pass the argument directly. const podNamespace = decodeURIComponent((req.query.podnamespace as string) || '') || undefined; + // Check access to namespace if podNamespace is provided + if (podNamespace) { + try { + const authError = await authorizeFn( + { + verb: AuthorizeRequestVerb.GET, + resources: AuthorizeRequestResources.VIEWERS, + namespace: podNamespace, + }, + req, + ); + if (authError) { + res.status(403).send('Access denied to namespace'); + return; + } + } catch (error) { + console.error('Authorization error:', error); + res.status(500).send('Authorization check failed'); + return; + } + } + try { const stream = await getPodLogsStream(podName, createdAt, podNamespace); stream.on('error', err => {