Add Keycloak Refresh Token Middleware

[bugraoz93]

---

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

[vincbeck]

I think this PR should be separated in multiple PRs, you are trying to implement multiple different features. I started to work on the create token API (https://github.com/orgs/apache/projects/500/views/1?pane=issue&itemId=112609811) but it seems you want to work on it as well?

[bugraoz93]

I think this PR should be separated in multiple PRs, you are trying to implement multiple different features. I started to work on the create token API (Keycloak auth manager (view)) but it seems you want to work on it as well?

I should have communicated this with you better. My bad, Vincent! Although I saw the issue, I first started adding these features because I thought we would have these endpoints to manage login, similar to FAB and SAM but then realised that we should manage the login and refresh part separately. Those things just stayed. I can also help on those parts if there isn't too much duplicate work already on your side 😅 Please let me know which one can stay and I can separate the PR and implement unit test on it

[vincbeck]

No worries at all Bugra, and thanks a lot for your hard work, I understand Keycloak can be confusing and auth in general is not easy to grasp at first sight. Let's do it step by step, could you update this PR to focus only on the refresh token logic? We'll see other points later. I think that will simplify the review.

[bugraoz93]

Thanks Vincent! It has been a great time spent to learn and connecting all the pieces for both Keycloak and our internal authentication. I only worked on the backend API part in other authentication managers, so I missed completely what and how the entire authentication system works.
Sure, Vincent, I will update the PR to include only the refresh_token logic. I will update the PR today or tomorrow. Today, I have a full day plan, so I will try the night or tomorrow morning to update the PR :)

[vincbeck]

Overall it is much better :) I like the direction and I think we are close. Thanks for being receptive to my comments :)

On top of my comments, one other thing: I think we should call refresh_token only when the token expired. A bit like your previous implementation. Basically:

• Detecting when Keycloak token expired by catching the exception. When that happens, raising another generic exception (e.g. using an exception from jwt package)
• In the middleware, catching this exception, and calling refresh_token when that happens

That allows to call refresh_token only when the token expires, otherwise we call refresh_token at every call, which can lead to severe latency increase.

[bugraoz93]

Overall it is much better :) I like the direction and I think we are close. Thanks for being receptive to my comments :)

On top of my comments, one other thing: I think we should call refresh_token only when the token expired. A bit like your previous implementation. Basically:

• Detecting when Keycloak token expired by catching the exception. When that happens, raising another generic exception (e.g. using an exception from jwt package)
• In the middleware, catching this exception, and calling refresh_token when that happens

That allows to call refresh_token only when the token expires, otherwise we call refresh_token at every call, which can lead to severe latency increase.

Thanks a lot, Vincent! I would be a lot faster in different areas but in auth managers, I have learned a lot during the implementation. Still small unit test is needed for refresh_token. Functionality is also working as expected. I will try to do it and finalise this one in the coming hours.

[vincbeck]

LGTM! Only this comment thread to resolve and we are good :)

[vincbeck]

One last ask (promise, the last one), can you add a test for the middleware? Thanks again for the work!

[bugraoz93]

One last ask (promise, the last one), can you add a test for the middleware? Thanks again for the work!

For sure, thanks for pointing out! I would say this is a good collaboration rather than an ask :)

[bugraoz93]

Added the test, which I had missed while deleting it for the CORS. That was on me

[bugraoz93]

This needs small care on the test. It worked in my local but seems not here. Will check this out