Use GitHub user ids instead of usernames for authentication

Otherwise, renaming a GitHub account could allow someone else
to gain access to a CDB account.

app/blueprints/admin/actions.py

@@ -28,6 +28,7 @@ from app.tasks.emails import send_pending_digests
 from app.tasks.forumtasks import import_topic_list, check_all_forum_accounts
 from app.tasks.importtasks import import_repo_screenshot, check_zip_release, check_for_updates, update_all_game_support, \
 	import_languages
+from app.tasks.usertasks import import_github_user_ids
 from app.utils import add_notification, get_system_user
 
 actions = {}
@@ -289,6 +290,13 @@ def do_send_pending_digests():
 	send_pending_digests.delay()
 
 
+@action("Import user ids from GitHub")
+def do_import_github_user_ids():
+	task_id = uuid()
+	import_github_user_ids.apply_async((), task_id=task_id)
+	return redirect(url_for("tasks.check", id=task_id, r=url_for("admin.admin_page")))
+
+
 @action("DANGER: Delete removed packages")
 def del_removed_packages():
 	query = Package.query.filter_by(state=PackageState.DELETED)

app/blueprints/github/__init__.py

@@ -23,7 +23,7 @@ bp = Blueprint("github", __name__)
 
 from flask import redirect, url_for, request, flash, jsonify, current_app
 from flask_login import current_user
-from sqlalchemy import func, or_, and_
+from sqlalchemy import or_, and_
 from app import github, csrf
 from app.models import db, User, APIToken, Package, Permission, AuditSeverity, PackageState
 from app.utils import abs_url_for, add_audit_log, login_user_set_active, is_safe_url
@@ -65,18 +65,23 @@ def callback(oauth_token):
 	# Get GitGub username
 	url = "https://api.github.com/user"
 	r = requests.get(url, headers={"Authorization": "token " + oauth_token})
-	username = r.json()["login"]
+	json = r.json()
+	user_id = json["id"]
+	username = json["login"]
 
-	# Get user by GitHub username
+	# Get user by GitHub user ID
-	userByGithub = User.query.filter(func.lower(User.github_username) == func.lower(username)).first()
+	userByGithub = User.query.filter(User.github_user_id == user_id).first()
 
 	# If logged in, connect
 	if current_user and current_user.is_authenticated:
 		if userByGithub is None:
 			current_user.github_username = username
+			current_user.github_user_id = user_id
 			db.session.commit()
 			flash(gettext("Linked GitHub to account"), "success")
 			return redirect(redirect_to)
+		elif userByGithub == current_user:
+			return redirect(redirect_to)
 		else:
 			flash(gettext("GitHub account is already associated with another user"), "danger")
 			return redirect(redirect_to)

app/blueprints/users/settings.py

@@ -25,6 +25,7 @@ from wtforms.validators import Length, Optional, Email, URL
 from app.models import User, AuditSeverity, db, UserRank, PackageAlias, EmailSubscription, UserNotificationPreferences, \
 	UserEmailVerification, Permission, NotificationType, UserBan
 from app.tasks.emails import send_verify_email
+from app.tasks.usertasks import update_github_user_id
 from app.utils import nonempty_or_none, add_audit_log, random_string, rank_required, has_blocked_domains
 from . import bp
 
@@ -335,7 +336,12 @@ def modtools(username):
 
 			user.display_name = form.display_name.data
 			user.forums_username = nonempty_or_none(form.forums_username.data)
-			user.github_username = nonempty_or_none(form.github_username.data)
+			github_username = nonempty_or_none(form.github_username.data)
+			if github_username is None:
+				user.github_username = None
+				user.github_user_id = None
+			else:
+				update_github_user_id.delay(user.id, github_username)
 
 		if user.check_perm(current_user, Permission.CHANGE_RANK):
 			new_rank = form["rank"].data

app/models/users.py

@@ -144,6 +144,7 @@ class User(db.Model, UserMixin):
 
 	# Account linking
 	github_username = db.Column(db.String(50, collation="NOCASE"), nullable=True, unique=True)
+	github_user_id = db.Column(db.Integer, nullable=True, unique=True)
 	forums_username = db.Column(db.String(50, collation="NOCASE"), nullable=True, unique=True)
 
 	# Access token for webhook setup

app/tasks/forumtasks.py

@@ -24,7 +24,7 @@ from app.models import User, db, PackageType, ForumTopic
 from app.tasks import celery
 from app.utils import is_username_valid
 from app.utils.phpbbparser import get_profile, get_topics_from_forum
