Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions server/authflow/callback.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,13 @@ func (h *Handler) handleConnectorCallback(w http.ResponseWriter, r *http.Request
authReq, err = h.finalizeLogin(ctx, identity, authReq, conn.Connector)
if err != nil {
h.Logger.ErrorContext(r.Context(), "failed to finalize login", "err", err)
if errors.Is(err, storage.ErrNotFound) {
// The auth request is gone from storage, most likely because an earlier,
// still-in-flight submission already finalized it (e.g. a
// double-submitted callback).
h.renderError(r, w, http.StatusBadRequest, ErrMsgRequestAlreadyCompleted)
return
}
h.renderError(r, w, http.StatusInternalServerError, "Login error.")
return
}
Expand Down
7 changes: 7 additions & 0 deletions server/authflow/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,11 @@ const (
// ErrMsgNotInRequiredGroups is shown when a user authenticates successfully
// but is not a member of any of the groups required by the connector.
ErrMsgNotInRequiredGroups = "You are not a member of any of the required groups to authenticate."

// ErrMsgRequestAlreadyCompleted is shown when a login request is resubmitted
// after its AuthRequest is no longer in storage: most commonly because it was
// already finalized by an earlier, still-in-flight submission (e.g. a
// double-submitted login form, or a stale page resubmitted via the browser
// back button), but also possible if the request expired in the meantime.
ErrMsgRequestAlreadyCompleted = "This login request is no longer valid. It may have already been completed, or it may have expired. Please close this tab, or go back and start over if you need to sign in again."
)
2 changes: 1 addition & 1 deletion server/authflow/finalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (h *Handler) finalizeLogin(ctx context.Context, identity connector.Identity
return a, nil
}
if err := h.Storage.UpdateAuthRequest(ctx, authReq.ID, updater); err != nil {
return storage.AuthRequest{}, fmt.Errorf("failed to update auth request: %v", err)
return storage.AuthRequest{}, fmt.Errorf("failed to update auth request: %w", err)
}
// Keep the in-memory copy in sync with what was persisted so later reads
// (the next-step decision below) see the identity we just stored.
Expand Down
27 changes: 27 additions & 0 deletions server/authflow/finalize_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package authflow

import (
"errors"
"testing"
"time"

Expand Down Expand Up @@ -58,3 +59,29 @@ func TestFinalizeLoginBlockedAccount(t *testing.T) {
_, err = server.finalizeLogin(ctx, ident, authReq, nil)
require.NoError(t, err)
}

// TestFinalizeLoginAlreadyFinalized reproduces a double-submitted login (e.g. the
// user double-clicks the submit button, or resubmits a stale page reached via the
// browser back button): by the time the duplicate request reaches finalizeLogin,
// the first submission has already completed and the AuthRequest is gone. The
// caller needs to distinguish this from a generic storage failure, so the
// returned error must still satisfy errors.Is(err, storage.ErrNotFound).
func TestFinalizeLoginAlreadyFinalized(t *testing.T) {
httpServer, server := newTestHandler(t, nil)
defer httpServer.Close()

ctx := t.Context()

ident := connector.Identity{UserID: "user-1", Email: "user@example.com"}
authReq := storage.AuthRequest{
ID: "login-req",
ClientID: "example-app",
Expiry: time.Now().Add(time.Hour),
ConnectorID: "mock",
}
// Do not create the AuthRequest, simulating that an earlier, still-in-flight
// submission already finalized and deleted it.
_, err := server.finalizeLogin(ctx, ident, authReq, nil)
require.Error(t, err)
require.True(t, errors.Is(err, storage.ErrNotFound), "expected error to wrap storage.ErrNotFound, got: %v", err)
}
12 changes: 12 additions & 0 deletions server/authflow/password.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package authflow
// form and the credential check for password connectors.

import (
"errors"
"net/http"
"net/url"

Expand Down Expand Up @@ -80,6 +81,10 @@ func (h *Handler) handlePasswordLogin(w http.ResponseWriter, r *http.Request) {
authReq, err = h.finalizeLogin(ctx, *ident, authReq, conn.Connector)
if err != nil {
h.Logger.ErrorContext(ctx, "failed to finalize login", "err", err)
if errors.Is(err, storage.ErrNotFound) {
h.renderError(r, w, http.StatusBadRequest, ErrMsgRequestAlreadyCompleted)
return
}
h.renderError(r, w, http.StatusInternalServerError, "Login error.")
return
}
Expand Down Expand Up @@ -116,6 +121,13 @@ func (h *Handler) handlePasswordLogin(w http.ResponseWriter, r *http.Request) {
authReq, err = h.finalizeLogin(r.Context(), identity, authReq, conn.Connector)
if err != nil {
h.Logger.ErrorContext(r.Context(), "failed to finalize login", "err", err)
if errors.Is(err, storage.ErrNotFound) {
// The auth request is gone from storage, most likely because an
// earlier submission already finalized it, e.g. the user
// double-clicked the login button.
h.renderError(r, w, http.StatusBadRequest, ErrMsgRequestAlreadyCompleted)
return
}
h.renderError(r, w, http.StatusInternalServerError, "Login error.")
return
}
Expand Down