Skip to content
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

Add Role Based Access Control [RBAC] #3305

Open
hmerk opened this issue Jan 9, 2023 · 30 comments
Open

Add Role Based Access Control [RBAC] #3305

hmerk opened this issue Jan 9, 2023 · 30 comments
Labels
enhancement An enhancement or new feature of the Core

Comments

@hmerk
Copy link

hmerk commented Jan 9, 2023

See community discussion and proposal
https://community.openhab.org/t/rbac-model-in-openhab-and-potential-security-vulnerability-found/136419

@hmerk
Copy link
Author

hmerk commented Jan 9, 2023

@openhab/core-maintainers Would you please add this to the project list for openHAB 4.0.
This would be a very good addition.

@hmerk
Copy link
Author

hmerk commented Jan 9, 2023

Could add it myself ;-)

@hmerk hmerk changed the title Add Role Bases Access Control Add Role Based Access Control [RBAC] Jan 9, 2023
@splatch
Copy link
Contributor

splatch commented Jan 20, 2023

@hmerk while not being a core contributor I might be able to work on this in my spare time based on earlier work I did with Open Smart House.

I outlined some of concepts in forums https://community.openhab.org/t/rbac-model-in-openhab-and-potential-security-vulnerability-found/136419/4. They are based on initial implementation of eclipse-archived/smarthome#6034 which covered first step of design made under eclipse-archived/smarthome#579. Long story short - we got authentication API which was adjusted/stripped for openHAB 3.0, we never reached step 2 which would involve wider support for user roles and permissions.

@hmerk
Copy link
Author

hmerk commented Jan 20, 2023

@splatch
To be honest, I don't care who is doing this. Just asked in the community, as it sounded like ready, just needs some code cleaning. So why not join forces with OP ?

@splatch
Copy link
Contributor

splatch commented Jan 20, 2023

@hmerk I could argue on some of statements made there, ie.:

The openHAB community has been visited 93 days and 257 topics and 1 800 posts have been viewed with a total read time of 22 hours. The activity in this community demonstrates the difficulty of incorporating the RBAC model into openHAB and the complexity of the platform.

;-)

@hmerk
Copy link
Author

hmerk commented Jan 20, 2023

@splatch Sorry, i might have been a bit unclear.
I have asked the OP if he could provide a PR for this, to have it implement in oh4.
https://community.openhab.org/t/rbac-model-in-openhab-and-potential-security-vulnerability-found/136419/12
He answered that the code needs some cleanup and he is going to provide a PR in de next weeks.
https://community.openhab.org/t/rbac-model-in-openhab-and-potential-security-vulnerability-found/136419/13

So under the bottom line, I only wanted to prevent anyone from double work...

@splatch
Copy link
Contributor

splatch commented Jan 20, 2023

Going over code made by @gennartn I found several troubles. There are multiple commits, including merges, which make moving it into current code base troublesome as we would need to make a "back merge" changes which are started on 3.2.x state of auth packages and UI. Both core and ui moved forward since then. I attempted to rebase/squash commits but its not going straight as there are conflicting changes between commits.
Latest version of changes (main branch of Nicolas fork) contains unresolved conflict markers which means it can't be compiled and tested.

Aside of above there are some more troubles I see:

  1. There is implicit assumption of JWT as a primary authentication mechanism which means that AudioServlet, ChartServlet and IconServlet are not secured. They do not work with JWT. I think some browsers have troubles with sending headers to SSE endpoints (we need to confirm that with @ghys or webui maintainers).
  2. Provided code duplicates JWT and basic credentials validation logic between two places, AuthFilter and newly introduced VerifyToken interface. This is not only duplicated code but also duplicate functionality and duplicate execution. The AuthFilter is fired by underlying frameworks for all resources, including ItemResource where VerifyToken is being used, making its execution redundant just to obtain principal which is indirectly available in jaxrs.SecurityContext or openhab.Authentication interfaces. The VerifyToken is till some degree duplicate of AuthenticationManager limited to just one credential and input kind (http headers).
  3. The additional securing of command line is nice addon, but administrator verification there again reaches UserRegistry which backs JWT checks but does not follow principles introduced in Add support for conditional access based on user role eclipse-archived/smarthome#579 (through calling AuthenticationManager with UsernamePasswordCredentials), question if it should? Generally speaking statement in thesis says that its better to have that than given the default password set to access shell.
  4. While related work points to extension of role handling mechanism it does not conclude how roles are mapped to items and probably other elements (things?, sitemaps?)
  5. The SSE endpoint is still not secured, effectively allowing any user who has access to system to listen for changes in all items, even these which he has no access through implemented REST checks.

