Skip to content

fix(login): send the sign-in form only once - #76

Open
alghanim wants to merge 1 commit into
bodaay:masterfrom
alghanim:fix/login-form-double-submit
Open

alghanim wants to merge 1 commit into
bodaay:masterfrom
alghanim:fix/login-form-double-submit

Conversation

@alghanim

Copy link
Copy Markdown
Collaborator

Pressing Sign In a second time while the first sign-in is still redirecting signs the user in twice, and the second one fails at the relying party. The user ends up on an error page even though the first login succeeded.

What happens

Seen in production-like testing with OpenProject as the OIDC client. The OpenProject callback takes 1–2 s, and during that time the SimpleAuth page looks unchanged, so users click again (or press Enter, then click).

# Request Result
1 POST …/openid-connect/auth (first click) credentials OK, 302 to the callback with code A
2 GET /auth/simpleauth/callback?code=A&state=S OpenProject redeems A and logs the user in, but the browser has already left (nginx logs 499)
3 POST …/openid-connect/auth (second click, same form) credentials OK, 302 with code B, same state S
4 GET /auth/simpleauth/callback?code=B&state=S 401 Unauthorized: the client deleted state S when it handled request 2

OpenProject's OIDC strategy reads the stored state with session.delete('omniauth.state') and answers a mismatch with a bare Rack::Response.new(['401 Unauthorized'], 401). Going back and signing in from the same page fails the same way, because that page still carries the spent state. Any client that treats state as one-time (as it should) reacts like this.

The OIDC authorize page (oidc.go) and the hosted /login page (hosted_login.go) share the same form markup, and neither guards against a second submit. The account page already disables its button while it submits (account.go).

Change

Both login pages:

  • ignore every submit after the first, which covers a second click, Enter, and requestSubmit();
  • disable the button and change its label to Signing in..., so the user can see something is happening;
  • on pageshow with persisted (back/forward cache restore), reset the form so the user can sign in again.

No server-side change. The snippet contains no %, so the fmt.Sprintf template in hosted_login.go is unaffected.

Verification

Chrome, against a local SimpleAuth whose redirect target is a callback that waits 2 s. The script clicks Sign In once, then again 0.5–0.7 s later:

master this PR
logins (App-local auth success) 2 1
callbacks received 2, same state, different code 1
button after the first click enabled, Sign In disabled, Signing in...
an extra requestSubmit() (Enter) 0.9 s later — ignored
browser Back to the form — Sign In enabled, form can be sent again

Unit test: TestLoginPagesSubmitOnce renders both pages and checks the guard is wired in. It fails on master and passes here. The behaviour itself is browser-side, hence the manual check above.

go test ./..., go vet and gofmt are clean.

🤖 Generated with Claude Code

A second Sign In while the first is still redirecting replaces the first
navigation. SimpleAuth issues a second authorization code, but the relying
party has already spent its one-time state on the first one, so the second
callback fails. OpenProject answers it with a bare "401 Unauthorized" and
the user is left on a dead page even though the first login succeeded.

The OIDC authorize page and the hosted /login page now ignore any submit
after the first one, disable the button and show "Signing in...". A page
restored from the back/forward cache accepts a new submit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant