[WIP] Add support for configuring Google OAuth logins#292
Conversation
se7entyse7en
changed the title
| # Should match `--timeout` value of gunicorn | ||
| SUPERSET_WEBSERVER_TIMEOUT = 300 | ||
|
|
||
| if get_env_variable('OAUTH_PROVIDER', False) == 'google': |
There was a problem hiding this comment.
maybe we should exit with an error if OAUTH_PROVIDER value is unknown.
There was a problem hiding this comment.
I've done a draft implementation of that, although I don't love it
| 'remote_app': { | ||
| 'consumer_key': get_env_variable('OAUTH_CONSUMER_KEY'), | ||
| 'consumer_secret': get_env_variable('OAUTH_CONSUMER_SECRET'), | ||
| 'base_url':'https://www.googleapis.com/oauth2/v2/', |
There was a problem hiding this comment.
It passes flake8 now (I didn't find a specific flake8 config in the repo)
There was a problem hiding this comment.
https://github.com/src-d/sourced-ui/pull/292/checks?check_run_id=244877259
sourced-ui uses flake8, pylint and black.
There was a problem hiding this comment.
flake8 and pylint look fine for this change (although I'm not 100% familiar with pylint).
This file doesn't follow black conventions, so I'd leave it as-is, but if you want, I can introduce a commit before applying black to the existing file, then add my change with black conventions.
|
From [private] slack:
Actually I meant issue 😅 but I wrote PR. I think that we could use this as a base and take ownership. But there's no urgency for this to be included now. |
Known issues: * Upon registration/login you are redirected to an ugly error message. If you change the URL you can access the application and things are working, though * User cannot access the user list! * No documentation for the new environment variables Signed-off-by: Alex Corcoles <alex@pdp7.net>
Don't worry, it took minutes to do it. It still lacks documentation but at least you have something to start with, I guess |
|
I have weird conflicts when I try to checkout to this branch which are related to the merge of new superset into srcd-ui. //cc @alexpdp7 @se7entyse7en |
|
Forks make my head hurt. Normally I wouldn't merge code which is not "ready", but I would ignore my principles to avoid having to deal with forks... |
|
another option, I can just close it, copy-paste the code into my PR and take all the praise for the implementation of the feature 😝 |
|
:D I really don't mind. I've had success with using git format-patch / am to get out of these situations. But I'm not proud of that- I feel like I'm doing a dumb thing instead of the "proper" solution. |
|
hahaha I'd vote for this BTW:
😂 I prefer to avoid having non-properly ready things. |
3 participants
Known issues:
Upon registration/login you are redirected to an ugly error message.
If you change the URL you can access the application and things are
working, though
User cannot access the user list!
No documentation for the new environment variables
Signed-off-by: Alex Corcoles alex@pdp7.net