Skip to content

Commit

Permalink
[Roles] Added transformation of application * privilege to all (#181400)
Browse files Browse the repository at this point in the history
## Summary

Added transformation of application wildcard `*` privilege to `all` to
correctly filter and display roles as `superuser`.

<details>
  <summary>[Screenshot] Kibana superuser role before change</summary>
 
<img width="1508" alt="Screenshot 2024-04-23 at 11 20 31"
src="https://github.com/elastic/kibana/assets/165678770/594cf27c-64a7-49cf-bf4b-c63adca76a55">

</details>

<details>
  <summary>[Screenshot] Kibana superuser role after change</summary>
<img width="1504" alt="Screenshot 2024-04-23 at 11 17 54"
src="https://github.com/elastic/kibana/assets/165678770/96dbdaa6-f5c9-4644-82ca-f88a0f6a15b3">
</details>

### Checklist

- [X] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios


### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

__Fixes: https://github.com/elastic/kibana/issues/106561__

## Release note
Added transformation of application wildcard `*` privilege to `all` to
correctly filter and display roles as `superuser`.

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
elena-shostak and kibanamachine committed Apr 30, 2024
1 parent e6dcdc7 commit 4023f92
Show file tree
Hide file tree
Showing 15 changed files with 193 additions and 22 deletions.
5 changes: 5 additions & 0 deletions x-pack/plugins/security/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ export const UNKNOWN_SPACE = '?';

export const APPLICATION_PREFIX = 'kibana-';

/**
* The wildcard identifier for all application privileges.
*/
export const PRIVILEGES_ALL_WILDCARD = '*';

/**
* Reserved application privileges are always assigned to this "wildcard" application.
* This allows them to be applied to any Kibana "tenant" (`kibana.index`). Since reserved privileges are always assigned to reserved (built-in) roles,
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export type {
InvalidRoleTemplate,
InlineRoleTemplate,
} from './model';
export { getUserDisplayName, isRoleReserved } from './model';
export { getUserDisplayName, isRoleReserved, isRoleWithWildcardBasePrivilege } from './model';

// Re-export types from the plugin directly to enhance the developer experience for consumers of the Security plugin.
export type {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/security/common/model/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export {
isRoleEnabled,
prepareRoleClone,
getExtendedRoleDeprecationNotice,
isRoleWithWildcardBasePrivilege,
} from './role';
export type {
InlineRoleTemplate,
Expand Down
19 changes: 17 additions & 2 deletions x-pack/plugins/security/common/model/role.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ export function isRoleSystem(role: Partial<Role>) {
*/
export function isRoleAdmin(role: Partial<Role>) {
return (
(isRoleReserved(role) && (role.name?.endsWith('_admin') || role.name === 'superuser')) ?? false
((isRoleReserved(role) && (role.name?.endsWith('_admin') || role.name === 'superuser')) ||
isRoleWithWildcardBasePrivilege(role)) ??
false
);
}

Expand All @@ -73,13 +75,26 @@ export function getExtendedRoleDeprecationNotice(role: Partial<Role>) {
});
}

/**
* Returns whether given role is editable through the UI or not.
*
* @param role the Role as returned by roles API
*/
export function isRoleWithWildcardBasePrivilege(role: Partial<Role>): boolean {
return role.kibana?.some((entry) => entry.base.includes('*')) ?? false;
}