Given the community inquiries about support for external oauth authentication, support for LDAP going forward with proposed way will not solve any of troubles we ended with after Eclipse SmartHome migration and will add more places to work with with eventual introduction of external authentication support for openHAB. Even if we do not wish to follow the whishlist then there is an issue of stronger authentication which is based ie. on client certificate - this will not be possible as well, because whole code right now does rely on user/password auth and does not leave any space for extension.

My proposal of workflow is following:

  1. Make UserRegistry implement AuthenticationProvider which supports UsernamePasswordCredential.
  2. Introduce JWTCredential for cases which rely on Authorization header with Bearer scheme.
  3. Extract JWT validation from AuthFilter logic into LocalJwtAuthenticationProvider which will rely on openHAB own RSA key.
  4. Introduce JWTAuthenticationProvider which would work with standard openid connect providers (google, okta, keycloak etc.), this will probably require an extra html form with related servlet to redirect user to external provider in order to obtain authentication and small UI adjustment to accept external token.
  5. Extract authorization logic into AuthorizationManager which would become a common place for access checks, so access control logic is not distributed across REST, rules (hopefully covered in future) and other layers. Introduce consistent set of Principal types which should be used by AuthenticationProvider implementations to map Group, Role and Permission entries.

Above steps will permit to switch authentication mechanism based on installed features and runtime configuration (possible to control using service configuration in OH webui) and also introduce a authorization logic which could be unified independently of user authentication source.

@hmerk
Copy link
Author

hmerk commented Mar 2, 2023

@splatch As you offered to help with this issue, did you make any progress ?

@splatch
Copy link
Contributor

splatch commented Mar 2, 2023

I have not touched openHAB side due to awaiting of publication of trademark policies which happened just few days ago.

@wborn wborn added the enhancement An enhancement or new feature of the Core label Mar 11, 2023
@straiforos
Copy link

straiforos commented Jan 5, 2025

Hey folks,

I apologize in advance, I am long winded...

I've been looking into this since about this time last year, gennartn did not complete the implementation and as @splatch noted there are challenges with bringing it up to date. I did most the work in my own fork (I accidentally PR'd to this repository this past week). I have the work in my 4.1.X-RBAC branch that I cherry picked their work in and resolved issues as I found them. However, the UI remains incomplete, leading me to consider whether this feature might be better suited as a plugin.

Initially, I worked on this for a project with friends that has since fallen through. Still, I’d love to see OpenHAB support local businesses like bars and shops, which I think could benefit from enhanced authorization features.

Would it be feasible to develop a plugin that extends and re-registers the service UserRegistryImpl to introduce roles and permissions? This way, home users can maintain simplicity while those needing advanced features can opt-in by installing the plugin. I believe gennartn's work/thesis provides a starting point.

@hmerk
Copy link
Author

hmerk commented Jan 5, 2025

Thanks @straiforos to bringing this back to life.
I would strongly recommend to change your development to openHAB 5, requiring Java 21, as I don't see this feature to be backported to openHAB 4. openHAB 4 won't get new features, just bug fixes.

@straiforos
Copy link

Will do! Wow 5 already! Time flies.

@florian-h05
Copy link
Contributor

However, the UI remains incomplete, leading me to consider whether this feature might be better suited as a plugin.

The core RBAC functionality, i.e. the parts securing Item access etc., need to be core, as well as the UI for creating and managing users/roles needs to be part of Main UI. Once core APIs are in place, I can help with the Main UI part.
I can however imagine having the ability to extend the RBAC system via plugins, e.g. to support connection to a OIDC server like Keycloak.

Would it be feasible to develop a plugin that extends and re-registers the service UserRegistryImpl to introduce roles and permissions?

