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

Migrate authentication subsystem to the new platform. #39446

Merged
merged 22 commits into from
Jul 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
4df8d89
Temporary Core workarounds.
azasypkin Jul 3, 2019
ba674e9
Move files to NP Security Plugin.
azasypkin Jul 3, 2019
02dbfdb
Fix references.
azasypkin Jul 3, 2019
b3abda4
Migrate to the New Platform.
azasypkin Jul 3, 2019
051a4d0
Review#1: remove unused `loginAttempt` from provider iterator, rely m…
azasypkin Jul 9, 2019
dc9ce76
Merge branch 'master' into issue-xxx-np-authc
azasypkin Jul 10, 2019
f5795d3
Integrate latest core changes: isTlsEnabled and get rid of legacy ES …
azasypkin Jul 10, 2019
9171e43
Merge branch 'master' into issue-xxx-np-authc
azasypkin Jul 10, 2019
a8b237e
Revert `deepFreeze` changes and rely on `src/core/utils`.
azasypkin Jul 10, 2019
411af06
Merge branch 'master' into issue-xxx-np-authc
azasypkin Jul 11, 2019
70e726d
Review#2: do not mutate injectedVars in onInit. Integrate latest upst…
azasypkin Jul 11, 2019
4b737fb
Merge branch 'master' into issue-xxx-np-authc
azasypkin Jul 11, 2019
85ae758
Use mocks provided by the Core.
azasypkin Jul 12, 2019
63c3eac
Expect ElasticsearchError instead of Boom errors as 401 Cluster clien…
azasypkin Jul 12, 2019
41774d4
Merge branch 'master' into issue-xxx-np-authc
azasypkin Jul 12, 2019
e09958c
Simplify session handling for `login`.
azasypkin Jul 15, 2019
a20b939
Merge branch 'master' into issue-xxx-np-authc
azasypkin Jul 15, 2019
99606ef
Merge branch 'master' into issue-xxx-np-authc
azasypkin Jul 16, 2019
07507cd
Review#3: properly handle session updates for `login`, remove redunda…
azasypkin Jul 16, 2019
8980946
Do not clear session on login if it does not exist.
azasypkin Jul 16, 2019
653b34a
Merge branch 'master' into issue-xxx-np-authc
azasypkin Jul 16, 2019
d8fdb13
Merge branch 'master' into issue-xxx-np-authc
azasypkin Jul 19, 2019
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
2 changes: 1 addition & 1 deletion src/test_utils/kbn_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export function createRootWithSettings(
repl: false,
basePath: false,
optimize: false,
oss: false,
oss: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

note: NP x-pack plugins are picked up by Kibana if oss is true and that's not what we want by default when we run test servers in OSS.

...cliArgs,
},
isDevClusterMaster: false,
Expand Down
54 changes: 0 additions & 54 deletions x-pack/legacy/plugins/security/__snapshots__/index.test.js.snap

This file was deleted.

7 changes: 5 additions & 2 deletions x-pack/legacy/plugins/security/common/model/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ export { Role, RoleIndexPrivilege, RoleKibanaPrivilege } from './role';
export { FeaturesPrivileges } from './features_privileges';
export { RawKibanaPrivileges, RawKibanaFeaturePrivileges } from './raw_kibana_privileges';
export { KibanaPrivileges } from './kibana_privileges';
export { User, EditUser, getUserDisplayName } from './user';
export { AuthenticatedUser, canUserChangePassword } from './authenticated_user';
export { User, EditUser, getUserDisplayName } from '../../../../../plugins/security/common/model';
Copy link
Member Author

Choose a reason for hiding this comment

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

note: To minimize number of changes I temporarily re-export "user" models.

export {
AuthenticatedUser,
canUserChangePassword,
} from '../../../../../plugins/security/common/model';
export { BuiltinESPrivileges } from './builtin_es_privileges';
4 changes: 0 additions & 4 deletions x-pack/legacy/plugins/security/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,12 @@

import { Legacy } from 'kibana';
import { AuthenticatedUser } from './common/model';
import { AuthenticationResult, DeauthenticationResult } from './server/lib/authentication';
import { AuthorizationService } from './server/lib/authorization/service';