/**
* Returns whether given role is editable through the UI or not.
*
* @param role the Role as returned by roles API
*/
export function isRoleReadOnly(role: Partial<Role>): boolean {
return isRoleReserved(role) || (role._transform_error?.length ?? 0) > 0;
return (
isRoleReserved(role) ||
isRoleWithWildcardBasePrivilege(role) ||
(role._transform_error?.length ?? 0) > 0
);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,30 @@ describe('<EditRolePage />', () => {
expectSaveFormButtons(wrapper);
});

it('render role with wildcard base privilege without edit/delete actions', async () => {
const wrapper = mountWithIntl(
<KibanaContextProvider services={coreStart}>
<EditRolePage
{...getProps({
action: 'edit',
role: {
name: 'my custom role',
metadata: {},
elasticsearch: { cluster: ['all'], indices: [], run_as: ['*'] },
kibana: [{ spaces: ['*'], base: ['*'], feature: {} }],
},
})}
/>
</KibanaContextProvider>
);

await waitForRender(wrapper);

expect(wrapper.find('[data-test-subj="privilegeEditAction-0"]')).toHaveLength(0);
expect(wrapper.find('[data-test-subj="privilegeDeleteAction-0"]')).toHaveLength(0);
expectReadOnlyFormButtons(wrapper);
});

describe('in create mode', () => {
it('renders an error for existing role name', async () => {
const props = getProps({ action: 'edit' });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -915,4 +915,30 @@ describe('PrivilegeFormCalculator', () => {
expect(calculator.hasSupersededInheritedPrivileges(0)).toEqual(false);
});
});

describe('#isWildcardBasePrivilege', () => {
it('returns true for the base privilege with wildcard', () => {
const kibanaPrivileges = createKibanaPrivileges(kibanaFeatures);
const role = createRole([
{
base: ['*'],
feature: {
with_sub_features: ['all'],
},
spaces: ['foo'],
},
{
base: ['all'],
feature: {
with_sub_features: ['read'],
},
spaces: ['*'],
},
]);

const calculator = new PrivilegeFormCalculator(kibanaPrivileges, role);

expect(calculator.isWildcardBasePrivilege(0)).toEqual(true);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,21 @@ export class PrivilegeFormCalculator {
public getBasePrivilege(privilegeIndex: number) {
const entry = this.role.kibana[privilegeIndex];
const basePrivileges = this.kibanaPrivileges.getBasePrivileges(entry);

return basePrivileges.find((bp) => entry.base.includes(bp.id));
}

/**
* Returns true if it is base wildcard (*) privilege.
*
* @param privilegeIndex the index of the kibana privileges role component
*/
public isWildcardBasePrivilege(privilegeIndex: number) {
const entry = this.role.kibana[privilegeIndex];

return entry.base.includes('*');
}

/**
* Returns the ID of the *displayed* Primary Feature Privilege for the indicated feature and privilege index.
* If the effective primary feature privilege is a "minimal" version, then this returns the corresponding non-minimal version.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,15 @@ describe('only global', () => {
]);
});

it('base *', () => {
const props = buildProps([{ spaces: ['*'], base: ['*'], feature: {} }]);
const component = mountWithIntl(<PrivilegeSpaceTable {...props} />);
const actualTable = getTableFromComponent(component);
expect(actualTable).toEqual([
{ spaces: ['*'], privileges: { summary: '*', overridden: false } },
]);
});

it('base read', () => {
const props = buildProps([{ spaces: ['*'], base: ['read'], feature: {} }]);
const component = mountWithIntl(<PrivilegeSpaceTable {...props} />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,14 @@ export class PrivilegeSpaceTable extends Component<Props, State> {
);
}

const basePrivilege =
privilegeCalculator.getBasePrivilege(record.privilegeIndex)?.id ??
CUSTOM_PRIVILEGE_VALUE;

const privilege = privilegeCalculator.isWildcardBasePrivilege(record.privilegeIndex)
? '*'
: basePrivilege;

let icon = <EuiIcon type="empty" size="s" />;
if (privilegeCalculator.hasSupersededInheritedPrivileges(record.privilegeIndex)) {
icon = (
Expand All @@ -202,13 +210,7 @@ export class PrivilegeSpaceTable extends Component<Props, State> {
<EuiFlexGroup gutterSize="xs" alignItems="center">
<EuiFlexItem grow={false}>{icon}</EuiFlexItem>
<EuiFlexItem>
<PrivilegeDisplay
privilege={
privilegeCalculator.getBasePrivilege(record.privilegeIndex)?.id ??
CUSTOM_PRIVILEGE_VALUE
}
data-test-subj={`privilegeColumn`}
/>
<PrivilegeDisplay privilege={privilege} data-test-subj={`privilegeColumn`} />
</EuiFlexItem>
</EuiFlexGroup>
);
Expand All @@ -234,6 +236,7 @@ export class PrivilegeSpaceTable extends Component<Props, State> {
color={'primary'}
iconType={'pencil'}
onClick={() => this.props.onEdit(record.privilegeIndex)}
data-test-subj={`privilegeEditAction-${record.privilegeIndex}`}
/>
);
},
Expand All @@ -252,6 +255,7 @@ export class PrivilegeSpaceTable extends Component<Props, State> {
color={'danger'}
iconType={'trash'}
onClick={() => this.onDeleteSpacePrivilege(record)}
data-test-subj={`privilegeDeleteAction-${record.privilegeIndex}`}
/>
);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,27 @@ describe('<SpaceAwarePrivilegeSection>', () => {
expect(wrapper.find('button[data-test-subj="addSpacePrivilegeButton"]')).toHaveLength(0);
});

it('hides privilege buttons if role has a base wildcard privilege', () => {
const props = buildProps({
role: {
elasticsearch: {
cluster: ['manage'],
},
kibana: [
{
spaces: ['*'],
base: ['*'],
feature: {},
},
],
},
});

const wrapper = mountWithIntl(<SpaceAwarePrivilegeSection {...props} />);
expect(wrapper.find('button[data-test-subj="addSpacePrivilegeButton"]')).toHaveLength(0);
expect(wrapper.find('button[data-test-subj="privilegeSummaryButton"]')).toHaveLength(0);
});

it('Renders flyout after clicking "Add space privilege" button', () => {
const props = buildProps({
role: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import type { Space, SpacesApiUi } from '@kbn/spaces-plugin/public';
import { PrivilegeSpaceForm } from './privilege_space_form';
import { PrivilegeSpaceTable } from './privilege_space_table';
import type { Role } from '../../../../../../../common';
import { isRoleReserved } from '../../../../../../../common';
import { isRoleReserved, isRoleWithWildcardBasePrivilege } from '../../../../../../../common';
import type { KibanaPrivileges } from '../../../../model';
import type { RoleValidator } from '../../../validate_role';
import { PrivilegeFormCalculator } from '../privilege_form_calculator';
Expand Down Expand Up @@ -188,7 +188,10 @@ export class SpaceAwarePrivilegeSection extends Component<Props, State> {
const hasAvailableSpaces = this.getAvailableSpaces().length > 0;

// This shouldn't happen organically...
if (!hasAvailableSpaces && !hasPrivilegesAssigned) {
if (
(!hasAvailableSpaces && !hasPrivilegesAssigned) ||
isRoleWithWildcardBasePrivilege(this.props.role)
) {
return null;
}

Expand Down Expand Up @@ -219,6 +222,7 @@ export class SpaceAwarePrivilegeSection extends Component<Props, State> {
kibanaPrivileges={this.props.kibanaPrivileges}
canCustomizeSubFeaturePrivileges={this.props.canCustomizeSubFeaturePrivileges}
spacesApiUi={this.props.spacesApiUi}
data-test-subj={'privilegeSummaryButton'}
/>
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import { PrivilegeSerializer } from './privilege_serializer';

describe(`#isSerializedGlobalBasePrivilege`, () => {
['all', 'read'].forEach((validValue) => {
['all', 'read', '*'].forEach((validValue) => {
test(`returns true for '${validValue}'`, () => {
expect(PrivilegeSerializer.isSerializedGlobalBasePrivilege(validValue)).toBe(true);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
* 2.0.
*/

import { PRIVILEGES_ALL_WILDCARD } from '../../common/constants';

const featurePrefix = 'feature_';
const spacePrefix = 'space_';
const reservedPrefix = 'reserved_';
const basePrivilegeNames = ['all', 'read'];
const basePrivilegeNames = ['all', 'read', PRIVILEGES_ALL_WILDCARD];
const globalBasePrivileges = [...basePrivilegeNames];
const spaceBasePrivileges = basePrivilegeNames.map(
(privilegeName) => `${spacePrefix}${privilegeName}`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,4 +289,39 @@ describe('#transformElasticsearchRoleToRole', () => {
{ name: 'default-malformed', _transform_error: ['kibana'] },
]
);

it('#When application privilege is set to * return it correctly', () => {
const role = {
name: 'global-all',
cluster: [],
indices: [],
applications: [
{
application: '*',
privileges: ['*'],
resources: ['*'],
},
],
run_as: [],
metadata: {},
transient_metadata: {
enabled: true,
},
};

const transformedRole = transformElasticsearchRoleToRole(
featuresWithRequireAllSpaces,
omit(role, 'name'),
role.name,
'kibana-.kibana',
loggerMock.create()
);

const [privilege] = transformedRole.kibana;
const [basePrivilege] = privilege.base;
const [spacePrivilege] = privilege.spaces;

expect(basePrivilege).toBe('*');
expect(spacePrivilege).toBe('*');
});
});

0 comments on commit 4023f92

Please sign in to comment.