Skip to content
20 changes: 15 additions & 5 deletions packages/kbn-mock-idp-plugin/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ export const plugin: PluginInitializer<void, void, PluginSetupDependencies> = as
}),
},
security: {
authc: { enabled: 'optional' },
authc: { enabled: 'optional', reason: 'Mock IDP plugin for testing' },
authz: { enabled: false, reason: 'Mock IDP plugin for testing' },
},
},
Expand Down Expand Up @@ -312,8 +312,13 @@ export const plugin: PluginInitializer<void, void, PluginSetupDependencies> = as
credential: schema.string(),
}),
},
options: { authRequired: 'optional' },
security: { authz: { enabled: false, reason: 'Mock IDP plugin for testing' } },
security: {
authc: {
enabled: 'optional',
reason: 'Mock IDP plugin for testing UIAM operations',
},
authz: { enabled: false, reason: 'Mock IDP plugin for testing' },
},
},
async (context, request, response) => {
try {
Expand Down Expand Up @@ -366,8 +371,13 @@ export const plugin: PluginInitializer<void, void, PluginSetupDependencies> = as
keys: schema.arrayOf(schema.string(), { minSize: 1 }),
}),
},
options: { authRequired: 'optional' },
security: { authz: { enabled: false, reason: 'Mock IDP plugin for testing' } },
security: {
authc: {
enabled: 'optional',
reason: 'Mock IDP plugin for testing UIAM operations',
},
authz: { enabled: false, reason: 'Mock IDP plugin for testing' },
},
},
async (context, request, response) => {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ describe('CapabilitiesService', () => {
},
authc: {
enabled: 'optional',
reason: expect.any(String),
},
},
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export function registerCapabilitiesRoutes(router: IRouter, resolver: Capabiliti
},
authc: {
enabled: 'optional',
reason: 'This route can be accessed by both authenticated and unauthenticated users',
},
},
validate: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
*/

import { validRouteSecurity } from './security_route_config_validator';
import { ReservedPrivilegesSet } from '@kbn/core-http-server';
import { ReservedPrivilegesSet, type RouteSecurity } from '@kbn/core-http-server';
import type { DeepPartial } from '@kbn/utility-types';

