api-gateway icon indicating copy to clipboard operation
api-gateway copied to clipboard

Enhancement Delayed start for OpenID Provider

Open precoder opened this issue 4 years ago • 3 comments

Hello, We have made a few derivations on our copy from the upstream. We want to share them with you so if you find them useful you can also benefit from the code changes.

This is the 4th of 4 changes.

Motivation: Hosting the Open ID provider and another router may cause startup ordering issues. If Open ID provider can be started after the router this is solved.

precoder avatar Oct 22 '20 15:10 precoder

Attention: The title of this pull request is a bit misleading A little bit of background: We're running a setup where at one point we were trying to have the OIDC Provider and the OIDC Client run in the same membrane instance. We're no longer doing this, having implemented the Auth handling in our own software by now, but this still is a somewhat useful setup when testing in small environments.

Now what happened with vanilla Membrane was that the OIDC Authorization Service (the client part) was starting up before the sockets were opened - causing the service to always fail.

With this change the Authorization Service is moved to the end of the queue, therefore always only attempting to start after everything else (including the Authorization Provider and the network stack) is started.

DJGummikuh avatar Oct 23 '20 09:10 DJGummikuh

Thank you both for your contributions.

If I understand it correctly, pull requests #330 and #364 try to solve the same problem. As we also have the same problem in our own infrastructure, we are eager to solve it.

Let's compare the approaches and implementations. We'll try to add some comments in the coming days.

rrayst avatar Oct 23 '20 09:10 rrayst

Oops these two pull-requests might actually be the same, at least they came from the same fork on our end, most likely this issue here is the more polished one, though. Edit: We started using membrane in 2018 but (due to the hacky appraoch we took at getting membrane working in our environment) never felt these changes would be a good idea to be pushed back to upstream. Now that we wanted to update to the latest version (after that security incident we already provided a pull request for) we took the effort of consolidating our changes - which is why you got 4 pull requests from us in very short succession.

DJGummikuh avatar Oct 23 '20 09:10 DJGummikuh

A solution is described in https://github.com/membrane/service-proxy/pull/364#issuecomment-1002909928.

rrayst avatar Nov 22 '22 12:11 rrayst