I would have this roles and permissions stuff part of core, but with a default config that matches the status quo. I don’t think it is reasonable to effectively override a core service.

@straiforos
Copy link

straiforos commented Jan 8, 2025

However, the UI remains incomplete, leading me to consider whether this feature might be better suited as a plugin.

The core RBAC functionality, i.e. the parts securing Item access etc., need to be core, as well as the UI for creating and managing users/roles needs to be part of Main UI. Once core APIs are in place, I can help with the Main UI part. I can however imagine having the ability to extend the RBAC system via plugins, e.g. to support connection to a OIDC server like Keycloak.

Would it be feasible to develop a plugin that extends and re-registers the service UserRegistryImpl to introduce roles and permissions?

I would have this roles and permissions stuff part of core, but with a default config that matches the status quo. I don’t think it is reasonable to effectively override a core service.

Thank you for the thoughtful and informative response, ruling out a plugin architecture.

I am excited to begin prototyping RBAC, personally I've worked on systems that use roles but use them as a collection of privileges/permissions, giving flexibility to developers and implementations to add roles and privileges that are more flexible, capturing the status quo will be the fun part. Privilege collections IMO would allow plugins to add to known roles supporting the current functionality while baking in security checks as it becomes a fundamental to the core of the application. I would be curious your thoughts on that kind of approach.

Right now we have a user and admin role if I am not mistaken.

@florian-h05
Copy link
Contributor

At the moment we only have the user and admin roles, correct.
From core RBAC, I would expect to keep all the admin functionality only available to the admin role (status quo), and introduce fine-grained permissions for Item access. This means (at least in my imagination):

  1. Allow specifying user groups. This could also be implemented with roles inheriting the user role.
  2. Grant permission to access Item state and command an Item to single users or user groups (this can be done through Item metadata).

With these changes alone, it would be possible to have one admin taking care of a openHAB installation for a building with multiple flats, and create UI pages for the individual users (already possible) and groups (not yet possible) and very importantly limit access to Items through the REST API (currently, permissions only apply to page visibility, but the REST API allows full access to all Items, so this is no security feature). The last step is what I have implemented with an external proxy and which has been picked up by others once I archived it, see https://community.openhab.org/t/detailed-access-control-and-user-management-by-reverse-proxy-it-works/66450/23?u=florian-h05.

In a second step I could imagine having an additional role to allow non-admin users to design UI pages.

Your proposal sounds interesting, but I currently cannot imagine the use case for it. Can you please elaborate a bit more?

@straiforos
Copy link

Thanks again for the response,

User groups is something I should consider for sure, I was not considering that idea originally.

My thoughts are aside from item access the functionality for adding, deleting, and editing items is something I would want. Maybe I want a user to be able to add items and not remove them or not be able to add them but edit them for configuration updates, say they are on a phone call with support for that flat/retail space. Granular permissions would allow for dynamic use cases and if you want the simpler out of the box change nothing but once you want to make more fine grained roles beyond admin and user you could change the default behavior in your instance to have IT admins with separate responsibilities like plugin installation but not item configuration. Allowing for more fine grain management and oversight of an instance in say a hotel. A managed software company could own the IT admin and allow plugins and managers/handymen can access item addition and removal.

Thats kinda my thought process how used in practice would it be, I am not sure. My ambition is to essentially make a local business that sells managed IT deployment of OpenHAB to hotels, venues etc that would want a level of confidence in the fine grain controls and various implementations of it.

I tried to GPT a small summary that's maybe easier to read since I tend to be long winded.

Expanded Roles and Use Cases
By introducing these granular permissions, the current admin-user dichotomy could be extended into more specialized roles:

  • IT Admins:
    • Responsible for technical tasks like plugin management or system updates but without access to individual Item configuration.
  • Maintenance/Managers:
    • Able to manage Item configurations but restricted from adding or removing Items.
  • Custom UI Designers:
    • Non-admins who can design and manage UI pages for specific roles or user groups.

@straiforos
Copy link

straiforos commented Jan 11, 2025

I missed it in the original topic discussion from @splatch. The OpenSmartHouse Permission is exactly where my head was at. Combining that idea with the work Nicolas Gennart was working on for dynamic roles.

