[UI Server] Refactor for configurable auth header (#3753)
* [UI] Make auth header configurable * Refactor authorizeFn to move side effects out * Refactor tests to reduce duplication
This commit is contained in:
parent
81dd6a259c
commit
5435e8724f
|
|
@ -681,15 +681,26 @@ describe('UIServer apis', () => {
|
|||
.expect(200, done);
|
||||
});
|
||||
|
||||
it('authorizes user requests from KFP auth api', done => {
|
||||
app = new UIServer(loadConfigs(argv, { ENABLE_AUTHZ: 'true' }));
|
||||
const appKfpApi = express();
|
||||
function setupMockKfpApiService({ port = 3001 }: { port?: number } = {}) {
|
||||
const receivedHeaders: any[] = [];
|
||||
appKfpApi.get('/apis/v1beta1/auth', (req, res) => {
|
||||
receivedHeaders.push(req.headers);
|
||||
res.status(200).send('{}'); // Authorized
|
||||
});
|
||||
kfpApiServer = appKfpApi.listen(3001);
|
||||
kfpApiServer = express()
|
||||
.get('/apis/v1beta1/auth', (req, res) => {
|
||||
receivedHeaders.push(req.headers);
|
||||
res.status(200).send('{}'); // Authorized
|
||||
})
|
||||
.listen(port);
|
||||
return { receivedHeaders, host: 'localhost', port };
|
||||
}
|
||||
|
||||
it('authorizes user requests from KFP auth api', done => {
|
||||
const { receivedHeaders, host, port } = setupMockKfpApiService();
|
||||
app = new UIServer(
|
||||
loadConfigs(argv, {
|
||||
ENABLE_AUTHZ: 'true',
|
||||
ML_PIPELINE_SERVICE_PORT: `${port}`,
|
||||
ML_PIPELINE_SERVICE_HOST: host,
|
||||
}),
|
||||
);
|
||||
k8sGetCustomObjectSpy.mockImplementation(() =>
|
||||
Promise.resolve(newGetTensorboardResponse()),
|
||||
);
|
||||
|
|
@ -712,20 +723,51 @@ describe('UIServer apis', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('uses configured KUBEFLOW_USERID_HEADER for user identity', done => {
|
||||
const { receivedHeaders, host, port } = setupMockKfpApiService();
|
||||
app = new UIServer(
|
||||
loadConfigs(argv, {
|
||||
ENABLE_AUTHZ: 'true',
|
||||
KUBEFLOW_USERID_HEADER: 'x-kubeflow-userid',
|
||||
ML_PIPELINE_SERVICE_PORT: `${port}`,
|
||||
ML_PIPELINE_SERVICE_HOST: host,
|
||||
}),
|
||||
);
|
||||
k8sGetCustomObjectSpy.mockImplementation(() =>
|
||||
Promise.resolve(newGetTensorboardResponse()),
|
||||
);
|
||||
requests(app.start())
|
||||
.get(`/apps/tensorboard?logdir=some-log-dir&namespace=test-ns`)
|
||||
.set('x-kubeflow-userid', 'user@kubeflow.org')
|
||||
.expect(200, err => {
|
||||
expect(receivedHeaders).toHaveLength(1);
|
||||
expect(receivedHeaders[0]).toHaveProperty('x-kubeflow-userid', 'user@kubeflow.org');
|
||||
done(err);
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects user requests when KFP auth api rejected', done => {
|
||||
const errorSpy = jest.spyOn(console, 'error');
|
||||
errorSpy.mockImplementation();
|
||||
app = new UIServer(loadConfigs(argv, { ENABLE_AUTHZ: 'true' }));
|
||||
const appKfpApi = express();
|
||||
appKfpApi.get('/apis/v1beta1/auth', (_, res) => {
|
||||
res.status(400).send(
|
||||
JSON.stringify({
|
||||
error: 'User xxx is not unauthorized to list viewers',
|
||||
details: ['unauthorized', 'callstack'],
|
||||
}),
|
||||
);
|
||||
});
|
||||
kfpApiServer = appKfpApi.listen(3001);
|
||||
|
||||
const apiServerPort = 3001;
|
||||
kfpApiServer = express()
|
||||
.get('/apis/v1beta1/auth', (_, res) => {
|
||||
res.status(400).send(
|
||||
JSON.stringify({
|
||||
error: 'User xxx is not unauthorized to list viewers',
|
||||
details: ['unauthorized', 'callstack'],
|
||||
}),
|
||||
);
|
||||
})
|
||||
.listen(apiServerPort);
|
||||
app = new UIServer(
|
||||
loadConfigs(argv, {
|
||||
ENABLE_AUTHZ: 'true',
|
||||
ML_PIPELINE_SERVICE_PORT: `${apiServerPort}`,
|
||||
ML_PIPELINE_SERVICE_HOST: 'localhost',
|
||||
}),
|
||||
);
|
||||
k8sGetCustomObjectSpy.mockImplementation(() =>
|
||||
Promise.resolve(newGetTensorboardResponse()),
|
||||
);
|
||||
|
|
@ -1004,13 +1046,19 @@ describe('UIServer apis', () => {
|
|||
let kfpApiServer: Server;
|
||||
|
||||
beforeEach(() => {
|
||||
app = new UIServer(loadConfigs(argv, {}));
|
||||
const kfpApiPort = 3001;
|
||||
kfpApiServer = express()
|
||||
.all('/*', (_, res) => {
|
||||
res.status(200).send('KFP API is working');
|
||||
})
|
||||
.listen(kfpApiPort);
|
||||
app = new UIServer(
|
||||
loadConfigs(argv, {
|
||||
ML_PIPELINE_SERVICE_PORT: `${kfpApiPort}`,
|
||||
ML_PIPELINE_SERVICE_HOST: 'localhost',
|
||||
}),
|
||||
);
|
||||
request = requests(app.start());
|
||||
const appKfpApi = express();
|
||||
appKfpApi.all('/*', (_, res) => {
|
||||
res.status(200).send('KFP API is working');
|
||||
});
|
||||
kfpApiServer = appKfpApi.listen(3001);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ import {
|
|||
getArtifactServiceGetter,
|
||||
} from './handlers/artifacts';
|
||||
import { getTensorboardHandlers } from './handlers/tensorboard';
|
||||
import { getAuthorizeFn } from './helpers/auth';
|
||||
import { getPodLogsHandler } from './handlers/pod-logs';
|
||||
import { podInfoHandler, podEventsHandler } from './handlers/pod-info';
|
||||
import { getClusterNameHandler, getProjectIdHandler } from './handlers/gke-metadata';
|
||||
|
|
@ -124,15 +125,15 @@ function createUIServer(options: UIConfigs) {
|
|||
);
|
||||
registerHandler(app.get, '/artifacts/get', getArtifactsHandler(options.artifacts));
|
||||
|
||||
/** Authorize function */
|
||||
const authorizeFn = getAuthorizeFn(options.auth, { apiServerAddress });
|
||||
|
||||
/** Tensorboard viewer */
|
||||
const {
|
||||
get: tensorboardGetHandler,
|
||||
create: tensorboardCreateHandler,
|
||||
delete: tensorboardDeleteHandler,
|
||||
} = getTensorboardHandlers(options.viewer.tensorboard, {
|
||||
apiServerAddress,
|
||||
authzEnabled: options.auth.enabled,
|
||||
});
|
||||
} = getTensorboardHandlers(options.viewer.tensorboard, authorizeFn);
|
||||
registerHandler(app.get, '/apps/tensorboard', tensorboardGetHandler);
|
||||
registerHandler(app.delete, '/apps/tensorboard', tensorboardDeleteHandler);
|
||||
registerHandler(app.post, '/apps/tensorboard', tensorboardCreateHandler);
|
||||
|
|
|
|||
|
|
@ -92,6 +92,18 @@ export function loadConfigs(argv: string[], env: ProcessEnv): UIConfigs {
|
|||
ENABLE_AUTHZ = 'false',
|
||||
/** Deployment type. */
|
||||
DEPLOYMENT: DEPLOYMENT_STR = '',
|
||||
/**
|
||||
* A header user requests have when authenticated. It carries user identity information.
|
||||
* The default value works with Google Cloud IAP.
|
||||
*/
|
||||
KUBEFLOW_USERID_HEADER = 'x-goog-authenticated-user-email',
|
||||
/**
|
||||
* KUBEFLOW_USERID_HEADER's value may have a prefix before user identity.
|
||||
* Use this header to specify what the prefix is.
|
||||
*
|
||||
* e.g. a valid header value for default values can be like `accounts.google.com:user@gmail.com`.
|
||||
*/
|
||||
KUBEFLOW_USERID_PREFIX = 'accounts.google.com:',
|
||||
} = env;
|
||||
|
||||
return {
|
||||
|
|
@ -162,6 +174,8 @@ export function loadConfigs(argv: string[], env: ProcessEnv): UIConfigs {
|
|||
},
|
||||
auth: {
|
||||
enabled: asBool(ENABLE_AUTHZ),
|
||||
kubeflowUserIdHeader: KUBEFLOW_USERID_HEADER,
|
||||
kubeflowUserIdPrefix: KUBEFLOW_USERID_PREFIX,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
|
@ -221,8 +235,10 @@ export interface ServerConfigs {
|
|||
export interface GkeMetadataConfigs {
|
||||
disabled: boolean;
|
||||
}
|
||||
export interface AuthorizationConfigs {
|
||||
export interface AuthConfigs {
|
||||
enabled: boolean;
|
||||
kubeflowUserIdHeader: string;
|
||||
kubeflowUserIdPrefix: string;
|
||||
}
|
||||
export interface UIConfigs {
|
||||
server: ServerConfigs;
|
||||
|
|
@ -238,5 +254,5 @@ export interface UIConfigs {
|
|||
viewer: ViewerConfigs;
|
||||
pipeline: PipelineConfigs;
|
||||
gkeMetadata: GkeMetadataConfigs;
|
||||
auth: AuthorizationConfigs;
|
||||
auth: AuthConfigs;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,60 +11,17 @@
|
|||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
import { Handler, Request, Response } from 'express';
|
||||
import { Handler } from 'express';
|
||||
import * as k8sHelper from '../k8s-helper';
|
||||
import { ViewerTensorboardConfig } from '../configs';
|
||||
import {
|
||||
AuthServiceApi,
|
||||
AuthorizeRequestResources,
|
||||
AuthorizeRequestVerb,
|
||||
} from '../src/generated/apis/auth';
|
||||
import portableFetch from 'portable-fetch';
|
||||
import { AuthorizeRequestResources, AuthorizeRequestVerb } from '../src/generated/apis/auth';
|
||||
import { parseError } from '../utils';
|
||||
|
||||
async function authorize(
|
||||
req: Request,
|
||||
res: Response,
|
||||
authService: AuthServiceApi,
|
||||
{
|
||||
resources,
|
||||
verb,
|
||||
namespace,
|
||||
}: { resources: AuthorizeRequestResources; verb: AuthorizeRequestVerb; namespace: string },
|
||||
): Promise<boolean> {
|
||||
try {
|
||||
// Resources and verb are string enums, they are used as string here, that
|
||||
// requires a force type conversion. If we generated client should accept
|
||||
// enums instead.
|
||||
await authService.authorize(namespace, resources as any, verb as any, {
|
||||
// Pass authentication header.
|
||||
// TODO: parameterize the header.
|
||||
headers: { [AUTH_EMAIL_HEADER]: req.headers[AUTH_EMAIL_HEADER] },
|
||||
});
|
||||
console.debug(`Authorized to ${verb} ${resources} in namespace ${namespace}.`);
|
||||
return true;
|
||||
} catch (err) {
|
||||
const details = await parseError(err);
|
||||
const message = `User is not authorized to ${verb} ${resources} in namespace ${namespace}: ${details.message}`;
|
||||
console.error(message, details.additionalInfo);
|
||||
res.status(401).send(message);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
import { AuthorizeFn } from '../helpers/auth';
|
||||
|
||||
export const getTensorboardHandlers = (
|
||||
tensorboardConfig: ViewerTensorboardConfig,
|
||||
otherConfig: { apiServerAddress: string; authzEnabled: boolean },
|
||||
authorizeFn: AuthorizeFn,
|
||||
): { get: Handler; create: Handler; delete: Handler } => {
|
||||
const { apiServerAddress, authzEnabled } = otherConfig;
|
||||
console.log('api server address ' + apiServerAddress);
|
||||
// TODO: Use portable-fetch instead of node-fetch in other parts too. The generated api here only
|
||||
// supports portable-fetch.
|
||||
const authService = new AuthServiceApi(
|
||||
{ basePath: apiServerAddress },
|
||||
undefined,
|
||||
portableFetch as any,
|
||||
);
|
||||
/**
|
||||
* A handler which retrieve the endpoint for a tensorboard instance. The
|
||||
* handler expects a query string `logdir`.
|
||||
|
|
@ -81,15 +38,17 @@ export const getTensorboardHandlers = (
|
|||
}
|
||||
|
||||
try {
|
||||
if (authzEnabled) {
|
||||
const authorized = await authorize(req, res, authService, {
|
||||
const authError = await authorizeFn(
|
||||
{
|
||||
verb: AuthorizeRequestVerb.GET,
|
||||
resources: AuthorizeRequestResources.VIEWERS,
|
||||
namespace,
|
||||
});
|
||||
if (!authorized) {
|
||||
return;
|
||||
}
|
||||
},
|
||||
req,
|
||||
);
|
||||
if (authError) {
|
||||
res.status(401).send(authError.message);
|
||||
return;
|
||||
}
|
||||
res.send(await k8sHelper.getTensorboardInstance(logdir, namespace));
|
||||
} catch (err) {
|
||||
|
|
@ -122,15 +81,17 @@ export const getTensorboardHandlers = (
|
|||
}
|
||||
|
||||
try {
|
||||
if (authzEnabled) {
|
||||
const authorized = await authorize(req, res, authService, {
|
||||
const authError = await authorizeFn(
|
||||
{
|
||||
verb: AuthorizeRequestVerb.CREATE,
|
||||
resources: AuthorizeRequestResources.VIEWERS,
|
||||
namespace,
|
||||
});
|
||||
if (!authorized) {
|
||||
return;
|
||||
}
|
||||
},
|
||||
req,
|
||||
);
|
||||
if (authError) {
|
||||
res.status(401).send(authError.message);
|
||||
return;
|
||||
}
|
||||
await k8sHelper.newTensorboardInstance(
|
||||
logdir,
|
||||
|
|
@ -168,15 +129,17 @@ export const getTensorboardHandlers = (
|
|||
}
|
||||
|
||||
try {
|
||||
if (authzEnabled) {
|
||||
const authorized = await authorize(req, res, authService, {
|
||||
const authError = await authorizeFn(
|
||||
{
|
||||
verb: AuthorizeRequestVerb.DELETE,
|
||||
resources: AuthorizeRequestResources.VIEWERS,
|
||||
namespace,
|
||||
});
|
||||
if (!authorized) {
|
||||
return;
|
||||
}
|
||||
},
|
||||
req,
|
||||
);
|
||||
if (authError) {
|
||||
res.status(401).send(authError.message);
|
||||
return;
|
||||
}
|
||||
await k8sHelper.deleteTensorboardInstance(logdir, namespace);
|
||||
res.send('Tensorboard deleted.');
|
||||
|
|
@ -193,5 +156,3 @@ export const getTensorboardHandlers = (
|
|||
delete: deleteHandler,
|
||||
};
|
||||
};
|
||||
|
||||
const AUTH_EMAIL_HEADER = 'x-goog-authenticated-user-email';
|
||||
|
|
|
|||
|
|
@ -0,0 +1,62 @@
|
|||
import { Request, Response } from 'express';
|
||||
import portableFetch from 'portable-fetch';
|
||||
import { AuthConfigs } from '../configs';
|
||||
import {
|
||||
AuthorizeRequestResources,
|
||||
AuthorizeRequestVerb,
|
||||
AuthServiceApi,
|
||||
} from '../src/generated/apis/auth';
|
||||
import { parseError, ErrorDetails } from '../utils';
|
||||
|
||||
export type AuthorizeFn = (
|
||||
{
|
||||
resources,
|
||||
verb,
|
||||
namespace,
|
||||
}: {
|
||||
resources: AuthorizeRequestResources;
|
||||
verb: AuthorizeRequestVerb;
|
||||
namespace: string;
|
||||
},
|
||||
req: Request,
|
||||
) => Promise<ErrorDetails | undefined>;
|
||||
|
||||
export const getAuthorizeFn = (
|
||||
authConfigs: AuthConfigs,
|
||||
otherConfigs: {
|
||||
apiServerAddress: string;
|
||||
},
|
||||
) => {
|
||||
const { apiServerAddress } = otherConfigs;
|
||||
// TODO: Use portable-fetch instead of node-fetch in other parts too. The generated api here only
|
||||
// supports portable-fetch.
|
||||
const authService = new AuthServiceApi(
|
||||
{ basePath: apiServerAddress },
|
||||
undefined,
|
||||
portableFetch as any,
|
||||
);
|
||||
const authorize: AuthorizeFn = async ({ resources, verb, namespace }, req) => {
|
||||
if (!authConfigs.enabled) {
|
||||
return undefined;
|
||||
}
|
||||
try {
|
||||
// Resources and verb are string enums, they are used as string here, that
|
||||
// requires a force type conversion. If we generated client should accept
|
||||
// enums instead.
|
||||
await authService.authorize(namespace, resources as any, verb as any, {
|
||||
// Pass authentication header.
|
||||
headers: {
|
||||
[authConfigs.kubeflowUserIdHeader]: req.headers[authConfigs.kubeflowUserIdHeader],
|
||||
},
|
||||
});
|
||||
console.debug(`Authorized to ${verb} ${resources} in namespace ${namespace}.`);
|
||||
return undefined;
|
||||
} catch (err) {
|
||||
const details = await parseError(err);
|
||||
const message = `User is not authorized to ${verb} ${resources} in namespace ${namespace}: ${details.message}`;
|
||||
console.error(message, details.additionalInfo);
|
||||
return { message, additionalInfo: details.additionalInfo };
|
||||
}
|
||||
};
|
||||
return authorize;
|
||||
};
|
||||
Loading…
Reference in New Issue