-
Notifications
You must be signed in to change notification settings - Fork 84
Add OpenID Connect IDP #1839
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: main
Are you sure you want to change the base?
Add OpenID Connect IDP #1839
Changes from all commits
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,134 @@ | ||
| from typing import TypedDict | ||
|
|
||
| import authlib.oauth2.rfc6749.grants as rfc6749_grants | ||
| import authlib.oidc.core.grants as oidc_core_grants | ||
| from authlib.integrations.flask_oauth2 import AuthorizationServer | ||
| from authlib.integrations.sqla_oauth2 import create_query_client_func, create_save_token_func | ||
| from authlib.oauth2.rfc6749 import OAuth2Request | ||
| from authlib.oauth2.rfc6749.util import scope_to_list | ||
| from authlib.oidc.core.claims import UserInfo | ||
| from flask import request | ||
|
|
||
| from main import db | ||
| from models.oidc import OAuth2AuthorizationCode, OAuth2Client, OAuth2Token | ||
| from models.user import User | ||
|
|
||
| SCOPES: dict[str, str] = { | ||
| "openid": "Your anonymous account identifier", | ||
|
Contributor
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. I think it's pseudonymous, rather than anonymous?
Member
Author
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. heh I originally had "pseudoanonymous" and was like.. that's not a word |
||
| "email": "The email address associated with your account", | ||
| "permissions": "Your EMF website permissions", | ||
| } | ||
|
|
||
|
|
||
| def get_issuer(): | ||
| return f"https://{request.host}" | ||
|
|
||
|
|
||
| def save_authorization_code(code: str, request: OAuth2Request) -> OAuth2AuthorizationCode: | ||
| client = request.client | ||
| item = OAuth2AuthorizationCode( | ||
| code=code, | ||
| client_id=client.client_id, | ||
| redirect_uri=request.payload.redirect_uri, | ||
| scope=request.payload.scope, | ||
| user_id=request.user.id, | ||
| ) | ||
| db.session.add(item) | ||
| db.session.commit() | ||
| return item | ||
|
|
||
|
|
||
| def exists_nonce(nonce, request) -> bool: | ||
| # TODO: implement me! | ||
| return False | ||
|
|
||
|
|
||
| class JWTConfig(TypedDict): | ||
| key: str | ||
| alg: str | ||
| iss: str | ||
| exp: int | ||
|
|
||
|
|
||
| def get_jwt_config(grant) -> JWTConfig: | ||
| return JWTConfig(key="foo", alg="HS256", iss=get_issuer(), exp=999999999999) | ||
|
|
||
|
|
||
| def generate_user_info(user: User, scope) -> UserInfo: | ||
| info = { | ||
| "sub": user.id, | ||
| } | ||
| scopes = scope_to_list(scope) | ||
| if "email" in scopes: | ||
| info["email"] = user.email | ||
|
Contributor
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. We should probably also include
Member
Author
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. Good shout!
Member
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. We also don't verify emails though. That would be a new feature.
Member
Author
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. But we send people login links?
Member
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. Yep, and once they've logged in for the first time we could say their email is verified. But we don't currently verify emails before use. Maybe we should, in theory there's the potential for someone to submit a CFP with someone else's email including a nasty message.
Member
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 I'm not sure what the point of including email_verified is when it's not explicitly verified and we don't currently have people asking to use it?
Member
Author
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. Ah, I hadn't realised that you get logged in immediately on cfp submit/ticket purchase - IMO that should probably change. Would also have the benefit of preventing people from buying tickets under a mis-spelled email, though IDK in reality how much that happens?
It's a standard claim, so OIDC clients may well use it - and if it were as simple as "every account's email is verified by the fact that they've logged in" seems like there's no reason to not include it?
Member
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.
Once or twice per event, and it's almost always an easy fix via support email. Requiring a verified email to buy tickets is a big change which is basically equivalent to adding a pre-registration stage, which we don't want to do. It might be good to handle email verified status better, but that's probably out of scope of this issue.
Member
Author
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. Yeah, it would make ticket buying much more annoying I guess. I think it could be good to put CFP submission behind an email link though.
I don't think that's the case - I hadn't realised when initially working on this that an EMF website account didn't automatically necessitate that the email was verified. IMO we should either:
Both are fairly easy to implement. I think the latter is probably preferable as it means that we don't have to rely on (potentially hacked-together by attendees) client apps checking
Member
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. Yep, agree the second option is less likely to fail open. And yeah it's not much work, we set it on when authenticating a user, and clear it if we ever change a user's email address (in theory we should transfer everything to a new user, but that's often harder than fixing the typo). Probably a good excuse to add an admin page for this. |
||
| if "permissions" in scopes: | ||
| info["permissions"] = [p.name for p in user.permissions] | ||
|
|
||
| return UserInfo(info) | ||
|
|
||
|
|
||
| class AuthorizationCodeGrant(rfc6749_grants.AuthorizationCodeGrant): | ||
| def save_authorization_code(self, code, request): | ||
| return save_authorization_code(code, request) | ||
|
|
||
| def parse_authorization_code(self, code, client) -> OAuth2AuthorizationCode | None: | ||
| authcode = OAuth2AuthorizationCode.query.filter_by(code=code, client_id=client.client_id).first() | ||
| if authcode and not authcode.is_expired(): | ||
| return authcode | ||
| return None | ||
|
|
||
| def delete_authorization_code(self, authorization_code): | ||
| db.session.delete(authorization_code) | ||
| db.session.commit() | ||
|
|
||
| def authenticate_user(self, authorization_code): | ||
| return User.query.get(authorization_code.user_id) | ||
|
|
||
|
|
||
| class OpenIDCode(oidc_core_grants.OpenIDCode): | ||
| def exists_nonce(self, nonce, request): | ||
| return exists_nonce(nonce, request) | ||
|
|
||
| def get_jwt_config(self, grant) -> JWTConfig: | ||
| return get_jwt_config(grant) | ||
|
|
||
| def generate_user_info(self, user, scope) -> UserInfo: | ||
| return generate_user_info(user, scope) | ||
|
|
||
|
|
||
| class ImplicitGrant(oidc_core_grants.OpenIDImplicitGrant): | ||
| def exists_nonce(self, nonce, request) -> bool: | ||
| return exists_nonce(nonce, request) | ||
|
|
||
| def get_jwt_config(self) -> JWTConfig: | ||
| return get_jwt_config(grant=None) | ||
|
|
||
| def generate_user_info(self, user, scope) -> UserInfo: | ||
| return generate_user_info(user, scope) | ||
|
|
||
|
|
||
| class HybridGrant(oidc_core_grants.OpenIDHybridGrant): | ||
| def save_authorization_code(self, code, request): | ||
| return save_authorization_code(code, request) | ||
|
|
||
| def exists_nonce(self, nonce, request) -> bool: | ||
| return exists_nonce(nonce, request) | ||
|
|
||
| def get_jwt_config(self) -> JWTConfig: | ||
| return get_jwt_config(grant=None) | ||
|
|
||
| def generate_user_info(self, user, scope) -> UserInfo: | ||
| return generate_user_info(user, scope) | ||
|
|
||
|
|
||
| authorization = AuthorizationServer() | ||
|
|
||
|
|
||
| def init_oauth(app): | ||
| query_client = create_query_client_func(db.session, OAuth2Client) | ||
| save_token = create_save_token_func(db.session, OAuth2Token) | ||
|
|
||
| authorization.init_app(app, query_client=query_client, save_token=save_token) | ||
| authorization.register_grant(AuthorizationCodeGrant, [OpenIDCode(require_nonce=True)]) | ||
| authorization.register_grant(ImplicitGrant) | ||
| authorization.register_grant(HybridGrant) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| import logging | ||
|
|
||
| import click | ||
| from authlib.oauth2 import OAuth2Error | ||
| from authlib.oauth2.rfc6749.util import scope_to_list | ||
| from authlib.oidc.discovery import OpenIDProviderMetadata | ||
| from flask import Blueprint, jsonify, render_template, request, url_for | ||
| from flask.typing import ResponseValue | ||
| from flask_login import current_user, login_required | ||
| from werkzeug.security import gen_salt | ||
|
|
||
| from main import db | ||
| from models.oidc import OAuth2Client | ||
|
|
||
| from .oauth import SCOPES, authorization, get_issuer | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
| oidc = Blueprint("oidc", "oidc") | ||
|
|
||
|
|
||
| @oidc.cli.command("create_client") | ||
| @click.option("--name", type=str) | ||
| @click.option("--redirecturi", type=str) | ||
| @click.option("--official/--unofficial") | ||
| @click.option("--scope", default=["openid"], multiple=True) | ||
| def create_client(name: str, redirecturi: str, official: bool, scope: list[str]): | ||
| if invalid := [s for s in scope if s not in SCOPES]: | ||
| logger.error("Invalid scopes: %s", ", ".join(invalid)) | ||
| raise click.exceptions.Exit(1) | ||
|
|
||
| client = OAuth2Client( | ||
| client_id=gen_salt(24), | ||
| official=official, | ||
| ) | ||
| client.set_client_metadata( | ||
| { | ||
| "client_name": name, | ||
| "redirect_uris": [redirecturi], | ||
| "grant_types": ["code"], | ||
| "response_types": ["code id_token"], | ||
| "scope": " ".join(scope), | ||
| } | ||
| ) | ||
| db.session.add(client) | ||
| db.session.commit() | ||
| logger.info("New OIDC client created. Client id: %s", client.client_id) | ||
|
|
||
|
|
||
| @oidc.get("/.well-known/openid-configuration") | ||
| def discovery() -> ResponseValue: | ||
| """Implements the OpenID Connect Discovery protocol. | ||
|
|
||
| https://openid.net/specs/openid-connect-discovery-1_0.html | ||
| """ | ||
| m = OpenIDProviderMetadata( | ||
| issuer=get_issuer(), | ||
| authorization_endpoint=url_for("oidc.authorize", _external=True), | ||
| token_endpoint=url_for("oidc.token", _external=True), | ||
| jwks_uri=url_for("oidc.jwks", _external=True), | ||
| response_types_supported=["code", "id_token", "code id_token"], | ||
| subject_types_supported=["public"], | ||
| id_token_signing_alg_values_supported=["RS256"], | ||
| ) | ||
| m.validate() | ||
| return m | ||
|
|
||
|
|
||
| @oidc.route("/oidc/authorize", methods=["GET", "POST"]) | ||
| @login_required | ||
| def authorize() -> ResponseValue: | ||
| if request.method == "GET": | ||
| try: | ||
| grant = authorization.get_consent_grant(end_user=current_user) | ||
| scope = grant.client.get_allowed_scope(grant.request.payload.scope) | ||
| except OAuth2Error as e: | ||
| return jsonify(dict(e.get_body())) | ||
|
|
||
| scopes = {s: SCOPES[s] for s in scope_to_list(scope)} | ||
| scopents = {scope: desc for scope, desc in SCOPES.items() if scope not in scopes} | ||
| return render_template("oidc/authorize.html", grant=grant, scopes=scopes, scopents=scopents) | ||
| res = authorization.create_authorization_response( | ||
| grant_user=current_user if "authorize" in request.form else None | ||
| ) | ||
| return res | ||
|
|
||
|
|
||
| @oidc.post("/oidc/token") | ||
| def token(): | ||
| return authorization.create_token_response() | ||
|
|
||
|
|
||
| @oidc.get("/.well-known/jwks.json") | ||
| def jwks(): ... |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| """oidc_stuff | ||
|
|
||
| Revision ID: 26b2a94192b3 | ||
| Revises: bcf21daa6073 | ||
| Create Date: 2025-08-25 21:28:12.004215 | ||
|
|
||
| """ | ||
|
|
||
| # revision identifiers, used by Alembic. | ||
| revision = '26b2a94192b3' | ||
| down_revision = 'bcf21daa6073' | ||
|
|
||
| from alembic import op | ||
| import sqlalchemy as sa | ||
|
|
||
|
|
||
| def upgrade(): | ||
| # ### commands auto generated by Alembic - please adjust! ### | ||
| op.create_table('oauth2_client', | ||
| sa.Column('id', sa.Integer(), nullable=False), | ||
| sa.Column('official', sa.Boolean(), nullable=True), | ||
| sa.Column('client_id', sa.String(length=48), nullable=True), | ||
| sa.Column('client_secret', sa.String(length=120), nullable=True), | ||
| sa.Column('client_id_issued_at', sa.Integer(), nullable=False), | ||
| sa.Column('client_secret_expires_at', sa.Integer(), nullable=False), | ||
| sa.Column('client_metadata', sa.Text(), nullable=True), | ||
| sa.PrimaryKeyConstraint('id', name=op.f('pk_oauth2_client')) | ||
| ) | ||
| with op.batch_alter_table('oauth2_client', schema=None) as batch_op: | ||
| batch_op.create_index(batch_op.f('ix_oauth2_client_client_id'), ['client_id'], unique=False) | ||
|
|
||
| op.create_table('oauth2_authcode', | ||
| sa.Column('id', sa.Integer(), nullable=False), | ||
| sa.Column('user_id', sa.Integer(), nullable=True), | ||
| sa.Column('code', sa.String(length=120), nullable=False), | ||
| sa.Column('client_id', sa.String(length=48), nullable=True), | ||
| sa.Column('redirect_uri', sa.Text(), nullable=True), | ||
| sa.Column('response_type', sa.Text(), nullable=True), | ||
| sa.Column('scope', sa.Text(), nullable=True), | ||
| sa.Column('nonce', sa.Text(), nullable=True), | ||
| sa.Column('auth_time', sa.Integer(), nullable=False), | ||
| sa.Column('acr', sa.Text(), nullable=True), | ||
| sa.Column('amr', sa.Text(), nullable=True), | ||
| sa.Column('code_challenge', sa.Text(), nullable=True), | ||
| sa.Column('code_challenge_method', sa.String(length=48), nullable=True), | ||
| sa.ForeignKeyConstraint(['user_id'], ['user.id'], name=op.f('fk_oauth2_authcode_user_id_user'), ondelete='CASCADE'), | ||
| sa.PrimaryKeyConstraint('id', name=op.f('pk_oauth2_authcode')), | ||
| sa.UniqueConstraint('code', name=op.f('uq_oauth2_authcode_code')) | ||
| ) | ||
| op.create_table('oauth2_token', | ||
| sa.Column('id', sa.Integer(), nullable=False), | ||
| sa.Column('user_id', sa.Integer(), nullable=True), | ||
| sa.Column('client_id', sa.String(length=48), nullable=True), | ||
| sa.Column('token_type', sa.String(length=40), nullable=True), | ||
| sa.Column('access_token', sa.String(length=255), nullable=False), | ||
| sa.Column('refresh_token', sa.String(length=255), nullable=True), | ||
| sa.Column('scope', sa.Text(), nullable=True), | ||
| sa.Column('issued_at', sa.Integer(), nullable=False), | ||
| sa.Column('access_token_revoked_at', sa.Integer(), nullable=False), | ||
| sa.Column('refresh_token_revoked_at', sa.Integer(), nullable=False), | ||
| sa.Column('expires_in', sa.Integer(), nullable=False), | ||
| sa.ForeignKeyConstraint(['user_id'], ['user.id'], name=op.f('fk_oauth2_token_user_id_user'), ondelete='CASCADE'), | ||
| sa.PrimaryKeyConstraint('id', name=op.f('pk_oauth2_token')), | ||
| sa.UniqueConstraint('access_token', name=op.f('uq_oauth2_token_access_token')) | ||
| ) | ||
| with op.batch_alter_table('oauth2_token', schema=None) as batch_op: | ||
| batch_op.create_index(batch_op.f('ix_oauth2_token_refresh_token'), ['refresh_token'], unique=False) | ||
|
|
||
| # ### end Alembic commands ### | ||
|
|
||
|
|
||
| def downgrade(): | ||
| # ### commands auto generated by Alembic - please adjust! ### | ||
| with op.batch_alter_table('oauth2_token', schema=None) as batch_op: | ||
| batch_op.drop_index(batch_op.f('ix_oauth2_token_refresh_token')) | ||
|
|
||
| op.drop_table('oauth2_token') | ||
| op.drop_table('oauth2_authcode') | ||
| with op.batch_alter_table('oauth2_client', schema=None) as batch_op: | ||
| batch_op.drop_index(batch_op.f('ix_oauth2_client_client_id')) | ||
|
|
||
| op.drop_table('oauth2_client') | ||
| # ### end Alembic commands ### |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| from authlib.integrations.sqla_oauth2 import OAuth2AuthorizationCodeMixin, OAuth2ClientMixin, OAuth2TokenMixin | ||
|
|
||
| from main import db | ||
|
|
||
| from . import BaseModel | ||
|
|
||
|
|
||
| class OAuth2Client(BaseModel, OAuth2ClientMixin): | ||
| __tablename__ = "oauth2_client" | ||
|
|
||
| id = db.Column(db.Integer, primary_key=True) | ||
| official = db.Column(db.Boolean) | ||
|
|
||
|
|
||
| class OAuth2Token(BaseModel, OAuth2TokenMixin): | ||
| __tablename__ = "oauth2_token" | ||
| id = db.Column(db.Integer, primary_key=True) | ||
| user_id = db.Column(db.Integer, db.ForeignKey("user.id", ondelete="CASCADE")) | ||
| user = db.relationship("User") | ||
|
|
||
|
|
||
| class OAuth2AuthorizationCode(BaseModel, OAuth2AuthorizationCodeMixin): | ||
| __tablename__ = "oauth2_authcode" | ||
|
|
||
| id = db.Column(db.Integer, primary_key=True) | ||
| user_id = db.Column(db.Integer, db.ForeignKey("user.id", ondelete="CASCADE")) | ||
| user = db.relationship("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.
It would be nice to have a
profilescope as well, returninguser.nameaspreferred_usernameandname(I assume we have Names for everyone, not just speakers?)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.
Technically we might also have data like the users gender and address, but i'm certain we shouldn't expose that data over openid unless someone explicitely requests to have that (and we decide that's a valid use case)
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 think
nameis fine, and I think we all agree that we shouldn't do the latter!