I started down this path as a proof of concept myself and I opted to make roles into a registry with a permission registry as well.

@florian-h05
Copy link
Contributor

That is way more complex than what I thought about, but really versatile and sounds very powerful!

My ambition is to essentially make a local business that sells managed IT deployment of OpenHAB to hotels, venues etc that would want a level of confidence in the fine grain controls and various implementations of it.

I would love to see openHAB used in such a context, this also explains why you want such powerful RBAC.

As long as the average openHAB user does not have to care about it, I fully support your proposal.

@straiforos
Copy link

straiforos commented Jan 14, 2025

As long as the average openHAB user does not have to care about it, I fully support your proposal.

I appreciate the support, I will continue down that path. The idea is no breaking changes, this needs to be transparent.

Thank you.

@openhab-bot
Copy link
Collaborator

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/ideas-and-discussion-what-features-do-you-want-in-openhab-5-0/160573/430

@jimtng
Copy link
Contributor

jimtng commented Jan 14, 2025

Yes! Using registry will allow us to extend it using add-ons to link this system using LDAP or whatever rbac back end you wish e.g.sql database. So it sounds great.

@straiforos
Copy link

Hey y'all,

I am making good progress scaffolding out the solution. @splatch your proof of concept work has been invaluable.

I had a couple questions for the community and @splatch.

@splatch mind if I put you as the author for initial contribution for NamedPermission and a few others where I feel I am copy pasta'ing your work?

For the community, I plan to implement RBAC as far as our users are concerned yet for us devs I am actually implementing a form of ABAC. Its a flavor as I described that gives the flexibility of granular access control, while also preserving what most are used too, Roles. Roles become a container/logical grouping of permissions which are the "Attributes" in Attribute Based Access Control. This allows for dynamic user defined roles.

Additional functionality has been requested like User Groups, and Item assignment so users can have essentially multi-tenancy support. I would like to ask to split that out as additional initiatives? Keeping focus on RBAC without breaking changes giving me time to prove that out and stew on the larger implementation of groups and item assignment/tenancy.

@splatch
Copy link
Contributor

splatch commented Jan 18, 2025

Hello @straiforos,
The permission work I did in OSH works fine since in all deployments I conducted based on it. It could always be better, but that's another thing. :-)

There are several places where implementation I did got a little bit tricky. Main trouble maker for me was SSE. The item resource (as well as other places) is not ready to host more complex security concepts. Be aware that amount of calls to registries is huge, and REST layer itself is just top of iceberg.

As of copyrights, code I did was contributed to OpenSmartHouse project for a reason. (IANAL) If you wish to copy it, you should be fine even copying whole files, as long as you retain original copyright statement from OSH ( * Copyright (c) 2019-202x Contributors to the OpenSmartHouse project). As per EPL v2 license (point 3.3):

Contributors may not remove or alter any copyright, patent, trademark, attribution notices, disclaimers of warranty, or limitations of liability (‘notices’) contained within the Program from any copy of the Program which they Distribute, provided that Contributors may add their own appropriate notices.

WRT ABAC, I always thought that it is more about object attributes which can be used to evaluate access. Like for items it would be category or tag, for things its type or so. However, given that all terminology in IT is rather fluid and subject of evolution permissions might actually be also considered a sort of ABAC. :-)

@straiforos
Copy link

Hello @straiforos, The permission work I did in OSH works fine since in all deployments I conducted based on it. It could always be better, but that's another thing. :-)

Wonderful, I plan to do something very similar in OpenHAB, I have a preference in aspect oriented programing so I plan to avoid some extra boiler plate that the OpenSmartHouse implementation required like manager patterns etc. Time will tell if thats my hubris or not LOL.

There are several places where implementation I did got a little bit tricky. Main trouble maker for me was SSE. The item resource (as well as other places) is not ready to host more complex security concepts. Be aware that amount of calls to registries is huge, and REST layer itself is just top of iceberg.

I appreciate the heads up here with SSE, I will have to keep and eye out for performance concerns.

As of copyrights, code I did was contributed to OpenSmartHouse project for a reason. (IANAL) If you wish to copy it, you should be fine even copying whole files, as long as you retain original copyright statement from OSH ( * Copyright (c) 2019-202x Contributors to the OpenSmartHouse project). As per EPL v2 license (point 3.3):

