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

OAuth2 makes it difficult to consume the REST API #1699

Closed
spacemanspiff2007 opened this issue Oct 7, 2020 · 12 comments · Fixed by #1713
Closed

OAuth2 makes it difficult to consume the REST API #1699

spacemanspiff2007 opened this issue Oct 7, 2020 · 12 comments · Fixed by #1713
Labels

Comments

@spacemanspiff2007
Copy link
Contributor

OAuth2 workflow was introduced in #1388.

Whilst it might make sense in an untrusted environment or in an environment with lots of users both are not the scope of openhab.
The scope is home automation which means a trusted environment.
I'd even argue that 99.99% of all openhab instance and all of its devices are fully managed by just one person so there is nothing to gain by expiring tokens.
I am not against authentication but Oauth2 is way overblown and adds nothing that can't be achieved with basic auth (for a trusted home environment!). It also adds a huge overhead (only 1% of a message is payload, the rest is auth) making the API even slower.

There are countless people that are already struggling with the Rest API (just look at the forum) and OAuth2 will effectively kill Rest for the majority of users.
The easy integration into scripts and other already existing things through the API always has been a strength of OH and will be mitigated if not removed by OAuth2.

I'd argue that OAuth2 should be removed again but you guys seem to really like doing authentication so please - for the sake of the normal users - add an configuration option which can disable OAuth2.

@rkoshak
Copy link

rkoshak commented Oct 7, 2020

I tend to agree. If we will require OAuth2 to send commands or post updates to Items through the REST API a good number of integrations where users are using a web callback feature built into their device will become broken and impossible, The mere fact that OH requires PUTs and POSTs already makes this challenging for a lot of users.

You might be surprised at the number of users who also run scripts that interact with OH with the equivalent of curl. There are a number of important stuff that has been done in this way like a script that can move data from one OH persistence database to another one, upgrading the Zwave add-on to a newer version, building up Items from Things (etc), etc. While these will not become impossible they become much more difficult for the average semi-tehnical user.

I'm less concerned about the larger integrations (e.g. custom UIs, HABApp (sorry spacemanspiff), etc.) as those are built by more technically capable developers who should be able to manage the OAuth2 workflow (NOTE: I'm also included in this category with my sensor_reporter). They won't like it as it will require a lot of work. I'm also not that concerned about efficiency or the web calls. If the OAuth2 headers in the calls make that big of a difference perhaps REST isn't the right integration approach in the first place. But I am concerned that a number of integrations some users have relied upon will become impossible if OAuth2 is required.

In the end, the main take away is that web UIs are not the only types of clients that will interact with OH's REST API and OAuth 2 is a real pain and occasionally impossible for those types of clients.

@wborn wborn changed the title [OH3] Revoke OAuth2 or make it optional OAuth2 makes it difficult to consume the REST API Oct 8, 2020
@wborn wborn added the REST/SSE label Oct 8, 2020
@kaikreuzer
Copy link
Member

If we will require OAuth2 to send commands or post updates to Items through the REST API

Just to make sure that it is clear: The authentication has only been introduced on any operation that is considered to be "admin only". All the rest ist still completely unsecured just as before.

I see the need for a solution for system-to-system integrations, but I absolutely do not agree that this should be to remove authentication again. Instead, I'd think that we should have something like apikeys that can be managed by admin users (a topic I briefly touched on in this comment already. We didn't require it for Swagger (which was the topic of that discussion), but maybe @ghys can comment on whether that is something that could fit into our auth infrastructure as a future addition.

@spacemanspiff2007
Copy link
Contributor Author

@kaikreuzer I did not ask to completely remove authentication but instead I asked to not use Oauth2 because it is too complicated. See my comment

I am not against authentication but Oauth2 is way overblown and adds nothing that can't be achieved with basic auth (for a trusted home environment!).


I'd think that we should have something like apikeys that can be managed by admin users

Could you please elaborate what benefit a token based authentication has?
Why not just create a user with a password and role?

This would be imho the much nicer concept, because it would allow creating the user:role:password combination through the gui and through file based configuration which is not possible with Oauth2.
Why not create a users file like mosquitto and attach the role behind the username:

admin:admin:UGxlYXNlIGRvbid0IHVzZSBPQXV0aDIhIEl0IGlzIG5vdCB2ZXJ5IGdvb2QgZm9yIG9wZW5oYWIh
my_kid:user:VXNlIGVhc3kgYXV0aGVudGlmaWNhdGlvbiBtZWNoYW5pc21zIQ==

Three rest endpoints could be used to manage users

