Skip to content

fix(frontend): Prevent namespace authorization bypass in artifact requests #12214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

fix(frontend): Prevent namespace authorization bypass in artifact requests #12214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fbda084
fix bypass namespace authorization by manipulating URL parameters in
akagami-harsh Sep 4, 2025
e057828
npm run format
akagami-harsh Sep 4, 2025
975f93b
fix aritfact proxy test
akagami-harsh Sep 5, 2025
754d87b
add namespace parameter by default
akagami-harsh Sep 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions frontend/server/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ function createUIServer(options: UIConfigs) {
}),
);

/** Authorize function */
const authorizeFn = getAuthorizeFn(options.auth, { apiServerAddress });

/** Artifact */
registerHandler(
app.get,
Expand All @@ -144,6 +147,7 @@ function createUIServer(options: UIConfigs) {
useParameter: false,
tryExtract: true,
options: options,
authorizeFn: authorizeFn,
}),
);
// /artifacts/ endpoint downloads the artifact as is, it does not try to unzip or untar.
Expand All @@ -160,12 +164,10 @@ function createUIServer(options: UIConfigs) {
useParameter: true,
tryExtract: false,
options: options,
authorizeFn: authorizeFn,
}),
);

/** Authorize function */
const authorizeFn = getAuthorizeFn(options.auth, { apiServerAddress });

