-
Notifications
You must be signed in to change notification settings - Fork 105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Make anitya use authlib instead of social_auth #1220
base: master
Are you sure you want to change the base?
Conversation
What is working so far: authentication with GitHub and Google. |
Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
|
This pull request introduces 2 alerts when merging ffbba79 into b16ca5e - view on LGTM.com new alerts:
|
requirements.txt
Outdated
@@ -11,7 +11,6 @@ ordered-set>=3.1, <5.0.0 | |||
packaging>=20.0 # (no upper limit, this is a calendar-based version) | |||
python-dateutil>=2.8.0, <3.0.0 | |||
semver>=2.8.1, <3.0.0 | |||
social-auth-app-flask>=1.0.0, <2.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to add the authlib to requirements.txt as well.
@@ -42,6 +42,7 @@ | |||
fedora-messaging, | |||
python3-alembic, | |||
python3-arrow, | |||
python-authlib, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's nice that the authlib is available in Fedora, one less package to download from PyPi.
SSE_FEED="http://firehose.libraries.io/events", | ||
CRON_POOL=10, # Number of workers for check service | ||
CHECK_TIMEOUT=600, # Timeout for check service | ||
# When this number of failed checks is reached, | ||
# project will be automatically removed, if no version was retrieved yet | ||
CHECK_ERROR_THRESHOLD=100, | ||
# Enabled authentication backends |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be also done in the example config and config for vagrant and container setup.
@@ -0,0 +1,59 @@ | |||
import flask |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file deserves some comments. :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw that the social_auth stuff was registered as flask blueprint in the app.py
, so I was trying to make it similar.
The rest is from authlib examples. The def oauthlogin
is usually named login
in the authlib documentation. I renamed it because there already is login function in ui.py.
Then I created the tokens in github and google for the anitya app and since both parties send a different user_info, so it is processed differently.
I will add docstrings and comments.
This would be nice for https://rpmfusion.org/ next. Good job. |
Because there is now initiative working on Flask OIDC to remove the deprecated parts, we should probably use it instead of using authlib directly. |
from sqlalchemy.exc import IntegrityError | ||
from sqlalchemy.orm.exc import NoResultFound | ||
|
||
<<<<<<< HEAD |
Check failure
Code scanning / CodeQL
Syntax error Error
#if user.oauth.count() == 0: | ||
# raise IntegrityError( | ||
# "Missing authlib table", | ||
# {"authlib": None, "email": user.email}, | ||
# None, |
Check notice
Code scanning / CodeQL
Commented-out code Note
Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci ❌ fi-tox-mypy FAILURE in 4m 09s |
Allow authentication with Fedora, GitHub and Google.
This PR aims to get rid of the social_auth library and use authlib instead.
Related issue #1139