🐛 Improve admin security over dnshole controls

[SeDemal]

Category

Bugfix / Security

Overview

1. Although DNS toggle all controls where hidden, the per item badge was still active.
2. Toggle function was not safe from CSS manipulation to regain access to it, made the toggle function only answer to admin users.
3. Non admins didn't have controls elements centered.

Change allow controls to server side.
Added ability to enable controls for non admins (But still users at least) with widget option.

Issue Number

Reported in #1685

[ajnart]

This is not how to protect the api call from being made by non admins. This call should be restricted to logged-in users and not only admins.
please read about “admin procedures” and try to understand how trpc works with that. Also you could’ve just put the conditions inside of the react query use mutation call

[ajnart]

There is no need for settings like these, it still will allow requests if the user just modifies the HTML, please read my comment and try to understand the trpc server

[SeDemal]

There is no need for settings like these, it still will allow requests if the user just modifies the HTML, please read my comment and try to understand the trpc server

Changed to protected procedure, but as mentioned in #1685 and as how the widget was initially changed, controlling network behaviour's is an admin procedure. The solution I'm adding is an extra to let normal users use it although they shouldn't be able to.
Since the router uses the config, there is no way to manipulate the html to somehow change the config and unlock the function. That's the whole point, if you manipulate the html, all you'll get is the TRPCError Unauthorized.

[ajnart]

There is no need for settings like these, it still will allow requests if the user just modifies the HTML, please read my comment and try to understand the trpc server

Changed to protected procedure, but as mentioned in #1685 and as how the widget was initially changed, controlling network behaviour's is an admin procedure. The solution I'm adding is an extra to let normal users use it although they shouldn't be able to. Since the router uses the config, there is no way to manipulate the html to somehow change the config and unlock the function. That's the whole point, if you manipulate the html, all you'll get is the TRPCError Unauthorized.

The thing is just that admin procedure = admin can do it and protected = logged-in users, there is no point to do a protected procedure and then check if the user is an admin, just use an admin procedure. The normal users can still click the buttons but it will return an error when doing so

[SeDemal]

There is no need for settings like these, it still will allow requests if the user just modifies the HTML, please read my comment and try to understand the trpc server

Changed to protected procedure, but as mentioned in #1685 and as how the widget was initially changed, controlling network behaviour's is an admin procedure. The solution I'm adding is an extra to let normal users use it although they shouldn't be able to. Since the router uses the config, there is no way to manipulate the html to somehow change the config and unlock the function. That's the whole point, if you manipulate the html, all you'll get is the TRPCError Unauthorized.

The thing is just that admin procedure = admin can do it and protected = logged-in users, there is no point to do a protected procedure and then check if the user is an admin, just use an admin procedure. The normal users can still click the buttons but it will return an error when doing so

Ok I might have not expressed it properly, I did at first only want admins to be able to use it to follow what was already done before, just improperly. I didn't know in my first draft about protected and admin procedure.
In the following modification, I did learn about those, both actually, but choose protected procedure because of the first comment you left.
I've only added the ability to let normal users interact with it because you mentioned it could be a protected procedure and not limited to admins.
I somewhat agree to that but the option is there to let sys admins choose whether or not they want to let their user access that function.
Now if we're all ok with protecting it only behind admin procedure and no normal user interaction, I'm all fine with it. I can remove the extra things and make it 100% admin.

[ajnart]

Oooh okay makes sense. Then we make it admin only

[ajnart]

This will be better whenever we have a more fine grained permission system

[SeDemal]

Ok, will do that, thanks :)

[Meierschlumpf]

lgtm