Security: domains not filtered when a user has a root content node set

[jamiehowarth0]

When logging in to view redirects, the domains list does not filter out nodes that the user does not have access to.
On a multi-domain site, this can expose other domains and redirects that the user should not have access to.

[jamiehowarth0]

Further to this, no security (beyond basic control applied with UmbracoAuthorizedApiController inheritance) is applied to actions to add, edit or remove redirects, thus allowing a user to potentially craft a HTTP request to manipulate redirects that they should not have access to.

I'll fork this issue out into a separate issue and subsequent PR.

[abjerner]

Hi @jamiehowarth0

There are probably a number of ways to go about for handling permissions, so if you're planning on working on something, it might be a good idea to share your thoughts here first. That way I can hopefully help you in the right direction, and it also improves the chances of a potential PR being accepted.

For now, the basis of this package has been that we trust the users, and that they are not creating, editing or deleting redirects that they shouldn't. For some clients, we've hidden the dashboard for regular users, so it's only available for admins.

It's true that users can then still call the API endpoint directly with the right payload, and thereby still be able to create, edit or delete redirects. Typically our users are not tech-savy enough to know how to do this, but I understand there are situations where this might be a problem.

I have thought about adding permissions to the package before, but as Umbraco doesn't offer a way to administer permissions for packages, I haven't really looked much into this yet.

Different sites may require different setups in terms of permissions, so ideally this should be something that is easily configurable. It could be through appsettings.json or somewhere through the backoffice. A potential PR should also try to avoid breaking changes, or keep it to a minimum.

I hope that makes sense 😉

[jamiehowarth0]

Sounds good - for now, the basic security I've coded up simply uses the user start node (set via the "Content start node" property either on a user group or the user object), which means that if a user only has access to certain content nodes, the dashboard won't leak domain or redirect data from other content nodes the user doesn't have access to. It's basic and minimal, adding just a couple of methods onto the API controller and RedirectsService, thus keeping signatures mostly the same for versioning.

I'll look at more advanced security options when I have more time, currently this is an essential fix for a client site that I'm working on. I'll push my PR in the meantime as a "this is basic but it prevents data leakage between users" fix.

BTW the dashboard visibility is hard-coded into the C# source code, so to make it more user-configurable for developers to manage, maybe switch the dashboard declaration into the package.manifest instead? :-)