search

amundsen-io / amundsen

Amundsen is a metadata driven application for improving the productivity of data analysts, data scientists and engineers when interacting with data.
https://www.amundsen.io/amundsen/
Apache License 2.0
4.44k stars 960 forks source link

FlaskOidc is not useful while using with k8s ingress #594

Closed pPanda-beta closed 4 years ago

pPanda-beta commented 4 years ago

I was using amundsen frontend with flaskoidc as auth layer wrapper, and following is the kubernetes deployment architecture :

Browser(Client) --(HTTPS)--> Nginx Ingress Controller --(HTTP, port:5000)--> Amundsen-Frontend-Pod

So flask_oidc was detecting the wrong redirect_uri, (which I was able to fix by using proxy_redirect, aka find and replace in location header for 3xx) as well as it is sending a "state" variable which holds the redirect_uri in a JWT (Now nginx can not help anymore). So no escape by hacking the nginx router. Only solution is injecting OVERWRITE_REDIRECT_URI.

Expected Behavior

redirect uri comes correctly on "Location" header with "https" scheme

Current Behavior

incorrect redirect uri comes correctly on "Location" header with "http" scheme

Possible Solution

A pre start command (or init container) and env variable FLASK_OIDC_OVERWRITE_REDIRECT_URI

echo "    OVERWRITE_REDIRECT_URI = os.environ.get('FLASK_OIDC_OVERWRITE_REDIRECT_URI', 'False') " \
    >>  /usr/local/lib/python3.7/site-packages/flaskoidc/config.py

Steps to Reproduce

deploy the k8s app and expose using ingress

Screenshots (if appropriate)

Context

More info at : https://github.com/verdan/flaskoidc/issues/1

Your Environment

Amunsen version used: dockerimages : amundsendev/amundsen-search:2.4.0 amundsendev/amundsen-metadata:2.5.4 amundsendev/amundsen-frontend-oidc:2.2.2

Deployment: k8s behind ingress

feng-tao commented 4 years ago

cc @verdan

verdan commented 4 years ago

cc @verdan

Will take a look once I am back from my holiday!!

feng-tao commented 4 years ago

cc @verdan

verdan commented 4 years ago

I have this in my agenda as the next feature (once I am done with the users/owners part). I'll be working on fixing issues with this and also OIDC for google.

pPanda-beta commented 4 years ago

hey @verdan Just one general update, in my organisation we first started with google oidc, later moved to our own keycloak, which is working as oidc broker.

With google it was working fine. But the refresh token lifetime was quite high, and no option in the frontend to logout. It got solved when things are in our control using keycloak.

With keycloak there are slight glitches (clearing out cookies helped), with which we can live with.

Anyway I believe the problem lies with https://github.com/verdan/flaskoidc/issues/1 and nothing can be done from amundsen frontend side.

mgorsk1 commented 4 years ago

I think the glitches part in keycloak oidc is also what we are experiencing @verdan . I assume that's going in the fix as well.

verdan commented 4 years ago

@mgorsk1 @pPanda-beta what glitches ? Can you please be more specific or examples?

flaskoidc is not very complex or a fancy code, can you please help me fix those issues and open a PR if you see this happening as I can not reproduce all the issues sometimes, especially in the case of google OIDC.

But please feel free to open a PR and I will prioritise the review.

mgorsk1 commented 4 years ago

The one I have in mind is what we sometimes notice (the same thing I mentioned before internally) when every Amundsen request returns 500 and what helps is clearing cookies/opening in incognito mode. It seems that not redirecting to login screen when token has expired might be the issue.

pPanda-beta commented 4 years ago

@verdan

This is issue is mainly a proxy for https://github.com/verdan/flaskoidc/issues/1 , there is no problem on amundsen frontend actually.

For glitches of flaskoidc, I've not debugged myself. But mostly it happens when tokens are expired and on the same browser we have logged out and logged in with a different user on keycloak. I'll share a detailed reproduction steps on a fresh issue on flaskoidc, not here. This is in favour of concentrating on the current issue (that is over-ridding the scheme).

As an improvement of flaskoidc the following I can suggest, (THIS IS ALIGNED WITH THE CURRENT ISSUE)

Identify scheme from x-forwarded-proto header IN RUNTIME since, both nginx and istio ingress automaticity sends this header.

This will help us to access any flaskoidc powered app from both an http ingress as well as https ingress. The solution mentioned here can not support multiple schemes.

verdan commented 4 years ago

I believe this can be closed now with the latest version of flaskoidc. cc: @pPanda-beta

pPanda-beta commented 4 years ago

Sure @verdan Though there are some other oidc related problems on amundsen but all of those can be solved from flaskoidc. I'll raise corresponding issues on flaskoidc.