-from .usertasks import set_profile_picture_from_url
+from .usertasks import set_profile_picture_from_url, update_github_user_id_raw
 
 
 @celery.task()
@@ -53,6 +53,7 @@ def check_forum_account(forums_username, force_replace_pic=False):
 	if github_username is not None and github_username.strip() != "":
 		print("Updated GitHub username for " + user.display_name + " to " + github_username)
 		user.github_username = github_username
+		update_github_user_id_raw(user)
 		needs_saving = True
 
 	pic = profile.avatar

app/tasks/usertasks.py

@@ -19,12 +19,13 @@ import datetime, requests
 import os
 import sys
 
+from flask import url_for
 from sqlalchemy import or_, and_
 
 from app import app
-from app.models import User, db, UserRank, ThreadReply, Package
+from app.models import User, db, UserRank, ThreadReply, Package, NotificationType
 from app.utils import random_string
-from app.utils.models import create_session
+from app.utils.models import create_session, add_notification, get_system_user
 from app.tasks import celery, TaskError
 
 
@@ -92,3 +93,60 @@ def set_profile_picture_from_url(username: str, url: str):
 	db.session.commit()
 
 	return filepath
+
+
+def update_github_user_id_raw(user: User, send_notif: bool = False):
+	github_api_token = app.config.get("GITHUB_API_TOKEN")
+	if github_api_token is None or github_api_token == "":
+		raise TaskError("Importing requires a GitHub API token")
+
+	url = f"https://api.github.com/users/{user.github_username}"
+	resp = requests.get(url, headers={"Authorization": "token " + github_api_token}, timeout=15)
+	if resp.status_code == 404:
+		print(" - not found", file=sys.stderr)
+		if send_notif:
+			system_user = get_system_user()
+			add_notification(user, system_user, NotificationType.BOT,
+					f"GitHub account {user.github_username} does not exist, so has been disconnected from your account",
+					url_for("users.profile", username=user.username), None)
+		user.github_username = None
+		return False
+	elif resp.status_code != 200:
+		print(" - " + resp.json()["message"], file=sys.stderr)
+		return False
+
+	json = resp.json()
+	user_id = json.get("id")
+	if type(user_id) is not int:
+		raise TaskError(f"{url} returned non-int id")
+
+	user.github_user_id = user_id
+	return True
+
+
+@celery.task()
+def update_github_user_id(user_id: int, github_username: str):
+	user = User.query.get(user_id)
+	if user is None:
+		raise TaskError("Unable to find that user")
+
+	user.github_username = github_username
+	if update_github_user_id_raw(user):
+		db.session.commit()
+	else:
+		raise TaskError(f"Unable to set the GitHub username to {github_username}")
+
+
+@celery.task()
+def import_github_user_ids():
+	users = User.query.filter(User.github_user_id.is_(None), User.github_username.is_not(None)).all()
+	total = len(users)
+	count = 0
+	for i, user in enumerate(users):
+		print(f"[{i + 1} / {total}] Getting GitHub user id for {user.github_username}", file=sys.stderr)
+		if update_github_user_id_raw(user, send_notif=True):
+			count += 1
+
+	db.session.commit()
+
+	print(f"Updated {count} users", file=sys.stderr)

config.example.cfg

@@ -11,6 +11,9 @@ SQLALCHEMY_TRACK_MODIFICATIONS = False
 GITHUB_CLIENT_ID = ""
 GITHUB_CLIENT_SECRET = ""
 
+# Optional, used for an admin action - import user ids from GitHub
+GITHUB_API_TOKEN = ""
+
 REDIS_URL = 'redis://redis:6379'
 CELERY_BROKER_URL = 'redis://redis:6379'
 CELERY_RESULT_BACKEND = 'redis://redis:6379'

migrations/versions/1fe2e44cf565_.py

@@ -0,0 +1,28 @@
+"""empty message
+
+Revision ID: 1fe2e44cf565
+Revises: d73078c5d619
+Create Date: 2024-03-30 16:19:47.384716
+
+"""
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import postgresql
+
+# revision identifiers, used by Alembic.
+revision = '1fe2e44cf565'
+down_revision = 'd73078c5d619'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    with op.batch_alter_table('user', schema=None) as batch_op:
+        batch_op.add_column(sa.Column('github_user_id', sa.Integer(), nullable=True))
+        batch_op.create_unique_constraint("_user_github_user_id", ['github_user_id'])
+
+
+def downgrade():
+    with op.batch_alter_table('user', schema=None) as batch_op:
+        batch_op.drop_constraint("_user_github_user_id", type_='unique')
+        batch_op.drop_column('github_user_id')