Skip to content
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.

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

Prepare the Security domain HTTP APIs for Serverless #162087

Merged
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
71 commits
Select commit Hold shift + click to select a range
f7ba619
Disables spaces public APIs in serverless
jeramysoucy Jul 17, 2023
5203a0b
Updates tests and snapshots
jeramysoucy Jul 17, 2023
08146f0
Fixes more tests
jeramysoucy Jul 18, 2023
c10abc7
Merge branch 'main' into disable-spaces-public-api-in-serverless
jeramysoucy Jul 19, 2023
dc727cd
Merge branch 'main' into disable-spaces-public-api-in-serverless
jeramysoucy Jul 20, 2023
70f06f7
Uses build flavor to determine correct API access. Adds tests for int…
jeramysoucy Jul 24, 2023
8b0c93d
Fixes unit tests for access-configurable endpoints.
jeramysoucy Jul 24, 2023
7a346b6
Merge branch 'main' into disable-spaces-public-api-in-serverless
jeramysoucy Jul 24, 2023
04f1c92
Reformatting
jeramysoucy Jul 24, 2023
6c53a54
Update to rely on default access in serverless.
jeramysoucy Jul 25, 2023
4f20016
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Jul 25, 2023
a44549d
Merge branch 'main' into disable-spaces-public-api-in-serverless
jeramysoucy Jul 25, 2023
8f20ee0
Merge branch 'main' into disable-spaces-public-api-in-serverless
jeramysoucy Jul 26, 2023
994654c
Disables security and encrypted saved objects APIs for serverless
jeramysoucy Jul 26, 2023
aa8c78e
Adds authentication API access tests
jeramysoucy Jul 27, 2023
e4f60db
Merge branch 'main' into disable-spaces-public-api-in-serverless
jeramysoucy Jul 27, 2023
131a771
Adds API access tests for security and encrypted SOs
jeramysoucy Jul 27, 2023
e3bb1be
Merge remote-tracking branch 'upstream/main' into disable-spaces-publ…
jeramysoucy Jul 27, 2023
8bb3ca2
Merge branch 'main' into disable-spaces-public-api-in-serverless
jeramysoucy Jul 28, 2023
074c0cc
Removes unnecessary comments
jeramysoucy Aug 1, 2023
2997135
Merge remote-tracking branch 'upstream/main' into disable-spaces-publ…
jeramysoucy Aug 1, 2023
78da626
Merge branch 'main' into disable-spaces-public-api-in-serverless
jeramysoucy Aug 1, 2023
ee8a12c
Merge branch 'main' into disable-spaces-public-api-in-serverless
jeramysoucy Aug 2, 2023
a366b54
Merge branch 'main' into disable-spaces-public-api-in-serverless
jeramysoucy Aug 4, 2023
6cb21ba
Merge branch 'main' into disable-spaces-public-api-in-serverless
jeramysoucy Aug 4, 2023
252cd72
Merge branch 'main' into disable-spaces-public-api-in-serverless
jeramysoucy Aug 7, 2023
d0b2364
Update x-pack/test_serverless/api_integration/services/svl_common_api.ts
jeramysoucy Aug 7, 2023
452ad5a
Adresses initial review feedback
jeramysoucy Aug 7, 2023
d1c3bdf
Additional comments for build flavor conditions
jeramysoucy Aug 7, 2023
a63d67b
Merge branch 'main' into disable-spaces-public-api-in-serverless
jeramysoucy Aug 7, 2023
14c00ae
Update x-pack/plugins/security/server/routes/authentication/common.ts
jeramysoucy Aug 8, 2023
9f0606d
Update x-pack/plugins/security/server/routes/authentication/common.ts
jeramysoucy Aug 8, 2023
ecb595c
Update x-pack/plugins/spaces/server/routes/api/external/index.ts
jeramysoucy Aug 8, 2023
4682542
Additional comment updates
jeramysoucy Aug 8, 2023
182273f
Merge branch 'main' into disable-spaces-public-api-in-serverless
jeramysoucy Aug 8, 2023
e7eec38
Temporarilty resolves failing test issues by registering the login en…
jeramysoucy Aug 8, 2023
0e43706
Adds basic authc http scheme to test config.
jeramysoucy Aug 8, 2023
5e54975
Fixes api key failing tests
jeramysoucy Aug 8, 2023
f69ff2f
Merge branch 'main' into disable-spaces-public-api-in-serverless
jeramysoucy Aug 8, 2023
8018d63
Temporarily allows access to login and role APIs. Skips user profiles…
jeramysoucy Aug 8, 2023
cf5dae0
Reverts temporary enabling of role APIs
jeramysoucy Aug 9, 2023
100ced7
Adds the basic authc provider to the base config temporarily.
jeramysoucy Aug 9, 2023
b94cc86
Removes override of HTTP schemes
jeramysoucy Aug 9, 2023
0e291c6
Merge branch 'main' into disable-spaces-public-api-in-serverless
jeramysoucy Aug 14, 2023
3608f85
Enables user and roles APIs to temporarily unblock cypress and UI tests
jeramysoucy Aug 15, 2023
b81b484
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Aug 15, 2023
3478460
Updates serverless authc config to be compatible with existing tests.
jeramysoucy Aug 15, 2023
24e21f4
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Aug 15, 2023
95b2f12
Uses existing user for user profile tests
jeramysoucy Aug 15, 2023
1edb353
Uses test user for user profile tests rather than admin user
jeramysoucy Aug 15, 2023
727b393
Enables login page routes.
jeramysoucy Aug 16, 2023
6ece943
Merge branch 'main' into disable-spaces-public-api-in-serverless
jeramysoucy Aug 16, 2023
761f23b
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Aug 16, 2023
f0fbfdf
Adds saml tools for saml login.
jeramysoucy Aug 16, 2023
65c23e4
Added ftr context for shared
jeramysoucy Aug 16, 2023
3c24c33
Update import order
jeramysoucy Aug 16, 2023
0d22303
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Aug 16, 2023
5c535d6
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Aug 16, 2023
a979e58
Moves saml tools to api integration
jeramysoucy Aug 16, 2023
051cf71
Removes reference to getSAMLResponse
jeramysoucy Aug 16, 2023
4ca4924
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Aug 16, 2023
305aa70
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Aug 16, 2023
ba78385
Moves saml tools back into shared.
jeramysoucy Aug 17, 2023
d2ee490
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Aug 17, 2023
c552a53
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Aug 17, 2023
084e495
Merge branch 'main' into pr-162087-apis
azasypkin Aug 21, 2023
d0292f8
Merge branch 'main' into pr-162087-apis
azasypkin Aug 22, 2023
e8b0bef
Merge branch 'main' into pr-162087-apis
azasypkin Aug 22, 2023
5e953fa
Review#1: fix typos, disable Spaces Update API, make cookie an intern…
azasypkin Aug 22, 2023
573a552
Review#1: move Saml Tools service to API integrations services as it …
azasypkin Aug 22, 2023
710ca09
Merge branch 'main' into disable-spaces-public-api-in-serverless
azasypkin Aug 22, 2023
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
3 changes: 3 additions & 0 deletions config/serverless.yml
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,6 @@ console.autocompleteDefinitions.endpointsAvailability: serverless

# Allow authentication via the Elasticsearch JWT realm with the `shared_secret` client authentication type.
elasticsearch.requestHeadersWhitelist: ["authorization", "es-client-authentication"]

# FOR LOCAL TESTING - AWAITING MERGE OF https://github.com/elastic/kibana/pull/162149
server.restrictInternalApis: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be present once #162149 is merged to main.

Copy link
Member

Choose a reason for hiding this comment

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

Aaaand, it has been merged!

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The revert was closed and not merged.

28 changes: 15 additions & 13 deletions x-pack/plugins/encrypted_saved_objects/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,19 +95,21 @@ export class EncryptedSavedObjectsPlugin
getStartServices: core.getStartServices,
});

defineRoutes({
router: core.http.createRouter(),
logger: this.initializerContext.logger.get('routes'),
encryptionKeyRotationService: Object.freeze(
new EncryptionKeyRotationService({
logger: this.logger.get('key-rotation-service'),
service,
getStartServices: core.getStartServices,
security: deps.security,
})
),
config,
});
if (this.initializerContext.env.packageInfo.buildFlavor !== 'serverless') {
jeramysoucy marked this conversation as resolved.
Show resolved Hide resolved
defineRoutes({
router: core.http.createRouter(),
logger: this.initializerContext.logger.get('routes'),
encryptionKeyRotationService: Object.freeze(
new EncryptionKeyRotationService({
logger: this.logger.get('key-rotation-service'),
service,
getStartServices: core.getStartServices,
security: deps.security,
})
),
config,
});
}

