fix: login race condition + login UI improvements

backend/api/schemas/authentication.ts

@@ -30,6 +30,11 @@ export async function registerSchemas(app: FastifyInstance): Promise<void> {
         description:
           'For `setupTfa` only: the `otpauth://` URI as an SVG QR code, to be rendered as-is.'
       },
+      tfaSecret: {
+        type: 'string',
+        description:
+          'For `setupTfa` only: the base32 secret that QR code encodes, for a user typing it into an authenticator app rather than scanning it.'
+      },
       redirect: {
         type: 'string',
         description: 'Where to send the user once logged in. A path within this wiki, or a URL.'

backend/api/users.ts

@@ -832,7 +832,7 @@ async function routes(app: FastifyInstance) {
               tfaSecret: {
                 type: 'string',
                 description:
-                  'The base32 secret the QR code encodes, for a user who would rather type it into an authenticator app than scan it. Only ever returned here, to the user setting 2FA up on their own account.'
+                  'The base32 secret the QR code encodes, for a user who would rather type it into an authenticator app than scan it. Returned only to the user setting 2FA up on their own account — here, and from the login flow that enforces it.'
               }
             }
           }

backend/locales/en.json

@@ -157,7 +157,6 @@
   "admin.auth.registration": "Registration",
   "admin.auth.registrationHint": "Allow any user successfully authorized by the strategy to access the wiki.",
   "admin.auth.registrationLocalHint": "Whether to allow guests to register new accounts.",
-  "admin.auth.registrationNotEnforced": "Saved but not enforced yet: self-registration is not implemented.",
   "admin.auth.saveFailed": "Failed to save {strategy}.",
   "admin.auth.saveSuccess": "Authentication configuration saved successfully.",
   "admin.auth.security": "Security",

backend/models/users.ts

@@ -226,6 +226,7 @@ export interface AfterLoginResult {
   nextAction: string
   continuationToken?: string
   tfaQRImage?: string
+  tfaSecret?: string
   redirect: string
 }
 