PUT /users  # add/update
GET /users  # list
DEL /users  # remove

@bwosborne2
Copy link

OAuth2 authorization make sense to me but the authentication required appears to be outside the REST API requiring a call to /auth with some unknown calculated parameters.
IMO before a milestone is released this needs to be documented to permit contributed services such as HABApp to function with OH3. If the owner of the OH system supplies the credentials to HABApp, for instance, the system should permit that.

ghys added a commit to ghys/openhab-core that referenced this issue Oct 11, 2020
Closes openhab#1699.

Note, this opens a minor security issue that allows an attacker
to brute force passwords by making calls to the API - contrary to
the authorization page, the credentials parsing for the REST API
is stateless & doesn't have a lock mechanism to lock user accounts
after too many failed login attempts.

Signed-off-by: Yannick Schaus <github@schaus.net>
@ghys
Copy link
Member

ghys commented Oct 11, 2020

There was a thought process with introducing OAuth2 as a supported authorization mechanism, which never meant it has to be the only one. OAuth2, despite being a little more involved, is a widely adopted standard that is implemented in countless libraries in virtually every language that can do HTTP.

The assessment of your home environment being "trusted" can be true, until it isn't, and using expiring or easily-revocable tokens has advantages over sharing passwords; the vast majority of openHAB instances being accessed with unencrypted HTTP, this means credentials are sent in clear text and multiplying requests with credentials attached increases the risk of these credentials being sniffed on the network. Limiting the number of password-based authentications to the infrequent signing in on a dedicated page ensure that passwords aren't known by potentially untrusted third party services, or stored inadequately - the less you see your password in clear text, the better. It's a known fact that people tend to reuse passwords.

It also allows potential interesting enhancements to the authentication flow like 2FA or SSO or timeouts after n failed logins.