describe('RouteSecurity validation', () => {
it('should pass validation for valid route security with authz enabled and valid required privileges', () => {
Expand All @@ -19,6 +20,7 @@ describe('RouteSecurity validation', () => {
},
authc: {
enabled: 'optional',
reason: 'some reason',
},
})
).not.toThrow();
Expand Down Expand Up @@ -161,6 +163,40 @@ describe('RouteSecurity validation', () => {
);
});

it('should fail validation when authc is minimal but reason is missing', () => {
const routeSecurity = {
authz: {
requiredPrivileges: ['read'],
},
authc: {
enabled: 'minimal',
},
};

expect(() =>
validRouteSecurity(routeSecurity as DeepPartial<RouteSecurity>)
).toThrowErrorMatchingInlineSnapshot(
`"[authc.reason]: expected value of type [string] but got [undefined]"`
);
});

it('should fail validation when authc is optional but reason is missing', () => {
const routeSecurity = {
authz: {
requiredPrivileges: ['read'],
},
authc: {
enabled: 'optional',
},
};

expect(() =>
validRouteSecurity(routeSecurity as DeepPartial<RouteSecurity>)
).toThrowErrorMatchingInlineSnapshot(
`"[authc.reason]: expected value of type [string] but got [undefined]"`
);
});

it('should fail validation when authc is disabled but reason is missing', () => {
const routeSecurity = {
authz: {
Expand Down Expand Up @@ -193,6 +229,20 @@ describe('RouteSecurity validation', () => {
);
});

it('should pass validation when authc is minimal', () => {
expect(() =>
validRouteSecurity({
authz: {
requiredPrivileges: ['read'],
},
authc: {
enabled: 'minimal',
reason: 'some reason',
},
})
).not.toThrow();
});

it('should pass validation when authc is optional', () => {
expect(() =>
validRouteSecurity({
Expand All @@ -201,6 +251,7 @@ describe('RouteSecurity validation', () => {
},
authc: {
enabled: 'optional',
reason: 'some reason',
},
})
).not.toThrow();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,17 @@ const authzSchema = schema.object({
});

const authcSchema = schema.object({
enabled: schema.oneOf([schema.literal(true), schema.literal('optional'), schema.literal(false)]),
enabled: schema.oneOf([
schema.literal(true),
schema.literal('optional'),
schema.literal('minimal'),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first glance, plugin owners might adopt minimal auth to avoid round-tripping to ES. We'll need to make sure use remains appropriate.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Eventually, however, we'd like to enable minimal authentication for all routes by default, as the absolute majority of routes either don't need any user information or need only what's already available in the Kibana session. For the remaining routes, we might expose dedicated asynchronous programmatic APIs to fetch that additional information (e.g., a list of roles).

schema.literal(false),
]),
reason: schema.conditional(
schema.siblingRef('enabled'),
schema.literal(false),
schema.string(),
schema.never()
schema.literal(true),
schema.never(),
schema.string()
),
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,7 @@ describe('Versioned route', () => {
},
authc: {
enabled: 'optional',
reason: 'some reason',
},
};
const securityConfig2: RouteSecurity = {
Expand Down Expand Up @@ -813,6 +814,7 @@ describe('Versioned route', () => {
},
authc: {
enabled: 'optional',
reason: 'some reason',
},
};

Expand Down Expand Up @@ -863,6 +865,7 @@ describe('Versioned route', () => {
},
authc: {
enabled: 'optional',
reason: 'some reason',
},
};

Expand All @@ -888,7 +891,7 @@ describe('Versioned route', () => {
// @ts-expect-error for test purpose
const security = route.getSecurity({ headers: { [ELASTIC_HTTP_VERSION_HEADER]: '1' } });

expect(security.authc).toEqual({ enabled: 'optional' });
expect(security.authc).toEqual({ enabled: 'optional', reason: 'some reason' });

expect(security.authz).toEqual({ requiredPrivileges: ['foo', 'bar'] });
});
Expand Down
45 changes: 43 additions & 2 deletions src/core/packages/http/server-internal/src/http_server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2139,7 +2139,7 @@ test('exposes authentication details of incoming request to a route handler', as
path: '/foo',
validate: false,
security: {
authc: { enabled: 'optional' },
authc: { enabled: 'optional', reason: 'test' },
authz: { enabled: false, reason: 'test' },
},
},
Expand Down Expand Up @@ -2181,7 +2181,48 @@ test('exposes authentication details of incoming request to a route handler', as
tags: [],
timeout: {},
security: {
authc: { enabled: 'optional' },
authc: { enabled: 'optional', reason: 'test' },
authz: { enabled: false, reason: 'test' },
},
},
});
});

test('properly treats minimal authentication as required', async () => {
const { registerRouter, registerAuth, server: innerServer } = await server.setup({ config$ });

const router = new Router('', logger, enhanceWithContext, routerOptions);
router.get(
{
path: '/',
validate: false,
security: {
authc: { enabled: 'minimal', reason: 'test' },
authz: { enabled: false, reason: 'test' },
},
},
(context, req, res) => res.ok({ body: req.route })
);

// mocking to have `authRegistered` filed set to true
registerAuth((req, res, auth) => auth.authenticated({ state: { alpha: 'beta' } }));
registerRouter(router);

await server.start();
await supertest(innerServer.listener)
.get('/')
.expect(200, {
method: 'get',
path: '/',
routePath: '/',
options: {
authRequired: true,
xsrfRequired: false,
access: 'internal',
tags: [],
timeout: {},
security: {
authc: { enabled: 'minimal', reason: 'test' },
authz: { enabled: false, reason: 'test' },
},
},
Expand Down
5 changes: 3 additions & 2 deletions src/core/packages/http/server-internal/src/http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,11 +396,12 @@ export class HttpServer {
}

private getAuthOption(
authRequired: RouteConfigOptions<any>['authRequired'] = true
authRequired: RouteConfigOptions<any>['authRequired'] | 'minimal' = true
): undefined | false | { mode: 'required' | 'try' } {
if (this.authRegistered === false) return undefined;

if (authRequired === true) {
// Minimal authentication still should go through the authentication handler.
if (authRequired === true || authRequired === 'minimal') {
return { mode: 'required' };
}
if (authRequired === 'optional') {
Expand Down
2 changes: 2 additions & 0 deletions src/core/packages/http/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ export type {
RouteAuthc,
AuthcDisabled,
AuthcEnabled,
AuthcMinimal,
AuthcOptional,
Privilege,
PrivilegeSet,
AllRequiredCondition,
Expand Down
2 changes: 2 additions & 0 deletions src/core/packages/http/server/src/router/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ export type {
RouteAuthc,
AuthcDisabled,
AuthcEnabled,
AuthcMinimal,
AuthcOptional,
RouteSecurity,
AllRequiredCondition,
AnyRequiredCondition,
Expand Down
29 changes: 23 additions & 6 deletions src/core/packages/http/server/src/router/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,12 +242,28 @@ export interface AuthzDisabled {
}

/**
* Describes the authentication status when authentication is enabled.
*
* - `enabled`: A boolean or string indicating the authentication status. Can be `true` (authentication required) or `'optional'` (authentication is optional).
* Describes the authentication status when authentication is enabled (default).
*/
export interface AuthcEnabled {
enabled: true | 'optional';
enabled: true;
}

/**
* Describes the authentication status when authentication is switched to a minimal mode (only existence of credentials
* is checked). Requires an explicit reason explaining why full authentication can be deferred to Elasticsearch.
*/
export interface AuthcMinimal {
enabled: 'minimal';
Comment thread
rgodfrey-elastic marked this conversation as resolved.
reason: string;
}

/**
* Describes the authentication status when authentication is optional. Requires an explicit reason explaining why
* authentication is optional.
*/
export interface AuthcOptional {
enabled: 'optional';
reason: string;
}

/**
Expand All @@ -262,9 +278,10 @@ export interface AuthcDisabled {
}

/**
* Represents the authentication status for a route. It can either be enabled (`AuthcEnabled`) or disabled (`AuthcDisabled`).
* Represents the authentication status for a route. It can either be enabled (`AuthcEnabled`), minimal (`AuthcMinimal`),
* optional (`AuthcOptional`), or disabled (`AuthcDisabled`).
*/
export type RouteAuthc = AuthcEnabled | AuthcDisabled;
export type RouteAuthc = AuthcEnabled | AuthcMinimal | AuthcOptional | AuthcDisabled;

/**
* Represents the authorization status for a route. It can either be enabled (`AuthzEnabled`) or disabled (`AuthzDisabled`).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,14 @@ describe('registerBootstrapRoute', () => {
expect(router.get).toHaveBeenNthCalledWith(
2,
expect.objectContaining({
security: {
authc: { enabled: 'optional', reason: expect.any(String) },
authz: { enabled: false, reason: expect.any(String) },
},
options: {
access: 'public',
excludeFromRateLimiter: true,
tags: ['api'],
authRequired: 'optional',
},
}),
expect.any(Function)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,17 @@ export const registerBootstrapRoute = ({
{
path: '/bootstrap-anonymous.js',
security: {
authc: {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicitly specifying auth mode with a clear reason is a Huge improvement over the someone obscure optional auth, nice!

enabled: 'optional',
reason:
'Anonymous bootstrap script must be loadable on pages like the login page where the user is not yet authenticated',
},
authz: {
enabled: false,
reason: 'This route is only used for serving the bootstrap script.',
},
},
options: {
authRequired: 'optional',
tags: ['api'],
access: 'public',
excludeFromRateLimiter: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,17 @@ export const registerStatusRoute = ({
{
path: '/api/status',
security: {
authc: {
enabled: 'optional',
reason:
'Status endpoint must be accessible by unauthenticated system users such as k8s readiness probes',
},
authz: {
enabled: false,
reason: 'Status route should be accessible without authorization.',
},
},
options: {
authRequired: 'optional',
// The `api` tag ensures that unauthenticated calls receive a 401 rather than a 302 redirect to login page.
// The `security:acceptJWT` tag allows route to be accessed with JWT credentials. It points to
// ROUTE_TAG_ACCEPT_JWT from '@kbn/security-plugin/server' that cannot be imported here directly.
Expand Down
Loading
Loading