diff --git a/code/DHMemberPortal/app.py b/code/DHMemberPortal/app.py index 990d3cb..dd381ad 100644 --- a/code/DHMemberPortal/app.py +++ b/code/DHMemberPortal/app.py @@ -81,8 +81,11 @@ def index(): @app.route('/signup') def signup_start(): """First step of signup - email entry""" - # Clear any existing session data to prevent showing previous data from any login or signup attempts - session.clear() + # Drop only the signup-scoped session key. Don't session.clear() — that + # logs out any signed-in user who visits /signup (weaponizable as a + # logout link, GET so no CSRF) and wipes the flash queue, so error + # redirects from signup_submit lose their flash message before render. + session.pop('signup_email', None) return render_template('signup_email.html') @app.route('/signup/check-email', methods=['POST']) @@ -220,57 +223,74 @@ def signup_submit(): logger.debug(f"Access data to be sent for signup: {access_data}") logger.debug(f"Authorizations data to be sent for signup: {authorizations_data}") logger.debug(f"Extras data to be sent for signup: {extras_data}") + # The member-row creation (add_member) is the only step that MUST succeed + # before we can hand the user off to Stripe. The scaffolding writes below + # are best-effort: any one failing leaves a half-populated row that admin + # tooling can address later, but the user still gets a Stripe checkout. + # See #283. try: - # Get access token for DHService access_token = dhservices.get_access_token( - dhservices.DH_CLIENT_ID, + dhservices.DH_CLIENT_ID, dhservices.DH_CLIENT_SECRET ) logger.debug("Obtained access token for DHService") - - # First we need to get/create the member ID for the email provided - member_id = dhservices.get_member_id(access_token, email).get("member_id") - # If member_id is None, it means the member does not exist and needs to be created - # otherwise the member exists with the email address and we gotta stop them from - # signing up again - if member_id is None: - member_id = dhservices.add_member(access_token, identity_data).get("member_id") - logger.info(f"Created new member with ID: {member_id}") - else: + + # If the email is already taken, bail before creating anything. + existing_member_id = dhservices.get_member_id(access_token, email).get("member_id") + if existing_member_id is not None: flash('A member with this email already exists', 'error') return redirect(url_for('signup_start')) - - # Now we can send the connections data to the service to create - # the phone number and discord handle entries - dhservices.update_member_connections(access_token, member_id, connections_data) - logger.info(f"Updated member {member_id} with connections data") - - # Now, we set the status data for the new member - dhservices.update_member_status(access_token, member_id, status_data) - logger.info(f"Updated member {member_id} with status data") - - # Finally, we log the waiver form submission - dhservices.update_member_forms(access_token, member_id, forms_data) - logger.info(f"Logged waiver form submission for member {member_id}") - - # And we add a note about the new signup - dhservices.update_member_notes(access_token, member_id, notes_data) - logger.info(f"Added note for new signup for member {member_id}") - - # These are empty but we want the record to show everything on - # first pass so that the admins can make changes as the fields - # would be null otherwise. - dhservices.update_member_access(access_token, member_id, access_data) - logger.info(f"Set initial access data for member {member_id}") - dhservices.update_member_authorizations(access_token, member_id, authorizations_data) - logger.info(f"Set initial authorizations data for member {member_id}") - dhservices.update_member_extras(access_token, member_id, extras_data) - logger.info(f"Set initial extras data for member {member_id}") + + # Server-side username uniqueness gate. /api/check-username is only + # the AJAX UX hint — without this, the form happily creates a second + # row with a duplicate active_directory_username, which collides in + # AD/B2C downstream. Case-insensitive (matches is_username_available + # in DHService, PR #252). Skip when no username was provided; the + # broader required-field enforcement is tracked separately. + username = (request.form.get("username") or "").strip() + if username and dhservices.is_username_taken(access_token, username): + flash('That username is already taken. Please choose another.', 'error') + return redirect(url_for('signup_start')) + + member_id = dhservices.add_member(access_token, identity_data).get("member_id") except Exception as e: logger.error(f"Error creating new member: {str(e)}") flash('Error creating new member', 'error') return redirect(url_for('signup_start')) - + + # DHService returns 200 with `member_id: null` on internal INSERT failure + # (see add_update_identity in DHService/db.py). Guard explicitly so we + # don't proceed with a null id into the scaffolding loop. + if not member_id: + logger.error(f"Signup add_member returned no member_id for email={email}") + flash('Error creating new member', 'error') + return redirect(url_for('signup_start')) + + logger.info(f"Created new member with ID: {member_id}") + + # Best-effort scaffolding initialization. We pre-populate these JSONB + # columns so admin tooling sees a complete record on first pass instead of + # nulls. Any single failure is logged and skipped — we still proceed to + # Stripe, and ST2DH will fill in stripe_product_id on the status row when + # payment lands. Admin can backfill any remaining blanks. + scaffolding_steps = [ + ("connections", dhservices.update_member_connections, connections_data), + ("status", dhservices.update_member_status, status_data), + ("forms", dhservices.update_member_forms, forms_data), + ("notes", dhservices.update_member_notes, notes_data), + ("access", dhservices.update_member_access, access_data), + ("authorizations", dhservices.update_member_authorizations, authorizations_data), + ("extras", dhservices.update_member_extras, extras_data), + ] + for label, fn, payload in scaffolding_steps: + try: + fn(access_token, member_id, payload) + logger.info(f"Signup member {member_id}: {label} initialized") + except Exception as e: + logger.error( + f"Signup member {member_id}: {label} init failed (continuing): {str(e)}" + ) + flash('Sign up successful! Please complete payment.', 'success') return redirect(url_for('signup_payment', email=email)) @@ -363,7 +383,7 @@ def authorized(): # very first post-login request, not just after the first dashboard hit. try: full_info = dhservices.get_full_member_info(access_token, member_id) or {} - session['membership_status'] = (full_info.get('status', {}) or {}).get('membership_status', '').lower() + session['membership_status'] = ((full_info.get('status') or {}).get('membership_status') or '').lower() except Exception as e: logger.warning(f"Could not fetch status at B2C login for member_id={member_id}: {e}") session['membership_status'] = '' @@ -405,7 +425,7 @@ def _get_authenticated_member_info(): # Refresh cached membership_status so the before_request gate catches # mid-session admin status flips (e.g. active → banned) on the next page. if isinstance(member_info, dict): - session['membership_status'] = (member_info.get('status', {}) or {}).get('membership_status', '').lower() + session['membership_status'] = ((member_info.get('status') or {}).get('membership_status') or '').lower() return member_info, None except Exception as e: logger.error(f"Error fetching member data: {str(e)}", exc_info=True) @@ -614,7 +634,7 @@ def member_update_profile(): if 'rfid_tags' in request.form: try: current_member_info = dhservices.get_full_member_info(access_token, member_id) or {} - current_status = (current_member_info.get('status') or {}).get('membership_status', '') + current_status = (current_member_info.get('status') or {}).get('membership_status') or '' except Exception as e: logger.error(f"Error fetching member status for RFID gate: {str(e)}", exc_info=True) current_status = '' @@ -816,7 +836,7 @@ def dev_login_select(): # very first post-login request, not just after the first dashboard hit. try: full_info = dhservices.get_full_member_info(access_token, member_id) or {} - session['membership_status'] = (full_info.get('status', {}) or {}).get('membership_status', '').lower() + session['membership_status'] = ((full_info.get('status') or {}).get('membership_status') or '').lower() except Exception as e: logger.warning(f"Could not fetch status at dev login for member_id={member_id}: {e}") session['membership_status'] = '' diff --git a/code/DHMemberPortal/dhservices.py b/code/DHMemberPortal/dhservices.py index 0182b91..d41614f 100644 --- a/code/DHMemberPortal/dhservices.py +++ b/code/DHMemberPortal/dhservices.py @@ -201,7 +201,7 @@ def update_member_status(access_token: str, member_id: str, status_data: dict): headers = {"Authorization": f"Bearer {access_token}"} headers["X-Member-ID"] = str(member_id) params = {"member_id": member_id} - response = requests.post(url, headers=headers, params=params, json=status_data, timeout=10) + response = requests.post(url, headers=headers, params=params, json=status_data, timeout=20) response.raise_for_status() return response.json() @@ -228,7 +228,7 @@ def update_member_extras(access_token: str, member_id: str, extras_data: dict): headers = {"Authorization": f"Bearer {access_token}"} headers["X-Member-ID"] = str(member_id) params = {"member_id": member_id} - response = requests.post(url, headers=headers, params=params, json=extras_data, timeout=10) + response = requests.post(url, headers=headers, params=params, json=extras_data, timeout=20) response.raise_for_status() return response.json() @@ -239,7 +239,7 @@ def update_member_authorizations(access_token: str, member_id: str, auth_data: d "X-Member-ID": str(member_id) # FastAPI expects dashes, not underscores, which came as a big surprise } logger.debug(f"Sending authorization update - member_id: {member_id}, data: {auth_data}") - response = requests.post(url, headers=headers, json=auth_data, timeout=10) + response = requests.post(url, headers=headers, json=auth_data, timeout=20) logger.debug(f"Response status: {response.status_code}") if not response.ok: logger.error(f"Error response: {response.text}") @@ -251,7 +251,7 @@ def update_member_notes(access_token: str, member_id: str, notes_data: dict): headers = {"Authorization": f"Bearer {access_token}"} headers["X-Member-ID"] = str(member_id) params = {"member_id": member_id} - response = requests.post(url, headers=headers, params=params, json=notes_data, timeout=10) + response = requests.post(url, headers=headers, params=params, json=notes_data, timeout=20) response.raise_for_status() return response.json() @@ -260,7 +260,7 @@ def update_member_access(access_token: str, member_id: str, access_data: dict): headers = {"Authorization": f"Bearer {access_token}"} headers["X-Member-ID"] = str(member_id) params = {"member_id": member_id} - response = requests.post(url, headers=headers, params=params, json=access_data, timeout=10) + response = requests.post(url, headers=headers, params=params, json=access_data, timeout=20) response.raise_for_status() return response.json() @@ -269,7 +269,7 @@ def update_member_forms(access_token: str, member_id: str, forms_data: dict): headers = {"Authorization": f"Bearer {access_token}"} headers["X-Member-ID"] = str(member_id) params = {"member_id": member_id} - response = requests.post(url, headers=headers, params=params, json=forms_data, timeout=10) + response = requests.post(url, headers=headers, params=params, json=forms_data, timeout=20) response.raise_for_status() return response.json() @@ -278,7 +278,7 @@ def update_member_connections(access_token: str, member_id: str, connections_dat headers = {"Authorization": f"Bearer {access_token}"} headers["X-Member-ID"] = str(member_id) params = {"member_id": member_id} - response = requests.post(url, headers=headers, params=params, json=connections_data, timeout=10) + response = requests.post(url, headers=headers, params=params, json=connections_data, timeout=20) response.raise_for_status() return response.json() diff --git a/code/DHMemberPortal/templates/signup_email.html b/code/DHMemberPortal/templates/signup_email.html index af51560..56ff21a 100644 --- a/code/DHMemberPortal/templates/signup_email.html +++ b/code/DHMemberPortal/templates/signup_email.html @@ -12,6 +12,17 @@