/** Tensorboard viewer */
const {
get: tensorboardGetHandler,
Expand Down
57 changes: 48 additions & 9 deletions frontend/server/handlers/artifacts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import { isAllowedDomain } from './domain-checker';
import { getK8sSecret } from '../k8s-helper';
import { StorageOptions } from '@google-cloud/storage/build/src/storage';
import { CredentialBody } from 'google-auth-library/build/src/auth/credentials';
import { AuthorizeFn } from '../helpers/auth';
import { AuthorizeRequestResources, AuthorizeRequestVerb } from '../src/generated/apis/auth';

/**
* ArtifactsQueryStrings describes the expected query strings key value pairs
Expand Down Expand Up @@ -81,6 +83,7 @@ export function getArtifactsHandler({
useParameter,
tryExtract,
options,
authorizeFn,
}: {
artifactsConfigs: {
aws: AWSConfigs;
Expand All @@ -91,17 +94,17 @@ export function getArtifactsHandler({
tryExtract: boolean;
useParameter: boolean;
options: UIConfigs;
authorizeFn?: AuthorizeFn;
}): Handler {
const { aws, http, minio, allowedDomain } = artifactsConfigs;
return async (req, res) => {
const source = useParameter ? req.params.source : req.query.source;
const bucket = useParameter ? req.params.bucket : req.query.bucket;
const key = useParameter ? req.params[0] : req.query.key;
const {
peek = 0,
providerInfo = '',
namespace = options.server.serverNamespace,
} = req.query as Partial<ArtifactsQueryStrings>;
const { peek = 0, providerInfo = '', namespace: requestedNamespace } = req.query as Partial<
ArtifactsQueryStrings
>;

if (!source) {
res.status(500).send('Storage source is missing from artifact request');
return;
Expand All @@ -114,7 +117,41 @@ export function getArtifactsHandler({
res.status(500).send('Storage key is missing from artifact request');
return;
}
console.log(`Getting storage artifact at: ${source}: ${bucket}/${key}`);

// Security: Namespace parameter must be provided and user must be authorized for it
let namespace: string;
if (!requestedNamespace) {
// If no namespace provided, reject the request to prevent unauthorized access
res.status(400).send('namespace parameter is required for artifact access');
return;
}

// Authorize user for the requested namespace when authorization is enabled
if (authorizeFn) {
try {
const authError = await authorizeFn(
{
verb: AuthorizeRequestVerb.GET,
resources: AuthorizeRequestResources.VIEWERS,
namespace: requestedNamespace,
},
req,
);
if (authError) {
res.status(401).send(authError.message);
return;
}
} catch (error) {
console.error('Authorization check failed:', error);
res.status(500).send('Failed to authorize artifact access');
return;
}
}

namespace = requestedNamespace;
console.log(
`Getting storage artifact at: ${source}: ${bucket}/${key} in namespace: ${namespace}`,
);

let client: MinioClient;
switch (source) {
Expand Down Expand Up @@ -202,13 +239,15 @@ function getHttpArtifactsHandler(
peek: number = 0,
) {
return async (req: Request, res: Response) => {
const headers = {};
const headers: { [key: string]: string } = {};

// add authorization header to fetch request if key is non-empty
if (auth.key.length > 0) {
// inject original request's value if exists, otherwise default to provided default value
headers[auth.key] =
req.headers[auth.key] || req.headers[auth.key.toLowerCase()] || auth.defaultValue;
const headerValue = req.headers[auth.key] || req.headers[auth.key.toLowerCase()];
headers[auth.key] = Array.isArray(headerValue)
? headerValue[0]
: headerValue || auth.defaultValue;
}
if (!isAllowedDomain(url, allowedDomain)) {
res.status(500).send(`Domain not allowed.`);
Expand Down
89 changes: 67 additions & 22 deletions frontend/server/integration-tests/artifact-get.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ describe('/artifacts', () => {

const request = requests(app.start());
request
.get('/artifacts/get?source=minio&bucket=ml-pipeline&key=hello%2Fworld.txt')
.get(
'/artifacts/get?source=minio&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=kubeflow',
)
.expect(200, artifactContent, err => {
expect(mockedMinioClient).toBeCalledWith({
accessKey: 'minio',
Expand All @@ -100,7 +102,7 @@ describe('/artifacts', () => {
process.env.AWS_SECRET_ACCESS_KEY = 'awsSecret123';
const request = requests(app.start());
request
.get('/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt')
.get('/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=kubeflow')
.expect(200, artifactContent, err => {
expect(mockedMinioClient).toBeCalledWith({
accessKey: 'aws123',
Expand All @@ -122,7 +124,7 @@ describe('/artifacts', () => {
app = new UIServer(configs);
const request = requests(app.start());
request
.get('/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt')
.get('/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=kubeflow')
.expect(200, artifactContent, err => {
expect(mockedMinioClient).toBeCalledWith({
accessKey: 'aws123',
Expand Down Expand Up @@ -184,7 +186,7 @@ describe('/artifacts', () => {
});
});

it('responds with artifact if source is AWS S3, and creds are sourced from Provider Configs, and uses default kubeflow namespace when no namespace is provided', done => {
it('responds with artifact if source is AWS S3, and creds are sourced from Provider Configs, and uses default kubeflow namespace when namespace is provided', done => {
const mockedGetK8sSecret: jest.Mock = getK8sSecret as any;
mockedGetK8sSecret.mockResolvedValue('somevalue');
const mockedMinioClient: jest.Mock = minio.Client as any;
Expand All @@ -206,7 +208,7 @@ describe('/artifacts', () => {
};
request
.get(
`/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&providerInfo=${JSON.stringify(
`/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=${namespace}&providerInfo=${JSON.stringify(
providerInfo,
)}`,
)
Expand Down Expand Up @@ -238,7 +240,7 @@ describe('/artifacts', () => {
});
});

it('responds with artifact if source is AWS S3, and creds are sourced from Provider Configs, and uses default namespace when no namespace is provided, as specified in ENV', done => {
it('responds with artifact if source is AWS S3, and creds are sourced from Provider Configs, and uses custom namespace when namespace is provided, as specified in ENV', done => {
const mockedGetK8sSecret: jest.Mock = getK8sSecret as any;
mockedGetK8sSecret.mockResolvedValue('somevalue');
const mockedMinioClient: jest.Mock = minio.Client as any;
Expand All @@ -260,7 +262,7 @@ describe('/artifacts', () => {
};
request
.get(
`/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&providerInfo=${JSON.stringify(
`/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=${namespace}&providerInfo=${JSON.stringify(
providerInfo,
)}`,
)
Expand Down Expand Up @@ -292,6 +294,31 @@ describe('/artifacts', () => {
});
});

it('rejects requests when no namespace parameter is provided (security fix)', done => {
const configs = loadConfigs(argv, {});
app = new UIServer(configs);
const request = requests(app.start());
const providerInfo = {
Params: {
accessKeyKey: 'AWS_ACCESS_KEY_ID',
disableSSL: 'false',
endpoint: 's3.amazonaws.com',
fromEnv: 'false',
region: 'us-east-2',
secretKeyKey: 'AWS_SECRET_ACCESS_KEY',
secretName: 'aws-s3-creds',
},
Provider: 's3',
};
request
.get(
`/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&providerInfo=${JSON.stringify(
providerInfo,
)}`,
)
.expect(400, 'namespace parameter is required for artifact access', done);
});

it('responds with artifact if source is s3-compatible, and creds are sourced from Provider Configs', done => {
const mockedMinioClient: jest.Mock = minio.Client as any;
const mockedGetK8sSecret: jest.Mock = getK8sSecret as any;
Expand Down Expand Up @@ -433,7 +460,9 @@ describe('/artifacts', () => {

const request = requests(app.start());
request
.get('/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&peek=5')
.get(
'/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&peek=5&namespace=kubeflow',
)
.expect(200, artifactContent.slice(0, 5), err => {
expect(mockedMinioClient).toBeCalledWith({
accessKey: 'aws123',
Expand All @@ -459,7 +488,7 @@ describe('/artifacts', () => {

const request = requests(app.start());
request
.get('/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt')
.get('/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=kubeflow')
.expect(200, artifactContent, err => {
expect(mockedMinioClient).toBeCalledWith({
accessKey: 'aws123',
Expand Down Expand Up @@ -489,7 +518,9 @@ describe('/artifacts', () => {

const request = requests(app.start());
request
.get('/artifacts/get?source=http&bucket=ml-pipeline&key=hello%2Fworld.txt')
.get(
'/artifacts/get?source=http&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=kubeflow',
)
.expect(200, artifactContent, err => {
expect(mockedFetch).toBeCalledWith('http://foo.bar/ml-pipeline/hello/world.txt', {
headers: {},
Expand All @@ -516,7 +547,9 @@ describe('/artifacts', () => {

const request = requests(app.start());
request
.get('/artifacts/get?source=http&bucket=ml-pipeline&key=hello%2Fworld.txt&peek=5')
.get(
'/artifacts/get?source=http&bucket=ml-pipeline&key=hello%2Fworld.txt&peek=5&namespace=kubeflow',
)
.expect(200, artifactContent.slice(0, 5), err => {
expect(mockedFetch).toBeCalledWith('http://foo.bar/ml-pipeline/hello/world.txt', {
headers: {},
Expand Down Expand Up @@ -545,7 +578,9 @@ describe('/artifacts', () => {

const request = requests(app.start());
request
.get('/artifacts/get?source=https&bucket=ml-pipeline&key=hello%2Fworld.txt')
.get(
'/artifacts/get?source=https&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=kubeflow',
)
.expect(200, artifactContent, err => {
expect(mockedFetch).toBeCalledWith('https://foo.bar/ml-pipeline/hello/world.txt', {
headers: {
Expand Down Expand Up @@ -574,7 +609,9 @@ describe('/artifacts', () => {

const request = requests(app.start());
request
.get('/artifacts/get?source=https&bucket=ml-pipeline&key=hello%2Fworld.txt')
.get(
'/artifacts/get?source=https&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=kubeflow',
)
.set('Authorization', 'inheritedToken')
.expect(200, artifactContent, err => {
expect(mockedFetch).toBeCalledWith('https://foo.bar/ml-pipeline/hello/world.txt', {
Expand Down Expand Up @@ -603,7 +640,9 @@ describe('/artifacts', () => {

const request = requests(app.start());
request
.get('/artifacts/get?source=gcs&bucket=ml-pipeline&key=hello%2Fworld.txt')
.get(
'/artifacts/get?source=gcs&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=kubeflow',
)
.expect(200, artifactContent + '\n', done);
});

Expand All @@ -623,7 +662,9 @@ describe('/artifacts', () => {

const request = requests(app.start());
request
.get('/artifacts/get?source=gcs&bucket=ml-pipeline&key=hello%2Fworld.txt&peek=5')
.get(
'/artifacts/get?source=gcs&bucket=ml-pipeline&key=hello%2Fworld.txt&peek=5&namespace=kubeflow',
)
.expect(200, artifactContent.slice(0, 5), done);
});

Expand Down Expand Up @@ -667,7 +708,9 @@ describe('/artifacts', () => {

const request = requests(app.start());
request
.get('/artifacts/get?source=volume&bucket=artifact&key=subartifact/content')
.get(
'/artifacts/get?source=volume&bucket=artifact&key=subartifact/content&namespace=kubeflow',
)
.expect(200, artifactContent, done);
});

Expand Down Expand Up @@ -710,7 +753,7 @@ describe('/artifacts', () => {

const request = requests(app.start());
request
.get(`/artifacts/get?source=volume&bucket=artifact&key=content&peek=5`)
.get(`/artifacts/get?source=volume&bucket=artifact&key=content&peek=5&namespace=kubeflow`)
.expect(200, artifactContent.slice(0, 5), done);
});

Expand Down Expand Up @@ -752,7 +795,7 @@ describe('/artifacts', () => {

const request = requests(app.start());
request
.get(`/artifacts/get?source=volume&bucket=notexist&key=content`)
.get(`/artifacts/get?source=volume&bucket=notexist&key=content&namespace=kubeflow`)
.expect(404, 'Failed to open volume.', done);
});

Expand Down Expand Up @@ -795,7 +838,7 @@ describe('/artifacts', () => {

const request = requests(app.start());
request
.get(`/artifacts/get?source=volume&bucket=artifact&key=notexist/config`)
.get(`/artifacts/get?source=volume&bucket=artifact&key=notexist/config&namespace=kubeflow`)
.expect(404, 'Failed to open volume.', done);
});

Expand Down Expand Up @@ -835,7 +878,9 @@ describe('/artifacts', () => {

const request = requests(app.start());
request
.get(`/artifacts/get?source=volume&bucket=artifact&key=subartifact/notxist.csv`)
.get(
`/artifacts/get?source=volume&bucket=artifact&key=subartifact/notxist.csv&namespace=kubeflow`,
)
.expect(500, 'Failed to open volume.', done);
});
});
Expand All @@ -847,7 +892,7 @@ describe('/artifacts', () => {

const request = requests(app.start());
request
.get('/artifacts/minio/ml-pipeline/hello/world.txt') // url
.get('/artifacts/minio/ml-pipeline/hello/world.txt?namespace=kubeflow') // url
.expect(200, artifactContent, done);
});

Expand All @@ -862,7 +907,7 @@ describe('/artifacts', () => {

const request = requests(app.start());
request
.get('/artifacts/minio/ml-pipeline/hello/world.txt') // url
.get('/artifacts/minio/ml-pipeline/hello/world.txt?namespace=kubeflow') // url
.expect(200, tarGzBuffer.toString(), done);
});
});
Expand Down
2 changes: 1 addition & 1 deletion frontend/server/integration-tests/artifact-proxy.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ describe('/artifacts/get namespaced proxy', () => {
namespace: undefined,
})}`,
)
.expect(200, 'text-data2', done);
.expect(400, 'namespace parameter is required for artifact access', done);
});

it('proxies a request with basePath too', done => {
Expand Down
Loading