search

verdan / flaskoidc

A wrapper of Flask with pre-configured OIDC support
Apache License 2.0
52 stars 35 forks source link

There are many more configurations which can be injected from env, e.g OVERWRITE_REDIRECT_URI #1

Closed pPanda-beta closed 4 years ago

pPanda-beta commented 4 years ago

Right now the support is given for a few OpenIDConnect configurations. But some important ones are missed.

Another problem is hardcoded value of SESSION_TYPE.

Where?

https://github.com/verdan/flaskoidc/blob/76d168d680d11bd29456825075a4b5e71c79b7bf/flaskoidc/config.py

What?

Support providing any number of configurations without knowing the keys. Some important ones are :

  1. SESSION_TYPE
  2. OVERWRITE_REDIRECT_URI
  3. LOG_LEVEL

How?

Follow a pattern of FLASK_OIDC_some_magic_key=same_fancy_value to generate BaseConfig.som_magic_key = some_fancy_value and scrape all env vars. Its good to have default values but bad to hardcode those and not giving a choice to override. Tomorrow if flask_oidc introduces a new config variable, u dont have to maintain ur lib for that.

Context?

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. So no escape by hacking the nginx router. Only solution is injecting OVERWRITE_REDIRECT_URI.

Also as I'm using k8s, things are under extreme isolation and high security. So I dont want to slow down my website with a sqlite session store. So want SESSION_TYPE also configurable.

Current hack

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

My suggestion here is to remove these variables setting part from Config altogether and use the environment variables instead. This is much more flexible to set and customise. I am going to implement all the variables for flask-oidc and also flaskoidc both via environment variables to remove the confusion. This way we'll be able to use all the flask-oidc config variables via environment variables without prefixing or postfixing extra strings too.

cc: @pPanda-beta @caddac

pPanda-beta commented 4 years ago

@verdan If things are configurable from env variable, then it would be the best solution as per my use case. Please give an update when this change will be done. Thanks for the update.

verdan commented 4 years ago

@pPanda-beta just released a new version of flaskoidc Can you please give it a try? 0.1.0

For more info on how to set environment variables for various packages: https://github.com/verdan/flaskoidc/blob/master/README.md

verdan commented 4 years ago

should be fixed in the newer version i.e., 0.1.0

pPanda-beta commented 4 years ago

@verdan I dont have a separate website / demo frontend to use flaskoidc. So I couldn’t try it individually.

Once this will be integrated in amundsen frontend, I'll try it there. Thanks for the version release.