Get Started

Step 1: Review the membership agreement.

Step 2: Enter your email address to begin your membership application.

+ {% with messages = get_flashed_messages(with_categories=true) %} + {% if messages %} + {% for category, message in messages %} + + {% endfor %} + {% endif %} + {% endwith %} + {% if error %}
{{ error }}
{% endif %} diff --git a/code/DHService/db.py b/code/DHService/db.py index e95881b..dd4dad2 100644 --- a/code/DHService/db.py +++ b/code/DHService/db.py @@ -522,19 +522,49 @@ def add_update_identity(identity_dict, member_id=None): logger.error(error_message) return prepare_return_payload(None, error_message) + # Signup-fallback path: the caller doesn't yet have a member_id, so we + # derive it from the primary email. We extract the email here but defer + # the lookup itself into the transaction below — see the advisory-lock + # comment in the `with` block for why. + email_address = None if not caller_provided_member_id: email_address = get_primary_email(identity_dict) if not email_address: error_message = "No primary email address found in payload and no member_id provided." logger.error(error_message) return prepare_return_payload(None, error_message) - member_id = get_member_id_from_email(email_address) error_message = "OK" try: with get_db_connection() as conn: with conn.cursor() as cur: + if not caller_provided_member_id: + # Concurrent signups for the same email used to both + # see no existing row and both INSERT, producing two + # member rows with the same primary email. Downstream + # scaffolding writes that re-resolve by email then + # silently overwrote the loser's data. Serialize + # those concurrent INSERTs with a transaction-scoped + # advisory lock keyed on the lowercased email. The + # lock auto-releases on commit/rollback; signups for + # different emails don't interact. + cur.execute( + "SELECT pg_advisory_xact_lock(hashtext(LOWER(%s)))", + (email_address,), + ) + cur.execute( + """SELECT id FROM member + WHERE EXISTS ( + SELECT 1 FROM jsonb_array_elements(identity->'emails') AS e + WHERE LOWER(e->>'email_address') = LOWER(%s) + AND e->>'type' = 'primary')""", + (email_address,), + ) + row = cur.fetchone() + if row: + member_id = row[0] + if member_id is not None: # Shallow JSONB merge so a partial POST (e.g. just # `{"birthday": ...}`) doesn't wipe other identity diff --git a/code/external/ST2DH/app.py b/code/external/ST2DH/app.py index 151c85b..883abac 100644 --- a/code/external/ST2DH/app.py +++ b/code/external/ST2DH/app.py @@ -64,7 +64,7 @@ def update_membership_status(access_token): # For sanity checking, let's convert the current membership status to lowercase so we can # compare it more easily, and also log it for debugging purposes. - membership_status = current_status["membership_status"].lower() + membership_status = (current_status.get("membership_status") or "").lower() logger.debug(f"Member ID: {member.id} current membership status (lowercase): {membership_status}") logger.debug(f"Determining how to update membership status for member ID: {member.id} based on current status: {membership_status} and Stripe event membership status: {member.membership_status}")