-
Notifications
You must be signed in to change notification settings - Fork 919
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
[RFC] User level settings #7821
Comments
Thanks for the nice explanation of how this feature works with its interfaces. I love a lot of things about this. Especially how clean the change is w.r.t how saved objects are created. A few questions:
Not related to this RFC but where can I find more details about permission based saved objects (ACL) |
I always thought if we do implement user-settings, it would simply inherit the value from the level above it. For a setting X, we would pick from user's saved object, and if missing we pick from the global saved object, then the value from config file, and finally schema defaults. In tenant-based systems, the tenant's saved object would be between user and global. I feel that this definition of scopes and the two settings interfaces would complicate the user experience as well as the maintainability of the code. I was thinking that Admin users will get this interface when the access Dashboards Settings where they get to set their user settings, as well as define tenant and global defaults. When viewing either of the defaults tabs, they get the ability to lock a setting to prevent any change to it: Users without Admin access will get to set set the user-level settings: The interface shows the user the source of any defaults and provides them the ability to update tenant settings to match theirs (for simplicity). OSD installations that don't have tenants will simply not show that portion of the interface. Internally, we would have different saved objects for each, like it is today and the original RFC envisioned. However, the client would internally maintain the values for all of the levels which will be new as the client kinda flattens the hierarchy as it reads them in. By having all of the values in the client, we facilitate access to the "default" values which are those before user settings overrides them. The locking mechanism provide full control over locking down settings and would eliminate the need for This alternative proposal doesn't accommodate defining scopes for a setting because i couldn't think of any reason for it. However, if there is a compelling reason, my proposal is compatible with it too. |
@ashwin-pc Thank you for your questions!
That's a really good point, to be honest, I've be thinking of including that in this RFC. But currently, workspace level setting is implemented in workspace plugin. I didn't include that into this RFC because it's workspace specific implementation at the moment, I'm afraid of including it will make the scope of this RFC too larger for the audience. I believe the same design could apply to workspace level setting as well, but maybe it could discussed in a separate issue.
Yes, the scope will be an optional property, and it's optional parameters as well when calling
Here is a screenshot, user default workspace will the the first user level setting to be added.
Thanks for the suggestion, that sounds also reasonable to me :)
When authentication is disabled, meaning we won't be able to get user info(user id), then user setting won't exist, there won't be a user setting page. For authentication, what we're expecting auth info been injected into http request, so that we can identify the current user and retrieve user id from it.
The current implementation of ACL is inside workspace plugin, you can find overview design from here #4633, but the PR is a bit outdated, I'm planning to update it soon. In a nut shell, it will attach permission field to saved object that requires permission check, for example: {
id: 'my-workspace',
type: 'workspace',
permissions: {
read: {
users: [<user_name>],
groups: [<backend_role>],
},
write: {
users: [<user_name>],
groups: [<backend_role>],
}
}
} And it will validate the current user's permission to the saved object when call saved object API, such as get/update/delete |
@AMoo-Miki Thank you for the amazing feedbacks!
That's exactly what I originally thought, the main reason of introducing I can imagine when we're splitting the current advance settings into different portions: application settings, user settings and workspace settings, there will be more such cases. @kgcreative may be can comment more on this.
There will be a separate page for user to update user settings, as far as I see by the time, the UX mockup looks pretty intuitive to me. I posted a screenshot in my previous comment. For maintainability, the
Btw, thank for providing the mockup, I love it! For "get the ability to lock a setting", I don't have a strong opinion about this, perhaps this requires more inputs from UX.
My understanding is a tenant has its own OpenSearch-Dashboards index, their saved object are stored isolated. Do you mean the tenant's advance setting is currently inheriting the Global tenant? Please correct me if I'm wrong :)
For the implementation details, there could various, we're thinking of two possible approaches:
But I think we can discuss the implementation separately.
If not all settings are global settings, like I mentioned above, it doesn't sound this proposal will fit(hope I fully get your points). Also, If we gonna support more levels of settings, for example, apart from global setting and user setting, if we added workspace level settings, could your approach flexible enough to support such case? |
That makes sense.
Has it been implemented yet? If so, is there a doc on how I can try it out? :) |
Yes, workspace is a great example, you can try it on future playground, the workspace you can see is the workspace that you have permission. Right now, you can already see a few workspaces that because the workspace permission is set to |
keep the version that will need to have upgrade logic when OSD version bumps for user level settings also, to make the change more simper we can use fixed document id that is username |
config is versioned on start up within the id so i think it would be consistent. makes sense. it actually makes it a lot easier to understand what is happening and which default values are being pulled from. |
Questions:
Do plugins define their settings independently of global settings instead of participating/contributing to the global settings? Is this the behavior we want? If we want user specific workspace settings, it seems undesirable to reimplement the same behavior again. |
No, it doesn't replace browser settings. They are stored in different places, user setting stores in
Yes.
It's a enum or a group of enums export enum UiSettingScope {
GLOBAL = 'global',
USER = 'user',
}
Yes, we have add scope query parameter in get api that supports get values from specified scope. |
Introduction
Currently, OpenSearch-Dashboards provide global settings that apply to all users. The purpose of this RFC is to introduce a new feature that allows to customize advance settings at user level. This feature ensures that when a user changes their settings, it will not affect other users.
Goals
UX changes
Design details
To support user-level settings we'll need to modify the current ui settings schema by introducing a new field
scopes
to indicate whether a setting isglobal
oruser
or both.Registering a setting with scope
The
scopes
is an array, it can contain{type: 'global'}
or{type: 'user'}
or both. If no scope provided, it defaults toglobal
.For example:
{ dateFormat: { name: i18n.translate('core.ui_settings.params.dateFormatTitle', { defaultMessage: 'Date format', }), value: 'MMM D, YYYY @ HH:mm:ss.SSS', description: '...', schema: schema.string(), + scopes: [{type: 'global'}, {type: 'user'}] } }
Reading a setting with scope
When calling
uiSettings.get(key)
, it returns the value based on the following rules:global
or empty, the global setting will be returned.user
, the user setting will be returneduser
andglobal
, the user setting will be returned if it's set, otherwise, return the global settinguiSettings.get(key, defaultValue, 'user')
with scope specified, it will return the setting of the specified scope. If the value doesn't not exist, then return thedefaultValue
. This requires to update the current uiSettings service's interfaceWriting a setting with scope
When calling
uiSettings.set(key, value)
, it follows the following rules:global
or empty, it will write the value to global setting saved object.user
, it will write the value to user setting saved object.user
andglobal
, it will by default update the user setting saved object.uiSettings.set(key, value, 'global')
with scope specified, it will write the value to the corresponding saved object. This requires to update the current uiSettings service's interfaceHow to store user setting?
The global setting saved object should remain unchanged: it is a saved object which type is
config
and id is the current OSD version.For user setting, the saved object type will also be
config
but the id will be the current login username. Thus, user setting requires OSD to enable authentication to provide the usernameAccess control
When saved object permission control is enabled(ACL), the permission will be attached to the user setting saved object with permissions which ensures users won't be able to read others setting. However, ACL is not mandatory.
Summary
The implementation of a new
scopes
field in the settings schema ensures a seamless integration with the existing uiSettings client while maintaining the compatibility with existing codebase and it should be non-breaking changes.Open questions
The text was updated successfully, but these errors were encountered: