search

opensmarthouse / opensmarthouse-core

Eclipse Public License 2.0
7 stars 0 forks source link

OH updates #8

Closed cdjackson closed 3 years ago

cdjackson commented 3 years ago

@splatch FYI this soaks up changes from OHC over the past few weeks. It requires testing, but builds fine. Signed-off-by: Chris Jackson chris@cd-jackson.com

splatch commented 3 years ago

I was looking at auth stuff and I started to drive it into a bit different direction. Current OH approach binds auth token handling with specific user store which doesn't seem right. I moved part of the logic to JWTAuthenticationProvider which is configurable to validate JWTCredentials and work with different token issuers. In such approach it is possible to integrate runtime with any token source, not just internal one. It breaks obviously clean merge path. I also re-used some parts of CredentialExtractors which come in first implementation of auth api I provided for ESH a while ago (JWT was part of concept back then, removed once considered unwanted). It is still a bit ugly in form, but should save us troubles in different handling of inputs between servlets and rest resource.

My changes are not published yet so they do not block merge of this one. Feel free to go with it - I will continue with my stuff on top of this PR.

cdjackson commented 3 years ago

@splatch thanks for this. I think your process might be better than mine 👍

Let me know when you're happy for this to be merged - I guess that since your assembly is running, it's good to go?

splatch commented 3 years ago

@splatch thanks for this. I think your process might be better than mine +1

I've updated patch script so it prompts to automatically create a commit message with author and sha picked from patch file. Thanks to that and git format-patch process is sequential and rather fast. In case when there are rejections there is a time to fix issue and get back to shell and nod to the prompt.

splatch commented 3 years ago

I've pushed updated commits - there are no changes, I just added signed-off to make DCO less concerned.

@cdjackson I have not updated commits which you ported, you might want to sign them or just merge this PR if you're fine with it.

[its rebased to latest master, ready to go]

splatch commented 3 years ago

FYI, I been running build from this branch for a week with no issues. If anything bad happens then it should be fixed in separate PR given that it already accumulated lots of changes.

cdjackson commented 3 years ago

Thanks Łukasz. I'll merge and squash this.