-
Notifications
You must be signed in to change notification settings - Fork 86
Add OIDC login with Authlib (Google, GitHub, Microsoft) – closes #355 #654
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
d87d19a
bdf27b1
e874d4f
410014b
cd0a3d6
2ae7b44
a93999d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
GOOGLE_CLIENT_ID=your_google_client_id | ||
GOOGLE_CLIENT_SECRET=your_google_client_secret | ||
|
||
# GitHub | ||
GITHUB_CLIENT_ID=your_github_client_id | ||
GITHUB_CLIENT_SECRET=your_github_client_secret | ||
|
||
# Microsoft | ||
MICROSOFT_CLIENT_ID=your_microsoft_client_id | ||
MICROSOFT_CLIENT_SECRET=your_microsoft_client_secret | ||
|
||
# General | ||
SECRET_KEY=supersecret | ||
BASE_URL=http://localhost:5000 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,16 +23,18 @@ | |
import os | ||
import warnings | ||
from typing import Any, Dict, Optional | ||
from auth.oidc import configure_oauth, oidc_bp | ||
from dotenv import load_dotenv | ||
|
||
from flask import Flask, abort, g, send_from_directory | ||
from flask import Flask, abort, g, send_from_directory, session | ||
from flask_compress import Compress | ||
from flask_cors import CORS | ||
from flask_jwt_extended import JWTManager | ||
from gramps.gen.config import config as gramps_config | ||
from gramps.gen.config import set as setconfig | ||
|
||
from .api import api_blueprint | ||
from .api.cache import request_cache, thumbnail_cache | ||
from .api.cache import thumbnail_cache | ||
from .api.ratelimiter import limiter | ||
from .api.search.embeddings import load_model | ||
from .api.util import close_db | ||
|
@@ -42,6 +44,7 @@ | |
from .dbmanager import WebDbManager | ||
from .util.celery import create_celery | ||
|
||
load_dotenv() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please do not add dotenv! The place to document needed environment variables is in the documentation repository. |
||
|
||
def deprecated_config_from_env(app): | ||
"""Add deprecated config from environment variables. | ||
|
@@ -76,10 +79,10 @@ def deprecated_config_from_env(app): | |
return app | ||
|
||
|
||
def create_app(config: Optional[Dict[str, Any]] = None, config_from_env: bool = True): | ||
def create_app(config: Optional[Dict[str, Any]] = None): | ||
"""Flask application factory.""" | ||
app = Flask(__name__) | ||
|
||
app.secret_key = os.getenv("SECRET_KEY") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please do not read |
||
app.logger.setLevel(logging.INFO) | ||
|
||
# load default config | ||
|
@@ -93,8 +96,7 @@ def create_app(config: Optional[Dict[str, Any]] = None, config_from_env: bool = | |
deprecated_config_from_env(app) | ||
|
||
# use prefixed environment variables if exist | ||
if config_from_env: | ||
app.config.from_prefixed_env(prefix="GRAMPSWEB") | ||
app.config.from_prefixed_env(prefix="GRAMPSWEB") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please rebase. |
||
|
||
# update config from dictionary if present | ||
if config: | ||
|
@@ -143,7 +145,9 @@ def create_app(config: Optional[Dict[str, Any]] = None, config_from_env: bool = | |
app.config["SQLALCHEMY_DATABASE_URI"] = app.config["USER_DB_URI"] | ||
user_db.init_app(app) | ||
|
||
request_cache.init_app(app, config=app.config["REQUEST_CACHE_CONFIG"]) | ||
configure_oauth(app) | ||
DavidMStraub marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will register the providers. Does this involve an HTTP request to the provider endpoint? If so, what if that request fails? Will the whole flask app fail to start? |
||
app.register_blueprint(oidc_bp) | ||
|
||
thumbnail_cache.init_app(app, config=app.config["THUMBNAIL_CACHE_CONFIG"]) | ||
|
||
# enable CORS for /api/... resources | ||
|
@@ -207,4 +211,47 @@ def close_user_db_connection(exception) -> None: | |
def ready(): | ||
return {"status": "ready"}, 200 | ||
|
||
return app | ||
@oidc_bp.route("/callback/<provider>") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please move this to the OIDC module. |
||
def authorize(provider): | ||
client = oauth.create_client(provider) | ||
token = client.authorize_access_token() | ||
oidc_user = client.parse_id_token(token) if provider == "google" else client.get('user').json() | ||
|
||
# Get email from OIDC user info | ||
email = oidc_user.get('email') | ||
if not email: | ||
return {"error": "No email provided by OIDC provider"}, 400 | ||
|
||
# Check if user exists | ||
query = user_db.session.query(User) | ||
user = query.filter_by(email=email).scalar() | ||
|
||
if not user: | ||
# Create new user with default role | ||
try: | ||
user = User( | ||
id=uuid.uuid4(), | ||
name=email.split('@')[0], # Use part before @ as username | ||
email=email, | ||
fullname=oidc_user.get('name', ''), | ||
role=0, # Default role | ||
pwhash='', # No password for OIDC users | ||
) | ||
user_db.session.add(user) | ||
user_db.session.commit() | ||
except IntegrityError: | ||
return {"error": "User creation failed"}, 400 | ||
|
||
# Set up session | ||
session['user_id'] = str(user.id) | ||
session['user_name'] = user.name | ||
session['user_role'] = user.role | ||
|
||
return {"status": "logged_in", "provider": provider, "user": { | ||
"name": user.name, | ||
"email": user.email, | ||
"full_name": user.fullname, | ||
"role": user.role | ||
}} | ||
|
||
return app |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
import os | ||
from authlib.integrations.flask_client import OAuth | ||
from flask import Blueprint, redirect, url_for, session, request | ||
from dotenv import load_dotenv | ||
|
||
load_dotenv() | ||
|
||
oauth = OAuth() | ||
|
||
oidc_bp = Blueprint("oidc", __name__, url_prefix="/auth") | ||
|
||
def configure_oauth(app): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The frontend needs a way of knowing which of the providers have been registered in the end, based on the configuration, otherwise the frontend doesn't know which buttons to show. I think the best solution is to add it to a list of registered providers to the |
||
oauth.init_app(app) | ||
|
||
oauth.register( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you actually using OIDC here or just OAuth? https://docs.authlib.org/en/latest/client/flask.html#flask-openid-connect-client There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that OIDC is just a defined way to use oauth. From the docs section that you linked:
|
||
name='google', | ||
client_id=os.getenv("GOOGLE_CLIENT_ID"), | ||
client_secret=os.getenv("GOOGLE_CLIENT_SECRET"), | ||
access_token_url='https://oauth2.googleapis.com/token', | ||
authorize_url='https://accounts.google.com/o/oauth2/auth', | ||
api_base_url='https://www.googleapis.com/oauth2/v1/', | ||
client_kwargs={'scope': 'openid email profile'}, | ||
) | ||
|
||
oauth.register( | ||
name='github', | ||
client_id=os.getenv("GITHUB_CLIENT_ID"), | ||
client_secret=os.getenv("GITHUB_CLIENT_SECRET"), | ||
access_token_url='https://github.com/login/oauth/access_token', | ||
authorize_url='https://github.com/login/oauth/authorize', | ||
api_base_url='https://api.github.com/', | ||
client_kwargs={'scope': 'read:user user:email'}, | ||
) | ||
|
||
oauth.register( | ||
name='microsoft', | ||
client_id=os.getenv("MICROSOFT_CLIENT_ID"), | ||
client_secret=os.getenv("MICROSOFT_CLIENT_SECRET"), | ||
access_token_url='https://login.microsoftonline.com/common/oauth2/v2.0/token', | ||
authorize_url='https://login.microsoftonline.com/common/oauth2/v2.0/authorize', | ||
api_base_url='https://graph.microsoft.com/v1.0/', | ||
client_kwargs={'scope': 'openid email profile'}, | ||
) | ||
|
||
@oidc_bp.route("/login/<provider>") | ||
def login(provider): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doc string please There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And please add an example to the doc string, I see there is a |
||
redirect_uri = url_for("oidc.authorize", provider=provider, _external=True) | ||
return oauth.create_client(provider).authorize_redirect(redirect_uri) | ||
|
||
@oidc_bp.route("/callback/<provider>") | ||
def authorize(provider): | ||
client = oauth.create_client(provider) | ||
token = client.authorize_access_token() | ||
user = client.parse_id_token(token) if provider == "google" else client.get('user').json() | ||
session['user'] = user | ||
return {"status": "logged_in", "provider": provider, "user": user} |
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.
Can't this be handled as part of the flask configuration? https://docs.authlib.org/en/latest/client/flask.html#configuration
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.
So this file is not used anymore?
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.
yes
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.
Then why is it still here?