From c77499bffe4d39178fa77a1d98c8c59e09f625fb Mon Sep 17 00:00:00 2001 From: rubin110 Date: Wed, 20 May 2026 19:56:29 -0500 Subject: [PATCH 1/6] DHMemberPortal: make signup scaffolding writes best-effort Issue #283. Once add_member succeeds, the seven secondary JSONB writes (connections/status/forms/notes/access/authorizations/extras) run in a loop with per-step try/except so a transient failure no longer aborts the redirect to /signup/payment. Also bumps the 10s timeout to 20s on the seven secondary helpers in dhservices.py for cheap insurance against slow DHService responses. A failed scaffolding step leaves the corresponding JSONB column null; admin tooling can address that later. The user still reaches Stripe, which is the bandaid's only goal. Co-Authored-By: Claude Opus 4.7 --- code/DHMemberPortal/app.py | 86 +++++++++++++++++-------------- code/DHMemberPortal/dhservices.py | 14 ++--- 2 files changed, 53 insertions(+), 47 deletions(-) diff --git a/code/DHMemberPortal/app.py b/code/DHMemberPortal/app.py index 990d3cb..0b30a07 100644 --- a/code/DHMemberPortal/app.py +++ b/code/DHMemberPortal/app.py @@ -220,57 +220,63 @@ 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}") + + 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)) 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() From a64fcce1417ef295df8819720cc957f756a83840 Mon Sep 17 00:00:00 2001 From: rubin110 Date: Fri, 22 May 2026 00:10:39 -0500 Subject: [PATCH 2/6] DHMemberPortal+ST2DH: guard membership_status against None values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dict.get('key', '') returns '' only when the key is missing — when the key is present with a None value (which happens when v_member_info materializes a NULL status column as {membership_status: null, ...}), .get returns None and the following .lower() crashes with AttributeError. The signup scaffolding bandaid in c77499b makes this state reachable in production: when update_member_status fails, Stripe can later write its stripe_* keys to status without anyone setting membership_status, leaving that key None. Swap to ((dict or {}).get('membership_status') or '').lower() at four sites in DHMemberPortal (B2C authorized, _get_authenticated_member_info, RFID gate in member_update_profile, dev_login_select) and the equivalent in ST2DH update_membership (which was using raw [] indexing, so also vulnerable to KeyError). Verified live against dev DB row 51 (status column NULL): dashboard, profile, keys, auths, storage pages all render. RFID gate refuses update with status='' instead of 500. Pending/active/suspended/banned flows still behave correctly. Co-Authored-By: Claude Opus 4.7 (1M context) --- code/DHMemberPortal/app.py | 8 ++++---- code/external/ST2DH/app.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/code/DHMemberPortal/app.py b/code/DHMemberPortal/app.py index 0b30a07..c628df6 100644 --- a/code/DHMemberPortal/app.py +++ b/code/DHMemberPortal/app.py @@ -369,7 +369,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'] = '' @@ -411,7 +411,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) @@ -620,7 +620,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 = '' @@ -822,7 +822,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/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}") From 67c31952e952b0b908abef99a7629c456dd1292d Mon Sep 17 00:00:00 2001 From: rubin110 Date: Sat, 23 May 2026 19:36:21 -0500 Subject: [PATCH 3/6] DHMemberPortal: scope session reset on /signup to signup-only keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The unconditional session.clear() in signup_start logs out any signed-in user who visits /signup (a GET, so trivially weaponizable as a logout link via attacker-crafted URL) and wipes the flash queue, swallowing the "A member with this email already exists" message when signup_submit redirects back to signup_start. Replace with a narrow session.pop('signup_email', None) — the only key the signup flow writes. Addresses Findings 5 and 14 from the 2026-05-20 signup-flow sweep. Co-Authored-By: Claude Opus 4.7 --- code/DHMemberPortal/app.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/code/DHMemberPortal/app.py b/code/DHMemberPortal/app.py index c628df6..92468fe 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']) From 2fdbcd9769911cfa0e6c00d895feb1ebc74d0543 Mon Sep 17 00:00:00 2001 From: rubin110 Date: Sat, 23 May 2026 19:37:18 -0500 Subject: [PATCH 4/6] DHMemberPortal: reject duplicate active_directory_username on signup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit signup_submit had no server-side check for duplicate usernames — the /api/check-username AJAX call on the form was only advisory, so a direct POST (or a normal submit after a races on the AJAX call) could create a second member row with a colliding active_directory_username. Downstream this collides in AD/B2C when the dispatcher tries to provision the account. Add a case-insensitive gate using the existing dhservices.is_username_taken helper (which calls is_username_available in DHService, LOWER() = LOWER() per #252). Closes Findings 7-username and 9 from the 2026-05-20 signup-flow sweep. Co-Authored-By: Claude Opus 4.7 --- code/DHMemberPortal/app.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/code/DHMemberPortal/app.py b/code/DHMemberPortal/app.py index 92468fe..dd381ad 100644 --- a/code/DHMemberPortal/app.py +++ b/code/DHMemberPortal/app.py @@ -241,6 +241,17 @@ def signup_submit(): flash('A member with this email already exists', 'error') return redirect(url_for('signup_start')) + # 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)}") From 72f8abeadf6ce8c0932a9e4ccad5b2dc34dbeaa6 Mon Sep 17 00:00:00 2001 From: rubin110 Date: Sat, 23 May 2026 19:38:47 -0500 Subject: [PATCH 5/6] DHService: serialize concurrent same-email INSERTs in add_update_identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two simultaneous signup_submit POSTs for the same primary email used to both call get_member_id_from_email (opens its own connection, sees no match), then both INSERT in separate connections — producing two member rows with the same primary email. Downstream scaffolding writes that re-resolve by email then silently overwrote the loser's data (Finding 8 in the 2026-05-20 signup-flow sweep). Move the email lookup inside the same transaction as the INSERT/UPDATE and take a pg_advisory_xact_lock keyed on hashtext(LOWER(email)) before the lookup. Concurrent same-email signups serialize on the lock; the loser sees the winner's row and falls through to the UPDATE branch instead of inserting a duplicate. Different-email signups don't contend. No schema change; the standard caller-supplied member_id path is untouched. A real UNIQUE constraint on primary email is the right long-term fix but requires a schema migration and per-row dedup work first; that's a separate follow-up. Co-Authored-By: Claude Opus 4.7 --- code/DHService/db.py | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) 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 From 657e93d69ddfc5995433cb1d453b49e04244a5ee Mon Sep 17 00:00:00 2001 From: rubin110 Date: Sat, 23 May 2026 19:45:43 -0500 Subject: [PATCH 6/6] DHMemberPortal: render flashed messages on signup_email.html MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous Finding 5+14 commit (67c3195) scoped the session reset so flash messages survive the signup_submit → signup_start redirect, but signup_email.html didn't actually render flashes — so the preserved "A member with this email already exists" message still wouldn't show on the resubmit-after-back path. Add the same get_flashed_messages block used by _dashboard_base.html so the flash actually surfaces. Co-Authored-By: Claude Opus 4.7 --- code/DHMemberPortal/templates/signup_email.html | 11 +++++++++++ 1 file changed, 11 insertions(+) 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 %}