Ah I will definitely need others feedback on copying it verbatim like NamedPermission, and bringing in the OpenSmartHouse copyright since this is not just my call even if I need to rename it enough to be my own. I do feel weird taking such inspiration from your work without attributing yourself to it. I am manually writing the files for my own sake let alone legal concerns. I will leave your initial contribution there and update the copyright to match. I will ask for others input as I am new to the community and as well am not a lawyer :)

WRT ABAC, I always thought that it is more about object attributes which can be used to evaluate access. Like for items it would be category or tag, for things its type or so. However, given that all terminology in IT is rather fluid and subject of evolution permissions might actually be also considered a sort of ABAC. :-)

Good point, I have not implemented these patterns enough to be able to state one way or another but certainly felt like an evolution of RBAC into something a little more dynamic like ABAC.

@splatch, thank you for all your insight for my own implementation as well as others like Nicholas we could not do it without ya!

@kaikreuzer
Copy link
Member

Wrt file headers: For simplicity reasons, we typically have a standard license header on all files in src/main/java - assuming that those files are created by openHAB contributors. This makes the bulk management of the files much easier than if every file had a different header... If we use 3rd party source files directly, we typically store them in a separate src/3rdparty/java folder and keep their file headers intact (and never touch them when updating our own headers, e.g. on a year change).

It is therefore not really easy to "merge" files from two different projects into a single file for us for practicality reasons.
The simplest solution would be if @splatch agrees to contribute the code also to openHAB; then we could mention him personally as the author of the initial contribution and thus give him the due attribution, while keeping our standard license header at the top.

@straiforos
Copy link

Wrt file headers: For simplicity reasons, we typically have a standard license header on all files in src/main/java - assuming that those files are created by openHAB contributors. This makes the bulk management of the files much easier than if every file had a different header... If we use 3rd party source files directly, we typically store them in a separate src/3rdparty/java folder and keep their file headers intact (and never touch them when updating our own headers, e.g. on a year change).

Wicked cool, good to know.

It is therefore not really easy to "merge" files from two different projects into a single file for us for practicality reasons.
The simplest solution would be if @splatch agrees to contribute the code also to openHAB; then we could mention him personally as the author of the initial contribution and thus give him the due attribution, while keeping our standard license header at the top.

Gotcha, that would be the simplest for sure. I'm still working out the kinks and approaching it using an annotation versus a helper method in each area so I can hopefully make it as reusable as possible.

@splatch
Copy link
Contributor

splatch commented Jan 28, 2025

@kaikreuzer @straiforos I don't want to stop you from improving openHAB. At the same time, my position as inventor of code in question, on copyright hasn't changed.

@straiforos
Copy link

@kaikreuzer @straiforos I don't want to stop you from improving openHAB. At the same time, my position as inventor of code in question, on copyright hasn't changed.

@splatch totally understandable, frankly I am writing this by hand and wanted to use your work as inspiration. I will make sure it's unique, and has openHAB only copyrights to comply with @kaikreuzer point about 3rd party code additions.

I appreciate the guidance.

@Davek145
Copy link

@straiforos, let me know if I can anyhow contribute to your effort. I have taken over the archived OH Multi-user proxy created by @florian-h05 and adapted it for current OH versions (https://github.com/Davek145/openhab-multiuser-proxy). It does not have so big ambitions you have, but solves at least the basic issues I see in OH = no ability to differentiate between individual users of OH instance.
I use it in residential houses, including my own house. I believe that proper RBAC is needed even for residential users as even there use case exists for different users having access to different Items/Pages/Sitemaps in OH (e.g. I do not want my kids to control various technologies in the house, but they are welcome to control lights, music, etc.).
Totally agree with @florian-h05, that at least RBAC at the level we solve by the proxy including strict control of the REST API access shall be in the OH Core directly.
Feel free to contact me in case you want to know more of the scenairos I'm using it for.

@openhab-bot
Copy link
Collaborator

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/ideas-and-discussion-what-features-do-you-want-in-openhab-5-0/160573/514

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature of the Core
Projects
Development

No branches or pull requests

9 participants