POC to migrate Code to use Feature Controls #35093
Conversation
legrego
added
:Code
Feature:Security/Feature Controls
|
Pinging @elastic/kibana-security |
| isCodeAdmin: false, | ||
| isCodeUser: false, | ||
| isCodeAdmin: Boolean(uiCapabilities.code.admin), | ||
| isCodeUser: Boolean(uiCapabilities.code.user), | ||
| }; | ||
|
|
||
| export const userProfile = handleActions( |
There was a problem hiding this comment.
It's likely that you don't need any of these userProfile actions anymore. I left it in place because I didn't feel comfortable refactoring. All you essentially need is the initialState above, however you wish to incorporate that into the application.
| icon: 'codeApp', | ||
| navLinkId: 'code', | ||
| app: ['code', 'kibana'], | ||
| catalogue: [], |
There was a problem hiding this comment.
If you add an entry to the Feature Catalogue, then you should add the ID of that entry to this array. This catalogue is what populates the Kibana Home page with the various features that users can enjoy.
| catalogue: [], | ||
| privileges: { | ||
| all: { | ||
| api: ['code_user', 'code_admin'], |
There was a problem hiding this comment.
This grants users with the all privilege the ability to execute any API endpoints that are tagged with either access:code_user or access:code_admin
| ui: ['show', 'user', 'admin'], | ||
| }, | ||
| read: { | ||
| api: ['code_user'], |
There was a problem hiding this comment.
This grants users with the all privilege the ability to execute any API endpoints that are tagged with access:code_user.
| @@ -66,7 +62,7 @@ async function checkWithRoles(server: any, roles: any) { | |||
| return response; | |||
| } | |||
|
|
|||
| it('should response 403 when roles check failed', async () => { | |||
| it.skip('should response 403 when roles check failed', async () => { | |||
There was a problem hiding this comment.
Skipping this test, but this entire suite should be revisited.
| const routeOptions: RouteOptions = (route.options || {}) as RouteOptions; | ||
| routeOptions.tags = [ | ||
| ...(routeOptions.tags || []), | ||
| `access:code_${route.requireAdmin ? 'admin' : 'user'}`, |
There was a problem hiding this comment.
Tags the route with either access:code_user or access:code_admin depending on if requireAdmin is set on the route.
| .callWithRequest.bind(null, req); | ||
| // FIXME: I swapped out `callWithRequest` for `callWithInternalUser` to test and verify functionality, but you'll want to refactor this since | ||
| // the names no longer make sense. | ||
| this.callWithRequest = req.server.plugins.elasticsearch.getCluster('data').callWithInternalUser; |
There was a problem hiding this comment.
Important
In order to test my changes, I simply changed the call being made here from callWithRequest to callWithInternalUser. I assume that we want this in most places, but I'm not sure if there are legitimate cases for callWithRequest -- Do you need to query any user indices, or are you only querying .code* indices?
Either way, this will need refactoring, since it is no longer "WithRequest".
A couple other notes:
- The
kibana_systemrole has access to.code-*. Are there any other indices it needs access to in order for Code to function correctly? - The
code_adminandcode_userroles that were introduced here will need to be removed. They are no longer needed, and we shouldn't confuse users with those options.
There was a problem hiding this comment.
Update: We should not blindly use callWithInternalUser. Instead, Code should check if the Kibana security plugin is available, and if it is, use its exposed authorization mode to determine what to do:
const securityPlugin = server.plugins.security;
if (securityPlugin) {
const useRbac = securityPlugin.authorization.mode.useRbacForRequest(request);
if (useRbac) {
// use `callWithInternalUser`
} else {
// use `callWithRequest`
}
}
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
|
I'm going to continue the work here: #35115 |
|
Sounds good, thanks! I’ll close this then. |
Summary
This is a rough POC which removes Code's custom security handling in favor of Feature Controls:
Added Code as a feature via
xpack_mainfeature registrya) Defined
allprivilege as the existing "Code Admin"b) Defined
readprivilege as the existing "Code User"Removed explicit role checks:
a) Rather than checking for
code_adminorcode_userroles, each route that needs securing is now tagged with one of two access tags:access:code_useroraccess:code_admin. These tags are used by the security plugin to automatically authorize access to these routes. These tags are registered as part of the feature registry in step 1, which is how the security plugin knows how to authorize them.Updated client-side state to use
uiCapabilitiesPreviously, the client-side state was requesting the details of the authenticated user, and determining if they were a code admin or a code user. Now, this logic is automatically handled by
uiCapabilities, so the state was updated to read from these capabilities instead.Replaced
callWithRequestwithcallWithInternalUserNow that Code is using Feature Controls, the end-users should not be calling Elasticsearch directly when interacting with the
.code*indices. Instead, the Kibana server should be connecting on their behalf.Note: This will need additional work by the Code team. I'll point out areas that I'm aware of that need attention, but there may be other areas too.