-
-
Notifications
You must be signed in to change notification settings - Fork 252
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
Allow editing "extended_settings" user property #1160
base: master
Are you sure you want to change the base?
Allow editing "extended_settings" user property #1160
Conversation
I'm not sure about this one @Puyodead1 |
why? |
Extending settings is just a string? What if two clients both want to store extended settings, does that mean they just fight over it? |
Maybe a better idea would be it being a {[key:String]:String} to let clients use the extended settings as an object so conflicts are generally more graceful |
It's |
Ok |
Not sure why I didn't get an email for this reply, but I'm concerned that a malicious application could just, upload some large data and try to harm the server/database. There's no restrictions at all on the kind of data stored here, which is my problem. Also, I'm not sure what applications would even use this for. Client apps can very easily just store settings locally and allow some sort of export functionality. |
ah, makes sense. |
Yeah, clients can just store the information locally, but like clients like mine that have more themes, or stuff like that, it'd make sense to store that non-locally |
can this please get merged? I'd like to have this for storing some settings, it doesn't have to get merged now, I'd just like to know when this should happen ish. |
just store settings in localstorage? |
Why do you want to force users to reconfigure all custom settings every time they're on new device or clear the browser cache? This rather encourages off-Spacebar storage which also gets the job done, but shouldn't be used... If there's already a place to store user settings, why not also store client specific settings? I'll happily implement some limits for this, but in general I'd prefer this over localStorage/external storage |
Or a thought I've had several times to just store this kind of stuff in the dms to yourself, which I'm not going to do as that'd be really jank and likely look bad on other clients |
Another problem I have with this is that allowing clients to edit extended settings directly makes it more difficult for clients to use it, because now they have to worry about not breaking other client's stored settings. It would be better if there was a system to register an app's settings to that specific app so that it doesn't conflict with other apps. This still doesn't fix a malicious client potentially wiping settings or adding things to other apps though. E.g. if we had a naive solution where when updating extended settings, you also provide your app name and the server just does Is this even a problem we need to think about? I suppose if we were in discord's position, and client mods made use of extended settings, then it'd be possible for like, a malicious app to add like
How often does that happen? But also, isn't this what the normal settings are for? I think those cover a good majority of use cases, and clients should use them. I have no problem with that, since the server controls exactly what type of data is stored in there, and we can do validation very easily with that. Extended settings as implemented here is just a plain json object that we have no control over. |
Sure, but it could also just view the (current) IP itself. That's an issue all clients have, and kinda the users fault if they use such a client imo
I'm developing Jank Client on two, sometimes even three different devices - so, quite often for me.
Technically yes, but I don't think these are going to support e.g. custom CSS. |
This PR just adds the user property
extended_settings
to the user modify schema, so applications can actually use it to store extended settings.