DHMemberPortal: make signup scaffolding writes best-effort (#283 bandaid)

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'] = ''

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()
 

code/DHMemberPortal/templates/signup_email.html

@@ -12,6 +12,17 @@ <h2 class="card-title mb-3">Get Started</h2>
                         <p>Step 1: Review <a href="https://wiki.pumpingstationone.org/wiki/Membership_agreement" target="_blank" rel="noopener noreferrer">the membership agreement</a>.</p>
                         <p>Step 2: Enter your email address to begin your membership application.</p>
 
+                        {% with messages = get_flashed_messages(with_categories=true) %}
+                        {% if messages %}
+                        {% for category, message in messages %}
+                        <div class="alert alert-{{ 'danger' if category == 'error' else category }} alert-dismissible fade show" role="alert">
+                            {{ message }}
+                            <button type="button" class="btn-close" data-bs-dismiss="alert" aria-label="Close"></button>
+                        </div>
+                        {% endfor %}
+                        {% endif %}
+                        {% endwith %}
+
                         {% if error %}
                         <div class="alert alert-danger">{{ error }}</div>
                         {% endif %}

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

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}")