return {
canEncrypt,
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/security/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ export class SecurityPlugin
getAnonymousAccessService: this.getAnonymousAccess,
getUserProfileService: this.getUserProfileService,
analyticsService: this.analyticsService.setup({ analytics: core.analytics }),
buildFlavor: this.initializerContext.env.packageInfo.buildFlavor,
});

return Object.freeze<SecurityPluginSetup>({
Expand Down
127 changes: 69 additions & 58 deletions x-pack/plugins/security/server/routes/authentication/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,14 @@ export function defineCommonRoutes({
basePath,
license,
logger,
buildFlavor,
}: RouteDefinitionParams) {
// Generate two identical routes with new and deprecated URL and issue a warning if route with deprecated URL is ever used.
for (const path of ['/api/security/logout', '/api/security/v1/logout']) {
// For a serverless build, do not register deprecated versioned routes
for (const path of [
'/api/security/logout',
...(buildFlavor !== 'serverless' ? ['/api/security/v1/logout'] : []),
]) {
router.get(
{
path,
Expand Down Expand Up @@ -79,7 +84,11 @@ export function defineCommonRoutes({
}

// Generate two identical routes with new and deprecated URL and issue a warning if route with deprecated URL is ever used.
for (const path of ['/internal/security/me', '/api/security/v1/me']) {
// For a serverless build, do not register deprecated versioned routes
for (const path of [
'/internal/security/me',
...(buildFlavor !== 'serverless' ? ['/api/security/v1/me'] : []),
]) {
router.get(
{ path, validate: false },
createLicensedRouteHandler((context, request, response) => {
Expand Down Expand Up @@ -123,66 +132,68 @@ export function defineCommonRoutes({
return undefined;
}

router.post(
{
path: '/internal/security/login',
validate: {
body: schema.object({
providerType: schema.string(),
providerName: schema.string(),
currentURL: schema.string(),
params: schema.conditional(
schema.siblingRef('providerType'),
schema.oneOf([
schema.literal(BasicAuthenticationProvider.type),
schema.literal(TokenAuthenticationProvider.type),
]),
basicParamsSchema,
schema.never()
),
}),
if (buildFlavor !== 'serverless') {
Copy link
Member

Choose a reason for hiding this comment

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

note: We should provide developers with some escape hatch to log in to Kibana locally before we disable these APIs, otherwise they won't be able to use Kibana at all. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Providing something like this could solve the issue brought up about how many tests are written (logging in as a user with specific privileges). Do you have any ideas how we could do that?

router.post(
{
path: '/internal/security/login',
validate: {
body: schema.object({
providerType: schema.string(),
providerName: schema.string(),
currentURL: schema.string(),
params: schema.conditional(
schema.siblingRef('providerType'),
schema.oneOf([
schema.literal(BasicAuthenticationProvider.type),
schema.literal(TokenAuthenticationProvider.type),
]),
basicParamsSchema,
schema.never()
),
}),
},
options: { authRequired: false },
},
options: { authRequired: false },
},
createLicensedRouteHandler(async (context, request, response) => {
const { providerType, providerName, currentURL, params } = request.body;
logger.info(`Logging in with provider "${providerName}" (${providerType})`);

const redirectURL = parseNext(currentURL, basePath.serverBasePath);
const authenticationResult = await getAuthenticationService().login(request, {
provider: { name: providerName },
redirectURL,
value: getLoginAttemptForProviderType(providerType, redirectURL, params),
});

if (authenticationResult.redirected() || authenticationResult.succeeded()) {
return response.ok({
body: { location: authenticationResult.redirectURL || redirectURL },
headers: authenticationResult.authResponseHeaders,
createLicensedRouteHandler(async (context, request, response) => {
const { providerType, providerName, currentURL, params } = request.body;
logger.info(`Logging in with provider "${providerName}" (${providerType})`);

const redirectURL = parseNext(currentURL, basePath.serverBasePath);
const authenticationResult = await getAuthenticationService().login(request, {
provider: { name: providerName },
redirectURL,
value: getLoginAttemptForProviderType(providerType, redirectURL, params),
});
}

return response.unauthorized({
body: authenticationResult.error,
headers: authenticationResult.authResponseHeaders,
});
})
);

router.post(
{ path: '/internal/security/access_agreement/acknowledge', validate: false },
createLicensedRouteHandler(async (context, request, response) => {
// If license doesn't allow access agreement we shouldn't handle request.
if (!license.getFeatures().allowAccessAgreement) {
logger.warn(`Attempted to acknowledge access agreement when license doesn't allow it.`);
return response.forbidden({
body: { message: `Current license doesn't support access agreement.` },
if (authenticationResult.redirected() || authenticationResult.succeeded()) {
return response.ok({
body: { location: authenticationResult.redirectURL || redirectURL },
headers: authenticationResult.authResponseHeaders,
});
}

return response.unauthorized({
body: authenticationResult.error,
headers: authenticationResult.authResponseHeaders,
});
}
})
);

router.post(
{ path: '/internal/security/access_agreement/acknowledge', validate: false },
createLicensedRouteHandler(async (context, request, response) => {
// If license doesn't allow access agreement we shouldn't handle request.
if (!license.getFeatures().allowAccessAgreement) {
logger.warn(`Attempted to acknowledge access agreement when license doesn't allow it.`);
return response.forbidden({
body: { message: `Current license doesn't support access agreement.` },
});
}

await getAuthenticationService().acknowledgeAccessAgreement(request);
await getAuthenticationService().acknowledgeAccessAgreement(request);

return response.noContent();
})
);
return response.noContent();
})
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ export function defineAuthenticationRoutes(params: RouteDefinitionParams) {
defineSAMLRoutes(params);
}

if (params.config.authc.sortedProviders.some(({ type }) => type === 'oidc')) {
if (
params.buildFlavor !== 'serverless' &&
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not necessary to make a build flavor check here because OIDC is not configured in serverless, but should have one anyway for good measure?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure, I'd probably just keep params.config.authc.sortedProviders.some(({ type }) => type === 'oidc') as we detect based on the capability already. But I don't have a strong opinion, up to you.

params.config.authc.sortedProviders.some(({ type }) => type === 'oidc')
) {
defineOIDCRoutes(params);
}
}
7 changes: 6 additions & 1 deletion x-pack/plugins/security/server/routes/authentication/saml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,14 @@ export function defineSAMLRoutes({
getAuthenticationService,
basePath,
logger,
buildFlavor,
}: RouteDefinitionParams) {
// Generate two identical routes with new and deprecated URL and issue a warning if route with deprecated URL is ever used.
for (const path of ['/api/security/saml/callback', '/api/security/v1/saml']) {
// For a serverless build, do not register deprecated versioned routes
for (const path of [
'/api/security/saml/callback',
...(buildFlavor !== 'serverless' ? ['/api/security/v1/saml'] : []),
]) {
router.post(
{
path,
Expand Down
25 changes: 15 additions & 10 deletions x-pack/plugins/security/server/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import type { Observable } from 'rxjs';

import type { BuildFlavor } from '@kbn/config/src/types';
import type { HttpResources, IBasePath, Logger } from '@kbn/core/server';
import type { KibanaFeature } from '@kbn/features-plugin/server';
import type { PublicMethodsOf } from '@kbn/utility-types';
Expand Down Expand Up @@ -54,20 +55,24 @@ export interface RouteDefinitionParams {
getUserProfileService: () => UserProfileServiceStartInternal;
getAnonymousAccessService: () => AnonymousAccessServiceStart;
analyticsService: AnalyticsServiceSetup;
buildFlavor: BuildFlavor;
}

export function defineRoutes(params: RouteDefinitionParams) {
defineAnalyticsRoutes(params);
defineApiKeysRoutes(params);
defineAuthenticationRoutes(params);
defineAuthorizationRoutes(params);
defineSessionManagementRoutes(params);
defineApiKeysRoutes(params);
defineIndicesRoutes(params);
defineUsersRoutes(params);
defineUserProfileRoutes(params);
defineRoleMappingRoutes(params);
defineViewRoutes(params);
defineDeprecationsRoutes(params);
defineAnonymousAccessRoutes(params);
defineSecurityCheckupGetStateRoutes(params);
defineAnalyticsRoutes(params);

if (params.buildFlavor !== 'serverless') {
defineAnonymousAccessRoutes(params);
defineAuthorizationRoutes(params);
jeramysoucy marked this conversation as resolved.
Show resolved Hide resolved
defineDeprecationsRoutes(params);
defineIndicesRoutes(params);
defineRoleMappingRoutes(params);
defineSecurityCheckupGetStateRoutes(params);
jeramysoucy marked this conversation as resolved.
Show resolved Hide resolved
defineUserProfileRoutes(params);
jeramysoucy marked this conversation as resolved.
Show resolved Hide resolved
defineUsersRoutes(params);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,8 @@ import { defineInvalidateSessionsRoutes } from './invalidate';
export function defineSessionManagementRoutes(params: RouteDefinitionParams) {
defineSessionInfoRoutes(params);
defineSessionExtendRoutes(params);
defineInvalidateSessionsRoutes(params);

if (params.buildFlavor !== 'serverless') {
jeramysoucy marked this conversation as resolved.
Show resolved Hide resolved
defineInvalidateSessionsRoutes(params);
}
}
28 changes: 14 additions & 14 deletions x-pack/plugins/security/server/routes/views/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ describe('View routes', () => {
expect(routeParamsMock.httpResources.register.mock.calls.map(([{ path }]) => path))
.toMatchInlineSnapshot(`
Array [
"/security/access_agreement",
"/security/account",
"/internal/security/capture-url",
"/security/logged_out",
"/logout",
"/security/overwritten_session",
"/internal/security/capture-url",
"/security/access_agreement",
]
`);
expect(routeParamsMock.router.get.mock.calls.map(([{ path }]) => path)).toMatchInlineSnapshot(`
Expand All @@ -44,19 +44,19 @@ describe('View routes', () => {
expect(routeParamsMock.httpResources.register.mock.calls.map(([{ path }]) => path))
.toMatchInlineSnapshot(`
Array [
"/login",
"/security/access_agreement",
"/security/account",
"/internal/security/capture-url",
"/security/logged_out",
"/logout",
"/security/overwritten_session",
"/internal/security/capture-url",
"/security/access_agreement",
"/login",
]
`);
expect(routeParamsMock.router.get.mock.calls.map(([{ path }]) => path)).toMatchInlineSnapshot(`
Array [
"/internal/security/login_state",
"/internal/security/access_agreement/state",
"/internal/security/login_state",
]
`);
});
Expand All @@ -71,19 +71,19 @@ describe('View routes', () => {
expect(routeParamsMock.httpResources.register.mock.calls.map(([{ path }]) => path))
.toMatchInlineSnapshot(`
Array [
"/login",
"/security/access_agreement",
"/security/account",
"/internal/security/capture-url",
"/security/logged_out",
"/logout",
"/security/overwritten_session",
"/internal/security/capture-url",
"/security/access_agreement",
"/login",
]
`);
expect(routeParamsMock.router.get.mock.calls.map(([{ path }]) => path)).toMatchInlineSnapshot(`
Array [
"/internal/security/login_state",
"/internal/security/access_agreement/state",
"/internal/security/login_state",
]
`);
});
Expand All @@ -98,19 +98,19 @@ describe('View routes', () => {
expect(routeParamsMock.httpResources.register.mock.calls.map(([{ path }]) => path))
.toMatchInlineSnapshot(`
Array [
"/login",
"/security/access_agreement",
"/security/account",
"/internal/security/capture-url",
"/security/logged_out",
"/logout",
"/security/overwritten_session",
"/internal/security/capture-url",
"/security/access_agreement",
"/login",
]
`);
expect(routeParamsMock.router.get.mock.calls.map(([{ path }]) => path)).toMatchInlineSnapshot(`
Array [
"/internal/security/login_state",
"/internal/security/access_agreement/state",
"/internal/security/login_state",
]
`);
});
Expand Down
Loading