@@ -1386,7 +1387,7 @@ class Users {
         */
       } else if (str.conf?.enforceTfa || authStr.tfaRequired) {
         try {
-          const { tfaQRImage } = await this.startTfaSetup(user, strategyId, context.siteId)
+          const { secret, tfaQRImage } = await this.startTfaSetup(user, strategyId, context.siteId)
           const tfaToken = await this.generateToken({
             kind: 'tfaSetup',
             userId: user.id,
@@ -1401,6 +1402,7 @@ class Users {
             nextAction: 'setupTfa',
             continuationToken: tfaToken,
             tfaQRImage,
+            tfaSecret: secret,
             redirect
           }
         } catch (errc) {

frontend/src/boot/api.js

@@ -14,7 +14,27 @@ export function initializeApi() {
   const client = ky.create({
     prefix: '/_api',
     credentials: 'same-origin',
-    throwHttpErrors: (statusNumber) => statusNumber > 400 // Don't throw for 400
+    throwHttpErrors: (statusNumber) => statusNumber > 400, // Don't throw for 400
+    /*
+      ky retries by default, and both halves of that default are wrong for this API.
+
+      `methods` includes `put` and `delete`, so a write that the server answered 500 or 503 to is
+      sent again, twice, with no way for the caller to know -- and every write here is a page save,
+      an upload or a login rather than something safe to repeat. Only GET and HEAD are.
+
+      `statusCodes` includes 429, and 429 is in `afterStatusCodes`, so ky reads `Retry-After` and
+      SLEEPS for it before trying again -- uncapped, since `maxRetryAfter` defaults to Infinity. This
+      wiki's auth limiter answers a banned client with the remaining ban, up to 900 seconds
+      (`helpers/rateLimit.ts`), so one rate-limited login sat silently under the "Signing in..."
+      overlay for half an hour: two waits of fifteen minutes, no error, no redirect, and two further
+      attempts spent against the very limit that refused it. A 429 from this API is a decision with a
+      duration attached, not a blip -- it belongs in front of the user, not in a sleep.
+    */
+    retry: {
+      methods: ['get', 'head'],
+      statusCodes: [408, 500, 502, 503, 504],
+      afterStatusCodes: []
+    }
   })
 
   if (import.meta.env.SSR) {

frontend/src/components/AuthLoginPanel.vue

@@ -116,6 +116,7 @@
           class="acrylic-btn w-full"
           flat
           color="primary"
+          :text-color="acrylicBtnTextColor"
           :label="t(`auth.passkeys.signin`)"
           no-caps
           icon="la:key"
@@ -134,6 +135,7 @@
           :key="str.id"
           flat
           color="primary"
+          :text-color="acrylicBtnTextColor"
           :label="t(`auth.actions.loginWith`, { provider: str.activeStrategy.displayName })"
           no-caps
           :icon="`img:` + str.activeStrategy.strategy.icon"
@@ -147,6 +149,7 @@
           v-if="selectedStrategy.activeStrategy.registration"
           flat
           color="primary"
+          :text-color="acrylicBtnTextColor"
           :label="t(`auth.switchToRegister.link`)"
           no-caps
           icon="la:user-plus"
@@ -158,6 +161,7 @@
           v-if="selectedStrategy.activeStrategy.allowForgotPassword"
           flat
           color="primary"
+          :text-color="acrylicBtnTextColor"
           :label="t(`auth.forgotPasswordLink`)"
           no-caps
           icon="la:life-ring"
@@ -195,6 +199,7 @@
         class="acrylic-btn w-full"
         flat
         color="primary"
+        :text-color="acrylicBtnTextColor"
         :label="t(`auth.forgotPasswordCancel`)"
         no-caps
         icon="la:arrow-circle-left"
@@ -224,6 +229,7 @@
         class="acrylic-btn w-full"
         flat
         color="primary"
+        :text-color="acrylicBtnTextColor"
         :label="t(`auth.switchToLogin.link`)"
         no-caps
         icon="la:arrow-circle-left"
@@ -279,6 +285,7 @@
         class="acrylic-btn w-full"
         flat
         color="primary"
+        :text-color="acrylicBtnTextColor"
         :label="t(`auth.switchToLogin.link`)"
         no-caps
         icon="la:arrow-circle-left"
@@ -357,6 +364,7 @@
         class="acrylic-btn w-full"
         flat
         color="primary"
+        :text-color="acrylicBtnTextColor"
         :label="t(`auth.switchToLogin.link`)"
         no-caps
         icon="la:arrow-circle-left"
@@ -455,6 +463,36 @@
       <div style="justify-content: center; display: flex">
         <div v-html="state.tfaQRImage" style="width: 200px" />
       </div>
+      <!--
+        The same secret in text, for an authenticator app that is not on the device showing this, or
+        a user who would rather type it than point a camera at the screen. Grouped in fours to be
+        readable; the copy button copies it without the spaces. Same block as `SetupTfaDialog`, which
+        is the other half of this — 2FA set up from the profile rather than forced at the login.
+      -->
+      <div class="mt-2 text-center text-caption text-grey">
+        {{ t('auth.tfaSetupInstrManual') }}
+      </div>
+      <div class="mt-1 flex items-center justify-center gap-2">
+        <!--
+          `text-caption`, a step down from the `text-body2` the same block uses in `SetupTfaDialog`:
+          the key is 32 characters and 7 spaces of monospace, and this panel is a 500px column with
+          4rem of padding either side -- at 14px it does not fit beside the copy button and wraps to
+          a second line, which for something being read a group at a time and typed into a phone is
+          worse than the smaller size.
+        -->
+        <code class="rounded bg-black/6 px-2 py-1 font-mono text-caption dark:bg-white/10">{{
+          groupedTfaSecret
+        }}</code>
+        <w-btn
+          class="acrylic-btn"
+          flat
+          dense
+          icon="la:copy"
+          :aria-label="t(`common.actions.copy`)"
+          color="primary"
+          :text-color="acrylicBtnTextColor"
+          @click="copyTfaSecret" />
+      </div>
       <p class="auth-subtitle">{{ t('auth.tfaSetupInstrSecond') }}</p>
       <v-otp-input
         v-model:value="state.securityCode"
@@ -492,6 +530,7 @@
         class="acrylic-btn w-full"
         flat
         color="primary"
+        :text-color="acrylicBtnTextColor"
         :label="t(`auth.switchToLogin.link`)"
         no-caps
         icon="la:arrow-circle-left"
@@ -508,6 +547,7 @@ import { loading } from '@/composables/loading'
 import { notify } from '@/composables/notify'
 import { useDark } from '@/composables/dark'
 import { apiErrorMessage } from '@/helpers/apiError'
+import { copyToClipboard } from '@/helpers/clipboard'
 import { localizeError } from '@/helpers/localization'
 
 import { useSiteStore } from '@/stores/site'
@@ -557,9 +597,22 @@ const state = reactive({
   /** What the error box above the panel says, if anything. Set by `showError()`. */
   errorMessage: '',
   errorCaption: '',
+  /*
+    Whether a request that could carry the flow forward is already out.
+
+    Every screen here can be submitted twice inside the time one request takes: a password manager
+    fills the form and presses the button, `v-otp-input` emits `on-complete` again as the last of the
+    six boxes settles, someone double-clicks. The second attempt is never a second login -- it spends
+    a continuation token the first one has already spent -- so the server refuses it, and the panel
+    paints that refusal over a login that in fact succeeded, or drops the reader back to the login
+    form a beat before the redirect fires. It also costs another attempt against the auth rate limit,
+    which is what eventually locks the address out for a quarter of an hour.
+  */
+  isSubmitting: false,
   isTFAShown: false,
   isTFASetupShown: false,
-  tfaQRImage: ''
+  tfaQRImage: '',
+  tfaSecret: ''
 })
 
 // REFS
@@ -639,6 +692,23 @@ const canUsePasskeys = computed(() => {
   return browserSupportsWebAuthn()
 })
 
+/** The 2FA secret in groups of four, which is how a 32-character string stays readable to type. */
+const groupedTfaSecret = computed(() => state.tfaSecret.replace(/.{4}(?=.)/g, '$& '))
+
+/*
+  What the labels on the panel's flat "acrylic" buttons are coloured -- the passkey and provider
+  buttons, and every link-like one at the foot of a screen.
+
+  `primary` is a mid-tone picked to read on white, and this panel sits straight on the page rather
+  than in a card: white in light mode, `dark-6` in dark, where the same blue is the ~2.7:1 that
+  `--color-primary-light` exists for. Only the text: the buttons keep their `primary` fill, which is
+  a 10% tint that a lighter colour would wash out.
+
+  Bound rather than written in the stylesheet because `WBtn` resolves both colours to inline styles,
+  which no rule outside it can beat without `!important`.
+*/
+const acrylicBtnTextColor = computed(() => (dark.isActive ? 'primary-light' : 'primary'))
+
 // VALIDATION RULES
 
 const loginUsernameValidation = [(val) => val.length > 0 || t('auth.errors.missingUsername')]
@@ -803,6 +873,7 @@ async function handleLoginResponse(resp) {
       state.securityCode = ''
       state.screen = 'tfasetup'
       state.tfaQRImage = resp.tfaQRImage
+      state.tfaSecret = resp.tfaSecret
       loading.hide()
       break
     }
@@ -854,6 +925,10 @@ async function handleLoginResponse(resp) {
  * LOGIN
  */
 async function login() {
+  if (state.isSubmitting) {
+    return
+  }
+  state.isSubmitting = true
   clearError()
   loading.show({
     message: t('auth.signingIn')
@@ -881,6 +956,8 @@ async function login() {
     console.warn(err)
     loading.hide()
     showError(localizeError(apiErrorMessage(err), t))
+  } finally {
+    state.isSubmitting = false
   }
 }
 
@@ -888,6 +965,10 @@ async function login() {
  * LOGIN WITH PASSKEY
  */
 async function loginWithPasskey() {
+  if (state.isSubmitting) {
+    return
+  }
+  state.isSubmitting = true
   clearError()
   loading.show({
     message: t('auth.signingIn')
@@ -919,6 +1000,8 @@ async function loginWithPasskey() {
       return
     }
     showError(localizeError(apiErrorMessage(err), t))
+  } finally {
+    state.isSubmitting = false
   }
 }
 
@@ -926,6 +1009,10 @@ async function loginWithPasskey() {
  * FORGOT PASSWORD
  */
 async function forgotPassword() {
+  if (state.isSubmitting) {
+    return
+  }
+  state.isSubmitting = true
   clearError()
   loading.show({
     message: t('auth.forgotPasswordLoading')
@@ -955,6 +1042,8 @@ async function forgotPassword() {
   } catch (err) {
     loading.hide()
     showError(localizeError(apiErrorMessage(err), t))
+  } finally {
+    state.isSubmitting = false
   }
 }
 
@@ -966,6 +1055,10 @@ async function forgotPassword() {
  * success screen, and the login form is a link away from it, with the new password to type into it.
  */
 async function resetPassword() {
+  if (state.isSubmitting) {
+    return
+  }
+  state.isSubmitting = true
   clearError()
   loading.show({
     message: t('auth.changePwd.loading')
@@ -994,6 +1087,8 @@ async function resetPassword() {
   } catch (err) {
     loading.hide()
     showError(localizeError(apiErrorMessage(err), t))
+  } finally {
+    state.isSubmitting = false
   }
 }
 
@@ -1001,6 +1096,10 @@ async function resetPassword() {
  * REGISTER
  */
 async function register() {
+  if (state.isSubmitting) {
+    return
+  }
+  state.isSubmitting = true
   clearError()
   try {
     const isFormValid = await registerForm.value.validate(true)
@@ -1031,6 +1130,8 @@ async function register() {
   } catch (err) {
     loading.hide()
     showError(localizeError(apiErrorMessage(err), t))
+  } finally {
+    state.isSubmitting = false
   }
 }
 
@@ -1038,6 +1139,10 @@ async function register() {
  * CHANGE PASSWORD
  */
 async function changePwd() {
+  if (state.isSubmitting) {
+    return
+  }
+  state.isSubmitting = true
   clearError()
   try {
     const isFormValid = await changePwdForm.value.validate(true)
@@ -1064,6 +1169,8 @@ async function changePwd() {
     }
   } catch (err) {
     showError(localizeError(apiErrorMessage(err), t))
+  } finally {
+    state.isSubmitting = false
   }
 }
 
@@ -1115,6 +1222,15 @@ async function handleTFAError(err) {
 }
 
 async function verifyTFA() {
+  /*
+    `continuationToken` as well as the flag: `submitTFA` spends the token on the way out, and the OTP
+    boxes keep their digits when the model behind them is cleared -- so an autofill that writes them
+    a second time re-emits a code the login it belonged to has already finished with.
+  */
+  if (state.isSubmitting || !state.continuationToken) {
+    return
+  }
+  state.isSubmitting = true
   clearError()
   loading.show({
     message: t('auth.signingIn')
@@ -1123,6 +1239,28 @@ async function verifyTFA() {
     await handleLoginResponse(await submitTFA(false))
   } catch (err) {
     await handleTFAError(err)
+  } finally {
+    state.isSubmitting = false
+  }
+}
+
+/**
+ * Copy the setup key shown under the QR code, for typing it into an authenticator app by hand.
+ */
+async function copyTfaSecret() {
+  try {
+    // -> Without the display grouping: a space is harmless in most authenticator apps, but not all
+    await copyToClipboard(state.tfaSecret)
+    notify({
+      type: 'positive',
+      message: t('auth.tfaSetupKeyCopied')
+    })
+  } catch (err) {
+    notify({
+      type: 'negative',
+      message: t('auth.tfaSetupKeyCopyFailed'),
+      caption: err.message
+    })
   }
 }
 
@@ -1130,6 +1268,11 @@ async function verifyTFA() {
  * FINISH TFA SETUP
  */
 async function finishSetupTFA() {
+  // -> As `verifyTFA`: no token left means the code in the boxes belongs to a finished login
+  if (state.isSubmitting || !state.continuationToken) {
+    return
+  }
+  state.isSubmitting = true
   clearError()
   loading.show({
     message: t('auth.tfaSetupVerifying')
@@ -1143,6 +1286,8 @@ async function finishSetupTFA() {
     await handleLoginResponse(resp)
   } catch (err) {
     await handleTFAError(err)
+  } finally {
+    state.isSubmitting = false
   }
 }
 
@@ -1155,6 +1300,10 @@ async function finishSetupTFA() {
  * the paths through `cancelVerifyEmail()`.
  */
 async function confirmEmail() {
+  if (state.isSubmitting) {
+    return
+  }
+  state.isSubmitting = true
   clearError()
   loading.show({
     message: t('auth.verifyEmail.loading')
@@ -1179,6 +1328,8 @@ async function confirmEmail() {
     cancelVerifyEmail()
     // -> After the screen change, which clears the box this is about to fill
     showError(localizeError(apiErrorMessage(err), t))
+  } finally {
+    state.isSubmitting = false
   }
 }
 

frontend/src/components/shared/WBtn.vue

@@ -46,13 +46,19 @@ const props = defineProps({
   },
   /**
    * Theme or palette color name (`primary`, `negative`, `grey-7`, ...), resolved against the
-   * Tailwind color variables. Drives the background for solid variants, the text for flat ones.
+   * Tailwind color variables. The button's own color: the background for solid variants, and the
+   * hover tint for unfilled ones (`acrylic-btn` builds its resting fill from it too).
    */
   color: {
     type: String,
     default: null
   },
-  /** Overrides the foreground color independently of `color`. */
+  /**
+   * The label and icon color, when it is not the button's. Defaults to `color` on an unfilled
+   * button and to white on a solid one, which is what every caller that names only `color` gets --
+   * so this is only ever set where the two genuinely differ, as on the login screen, where the
+   * flat buttons keep a `primary` fill under a lighter `primary-light` label.
+   */
   textColor: {
     type: String,
     default: null
@@ -222,8 +228,12 @@ const classes = computed(() => [
   isSolid.value && !props.unelevated ? 'shadow-card' : '',
   props.outline ? 'border border-current' : '',
   isDisabled.value ? 'pointer-events-none opacity-60' : 'cursor-pointer',
-  // -> Flat buttons have no background of their own, so hover tints with the current text color
-  isSolid.value ? 'hover:brightness-110' : 'hover:bg-current/10',
+  /*
+    Flat buttons have no background of their own, so hover tints with the button's own color rather
+    than with `currentcolor` -- which is the label, and on a button whose `textColor` differs from
+    its `color` those are two different answers.
+  */
+  isSolid.value ? 'hover:brightness-110' : 'hover:bg-(--w-btn-tint)',
   props.glossy ? 'w-glossy' : ''
 ])
 
@@ -251,6 +261,22 @@ const styles = computed(() => {
     out.padding = `${SIZES[v] ?? v} ${SIZES[h] ?? h}`
   }
 
+  /*
+    The button's own color, for the tints that are drawn from it: the hover fill above, and
+    `acrylic-btn`'s resting one in `css/_base.scss`. Always set, so neither has to name a fallback,
+    and `currentcolor` where there is no `color` -- which is what both used unconditionally before
+    text and fill could differ.
+  */
+  out['--w-btn-color'] = props.color ? `var(--color-${props.color})` : 'currentcolor'
+
+  /*
+    The hover tint, as a variable rather than as `bg-[color-mix(...)]` in the class list: Tailwind
+    reads an arbitrary `color-mix()` as a color it cannot resolve and emits `background-color:
+    var(--w-btn-color)` ahead of it as the pre-`color-mix` fallback -- an opaque fill under a label
+    of the same color, on a button whose whole point is that it has no fill until it is hovered.
+  */
+  out['--w-btn-tint'] = 'color-mix(in srgb, var(--w-btn-color) 10%, transparent)'
+
   if (isSolid.value && props.color) {
     out.backgroundColor = `var(--color-${props.color})`
     // -> Solid buttons default to white text, matching the palette's intended contrast

frontend/src/css/_base.scss

@@ -152,15 +152,20 @@ body::-webkit-scrollbar-thumb {
 // own hover state -- so the tint is a background on the button itself. Kept alongside the rule
 // above rather than replacing it, because both kinds of button are in play until Quasar is gone.
 //
-// Unlayered (this file is plain CSS, not a Tailwind layer), so it beats the `hover:bg-current/10`
-// utility WBtn carries for its flat variant, which would otherwise cancel the hover step out.
+// Unlayered (this file is plain CSS, not a Tailwind layer), so it beats the hover-tint utility WBtn
+// carries for its flat variant, which would otherwise cancel the hover step out.
+//
+// `--w-btn-color` rather than `currentcolor`: that is the button's `color` prop, which is the label
+// too unless a `text-color` says otherwise -- and where it does, the fill is meant to stay with the
+// button rather than follow the text. WBtn always sets it, falling back to `currentcolor` itself,
+// so a Quasar-era button that somehow reaches this rule still tints.
 .w-btn.acrylic-btn {
-  background-color: color-mix(in srgb, currentcolor 10%, transparent);
+  background-color: color-mix(in srgb, var(--w-btn-color, currentcolor) 10%, transparent);
 }
 
 @media (hover: hover) {
   .w-btn.acrylic-btn:hover {
-    background-color: color-mix(in srgb, currentcolor 30%, transparent);
+    background-color: color-mix(in srgb, var(--w-btn-color, currentcolor) 30%, transparent);
   }
 }
 

frontend/src/pages/AdminAuth.vue

@@ -165,11 +165,6 @@
                   ? t(`admin.auth.registrationLocalHint`)
                   : t(`admin.auth.registrationHint`)
               }}</w-item-label>
-              <!-- Saved, but there is no self-registration path in the server yet — say so rather than -->
-              <!-- let the toggle read as a working setting -->
-              <w-item-label class="text-orange" caption>{{
-                t(`admin.auth.registrationNotEnforced`)
-              }}</w-item-label>
             </w-item-section>
             <w-item-section avatar>
               <w-toggle