(sidebar: you'll forgive a rare little public rant, but I do find it a little rude and inconsiderate to the unpaid work of other volunteers that at the slightest inconvenience to your use case, your immediate knee-jerk preference on said work seems to be basically to trash it. That's another occurrence of the negativity, abrasive commentary and baseless "developers are out of touch" nonsense that shouldn't happen in a community that aspires to be friendly and healthy, yet I have to say that I see that left and right lately, and it can be very disheartening at times. But I digress... /rant /sidebar).

On a more constructive note, I opened #1713 to let @openhab/core-maintainers decide if they'll deem appropriate to allow Basic authentication with user/password credentials that will be validated against the user registry. There's a risk of this being abused to perform brute force attacks (I commented on that in the PR description), and it's up to the user to decide if they're willing to trust their password to third-party services and transmitted potentially in clear text with every API request.

But I still think opaque, non-expiring "API tokens" tied to a user account and created/revoked with console commands or the UI would be more adequate - they've been planned from the start but the implementation was never finished, due to time & resource constraints as it was not deemed too urgent at the time to be able to make changes to the system by tools that don't support OAuth2.

One of the big advantages of these API tokens, apart from not divulging your actual password, is that they can be limited to a certain scope. That's a weak argument right now because we only have one scope defined (admin) and they're not even checked, but if we ever add more of them, that would allow limited-purpose API tokens with a scope that for instance authorizes it for managing items, and rules, but not things or services. When you think an API token could have been compromised, you can easily revoke it for the service in question without having to change your password or update the credentials everywhere else.

@bwosborne2
Copy link

bwosborne2 commented Oct 11, 2020

But I still think opaque, non-expiring "API tokens" tied to a user account and created/revoked with console commands or the UI would be more adequate - they've been planned from the start but the implementation was never finished, due to time & resource constraints as it was not deemed too urgent at the time to be able to make changes to the system by tools that don't support OAuth2.

I agree that would be good option. My main concerns were"

  1. OH3 broke the openness and purpose of a REST API and yet we are within days of the propssed Milestone1 (forst half of October).

  2. With OH2, app UIs accessed OH only through the REST API. That matches other open source products and permits contribution of other UIs for individual needs. Apparently this new UI does something outside the API for authentication. According to this, @kaikreuzer has a differing opinion on permitting other UIs.

  3. OpenHAB says they are open & transparent but how get access to much of the API has not been documented even tough a Milestone release is so close at hand.

One other commercial product I use lets you assign a secret to a particular REST client that can then use that to request short lived authorization token. That would be another option, revocable per client.

@spacemanspiff2007
Copy link
Contributor Author

The assessment of your home environment being "trusted" can be true, until it isn't, and using expiring or easily-revocable tokens has advantages over sharing passwords; the vast majority of openHAB instances being accessed with unencrypted HTTP, this means credentials are sent in clear text and multiplying requests with credentials attached increases the risk of these credentials being sniffed on the network. Limiting the number of password-based authentications to the infrequent signing in on a dedicated page ensure that passwords aren't known by potentially untrusted third party services, or stored inadequately - the less you see your password in clear text, the better. It's a known fact that people tend to reuse passwords.

In my understanding the token could be sniffed, too so the only difference would be the leakage of the password. Access to openhab would be possible in both scenarios, right?

timeouts after n failed logins.

Isn't it possible to react on a failed basic auth and prevent serving anything to the source?

One of the big advantages of these API tokens, apart from not divulging your actual password, is that they can be limited to a certain scope. That's a weak argument right now because we only have one scope defined (admin) and they're not even checked, but if we ever add more of them, that would allow limited-purpose API tokens with a scope that for instance authorizes it for managing items, and rules, but not things or services. When you think an API token could have been compromised, you can easily revoke it for the service in question without having to change your password or update the credentials everywhere else.

But if a token can be bound to a specific role so can a user. And if I create a fresh user for every service I integrate this would result in the same granularity as with tokens. As long as I don't use the initially created admin and password I can always revoke the rights for the integration thus resulting in feature parity with api tokens or am I fundamentally missing something?

(sidebar: you'll forgive a rare little public rant, but I do find it a little rude and inconsiderate to the unpaid work of other volunteers that at the slightest inconvenience to your use case, your immediate knee-jerk preference on said work seems to be basically to trash it. That's another occurrence of the negativity, abrasive commentary and baseless "developers are out of touch" nonsense that shouldn't happen in a community that aspires to be friendly and healthy, yet I have to say that I see that left and right lately, and it can be very disheartening at times. But I digress... /rant /sidebar).

Yes - no problem. I was a little bit frustrated and my wording was probably too harsh so I am sorry for that. My intention was not to trash you, your efforts or time spent implementing Oauth even if I still think there are better alternatives.

@bwosborne2
Copy link

bwosborne2 commented Oct 11, 2020

In my understanding the token could be sniffed, too so the only difference would be the leakage of the password. Access to openhab would be possible in both scenarios, right?

If https is required, the token is encrypted along with the payload. Otherwise, that statement is correct.

Isn't it possible to react on a failed basic auth and prevent serving anything to the source?

Possible, but unnecessary complexity and. if https is enforced. reduced security. IMO without enforcing https, the security is jus a complex illusion, sometimes referred to as "security theatre".

I was a little bit frustrated and my wording was probably too harsh so I am sorry for that.

Considering we are days away from the Milestone1 goal (Early October), I think it was justified, You will likely get blamed by users for not being OH3 compatible.

@spacemanspiff2007
Copy link
Contributor Author

If https is required, the token is encrypted along with the payload. Otherwise, that statement is correct.

But if we use https, basic auth is also protected 😉

Considering we are days away from the Milestone1 goal (Early October),

Milestones and and Timelines can easily be shifted.

@bwosborne2
Copy link

bwosborne2 commented Oct 11, 2020

Milestones and and Timelines can easily be shifted.

But openHAB is transparent and has not indicated the goal has slipped. 😉

@bwosborne2
Copy link

But if we use https, basic auth is also protected 😉

That is not happening before M1. Kai has spoken, ☹️

#1713 (comment)

@spacemanspiff2007
Copy link
Contributor Author

Just so it doesn't get lost after the milestone arguments I'll ping @ghys .
Thanks for quickly implementing basic auth - I still think it is the way to go.
But for the sake of a factual discussion I'd really like to hear your ideas and thoughts about my objections.

kaikreuzer pushed a commit that referenced this issue Oct 18, 2020
* Allow basic authentication to authorize API access

Closes #1699.

Note, this opens a minor security issue that allows an attacker
to brute force passwords by making calls to the API - contrary to
the authorization page, the credentials parsing for the REST API
is stateless & doesn't have a lock mechanism to lock user accounts
after too many failed login attempts.

Signed-off-by: Yannick Schaus <github@schaus.net>
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this issue Jul 11, 2023
* Allow basic authentication to authorize API access

Closes openhab#1699.

Note, this opens a minor security issue that allows an attacker
to brute force passwords by making calls to the API - contrary to
the authorization page, the credentials parsing for the REST API
is stateless & doesn't have a lock mechanism to lock user accounts
after too many failed login attempts.

Signed-off-by: Yannick Schaus <github@schaus.net>
GitOrigin-RevId: e26c49b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants