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

Add support for deprecated roles #57209

Merged
merged 14 commits into from Mar 3, 2020

Conversation

legrego
Copy link
Member

@legrego legrego commented Feb 10, 2020

Summary

Updates the User, Role, and Role Mappings management pages to warn about the use of deprecated roles.

Resolves #25722

Dashboard Only Mode Roles

Deprecates the advanced setting:
image

User Grid Page

image

Edit User Page

image

image

Role Grid Page

image

View Deprecated Role

image

Role Mappings Grid Page

image

Edit Role Mapping Page

image

@legrego legrego force-pushed the security/support-deprecated-roles branch from f32c39c to e5141b7 Compare February 10, 2020 20:02
@legrego legrego force-pushed the security/support-deprecated-roles branch from e5141b7 to 58a25fa Compare February 11, 2020 16:04
@legrego legrego added Feature:Security/Authorization Platform Security - Authorization release_note:enhancement Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v7.7.0 v8.0.0 labels Feb 12, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

@legrego legrego marked this pull request as ready for review February 12, 2020 12:58
@legrego legrego requested review from a team as code owners February 12, 2020 12:58
/**
* Returns whether given role is editable through the UI or not.
*
* @param role the Role as returned by roles API
*/
export function isReadOnlyRole(role: Partial<Role>): boolean {
return isReservedRole(role) || (role._transform_error?.length ?? 0) > 0;
export function isRoleReadOnly(role: Partial<Role>): boolean {
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 Just a rename to be more consistent with the other functions in this module

@@ -14,11 +14,11 @@ import { PersonalInfo } from './personal_info';

interface Props {
authc: AuthenticationServiceSetup;
apiClient: PublicMethodsOf<UserAPIClient>;
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: just a rename to make the client instance clearer, since this component now requires two API clients

@@ -120,6 +120,7 @@ export class DocLinksService {
},
management: {
kibanaSearchSettings: `${ELASTIC_WEBSITE_URL}guide/en/kibana/${DOC_LINK_VERSION}/advanced-options.html#kibana-search-settings`,
dashboardSettings: `${ELASTIC_WEBSITE_URL}guide/en/kibana/${DOC_LINK_VERSION}/advanced-options.html#kibana-dashboard-settings`,
Copy link
Member Author

Choose a reason for hiding this comment

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

Added to support the Deprecated badge for the xpackDashboardMode:roles Advanced Setting added in:

https://github.com/legrego/kibana/blob/58a25faaad88409fcda649dbb204c37d4b0a360b/x-pack/legacy/plugins/dashboard_mode/index.js#L36-L43

defaultMessage="Enable mapping"
/>
}
label={i18n.translate(
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 This just resolves an EUI warning where EuiSwitch's label must be a string when showLabel={false}.

Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

LGTM for platform changes

@legrego
Copy link
Member Author

legrego commented Feb 13, 2020

@elastic/kibana-security this is ready for review

@kobelb
Copy link
Contributor

kobelb commented Feb 14, 2020

ACK: reviewing

Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

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

Snapshot changes.
LGTM

@legrego
Copy link
Member Author

legrego commented Feb 25, 2020

@elasticmachine merge upstream

@legrego legrego requested a review from kobelb February 25, 2020 20:00
@legrego
Copy link
Member Author

legrego commented Feb 27, 2020

@elasticmachine merge upstream

@legrego
Copy link
Member Author

legrego commented Feb 27, 2020

@elasticmachine merge upstream

@legrego
Copy link
Member Author

legrego commented Feb 28, 2020

@elasticmachine merge upstream

@legrego
Copy link
Member Author

legrego commented Feb 28, 2020

@kobelb this is ready for another pass. Flaky test result appears unrelated to any changes here.

@gchaps
Copy link
Contributor

gchaps commented Mar 2, 2020

Here are some suggestions to tighten up the text a bit:

Tooltip
The kibana_user role is deprecated. Use kibana_admin instead.

Roles
This user is assigned a deprecated role. Please migrate to a supported role.

Feature privileges
This role is deprecated. Configure a custom role to grant access to the Dashboard feature.

Mapping
This mapping is assigned a deprecated role. Please migrate to a supported role.

@legrego
Copy link
Member Author

legrego commented Mar 3, 2020

Thanks @gchaps!

I'd like to make the copy for the "Tooltip" and "Feature privileges" suggestions match, as they're currently generated using a shared mechanism.

Would you be ok replacing

This role is deprecated. Configure a custom role to grant access to the Dashboard feature.

with

The kibana_dashboard_only_user role is deprecated. Configure a custom role to grant access to the Dashboard feature.
?

More generically, the message looks like this:

The {roleName} role is deprecated. {reason}.

Where {roleName} is the name of the deprecated role, and {reason} is the second half of the explanation, which is the part provided by Elasticsearch.

…o/kibana into security/support-deprecated-roles
@gchaps
Copy link
Contributor

gchaps commented Mar 3, 2020

@legrego good idea to make the text match. I edited the tooltip to match our marketing guidelines--we prefer to use Dashboard without "feature" or "app". Also, it makes the text shorter.

The kibana_dashboard_only_user role is deprecated. Configure a custom role to grant access to Dashboard.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@legrego legrego merged commit 74030c9 into elastic:master Mar 3, 2020
@legrego legrego deleted the security/support-deprecated-roles branch March 3, 2020 18:23
legrego added a commit to legrego/kibana that referenced this pull request Mar 3, 2020
* Add support for deprecated roles

* address PR feedback

* remove unused import

* copy edits

* fix snapshots

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@legrego
Copy link
Member Author

legrego commented Mar 3, 2020

Opened corresponding Elasticsearch PR to update deprecation reasons: elastic/elasticsearch#53074

legrego added a commit that referenced this pull request Mar 3, 2020
* Add support for deprecated roles

* address PR feedback

* remove unused import

* copy edits

* fix snapshots

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Security/Authorization Platform Security - Authorization release_note:enhancement Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce kibana_admin role, deprecate kibana_user and kibana_dashboard_only_userroles
8 participants