Add built-in user and role for code plugin

[spacedragon]

This pr add a code_system as a reserved role, and grant index privilege .code-* to both code_system and kibana_system.

[elasticmachine]

Pinging @elastic/es-security

[zfy0701]

@spalger @polyfractal could you help reviewing this PR?

[tvernum]

Why are we creating a new role, but not a new user? How will the code_system role be used?

[spacedragon]

@tvernum By default, the kibana user has sufficient permissions to manage code plugins, but if a customer wants to grant other users permission to manage code, they can assign code_system to them.

[polyfractal]

@tvernum would you mind taking the review on this? Not sure I know enough about security to intelligently review past syntax/style :)

[tvernum]

I will review, but I'll be out for the next couple of days, so I won't get to it until mid next week.

It would be helpful if you could provide some background about how you intend for this to work. I can review the change at a purely technical level, but in order to understand whether it's the right change to be making I'll need to understand what these .code* indices are used for and which users/applications read/write to them.
At the moment I'm confused by the fact that we're changing the kibana_system user (which the Kibana backend uses) and creating a code_admin user (which seems to be intended for real people). They seem very different and I'm not sure how they fit together.

Also, before I can approve this, we'll need corresponding changes to ReservedRolesStoreTests.

[zfy0701]

so we are trying to follow how beats/apm/monitoring security works in Kibana

first, the kibana_system role would have read/write access to all code-* indices because it's the root role. since it has access to monitoring/reporting indices so we assume we should do the same. There is some background process we run inside kibana that need to read/write to code-* indices.

then, for users that are not root user but need to have permission to manage code, they need to be granted code_admin role that read/writes code indices, this is similar to beats_admin I assume

last, for normal users, they need to be granted read access to all code-* indices, we initially planned to create a code_user role, but it's seems beats and apm don't do that, so we just follow the pattern.

Please let us known if we have misunderstood how beats/apm works in Kibana, it would be super helpful.

[tvernum]

Thanks for the explanation @zfy0701.

Is .code-* really the index pattern you want for the admin role? I don't know your backend implementation so it's hard for me to tell, but it surprises me that an admin would be expected to have write access to all of the code-search indices. Wouldn't they typically be storing parsed representations of the code - do you expect admins to need to modify that by hand? (Bear in mind that in the case of a major issue, someone can create their own role, or login as a superuser). I'm not objecting, it just seems a little off.

I think you do want a code_user role if you are going to have code_admin.
While other apps might not have one, it does seems like it would be a better user-experience to add it.

[spacedragon]

added a code_user role

[tvernum]

Thanks @spacedragon.
I'm happy with the change - we just need to update ReservedRolesStoreTests to have tests for these 2 new roles that mimic the existing tests. It should be self-explanatory, but ping me if you need more info.

[spacedragon]

@tvernum I added those unit tests, but It seems the ci jobs are failed. I don't know if these failures are related to my changes.

[tvernum]

Those failures don't look related.

@elasticmachine run gradle build tests 1 ; run gradle build tests 2

[tvernum]

@spacedragon I fixed a couple of tests, we'll see how CI goes.

[spacedragon]

@tvernum Thanks a lot for your help, could I merge this pr now?

[tvernum]

@spacedragon We need to get the build green before we can merge. I haven't been checking the most recent failures because Elasticsearch CI has had a bunch of internal problems, but they should be resolved now.
If this build fails we'll need to look at why that's happening.

[spacedragon]

It's all green now.

[tvernum]

@spacedragon Feel free to merge this.

[tvernum]

@spacedragon I've labelled this on the assumption that you will want to backport this to 6.7
Let me know if you need assistance with that.

[spacedragon]

@tvernum Thanks.
@zfy0701 What's the requirement of elasticsearch for our first code release? I think it's 7.0.0, right?

[tvernum]

I dropped the 6.7.0 label since it didn't get backported. Hopefully you only wanted this on 7.0

[legrego]

I would like to partially revert this PR. With the introduction of Kibana Feature Controls (and Code's support for it), the reserved code_admin and code_user roles are no longer necessary.
Is is possible to remove reserve roles in a minor release?

As far as I can tell, those roles aren't doing anything for customers today, as the corresponding code app isn't available yet. So if we remove this role, we shouldn't be revoking any access that users are relying on today.

@elastic/codesearch do you agree, or am I overlooking anything here?

[zfy0701]

I would like to partially revert this PR. With the introduction of Kibana Feature Controls (and Code's support for it), the reserved code_admin and code_user roles are no longer necessary.
Is is possible to remove reserve roles in a minor release?

As far as I can tell, those roles aren't doing anything for customers today, as the corresponding code app isn't available yet. So if we remove this role, we shouldn't be revoking any access that users are relying on today.

@elastic/codesearch do you agree, or am I overlooking anything here?

agree, I don't think we need them