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

Register "minimal" feature privileges regardless of the current license level #115992

Merged
merged 10 commits into from
Oct 26, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ describe('FeatureTable', () => {
});
});

it('renders with no privileges granted when minimal feature privileges are assigned, and sub-feature privileges are disallowed', () => {
it('renders with privileges granted when minimal feature privileges are assigned, and sub-feature privileges are disallowed', () => {
const role = createRole([
{
spaces: ['foo'],
Expand Down Expand Up @@ -710,13 +710,13 @@ describe('FeatureTable', () => {
subFeaturePrivileges: [],
},
with_sub_features: {
primaryFeaturePrivilege: 'none',
primaryFeaturePrivilege: 'all',
subFeaturePrivileges: [],
},
});
});

it('renders with no privileges granted when sub feature privileges are assigned, and sub-feature privileges are disallowed', () => {
it('renders with privileges granted when sub feature privileges are assigned, and sub-feature privileges are disallowed', () => {
const role = createRole([
{
spaces: ['foo'],
Expand Down Expand Up @@ -748,7 +748,7 @@ describe('FeatureTable', () => {
subFeaturePrivileges: [],
},
with_sub_features: {
primaryFeaturePrivilege: 'none',
primaryFeaturePrivilege: 'read',
subFeaturePrivileges: [],
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,13 +278,11 @@ export class PrivilegeFormCalculator {
.getMinimalFeaturePrivileges()
.find((mp) => mp.id === correspondingMinimalPrivilegeId)!;

// There are two cases where the minimal privileges aren't available:
// 1. The feature has no registered sub-features
// 2. Sub-feature privileges cannot be customized. When this is the case, the minimal privileges aren't registered with ES,
// There is only one case where the minimal privileges aren't available:
// 1. Sub-feature privileges cannot be customized. When this is the case, the minimal privileges aren't registered with ES,
// so they end up represented in the UI as an empty privilege. Empty privileges cannot be granted other privileges, so if we
// encounter a minimal privilege that isn't granted by it's correspending primary, then we know we've encountered this scenario.
const hasMinimalPrivileges =
XavierM marked this conversation as resolved.
Show resolved Hide resolved
feature.subFeatures.length > 0 && fp.grantsPrivilege(correspendingMinimalPrivilege);
const hasMinimalPrivileges = fp.grantsPrivilege(correspendingMinimalPrivilege);
return (
selectedFeaturePrivileges.includes(fp.id) ||
(hasMinimalPrivileges &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ describe('PrivilegeSummaryTable', () => {
with_sub_features: {
'default, space-1': {
hasCustomizedSubFeaturePrivileges: allowSubFeaturePrivileges,
primaryFeaturePrivilege: allowSubFeaturePrivileges ? 'Read' : 'None',
primaryFeaturePrivilege: 'Read',
...maybeExpectSubFeaturePrivileges(allowSubFeaturePrivileges, {
'Cool Sub Feature': [],
}),
Expand Down Expand Up @@ -693,7 +693,7 @@ describe('PrivilegeSummaryTable', () => {
with_sub_features: {
'*': {
hasCustomizedSubFeaturePrivileges: allowSubFeaturePrivileges,
primaryFeaturePrivilege: allowSubFeaturePrivileges ? 'Read' : 'None',
primaryFeaturePrivilege: 'Read',
...maybeExpectSubFeaturePrivileges(allowSubFeaturePrivileges, {
'Cool Sub Feature': ['All'],
}),
Expand Down Expand Up @@ -787,7 +787,7 @@ describe('PrivilegeSummaryTable', () => {
with_sub_features: {
'*': {
hasCustomizedSubFeaturePrivileges: allowSubFeaturePrivileges,
primaryFeaturePrivilege: allowSubFeaturePrivileges ? 'Read' : 'None',
primaryFeaturePrivilege: 'Read',
...maybeExpectSubFeaturePrivileges(allowSubFeaturePrivileges, {
'Cool Sub Feature': ['All'],
}),
Expand Down Expand Up @@ -859,7 +859,7 @@ describe('PrivilegeSummaryTable', () => {
},
'space-1, space-2': {
hasCustomizedSubFeaturePrivileges: allowSubFeaturePrivileges,
primaryFeaturePrivilege: allowSubFeaturePrivileges ? 'All' : 'None',
primaryFeaturePrivilege: 'All',
...maybeExpectSubFeaturePrivileges(allowSubFeaturePrivileges, {
'Cool Sub Feature': ['Cool toggle 2'],
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,10 @@ export class SecuredFeature extends KibanaFeature {
([id, privilege]) => new PrimaryFeaturePrivilege(id, privilege, actionMapping[id])
);

if (this.config.subFeatures?.length ?? 0 > 0) {
this.minimalPrimaryFeaturePrivileges = Object.entries(this.config.privileges || {}).map(
([id, privilege]) =>
new PrimaryFeaturePrivilege(`minimal_${id}`, privilege, actionMapping[`minimal_${id}`])
);
} else {
this.minimalPrimaryFeaturePrivileges = [];
}
this.minimalPrimaryFeaturePrivileges = Object.entries(this.config.privileges || {}).map(
([id, privilege]) =>
new PrimaryFeaturePrivilege(`minimal_${id}`, privilege, actionMapping[`minimal_${id}`])
);

this.securedSubFeatures =
this.config.subFeatures?.map((sf) => new SecuredSubFeature(sf, actionMapping)) ?? [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ describe('features', () => {
expect(actual).toHaveProperty('features.foo-feature', {
all: [actions.login, actions.version],
read: [actions.login, actions.version],
minimal_all: [actions.login, actions.version],
minimal_read: [actions.login, actions.version],
});
});

Expand Down Expand Up @@ -175,6 +177,8 @@ describe('features', () => {
expect(actual).toHaveProperty('features.foo', {
all: [...expectedAllPrivileges],
read: [...expectedReadPrivileges],
minimal_all: [...expectedAllPrivileges],
minimal_read: [...expectedReadPrivileges],
});
});

Expand Down Expand Up @@ -1627,7 +1631,7 @@ describe('subFeatures', () => {
});

describe(`when license does not allow sub features`, () => {
test(`should augment the primary feature privileges, and should not create minimal or sub-feature privileges`, () => {
test(`should augment the primary feature privileges, and should not create sub-feature privileges`, () => {
const features: KibanaFeature[] = [
new KibanaFeature({
id: 'foo',
Expand Down Expand Up @@ -1705,7 +1709,11 @@ describe('subFeatures', () => {
actions.ui.get('foo', 'sub-feature-ui'),
]);

expect(actual.features).not.toHaveProperty(`foo.minimal_all`);
expect(actual.features).toHaveProperty(`foo.minimal_all`, [
actions.login,
actions.version,
actions.ui.get('foo', 'foo'),
]);

expect(actual.features).toHaveProperty(`foo.read`, [
actions.login,
Expand All @@ -1730,7 +1738,11 @@ describe('subFeatures', () => {
actions.ui.get('foo', 'sub-feature-ui'),
]);

expect(actual.features).not.toHaveProperty(`foo.minimal_read`);
expect(actual.features).toHaveProperty(`foo.minimal_read`, [
actions.login,
actions.version,
actions.ui.get('foo', 'foo'),
]);

expect(actual).toHaveProperty('global.all', [
actions.login,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,18 @@ export function privilegesFactory(
];
}

if (allowSubFeaturePrivileges && feature.subFeatures?.length > 0) {
for (const featurePrivilege of featuresService.featurePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: false,
licenseHasAtLeast,
})) {
featurePrivileges[feature.id][`minimal_${featurePrivilege.privilegeId}`] = [
actions.login,
actions.version,
...uniq(featurePrivilegeBuilder.getActions(featurePrivilege.privilege, feature)),
];
}
for (const featurePrivilege of featuresService.featurePrivilegeIterator(feature, {
augmentWithSubFeaturePrivileges: false,
licenseHasAtLeast,
})) {
featurePrivileges[feature.id][`minimal_${featurePrivilege.privilegeId}`] = [
actions.login,
actions.version,
...uniq(featurePrivilegeBuilder.getActions(featurePrivilege.privilege, feature)),
];
}

if (allowSubFeaturePrivileges && feature.subFeatures?.length > 0) {
for (const subFeaturePrivilege of featuresService.subFeaturePrivilegeIterator(
feature,
licenseHasAtLeast
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export default function ({ getService }: FtrProviderContext) {
});

// Verify that privileges were re-registered.
const expectedBasicLicenseDiscoverPrivileges = ['all', 'read'];
const expectedBasicLicenseDiscoverPrivileges = ['all', 'read', 'minimal_all', 'minimal_read'];
const basicPrivileges = await supertest
.get('/api/security/privileges')
.set('kbn-xsrf', 'xxx')
Expand Down
38 changes: 19 additions & 19 deletions x-pack/test/api_integration/apis/security/privileges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,21 @@ export default function ({ getService }: FtrProviderContext) {
global: ['all', 'read'],
space: ['all', 'read'],
features: {
graph: ['all', 'read'],
savedObjectsTagging: ['all', 'read'],
canvas: ['all', 'read'],
maps: ['all', 'read'],
observabilityCases: ['all', 'read'],
fleet: ['all', 'read'],
actions: ['all', 'read'],
stackAlerts: ['all', 'read'],
ml: ['all', 'read'],
siem: ['all', 'read'],
uptime: ['all', 'read'],
securitySolutionCases: ['all', 'read'],
infrastructure: ['all', 'read'],
logs: ['all', 'read'],
apm: ['all', 'read'],
graph: ['all', 'read', 'minimal_all', 'minimal_read'],
savedObjectsTagging: ['all', 'read', 'minimal_all', 'minimal_read'],
canvas: ['all', 'read', 'minimal_all', 'minimal_read'],
maps: ['all', 'read', 'minimal_all', 'minimal_read'],
observabilityCases: ['all', 'read', 'minimal_all', 'minimal_read'],
fleet: ['all', 'read', 'minimal_all', 'minimal_read'],
actions: ['all', 'read', 'minimal_all', 'minimal_read'],
stackAlerts: ['all', 'read', 'minimal_all', 'minimal_read'],
ml: ['all', 'read', 'minimal_all', 'minimal_read'],
siem: ['all', 'read', 'minimal_all', 'minimal_read'],
uptime: ['all', 'read', 'minimal_all', 'minimal_read'],
securitySolutionCases: ['all', 'read', 'minimal_all', 'minimal_read'],
infrastructure: ['all', 'read', 'minimal_all', 'minimal_read'],
logs: ['all', 'read', 'minimal_all', 'minimal_read'],
apm: ['all', 'read', 'minimal_all', 'minimal_read'],
discover: [
'all',
'read',
Expand All @@ -56,10 +56,10 @@ export default function ({ getService }: FtrProviderContext) {
'url_create',
'store_search_session',
],
dev_tools: ['all', 'read'],
advancedSettings: ['all', 'read'],
indexPatterns: ['all', 'read'],
savedObjectsManagement: ['all', 'read'],
dev_tools: ['all', 'read', 'minimal_all', 'minimal_read'],
advancedSettings: ['all', 'read', 'minimal_all', 'minimal_read'],
indexPatterns: ['all', 'read', 'minimal_all', 'minimal_read'],
savedObjectsManagement: ['all', 'read', 'minimal_all', 'minimal_read'],
osquery: [
'all',
'read',
Expand Down
46 changes: 23 additions & 23 deletions x-pack/test/api_integration/apis/security/privileges_basic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,29 +20,29 @@ export default function ({ getService }: FtrProviderContext) {
// Roles are associated with these privileges, and we shouldn't be removing them in a minor version.
const expected = {
features: {
discover: ['all', 'read'],
visualize: ['all', 'read'],
dashboard: ['all', 'read'],
dev_tools: ['all', 'read'],
advancedSettings: ['all', 'read'],
indexPatterns: ['all', 'read'],
savedObjectsManagement: ['all', 'read'],
savedObjectsTagging: ['all', 'read'],
graph: ['all', 'read'],
maps: ['all', 'read'],
observabilityCases: ['all', 'read'],
canvas: ['all', 'read'],
infrastructure: ['all', 'read'],
logs: ['all', 'read'],
uptime: ['all', 'read'],
apm: ['all', 'read'],
osquery: ['all', 'read'],
ml: ['all', 'read'],
siem: ['all', 'read'],
securitySolutionCases: ['all', 'read'],
fleet: ['all', 'read'],
stackAlerts: ['all', 'read'],
actions: ['all', 'read'],
discover: ['all', 'read', 'minimal_all', 'minimal_read'],
visualize: ['all', 'read', 'minimal_all', 'minimal_read'],
dashboard: ['all', 'read', 'minimal_all', 'minimal_read'],
dev_tools: ['all', 'read', 'minimal_all', 'minimal_read'],
advancedSettings: ['all', 'read', 'minimal_all', 'minimal_read'],
indexPatterns: ['all', 'read', 'minimal_all', 'minimal_read'],
savedObjectsManagement: ['all', 'read', 'minimal_all', 'minimal_read'],
savedObjectsTagging: ['all', 'read', 'minimal_all', 'minimal_read'],
graph: ['all', 'read', 'minimal_all', 'minimal_read'],
maps: ['all', 'read', 'minimal_all', 'minimal_read'],
observabilityCases: ['all', 'read', 'minimal_all', 'minimal_read'],
canvas: ['all', 'read', 'minimal_all', 'minimal_read'],
infrastructure: ['all', 'read', 'minimal_all', 'minimal_read'],
logs: ['all', 'read', 'minimal_all', 'minimal_read'],
uptime: ['all', 'read', 'minimal_all', 'minimal_read'],
apm: ['all', 'read', 'minimal_all', 'minimal_read'],
osquery: ['all', 'read', 'minimal_all', 'minimal_read'],
ml: ['all', 'read', 'minimal_all', 'minimal_read'],
siem: ['all', 'read', 'minimal_all', 'minimal_read'],
securitySolutionCases: ['all', 'read', 'minimal_all', 'minimal_read'],
fleet: ['all', 'read', 'minimal_all', 'minimal_read'],
stackAlerts: ['all', 'read', 'minimal_all', 'minimal_read'],
actions: ['all', 'read', 'minimal_all', 'minimal_read'],
},
global: ['all', 'read'],
space: ['all', 'read'],
Expand Down