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

[Enterprise Search] Require security plugin in 8.0 #106307

Merged
merged 5 commits into from
Jul 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions x-pack/plugins/enterprise_search/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
"id": "enterpriseSearch",
"version": "kibana",
"kibanaVersion": "kibana",
"requiredPlugins": ["features", "spaces", "licensing", "data", "charts", "infra"],
"requiredPlugins": ["features", "spaces", "security", "licensing", "data", "charts", "infra"],
"configPath": ["enterpriseSearch"],
"optionalPlugins": ["usageCollection", "security", "home", "cloud"],
"optionalPlugins": ["usageCollection", "home", "cloud"],
"server": true,
"ui": true,
"requiredBundles": ["home", "kibanaReact"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,6 @@ describe('KibanaLogic', () => {
expect(KibanaLogic.values.config).toEqual({});
});

it('gracefully handles disabled security', () => {
mountKibanaLogic({ ...mockKibanaValues, security: undefined } as any);

expect(KibanaLogic.values.security).toEqual({});
});

it('gracefully handles non-cloud installs', () => {
mountKibanaLogic({ ...mockKibanaValues, cloud: undefined } as any);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,13 @@ interface KibanaLogicProps {
renderHeaderActions(HeaderActions: FC): void;
// Required plugins
charts: ChartsPluginStart;
security: SecurityPluginStart;
// Optional plugins
cloud?: CloudSetup;
security?: SecurityPluginStart;
}
export interface KibanaValues extends Omit<KibanaLogicProps, 'cloud' | 'security'> {
export interface KibanaValues extends Omit<KibanaLogicProps, 'cloud'> {
navigateToUrl(path: string, options?: CreateHrefOptions): Promise<void>;
cloud: Partial<CloudSetup>;
security: Partial<SecurityPluginStart>;
}

export const KibanaLogic = kea<MakeLogicType<KibanaValues>>({
Expand All @@ -54,7 +53,7 @@ export const KibanaLogic = kea<MakeLogicType<KibanaValues>>({
},
{},
],
security: [props.security || {}, {}],
security: [props.security, {}],
setBreadcrumbs: [props.setBreadcrumbs, {}],
setChromeIsVisible: [props.setChromeIsVisible, {}],
setDocTitle: [props.setDocTitle, {}],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ export const AccountSettings: React.FC = () => {
const [currentUser, setCurrentUser] = useState<AuthenticatedUser | null>(null);

useEffect(() => {
security!.authc!.getCurrentUser().then(setCurrentUser);
security.authc.getCurrentUser().then(setCurrentUser);
}, [security.authc]);

const PersonalInfo = useMemo(() => security!.uiApi!.components.getPersonalInfo, [security.uiApi]);
const ChangePassword = useMemo(() => security!.uiApi!.components.getChangePassword, [
const PersonalInfo = useMemo(() => security.uiApi.components.getPersonalInfo, [security.uiApi]);
const ChangePassword = useMemo(() => security.uiApi.components.getChangePassword, [
security.uiApi,
]);

Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/enterprise_search/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ export interface ClientData extends InitialAppData {
interface PluginsSetup {
cloud?: CloudSetup;
home?: HomePublicPluginSetup;
security?: SecurityPluginSetup;
security: SecurityPluginSetup;
}
export interface PluginsStart {
cloud?: CloudSetup;
licensing: LicensingPluginStart;
charts: ChartsPluginStart;
data: DataPublicPluginStart;
security?: SecurityPluginStart;
security: SecurityPluginStart;
}

export class EnterpriseSearchPlugin implements Plugin {
Expand Down
45 changes: 24 additions & 21 deletions x-pack/plugins/enterprise_search/server/lib/check_access.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,30 @@ describe('checkAccess', () => {
spaces: mockSpaces,
} as any;

describe('when security is disabled', () => {
it('should deny all access', async () => {
const security = {
authz: { mode: { useRbacForRequest: () => false } },
};
expect(await checkAccess({ ...mockDependencies, security })).toEqual({
hasAppSearchAccess: false,
hasWorkplaceSearchAccess: false,
});
});
});

describe('when the current request is unauthenticated', () => {
it('should deny all access', async () => {
const request = {
auth: { isAuthenticated: false },
};
expect(await checkAccess({ ...mockDependencies, request })).toEqual({
hasAppSearchAccess: false,
hasWorkplaceSearchAccess: false,
});
});
});

describe('when the space is disabled', () => {
it('should deny all access', async () => {
mockSpaces.spacesService.getActiveSpace.mockResolvedValueOnce(disabledSpace);
Expand All @@ -63,17 +87,6 @@ describe('checkAccess', () => {
});

describe('when the Spaces plugin is unavailable', () => {
describe('when security is disabled', () => {
it('should allow all access', async () => {
const spaces = undefined;
const security = undefined;
expect(await checkAccess({ ...mockDependencies, spaces, security })).toEqual({
hasAppSearchAccess: true,
hasWorkplaceSearchAccess: true,
});
});
});

describe('when getActiveSpace returns 403 forbidden', () => {
it('should deny all access', async () => {
mockSpaces.spacesService.getActiveSpace.mockReturnValueOnce(
Expand Down Expand Up @@ -105,16 +118,6 @@ describe('checkAccess', () => {
mockSpaces.spacesService.getActiveSpace.mockResolvedValueOnce(enabledSpace);
});

describe('when security is disabled', () => {
it('should allow all access', async () => {
const security = undefined;
expect(await checkAccess({ ...mockDependencies, security })).toEqual({
hasAppSearchAccess: true,
hasWorkplaceSearchAccess: true,
});
});
});

describe('when the user is a superuser', () => {
it('should allow all access when enabled at the space ', async () => {
const security = {
Expand Down
30 changes: 11 additions & 19 deletions x-pack/plugins/enterprise_search/server/lib/check_access.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { callEnterpriseSearchConfigAPI } from './enterprise_search_config_api';

interface CheckAccess {
request: KibanaRequest;
security?: SecurityPluginSetup;
security: SecurityPluginSetup;
spaces: SpacesPluginStart;
config: ConfigType;
log: Logger;
Expand All @@ -43,21 +43,18 @@ export const checkAccess = async ({
request,
log,
}: CheckAccess): Promise<ProductAccess> => {
const isRbacEnabled = security?.authz.mode.useRbacForRequest(request) ?? false;
const isRbacEnabled = security.authz.mode.useRbacForRequest(request);

// We can only retrieve the active space when either:
// 1) security is enabled, and the request has already been authenticated
// 2) security is disabled
const attemptSpaceRetrieval = !isRbacEnabled || request.auth.isAuthenticated;
// If security has been disabled, always hide the plugin
if (!isRbacEnabled) {
return DENY_ALL_PLUGINS;
}

// If we can't retrieve the current space, then assume the feature is available
// We can only retrieve the active space when security is enabled and the request has already been authenticated
const attemptSpaceRetrieval = request.auth.isAuthenticated;
let allowedAtSpace = false;

if (!spaces) {
allowedAtSpace = true;
}

if (spaces && attemptSpaceRetrieval) {
if (attemptSpaceRetrieval) {
try {
const space = await spaces.spacesService.getActiveSpace(request);
allowedAtSpace = !space.disabledFeatures?.includes('enterpriseSearch');
Expand All @@ -75,17 +72,12 @@ export const checkAccess = async ({
return DENY_ALL_PLUGINS;
}

// If security has been disabled, always show the plugin
if (!isRbacEnabled) {
return ALLOW_ALL_PLUGINS;
}

// If the user is a "superuser" or has the base Kibana all privilege globally, always show the plugin
const isSuperUser = async (): Promise<boolean> => {
try {
const { hasAllRequested } = await security!.authz
const { hasAllRequested } = await security.authz
.checkPrivilegesWithRequest(request)
.globally({ kibana: security!.authz.actions.ui.get('enterpriseSearch', 'all') });
.globally({ kibana: security.authz.actions.ui.get('enterpriseSearch', 'all') });
return hasAllRequested;
} catch (err) {
if (err.statusCode === 401 || err.statusCode === 403) {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/enterprise_search/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ import { ConfigType } from './';

interface PluginsSetup {
usageCollection?: UsageCollectionSetup;
security?: SecurityPluginSetup;
security: SecurityPluginSetup;
features: FeaturesPluginSetup;
infra: InfraPluginSetup;
}
Expand Down