/**
* Public interface of the security plugin.
*/
export interface SecurityPlugin {
authorization: Readonly<AuthorizationService>;
authenticate: (request: Legacy.Request) => Promise<AuthenticationResult>;
deauthenticate: (request: Legacy.Request) => Promise<DeauthenticationResult>;
getUser: (request: Legacy.Request) => Promise<AuthenticatedUser>;
Copy link
Member Author

Choose a reason for hiding this comment

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

note: there is still a bunch of places outside of security plugin where getUser is used. We can consider migrating them to NP security setup contract instead in follow-ups.

isAuthenticated: (request: Legacy.Request) => Promise<boolean>;
}
81 changes: 32 additions & 49 deletions x-pack/legacy/plugins/security/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/

import { resolve } from 'path';
import { getUserProvider } from './server/lib/get_user';
import { initAuthenticateApi } from './server/routes/api/v1/authenticate';
import { initUsersApi } from './server/routes/api/v1/users';
import { initExternalRolesApi } from './server/routes/api/external/roles';
Expand All @@ -16,10 +15,7 @@ import { initOverwrittenSessionView } from './server/routes/views/overwritten_se
import { initLoginView } from './server/routes/views/login';
import { initLogoutView } from './server/routes/views/logout';
import { initLoggedOutView } from './server/routes/views/logged_out';
import { validateConfig } from './server/lib/validate_config';
import { authenticateFactory } from './server/lib/auth_redirect';
import { checkLicense } from './server/lib/check_license';
import { initAuthenticator } from './server/lib/authentication/authenticator';
import { SecurityAuditLogger } from './server/lib/audit_logger';
import { AuditLogger } from '../../server/lib/audit_logger';
import {
Expand All @@ -34,6 +30,7 @@ import { watchStatusAndLicenseToInitialize } from '../../server/lib/watch_status
import { SecureSavedObjectsClientWrapper } from './server/lib/saved_objects_client/secure_saved_objects_client_wrapper';
import { deepFreeze } from './server/lib/deep_freeze';
import { createOptionalPlugin } from '../../server/lib/optional_plugin';
import { KibanaRequest } from '../../../../src/core/server';

export const security = (kibana) => new kibana.Plugin({
id: 'security',
Expand All @@ -42,23 +39,12 @@ export const security = (kibana) => new kibana.Plugin({
require: ['kibana', 'elasticsearch', 'xpack_main'],

config(Joi) {
const providerOptionsSchema = (providerName, schema) => Joi.any()
.when('providers', {
is: Joi.array().items(Joi.string().valid(providerName).required(), Joi.string()),
then: schema,
otherwise: Joi.any().forbidden(),
});

return Joi.object({
Copy link
Member Author

Choose a reason for hiding this comment

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

note: potentially we could move entire config schema to NP in this PR and expose values we need in LP through setup contract and remove them as soon as we fully migrate to NP, but I didn't do that since we don't have a proper config deprecations support in NP yet (required for authorization.legacyFallback.enabled deprecation transformation).

enabled: Joi.boolean().default(true),
cookieName: Joi.string().default('sid'),
encryptionKey: Joi.when(Joi.ref('$dist'), {
is: true,
then: Joi.string(),
otherwise: Joi.string().default('a'.repeat(32)),
}),
sessionTimeout: Joi.number().allow(null).default(null),
secureCookies: Joi.boolean().default(false),
cookieName: Joi.any().description('This key is handled in the new platform security plugin ONLY'),
encryptionKey: Joi.any().description('This key is handled in the new platform security plugin ONLY'),
sessionTimeout: Joi.any().description('This key is handled in the new platform security plugin ONLY'),
secureCookies: Joi.any().description('This key is handled in the new platform security plugin ONLY'),
authorization: Joi.object({
legacyFallback: Joi.object({
enabled: Joi.boolean().default(true) // deprecated
Expand All @@ -67,11 +53,7 @@ export const security = (kibana) => new kibana.Plugin({
audit: Joi.object({
enabled: Joi.boolean().default(false)
}).default(),
authc: Joi.object({
providers: Joi.array().items(Joi.string()).default(['basic']),
oidc: providerOptionsSchema('oidc', Joi.object({ realm: Joi.string().required() }).required()),
saml: providerOptionsSchema('saml', Joi.object({ realm: Joi.string().required() }).required()),
}).default()
authc: Joi.any().description('This key is handled in the new platform security plugin ONLY')
}).default();
},

Expand Down Expand Up @@ -112,15 +94,18 @@ export const security = (kibana) => new kibana.Plugin({
'plugins/security/hacks/on_unauthorized_response'
],
home: ['plugins/security/register_feature'],
injectDefaultVars: function (server) {
const config = server.config();
injectDefaultVars: (server) => {
const securityPlugin = server.newPlatform.setup.plugins.security;
if (!securityPlugin) {
throw new Error('New Platform XPack Security plugin is not available.');
}

return {
secureCookies: config.get('xpack.security.secureCookies'),
sessionTimeout: config.get('xpack.security.sessionTimeout'),
enableSpaceAwarePrivileges: config.get('xpack.spaces.enabled'),
secureCookies: securityPlugin.config.secureCookies,
sessionTimeout: securityPlugin.config.sessionTimeout,
enableSpaceAwarePrivileges: server.config().get('xpack.spaces.enabled'),
};
}
},
},

async postInit(server) {
Expand All @@ -138,28 +123,29 @@ export const security = (kibana) => new kibana.Plugin({
},

async init(server) {
const plugin = this;
const securityPlugin = server.newPlatform.setup.plugins.security;
if (!securityPlugin) {
throw new Error('New Platform XPack Security plugin is not available.');
}

const config = server.config();
const xpackMainPlugin = server.plugins.xpack_main;
const xpackInfo = xpackMainPlugin.info;
securityPlugin.registerLegacyAPI({
xpackInfo,
isSystemAPIRequest: server.plugins.kibana.systemApi.isSystemApiRequest.bind(
Copy link
Member Author

Choose a reason for hiding this comment

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

question: @restrry wondering if Core should eventually take systemApi over from Kibana plugin?

Copy link
Contributor

@mshustov mshustov Jul 8, 2019

Choose a reason for hiding this comment

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

seems so. as other meta-flags like kibana-xsrf, I will create an issue to track

Copy link
Contributor

Choose a reason for hiding this comment

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

added to #39330

server.plugins.kibana.systemApi
),
});

const plugin = this;
const config = server.config();
const xpackInfoFeature = xpackInfo.feature(plugin.id);

// Register a function that is called whenever the xpack info changes,
// to re-compute the license check results for this plugin
xpackInfoFeature.registerLicenseCheckResultsGenerator(checkLicense);

validateConfig(config, message => server.log(['security', 'warning'], message));

// Create a Hapi auth scheme that should be applied to each request.
server.auth.scheme('login', () => ({ authenticate: authenticateFactory(server) }));

server.auth.strategy('session', 'login');

// The default means that the `session` strategy that is based on `login` schema defined above will be
// automatically assigned to all routes that don't contain an auth config.
server.auth.default('session');
server.expose({ getUser: request => securityPlugin.authc.getCurrentUser(KibanaRequest.from(request)) });

const { savedObjects } = server;

Expand Down Expand Up @@ -203,20 +189,17 @@ export const security = (kibana) => new kibana.Plugin({
return client;
});

getUserProvider(server);

await initAuthenticator(server);
initAuthenticateApi(server);
initAuthenticateApi(securityPlugin, server);
initAPIAuthorization(server, authorization);
initAppAuthorization(server, xpackMainPlugin, authorization);
initUsersApi(server);
initUsersApi(securityPlugin, server);
initExternalRolesApi(server);
initIndicesApi(server);
initPrivilegesApi(server);
initGetBuiltinPrivilegesApi(server);
initLoginView(server, xpackMainPlugin);
initLoginView(securityPlugin, server, xpackMainPlugin);
initLogoutView(server);
initLoggedOutView(server);
initLoggedOutView(securityPlugin, server);
initOverwrittenSessionView(server);

server.injectUiAppVars('login', () => {
Expand Down
116 changes: 0 additions & 116 deletions x-pack/legacy/plugins/security/index.test.js

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { EuiFieldText } from '@elastic/eui';
import { ReactWrapper } from 'enzyme';
import React from 'react';
import { mountWithIntl } from 'test_utils/enzyme_helpers';
import { User } from '../../../../common/model/user';
import { User } from '../../../../common/model';
import { UserAPIClient } from '../../../lib/api';
import { ChangePasswordForm } from './change_password_form';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
import React, { ChangeEvent, Component } from 'react';
import { toastNotifications } from 'ui/notify';
import { User } from '../../../../common/model/user';
import { User } from '../../../../common/model';
import { UserAPIClient } from '../../../lib/api';

interface Props {
Expand Down
Loading