Various auth related fixes#4371
Conversation
…s setting token in SSE Signed-off-by: Florian Hotze <dev@florianhotze.com>
Fixes openhab#4354. Signed-off-by: Florian Hotze <dev@florianhotze.com>
Signed-off-by: Florian Hotze <dev@florianhotze.com>
|
@jsjames This should be backported to 5.2.x. |
#6043 Bundle Size — 13.89MiB (~-0.01%).2c594f0(current) vs 6e95b9c main#6036(baseline) Warning Bundle contains 2 duplicate packages – View duplicate packages Bundle metrics
Bundle size by type
Bundle analysis report Branch florian-h05:auth Project dashboard Generated by RelativeCI Documentation Report issue |
|
@florian-h05 Did you even look at what I did in #4365? I bet it's pretty much the same things. |
|
Ok, I see that you have changed things much more drastically. I actually got the existing logic to work by fixing the fact that it didn't pick up on 401s where it should. |
|
If there's no need to ever do SSE without a token, your simplification makes sense - I just don't know if that's the case. The existing logic allows not keep sending the token when the implicit user role is enabled. I don't know if the bandwidth use is of any relevance. |
Signed-off-by: Florian Hotze <dev@florianhotze.com>
No, I didn't look at it yet because its add-on related fixes acc. to its title and I decided to look at it later.
While looking at issue #4354 I noticed that not only the 401 detection is broken, but very likely the whole detection of reverse proxy authentication in front of openHAB, so I reverted the changes I considered problematic as well.
I don't think it is of any relevance. |
It's not always easy to name a PR. What the PR is, is me browsing around in MainUI and finding warnings and errors in the browser console, which I then address. It started out with doing that for add-ons against my core PR for add-ons, but those changes are just a few So, it's more of a "universal warnings and errors fixes" PR. It should be very easy to review, the only thing in it that isn't immediately obvious is that I made it stop trying to reconnect SSE if it fails because of invalid credentials. There is just no point in it doing that over and over again, causing "error logging" on both sides. To fix it, the user must log in, after which new SSE's will be initialized, so that particular connection will never connect. |
I'm just wondering why it was made that way in the first place. I agree that it shouldn't matter to send the token for those few requests, but when I see something like this, I always assume that there's a reason I don't know about, so I try to make it work without changing what I don't understand. But, I agree with the assessment, it's very hard to see why it would serve any purpose, and the code can be simplified without it. |
|
I hope you guys are planning on backporting this, we can't wait for 5.3.0 to make MainUI work with the implicit user role is disabled. |
Signed-off-by: Florian Hotze <dev@florianhotze.com> (cherry picked from commit b409121)
Fixes #4354: Implicit user role disabled breaks Main UI.
Fixes a regression from #3575 with reverse proxy-based AA. The more advanced error handling in
App.vue::fetchData`` is designed for XHR, notfetch`.Also simplifies the
auth.ts&auth-mixin.jscode by always sending an access token with SSE and removing thegetRequireToken()method.