Merge pull request #691 from rajbos/rajbos/sharing-server-admin-github-logins

rajbos · web-flow · commit 7c809dfe2a86 · 2026-04-29T11:00:03.000+02:00
feat(sharing-server): add ADMIN_GITHUB_LOGINS for declarative admin management
diff --git a/.github/workflows/sharing-server-deploy.yml b/.github/workflows/sharing-server-deploy.yml
@@ -11,6 +11,14 @@ on:
       - ".github/workflows/sharing-server-deploy.yml"
   workflow_dispatch:
 
+# One deploy per branch at a time — prevents concurrent Terraform runs from
+# conflicting on the same remote state file (state blob already locked errors).
+# Cancel in-progress for branch pushes so stale deploys don't block newer ones;
+# queue (no cancel) for main so production deploys always complete.
+concurrency:
+  group: sharing-server-deploy-${{ github.ref }}
+  cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}
+
 # Minimal baseline; jobs declare only what they need.
 permissions:
   contents: read
@@ -179,6 +187,7 @@ jobs:
           TF_VAR_session_secret:         ${{ secrets.SHARING_SESSION_SECRET }}
           TF_VAR_allowed_github_org:     ${{ vars.SHARING_ALLOWED_GITHUB_ORG }}
           TF_VAR_github_org_check_token: ${{ secrets.ORG_CHECK_TOKEN }}
+          TF_VAR_admin_github_logins:    ${{ vars.SHARING_ADMIN_GITHUB_LOGINS }}
           TF_VAR_min_replicas:           ${{ needs.setup.outputs.min_replicas }}
           TF_VAR_custom_domain:          ${{ vars.SHARING_CUSTOM_DOMAIN }}
         run: |
@@ -196,35 +205,71 @@ jobs:
             | grep -E '^\s+name\s+=' | head -1 \
             | sed 's/.*= "\(.*\)".*/\1/' || true)
 
-          if [[ "$CURRENT_CERT_NAME" != "$EXPECTED_CERT_NAME" ]]; then
-            echo "Cert not TF-managed (current: '${CURRENT_CERT_NAME:-none}'). Cleaning up Azure resources so Terraform can recreate them."
-
-            # Remove hostname binding first (cert cannot be deleted while a domain uses it)
-            az containerapp hostname delete \
-              --name "$TF_VAR_app_name" \
-              --resource-group "$TF_VAR_resource_group_name" \
-              --hostname "$TF_VAR_custom_domain" --yes 2>/dev/null || true
-
-            # Find the cert by subject name and delete it
-            AZURE_CERT_NAME=$(az containerapp env certificate list \
+          if [[ "$CURRENT_CERT_NAME" == "$EXPECTED_CERT_NAME" ]]; then
+            echo "Cert already TF-managed as '$EXPECTED_CERT_NAME'. No cleanup needed."
+          else
+            # Cert is absent from TF state or has a mismatched name.
+            # Before deleting anything, check whether the correctly-named cert already
+            # exists in Azure (e.g. a previous apply timed out while polling for the cert
+            # to become Succeeded, leaving it stranded in Azure but dropped from TF state).
+            AZURE_CERT_ID=$(az containerapp env certificate list \
               --name "$ENV_NAME" \
               --resource-group "$TF_VAR_resource_group_name" \
-              --query "[?properties.subjectName=='$TF_VAR_custom_domain'].name | [0]" \
+              --query "[?name=='$EXPECTED_CERT_NAME'].id | [0]" \
               -o tsv 2>/dev/null || true)
-            if [[ -n "$AZURE_CERT_NAME" && "$AZURE_CERT_NAME" != "None" ]]; then
-              echo "Deleting Azure cert: $AZURE_CERT_NAME"
-              az containerapp env certificate delete \
+
+            if [[ -n "$AZURE_CERT_ID" && "$AZURE_CERT_ID" != "None" ]]; then
+              # The correctly-named cert exists in Azure but TF lost track of it.
+              # Import it so apply doesn't delete-and-recreate (which resets provisioning
+              # and triggers another 60-minute wait).
+              echo "Cert '$EXPECTED_CERT_NAME' found in Azure but not in TF state. Importing..."
+              if terraform import 'azurerm_container_app_environment_managed_certificate.this[0]' "$AZURE_CERT_ID"; then
+                # Drop stale custom-domain state so cert_binding re-runs to re-bind.
+                terraform state rm 'azurerm_container_app_custom_domain.this[0]' 2>/dev/null || true
+                echo "Import done. Terraform will rebind the cert without recreating it."
+              else
+                # Import failed; delete the Azure cert so apply doesn't hit "already exists".
+                echo "Import failed. Deleting Azure cert so Terraform can create a fresh one."
+                az containerapp hostname delete \
+                  --name "$TF_VAR_app_name" \
+                  --resource-group "$TF_VAR_resource_group_name" \
+                  --hostname "$TF_VAR_custom_domain" --yes 2>/dev/null || true
+                az containerapp env certificate delete \
+                  --name "$ENV_NAME" \
+                  --resource-group "$TF_VAR_resource_group_name" \
+                  --certificate "$EXPECTED_CERT_NAME" --yes 2>/dev/null || true
+                terraform state rm 'azurerm_container_app_custom_domain.this[0]' 2>/dev/null || true
+                terraform state rm 'azurerm_container_app_environment_managed_certificate.this[0]' 2>/dev/null || true
+                echo "Cleanup done. Terraform will create cert and domain binding from scratch."
+              fi
+            else
+              echo "Cert not TF-managed (current: '${CURRENT_CERT_NAME:-none}'). Cleaning up Azure resources so Terraform can recreate them."
+
+              # Remove hostname binding first (cert cannot be deleted while a domain uses it)
+              az containerapp hostname delete \
+                --name "$TF_VAR_app_name" \
+                --resource-group "$TF_VAR_resource_group_name" \
+                --hostname "$TF_VAR_custom_domain" --yes 2>/dev/null || true
+
+              # Find the cert by subject name and delete it
+              AZURE_CERT_NAME=$(az containerapp env certificate list \
                 --name "$ENV_NAME" \
                 --resource-group "$TF_VAR_resource_group_name" \
-                --certificate "$AZURE_CERT_NAME" --yes 2>/dev/null || true
-            fi
+                --query "[?properties.subjectName=='$TF_VAR_custom_domain'].name | [0]" \
+                -o tsv 2>/dev/null || true)
+              if [[ -n "$AZURE_CERT_NAME" && "$AZURE_CERT_NAME" != "None" ]]; then
+                echo "Deleting Azure cert: $AZURE_CERT_NAME"
+                az containerapp env certificate delete \
+                  --name "$ENV_NAME" \
+                  --resource-group "$TF_VAR_resource_group_name" \
+                  --certificate "$AZURE_CERT_NAME" --yes 2>/dev/null || true
+              fi
 
-            # Remove stale TF state entries so Terraform creates fresh resources
-            terraform state rm 'azurerm_container_app_custom_domain.this[0]' 2>/dev/null || true
-            terraform state rm 'azurerm_container_app_environment_managed_certificate.this[0]' 2>/dev/null || true
-            echo "Cleanup done. Terraform will create cert and domain binding from scratch."
-          else
-            echo "Cert already TF-managed as '$EXPECTED_CERT_NAME'. No cleanup needed."
+              # Remove stale TF state entries so Terraform creates fresh resources
+              terraform state rm 'azurerm_container_app_custom_domain.this[0]' 2>/dev/null || true
+              terraform state rm 'azurerm_container_app_environment_managed_certificate.this[0]' 2>/dev/null || true
+              echo "Cleanup done. Terraform will create cert and domain binding from scratch."
+            fi
           fi
 
       - name: Terraform plan
@@ -240,6 +285,7 @@ jobs:
           TF_VAR_session_secret:      ${{ secrets.SHARING_SESSION_SECRET }}
           TF_VAR_allowed_github_org:  ${{ vars.SHARING_ALLOWED_GITHUB_ORG }}
           TF_VAR_github_org_check_token: ${{ secrets.ORG_CHECK_TOKEN }}
+          TF_VAR_admin_github_logins: ${{ vars.SHARING_ADMIN_GITHUB_LOGINS }}
           TF_VAR_min_replicas:        ${{ needs.setup.outputs.min_replicas }}
           TF_VAR_custom_domain:       ${{ vars.SHARING_CUSTOM_DOMAIN }}
         run: terraform plan -out=tfplan
@@ -257,6 +303,7 @@ jobs:
           TF_VAR_session_secret:      ${{ secrets.SHARING_SESSION_SECRET }}
           TF_VAR_allowed_github_org:  ${{ vars.SHARING_ALLOWED_GITHUB_ORG }}
           TF_VAR_github_org_check_token: ${{ secrets.ORG_CHECK_TOKEN }}
+          TF_VAR_admin_github_logins: ${{ vars.SHARING_ADMIN_GITHUB_LOGINS }}
           TF_VAR_min_replicas:        ${{ needs.setup.outputs.min_replicas }}
           TF_VAR_custom_domain:       ${{ vars.SHARING_CUSTOM_DOMAIN }}
         run: terraform apply -auto-approve tfplan
diff --git a/sharing-server/.env.example b/sharing-server/.env.example
@@ -34,3 +34,9 @@ ALLOWED_GITHUB_ORG=
 #   3. Click "Configure SSO" next to the token and authorize it for ALLOWED_GITHUB_ORG
 #   4. Paste the generated token below
 GITHUB_ORG_CHECK_TOKEN=
+
+# Optional: comma-separated GitHub logins to auto-grant admin access on the dashboard.
+# When set, this list is authoritative: listed users get is_admin=1, all others get is_admin=0.
+# Leave empty (or unset) to manage admin access manually via the SQLite database.
+# Example: ADMIN_GITHUB_LOGINS=alice,bob
+ADMIN_GITHUB_LOGINS=
diff --git a/sharing-server/README.md b/sharing-server/README.md
@@ -167,6 +167,7 @@ Open `http://localhost:3000/dashboard` in your browser to test the OAuth login f
 | `PORT` | ❌ | HTTP port (default: `3000`) |
 | `DB_PATH` | ❌ | SQLite database path (default: `/data/sharing.db`) |
 | `ALLOWED_GITHUB_ORG` | ❌ | If set, only members of this GitHub org can upload data |
+| `ADMIN_GITHUB_LOGINS` | ❌ | Comma-separated GitHub logins to auto-grant admin access (e.g. `alice,bob`). When set, this list is authoritative: listed users get admin, all others do not. Leave unset to manage admins manually via SQLite. |
 
 ## REST API
 
diff --git a/sharing-server/infra/main.tf b/sharing-server/infra/main.tf
@@ -161,6 +161,14 @@ resource "azurerm_container_app" "this" {
           secret_name = "org-check-token"
         }
       }
+      # Only set ADMIN_GITHUB_LOGINS when a value is provided.
+      dynamic "env" {
+        for_each = var.admin_github_logins != "" ? [1] : []
+        content {
+          name  = "ADMIN_GITHUB_LOGINS"
+          value = var.admin_github_logins
+        }
+      }
 
       liveness_probe {
         path             = "/health"
@@ -200,6 +208,12 @@ resource "null_resource" "hostname_registration" {
     app_id   = azurerm_container_app.this.id
   }
 
+  # Re-run whenever the container app is modified, because ACA updates reset
+  # the ingress configuration and strip any previously-registered custom hostnames.
+  lifecycle {
+    replace_triggered_by = [azurerm_container_app.this]
+  }
+
   provisioner "local-exec" {
     command = "az containerapp hostname add --name '${azurerm_container_app.this.name}' --resource-group '${var.resource_group_name}' --hostname '${var.custom_domain}' 2>/dev/null || true"
   }
@@ -219,6 +233,13 @@ resource "azurerm_container_app_environment_managed_certificate" "this" {
   lifecycle {
     create_before_destroy = true
   }
+
+  timeouts {
+    create = "60m"
+    read   = "5m"
+    update = "30m"
+    delete = "30m"
+  }
 }
 
 # azurerm_container_app_custom_domain cannot be used here because it only accepts
@@ -234,6 +255,12 @@ resource "null_resource" "cert_binding" {
     hostname = var.custom_domain
   }
 
+  # Re-run whenever the container app is modified, because ACA updates reset
+  # the ingress configuration and strip any previously-applied cert bindings.
+  lifecycle {
+    replace_triggered_by = [azurerm_container_app.this]
+  }
+
   provisioner "local-exec" {
     environment = {
       CERT_ID  = azurerm_container_app_environment_managed_certificate.this[0].id
diff --git a/sharing-server/infra/variables.tf b/sharing-server/infra/variables.tf
@@ -62,6 +62,12 @@ variable "min_replicas" {
   default     = 1
 }
 
+variable "admin_github_logins" {
+  description = "Optional: comma-separated GitHub logins to auto-grant admin access (e.g. 'alice,bob'). When set, this list is authoritative — listed users get is_admin=1, all others get is_admin=0. Leave empty to manage admin access manually via the SQLite database."
+  type        = string
+  default     = ""
+}
+
 variable "tags" {
   description = "Azure resource tags"
   type        = map(string)
diff --git a/sharing-server/src/db.ts b/sharing-server/src/db.ts
@@ -227,6 +227,26 @@ function initSchema(db: DatabaseSync): void {
 	}
 }
 
+/** Returns the parsed ADMIN_GITHUB_LOGINS env var as an array of lowercase logins. */
+function getAdminLoginsFromEnv(): string[] {
+	const raw = process.env.ADMIN_GITHUB_LOGINS ?? '';
+	return raw.split(',').map(s => s.trim().toLowerCase()).filter(Boolean);
+}
+
+/**
+ * Sync admin status for all existing users based on ADMIN_GITHUB_LOGINS.
+ * When the env var is set and non-empty, it is authoritative: users in the list
+ * get is_admin=1, all others get is_admin=0. When unset or empty, no changes are made.
+ */
+export function syncAdminLogins(): void {
+	const logins = getAdminLoginsFromEnv();
+	if (logins.length === 0) return;
+	const db = getDb();
+	const placeholders = logins.map(() => '?').join(', ');
+	db.prepare(`UPDATE users SET is_admin = CASE WHEN LOWER(github_login) IN (${placeholders}) THEN 1 ELSE 0 END`).run(...logins);
+	console.log(`[db] Admin sync from ADMIN_GITHUB_LOGINS: ${logins.join(', ')}`);
+}
+
 export function upsertUser(
 	githubId: number,
 	login: string,
@@ -243,6 +263,11 @@ export function upsertUser(
 			avatar_url   = excluded.avatar_url,
 			last_seen_at = datetime('now')
 	`).run(githubId, login, name, avatarUrl);
+	// Apply env-var admin grant immediately so new users get the right role on first login.
+	const adminLogins = getAdminLoginsFromEnv();
+	if (adminLogins.includes(login.toLowerCase())) {
+		db.prepare('UPDATE users SET is_admin = 1 WHERE github_id = ?').run(githubId);
+	}
 	return db.prepare('SELECT * FROM users WHERE github_id = ?').get(githubId) as unknown as UserRow;
 }
 
diff --git a/sharing-server/src/server.ts b/sharing-server/src/server.ts
@@ -2,7 +2,7 @@ import { serve } from '@hono/node-server';
 import { Hono } from 'hono';
 import { api } from './routes/api.js';
 import { dashboard } from './routes/dashboard.js';
-import { getDb, closeDb, restoreFromBackup, backupToAzureFiles } from './db.js';
+import { getDb, closeDb, restoreFromBackup, backupToAzureFiles, syncAdminLogins } from './db.js';
 
 const app = new Hono();
 
@@ -35,8 +35,6 @@ function shutdown(signal: string): void {
 process.on('SIGTERM', () => shutdown('SIGTERM'));
 process.on('SIGINT',  () => shutdown('SIGINT'));
 
-const PORT = parseInt(process.env.PORT ?? '3000', 10);
-
 async function initDbWithRetry(maxAttempts = 20): Promise<void> {
 	for (let attempt = 1; attempt <= maxAttempts; attempt++) {
 		try {
@@ -50,24 +48,40 @@ async function initDbWithRetry(maxAttempts = 20): Promise<void> {
 				await new Promise(r => setTimeout(r, delay));
 			} else {
 				console.error('[db] All init attempts exhausted:', err);
+				throw err;
 			}
 		}
 	}
 }
 
-serve({ fetch: app.fetch, port: PORT }, (info) => {
-	const org = process.env.ALLOWED_GITHUB_ORG;
-	console.log(`Token Tracker sharing server listening on port ${info.port}`);
-	if (org) {
-		console.log(`  Access restricted to members of GitHub org: ${org}`);
-	} else {
-		console.log('  Access: open to any GitHub user (set ALLOWED_GITHUB_ORG to restrict)');
-	}
+const PORT = parseInt(process.env.PORT ?? '3000', 10);
+
+async function main(): Promise<void> {
 	// Restore database from Azure Files backup before opening SQLite.
 	// SQLite runs on local container disk (/tmp/db) to avoid Azure Files SMB
 	// locking issues. Azure Files is used only as a backup/restore store.
 	restoreFromBackup();
-	initDbWithRetry();
+	await initDbWithRetry();
+	syncAdminLogins();
 	// Periodic backup every 5 minutes in case of unexpected SIGKILL.
 	setInterval(() => backupToAzureFiles(), 5 * 60 * 1000).unref();
+
+	const org = process.env.ALLOWED_GITHUB_ORG;
+	const adminLogins = process.env.ADMIN_GITHUB_LOGINS;
+	serve({ fetch: app.fetch, port: PORT }, (info) => {
+		console.log(`Token Tracker sharing server listening on port ${info.port}`);
+		if (org) {
+			console.log(`  Access restricted to members of GitHub org: ${org}`);
+		} else {
+			console.log('  Access: open to any GitHub user (set ALLOWED_GITHUB_ORG to restrict)');
+		}
+		if (adminLogins) {
+			console.log(`  Admin logins (ADMIN_GITHUB_LOGINS): ${adminLogins}`);
+		}
+	});
+}
+
+main().catch(err => {
+	console.error('Fatal startup error:', err);
+	process.exit(1);
 });
