fix(auth): garder l identite Discord du cote verifie
This commit is contained in:
+93
-15
@@ -21,10 +21,14 @@ from app.extensions import check_password, db, hash_password, limiter
|
||||
from app.i18n import LOCALE_SESSION_KEY
|
||||
from app.logging_config import log_auth_event
|
||||
from app.models import ESPORT_GAMES, Player, User
|
||||
from app.validators import LoginSchema, RegisterSchema
|
||||
from app.validators import LoginSchema, RegisterSchema, validate_discord_user_id
|
||||
|
||||
#: Session key holding the pending OAuth2 anti-forgery token.
|
||||
DISCORD_STATE_KEY = 'discord_oauth_state'
|
||||
#: Whether the OAuth result should create a registration draft or relink the
|
||||
#: signed-in account. Kept server-side and covered by the same signed session
|
||||
#: as the anti-forgery state.
|
||||
DISCORD_PURPOSE_KEY = 'discord_oauth_purpose'
|
||||
|
||||
# Failed-attempt tracking. The tally is kept for the audit trail and for the
|
||||
# cool-off marker below; it no longer refuses a correct password (SEC-018).
|
||||
@@ -354,6 +358,15 @@ def register():
|
||||
form_data = dict(request.form)
|
||||
form_data['games'] = request.form.getlist('games')
|
||||
|
||||
# Once Discord has authenticated the identity, neither its display
|
||||
# name nor its snowflake is input data anymore. Remove any client
|
||||
# copies before validation as well as before persistence: otherwise a
|
||||
# forged, malformed hidden value can still make the verified flow fail.
|
||||
discord_oauth = session.get('discord_oauth') or {}
|
||||
if discord_oauth.get('id'):
|
||||
form_data.pop('discord_username', None)
|
||||
form_data.pop('discord_user_id', None)
|
||||
|
||||
refusal = check_registration_challenge(request.form)
|
||||
if refusal is not None:
|
||||
# Logged, because this is the only place abuse of the sign-up
|
||||
@@ -380,8 +393,18 @@ def register():
|
||||
full_name = validated['full_name']
|
||||
phone = validated.get('phone')
|
||||
selected_games = validated.get('games', [])
|
||||
discord_username = validated.get('discord_username')
|
||||
discord_user_id = validated.get('discord_user_id')
|
||||
# The OAuth identity is server-side state. It used to be copied into
|
||||
# hidden inputs and read back from request.form, which let anyone
|
||||
# replace the verified Discord account before submitting (SEC-AUTH-005).
|
||||
# A manual registration may still provide a display name, but never a
|
||||
# Discord snowflake: that identifier is an authentication factor for
|
||||
# bot reactions and must come from Discord itself.
|
||||
discord_user_id = discord_oauth.get('id')
|
||||
if discord_user_id:
|
||||
discord_user_id = str(discord_user_id)
|
||||
discord_username = (
|
||||
discord_oauth.get('username') if discord_user_id else validated.get('discord_username')
|
||||
)
|
||||
league_os_profile = validated.get('league_os_profile')
|
||||
|
||||
if User.query.filter_by(username=username).first():
|
||||
@@ -392,6 +415,13 @@ def register():
|
||||
flash(_('Email already registered.'), 'danger')
|
||||
return _rerender_registration(form_data)
|
||||
|
||||
# The database constraint belongs to DB-002, after production has
|
||||
# been backed up and deduplicated. Refuse new duplicates now instead
|
||||
# of leaving the critical impersonation path open until then.
|
||||
if discord_user_id and User.query.filter_by(discord_user_id=discord_user_id).first():
|
||||
flash(_('This Discord account is already linked to another account.'), 'danger')
|
||||
return _rerender_registration(form_data)
|
||||
|
||||
hashed_password = hash_password(password)
|
||||
user = Player(
|
||||
username=username,
|
||||
@@ -451,11 +481,15 @@ def discord_login():
|
||||
Returns:
|
||||
Response: Redirect to Discord authorization URL.
|
||||
"""
|
||||
purpose = 'profile' if current_user.is_authenticated else 'registration'
|
||||
session[DISCORD_PURPOSE_KEY] = purpose
|
||||
return_endpoint = 'users.edit_profile' if purpose == 'profile' else 'auth.register'
|
||||
|
||||
# DISCORD_REDIRECT_URI is checked too: quoting it when unset used to
|
||||
# raise inside the query builder rather than report a configuration error.
|
||||
if not DISCORD_CLIENT_ID or not DISCORD_REDIRECT_URI:
|
||||
flash(_('Discord OAuth2 is not configured.'), 'danger')
|
||||
return redirect(url_for('auth.register'))
|
||||
return redirect(url_for(return_endpoint))
|
||||
|
||||
# Anti-forgery token, required by RFC 6749 §10.12. Without it, an
|
||||
# attacker could have the victim's browser consume an authorization code
|
||||
@@ -479,16 +513,24 @@ def discord_login():
|
||||
def discord_callback():
|
||||
"""Handle the OAuth2 callback from Discord.
|
||||
|
||||
Exchanges the authorization code for an access token, then fetches
|
||||
the user's profile (/users/@me) and connections (/users/@me/connections).
|
||||
Results are stored in the session and the user is redirected back to
|
||||
the registration form where fields will be pre-filled.
|
||||
Exchanges the authorization code for an access token, then fetches the
|
||||
user's profile. During registration, connected game accounts are also
|
||||
loaded into server-side draft state. For a signed-in profile relink, the
|
||||
verified identity is written directly without passing through a form.
|
||||
|
||||
Returns:
|
||||
Response: Redirect to registration page.
|
||||
Response: Redirect to the registration form or profile editor.
|
||||
"""
|
||||
# The state is consumed whatever happens next: a token is single-use, and
|
||||
# leaving it in the session would allow a replay.
|
||||
purpose = session.pop(DISCORD_PURPOSE_KEY, 'registration')
|
||||
if purpose == 'profile' and current_user.is_authenticated:
|
||||
return_endpoint = 'users.edit_profile'
|
||||
elif purpose == 'profile':
|
||||
return_endpoint = 'auth.login'
|
||||
else:
|
||||
return_endpoint = 'auth.register'
|
||||
|
||||
expected_state = session.pop(DISCORD_STATE_KEY, None)
|
||||
received_state = request.args.get('state', '')
|
||||
|
||||
@@ -500,12 +542,12 @@ def discord_callback():
|
||||
),
|
||||
'danger',
|
||||
)
|
||||
return redirect(url_for('auth.register'))
|
||||
return redirect(url_for(return_endpoint))
|
||||
|
||||
code = request.args.get('code')
|
||||
if not code:
|
||||
flash(_('Discord authorization failed. No code received.'), 'danger')
|
||||
return redirect(url_for('auth.register'))
|
||||
return redirect(url_for(return_endpoint))
|
||||
|
||||
# Exchange the authorization code for an access token
|
||||
token_data = {
|
||||
@@ -529,11 +571,11 @@ def discord_callback():
|
||||
access_token = token_json.get('access_token')
|
||||
except requests.RequestException:
|
||||
flash(_('Failed to connect to Discord. Please try again.'), 'danger')
|
||||
return redirect(url_for('auth.register'))
|
||||
return redirect(url_for(return_endpoint))
|
||||
|
||||
if not access_token:
|
||||
flash(_('Failed to obtain Discord access token.'), 'danger')
|
||||
return redirect(url_for('auth.register'))
|
||||
return redirect(url_for(return_endpoint))
|
||||
|
||||
auth_headers = {'Authorization': f'Bearer {access_token}'}
|
||||
|
||||
@@ -548,7 +590,43 @@ def discord_callback():
|
||||
user_data = user_response.json()
|
||||
except requests.RequestException:
|
||||
flash(_('Failed to fetch Discord user profile.'), 'danger')
|
||||
return redirect(url_for('auth.register'))
|
||||
return redirect(url_for(return_endpoint))
|
||||
|
||||
discord_user_id = user_data.get('id')
|
||||
try:
|
||||
if not discord_user_id:
|
||||
raise ValidationError('missing Discord user id')
|
||||
discord_user_id = str(discord_user_id)
|
||||
validate_discord_user_id(discord_user_id)
|
||||
except ValidationError:
|
||||
flash(_('Failed to fetch Discord user profile.'), 'danger')
|
||||
return redirect(url_for(return_endpoint))
|
||||
|
||||
if purpose == 'profile':
|
||||
# If the session expired while Discord was open, do not turn a profile
|
||||
# relink into registration state for an anonymous browser.
|
||||
if not current_user.is_authenticated:
|
||||
flash(_('Please log in to connect your Discord account.'), 'danger')
|
||||
return redirect(url_for('auth.login'))
|
||||
|
||||
clash = User.query.filter(
|
||||
User.discord_user_id == discord_user_id,
|
||||
User.id != current_user.id,
|
||||
).first()
|
||||
if clash:
|
||||
flash(_('This Discord account is already linked to another account.'), 'danger')
|
||||
return redirect(url_for('users.edit_profile'))
|
||||
|
||||
current_user.discord_user_id = discord_user_id
|
||||
current_user.discord_username = user_data.get('username') or None
|
||||
db.session.commit()
|
||||
log_auth_event(
|
||||
'account.discord_linked',
|
||||
username=current_user.username,
|
||||
user_id=current_user.id,
|
||||
)
|
||||
flash(_('Discord account connected!'), 'success')
|
||||
return redirect(url_for('users.edit_profile'))
|
||||
|
||||
# Fetch the user's connected gaming accounts
|
||||
connections = []
|
||||
@@ -587,7 +665,7 @@ def discord_callback():
|
||||
|
||||
# Store in session for the registration form to use
|
||||
session['discord_oauth'] = {
|
||||
'id': user_data.get('id'),
|
||||
'id': discord_user_id,
|
||||
'username': user_data.get('username'),
|
||||
'avatar': user_data.get('avatar'),
|
||||
'gamertag_suggestions': gamertag_suggestions,
|
||||
|
||||
@@ -111,6 +111,16 @@ def edit_user(user_id):
|
||||
flash(_('Email already in use by another account.'), 'danger')
|
||||
return _rerender()
|
||||
|
||||
discord_clash = None
|
||||
if discord_user_id:
|
||||
discord_clash = User.query.filter(
|
||||
User.discord_user_id == discord_user_id,
|
||||
User.id != user.id,
|
||||
).first()
|
||||
if discord_clash:
|
||||
flash(_('This Discord account is already linked to another account.'), 'danger')
|
||||
return _rerender()
|
||||
|
||||
role_changed = user.role != role
|
||||
previous_role = user.role
|
||||
|
||||
|
||||
@@ -76,7 +76,6 @@ def edit_profile():
|
||||
phone = validated.get('phone')
|
||||
selected_games = validated.get('games', [])
|
||||
discord_username = validated.get('discord_username')
|
||||
discord_user_id = validated.get('discord_user_id')
|
||||
league_os_profile = validated.get('league_os_profile')
|
||||
|
||||
if username != current_user.username and User.query.filter_by(username=username).first():
|
||||
@@ -105,7 +104,6 @@ def edit_profile():
|
||||
current_user.phone = phone
|
||||
current_user.games = ','.join(selected_games) if selected_games else None
|
||||
current_user.discord_username = discord_username or None
|
||||
current_user.discord_user_id = discord_user_id or None
|
||||
current_user.league_os_profile = league_os_profile or None
|
||||
|
||||
update_user_gamertags(current_user, selected_games)
|
||||
|
||||
Reference in New Issue
Block a user