-
Notifications
You must be signed in to change notification settings - Fork 894
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
Read Only Tenant Mode #4498
Read Only Tenant Mode #4498
Changes from all commits
b5fd84b
34b6105
ad6bac0
39a08e6
8e34af2
3f7799a
95ed066
83dfe9a
edffd9d
e71d72f
97ffecf
1bcf651
27ae5b5
b02687d
5b921d9
e55caae
b64a6af
e1b0d8e
4899570
77d3e69
79da52d
75b755a
66100c0
7f5ad08
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
# Read-only Mode | ||
|
||
There are two distinct functionalities for "read-only" access in Dashboards. One of them is associated with roles and one is associated with tenants. Regarding the first one, the Dashboards Security plugin contains a feature of hiding all plugin navigation links except Dashboards and Visualizations when the logged-in user has a certain role (more about it in [Read-only Role](#read-only-role)). | ||
|
||
The second one is limiting Dashboards access rights via assigning a specific role to a tenant (therefore, making a tenant read-only). Due to past issues and the deprecation of the first functionality, using read-only tenants is now the recommended way to limit users' access to Dashboards. | ||
|
||
## Design | ||
|
||
Whenever a plugin registers capabilities that should be limited (in other words, set to false) for read-only tenants, such capabilities should be registered through `registerSwitcher` with using method `core.security.readonlyService().hideForReadonly()` | ||
|
||
### Example | ||
|
||
```ts | ||
public setup(core: CoreSetup) { | ||
core.capabilities.registerProvider({ | ||
myAwesomePlugin: { | ||
show: true, | ||
save: true, | ||
delete: true, | ||
} | ||
}); | ||
|
||
core.capabilities.registerSwitcher(async (request, capabilites) => { | ||
return await core.security.readonlyService().hideForReadonly(request, capabilites, { | ||
myAwesomePlugin: { | ||
save: false, | ||
delete: false, | ||
}, | ||
}); | ||
}); | ||
} | ||
``` | ||
|
||
In this case, we might assume that a plugin relies on the `save` and `delete` capabilities to limit changes somewhere in the UI. Therefore, those capabilities are processed through `registerSwitcher`, they will be set to `false` whenever a read-only tenant is accessed. | ||
|
||
If `registerSwitcher` will try to provide or remove capabilites when invoking the switcher will be ignored. | ||
|
||
*In case of a disabled / not installed `security` plugin changes will be never applied to a capabilites.* | ||
|
||
## Requirements | ||
|
||
This feature will only work if you have the [`security` plugin](https://github.com/opensearch-project/security) installed on your OpenSearch cluster with https/authentication enabled. | ||
|
||
## Read-only Role | ||
|
||
The role is called `kibana_read_only` by default, but the name can be changed using the dashboard config option `opensearch_security.readonly_mode.roles`. One big issue with this feature is that the backend site of a Dashboard Security plugin is completely unaware of it. Thus, users in this mode still have write access to the Dashboards saved objects via the API as the implementation effectively hides everything except the Dashboards and Visualization plugins. | ||
|
||
**We highly do not recommend using it!** | ||
|
||
For more context, see [this group issues of problems connected with read-only roles](https://github.com/opensearch-project/security/issues/2701). | ||
|
||
### Usage | ||
|
||
1. Go to `Management > Security > Internal users` | ||
2. Create or select an already existing user | ||
3. Add a new `Backend role` called `kibana_read_only` (or use name used in `opensearch_security.readonly_mode.roles`) | ||
4. Save changes | ||
|
||
## Read-only Tenant (recommended) | ||
|
||
Dashboards Security plugin recognizes the selection of read-only tenant after logging in and sets the capabilities associated with write access or showing write controls to false for a variety of plugins. This can be easily checked for example by trying to re-arrange some visualizations on Dashboards. Such action will be resulting in a 403 error due to limited read-only access. | ||
|
||
### Usage | ||
|
||
1. Prepare tenant: | ||
* Use an existing tenant or create a new one in `Management > Security > Tenants` | ||
2. Prepare role: | ||
* Go to `Management > Security > Roles` | ||
* Use an existing role or create a new one | ||
* Fill **index permissions** with: | ||
* `indices:data/read/search` | ||
* `indices:data/read/get` | ||
* Add new **tenant permission** with: | ||
* your name of the tenant | ||
* read only | ||
3. Assign a role to a user: | ||
* Go to role | ||
* Click the tab `Mapped users` | ||
* Click `Manage mapping` | ||
* In `Users` select the user that will be affected |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
import { OpenSearchDashboardsRequest } from '../index'; | ||
import { ReadonlyService } from './readonly_service'; | ||
import { httpServerMock } from '../http/http_server.mocks'; | ||
|
||
describe('ReadonlyService', () => { | ||
let readonlyService: ReadonlyService; | ||
let request: OpenSearchDashboardsRequest; | ||
|
||
beforeEach(() => { | ||
readonlyService = new ReadonlyService(); | ||
request = httpServerMock.createOpenSearchDashboardsRequest(); | ||
}); | ||
|
||
it('isReadonly returns false by default', () => { | ||
expect(readonlyService.isReadonly(request)).resolves.toBeFalsy(); | ||
}); | ||
|
||
it('hideForReadonly merges capabilites to hide', () => { | ||
readonlyService.isReadonly = jest.fn(() => new Promise(() => true)); | ||
const result = readonlyService.hideForReadonly( | ||
request, | ||
{ foo: { show: true } }, | ||
{ foo: { show: false } } | ||
); | ||
|
||
expect(readonlyService.isReadonly).toBeCalledTimes(1); | ||
expect(result).resolves.toEqual({ foo: { show: false } }); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
import { merge } from 'lodash'; | ||
import { OpenSearchDashboardsRequest, Capabilities } from '../index'; | ||
import { IReadOnlyService } from './types'; | ||
|
||
export class ReadonlyService implements IReadOnlyService { | ||
async isReadonly(request: OpenSearchDashboardsRequest): Promise<boolean> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ananzh purpose is to keep working even without |
||
return false; | ||
} | ||
|
||
async hideForReadonly( | ||
request: OpenSearchDashboardsRequest, | ||
capabilites: Partial<Capabilities>, | ||
hideCapabilities: Partial<Capabilities> | ||
): Promise<Partial<Capabilities>> { | ||
return (await this.isReadonly(request)) ? merge(capabilites, hideCapabilities) : capabilites; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
import { SecurityServiceSetup } from './types'; | ||
|
||
const createSetupContractMock = () => { | ||
const setupContract: jest.Mocked<SecurityServiceSetup> = { | ||
readonlyService: jest.fn(), | ||
registerReadonlyService: jest.fn(), | ||
}; | ||
return setupContract; | ||
}; | ||
|
||
export const securityServiceMock = { | ||
createSetupContract: createSetupContractMock, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
import { OpenSearchDashboardsRequest } from '../index'; | ||
import { mockCoreContext } from '../core_context.mock'; | ||
import { SecurityService } from './security_service'; | ||
import { httpServerMock } from '../http/http_server.mocks'; | ||
import { IReadOnlyService } from './types'; | ||
|
||
describe('SecurityService', () => { | ||
let securityService: SecurityService; | ||
let request: OpenSearchDashboardsRequest; | ||
|
||
beforeEach(() => { | ||
const coreContext = mockCoreContext.create(); | ||
securityService = new SecurityService(coreContext); | ||
request = httpServerMock.createOpenSearchDashboardsRequest(); | ||
}); | ||
|
||
afterEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
describe('#readonlyService', () => { | ||
it("uses core's readonly service by default", () => { | ||
const setupContext = securityService.setup(); | ||
expect(setupContext.readonlyService().isReadonly(request)).resolves.toBeFalsy(); | ||
}); | ||
|
||
it('registers custom readonly service and it uses it', () => { | ||
const setupContext = securityService.setup(); | ||
const readonlyServiceMock: jest.Mocked<IReadOnlyService> = { | ||
isReadonly: jest.fn(), | ||
hideForReadonly: jest.fn(), | ||
}; | ||
|
||
setupContext.registerReadonlyService(readonlyServiceMock); | ||
setupContext.readonlyService().isReadonly(request); | ||
|
||
expect(readonlyServiceMock.isReadonly).toBeCalledTimes(1); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
import { CoreService } from '../../types'; | ||
import { IReadOnlyService, InternalSecurityServiceSetup } from './types'; | ||
import { CoreContext } from '../core_context'; | ||
import { Logger } from '../logging'; | ||
import { ReadonlyService } from './readonly_service'; | ||
|
||
export class SecurityService implements CoreService<InternalSecurityServiceSetup> { | ||
private logger: Logger; | ||
private readonlyService: IReadOnlyService; | ||
|
||
constructor(coreContext: CoreContext) { | ||
this.logger = coreContext.logger.get('security-service'); | ||
this.readonlyService = new ReadonlyService(); | ||
} | ||
|
||
public setup() { | ||
this.logger.debug('Setting up Security service'); | ||
|
||
const securityService = this; | ||
|
||
return { | ||
registerReadonlyService(service: IReadOnlyService) { | ||
securityService.readonlyService = service; | ||
}, | ||
readonlyService() { | ||
return securityService.readonlyService; | ||
}, | ||
}; | ||
} | ||
|
||
public start() { | ||
this.logger.debug('Starting plugin'); | ||
} | ||
|
||
public stop() { | ||
this.logger.debug('Stopping plugin'); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of changing the output from being boolean to be an enum that has values like
ReadOnly, ReadWrite, Unknown
?I think this would better describe results for inline reading especially in the controllers where its hard to 'see' what the impact of a true vs false will be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peternied then it wouldn't be a
ReadonlyService
butTenantCapabilityService
or something like that. As long as it has an impact only on capabilities I suggest keeping it as it is right now and extending that in the next PR :) Especially as long capabilities are only boolean and will not contain enum values.