mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-25 14:00:40 +00:00
fix: an invite asks an existing user instead of adding them; decline; CLI invitations (BUG-2136 U1) (#1831)
* test(server): an invite asks an existing user; re-invite replaces; decline by id and by code (BUG-2136, red)
* fix(server): an invite asks an existing user instead of adding them; a re-invite replaces the pending invitation; decline by id and by code (BUG-2136)
* feat(cli,mcp): pad workspace invitations/accept/decline; invite output says pending; ToolSurfaceVersion 0.66 (BUG-2136)
* test(e2e): invitees accept their invitation before the specs use the membership (BUG-2136)
* test(store): concurrent re-invites of one address leave one pending invitation (BUG-2136)
* test(store): an accept that loses to a decline adds no membership (BUG-2136, codex r1, red)
* test(server): an unverified invitee can decline by code, as they can accept (BUG-2136, codex r1, red)
* fix: an accept that loses to a decline or replace adds no membership; decline by code passes the verified-email gate; honest oracle comment (BUG-2136, codex r1)
* fix(server): the verified-email exemption matches exactly /invitations/{code}/accept|decline (BUG-2136, codex r2)
* test(server): drop the plan-limit harness's now-unused newUser (BUG-2136)
This commit is contained in:
@@ -390,7 +390,7 @@ directory for `claude-code`, and an `[mcp_servers.pad]` table in
|
||||
project-scoped, it's install-on-request only — `--all` and `pad mcp status` cover
|
||||
the per-user clients (including Codex) and skip it.
|
||||
|
||||
**Tool catalog (v0.65)** — ten resource × action tools plus `pad_set_workspace`. Undeclared input keys are rejected with a structured error rather than silently dropped, and field values are typed against the collection schema on the server:
|
||||
**Tool catalog (v0.66)** — ten resource × action tools plus `pad_set_workspace`. Undeclared input keys are rejected with a structured error rather than silently dropped, and field values are typed against the collection schema on the server:
|
||||
|
||||
| Tool | Actions |
|
||||
|---|---|
|
||||
@@ -414,7 +414,7 @@ Plus resources at `pad://workspaces`, `pad://workspace/{ws}/dashboard`,
|
||||
and `pad://_meta/version`.
|
||||
|
||||
**Stability contract.** The catalog is versioned
|
||||
(`tool_surface_version: "0.65"`) and advertised in the initialize handshake and at
|
||||
(`tool_surface_version: "0.66"`) and advertised in the initialize handshake and at
|
||||
`pad://_meta/version`, so external agents can pin against it; the per-version
|
||||
changelog lives in [`internal/mcp/version.go`](internal/mcp/version.go). Errors
|
||||
come back as structured envelopes (`{error: {code, message, hint, ...}}`) from a
|
||||
|
||||
@@ -212,13 +212,15 @@ func inviteCmd() *cobra.Command {
|
||||
|
||||
green := color.New(color.FgGreen).SprintFunc()
|
||||
|
||||
// A server before BUG-2136 still adds an existing account directly.
|
||||
if added, ok := result["added"].(bool); ok && added {
|
||||
name, _ := result["name"].(string)
|
||||
role, _ := result["role"].(string)
|
||||
fmt.Printf("%s Added %s (%s) as %s\n", green("✓"), name, email, role)
|
||||
} else {
|
||||
role, _ := result["role"].(string)
|
||||
fmt.Printf("%s Invitation created for %s (%s)\n", green("✓"), email, role)
|
||||
fmt.Printf("%s Invited %s (%s): pending until they accept\n", green("✓"), email, role)
|
||||
fmt.Printf(" They will see it in Pad (the + menu), or with: pad workspace invitations\n")
|
||||
if joinURL, ok := result["join_url"].(string); ok && joinURL != "" {
|
||||
fmt.Printf(" Share this link: %s\n", joinURL)
|
||||
} else {
|
||||
@@ -257,6 +259,76 @@ func joinCmd() *cobra.Command {
|
||||
}
|
||||
}
|
||||
|
||||
// myInvitationsCmd lists the pending invitations addressed to the signed-in
|
||||
// account (BUG-2136): what an invitee answers with accept or decline.
|
||||
func myInvitationsCmd() *cobra.Command {
|
||||
return &cobra.Command{
|
||||
Use: "invitations",
|
||||
Short: "List workspace invitations waiting for your answer",
|
||||
Args: cobra.NoArgs,
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
client, _ := getClient()
|
||||
invs, verified, err := client.ListMyInvitations()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if formatFlag == "json" {
|
||||
return cli.PrintJSON(map[string]any{"invitations": invs, "email_verified": verified})
|
||||
}
|
||||
if !verified {
|
||||
fmt.Println("Your email address is not verified, so no invitations are listed. Verify it, or use the link in the invitation email.")
|
||||
return nil
|
||||
}
|
||||
if len(invs) == 0 {
|
||||
fmt.Println("No pending invitations.")
|
||||
return nil
|
||||
}
|
||||
w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0)
|
||||
fmt.Fprintln(w, "ID\tWORKSPACE\tROLE\tINVITED BY")
|
||||
for _, inv := range invs {
|
||||
fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", inv.ID, inv.WorkspaceName, inv.Role, inv.InvitedByName)
|
||||
}
|
||||
w.Flush()
|
||||
fmt.Println("\nAnswer with: pad workspace accept <id> or pad workspace decline <id>")
|
||||
return nil
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func acceptInvitationCmd() *cobra.Command {
|
||||
return &cobra.Command{
|
||||
Use: "accept <invitation-id>",
|
||||
Short: "Accept a workspace invitation from pad workspace invitations",
|
||||
Args: cobra.ExactArgs(1),
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
client, _ := getClient()
|
||||
role, slug, err := client.AcceptMyInvitation(args[0])
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to accept invitation: %w", err)
|
||||
}
|
||||
green := color.New(color.FgGreen).SprintFunc()
|
||||
fmt.Printf("%s Joined %s as %s\n", green("✓"), slug, role)
|
||||
return nil
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func declineInvitationCmd() *cobra.Command {
|
||||
return &cobra.Command{
|
||||
Use: "decline <invitation-id>",
|
||||
Short: "Decline a workspace invitation from pad workspace invitations",
|
||||
Args: cobra.ExactArgs(1),
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
client, _ := getClient()
|
||||
if err := client.DeclineMyInvitation(args[0]); err != nil {
|
||||
return fmt.Errorf("failed to decline invitation: %w", err)
|
||||
}
|
||||
fmt.Println("Invitation declined.")
|
||||
return nil
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// workspaceRestoreCmd un-soft-deletes a workspace by slug, provided it is
|
||||
// still inside the 30-day restore window (owner-only server-side).
|
||||
func workspaceRestoreCmd() *cobra.Command {
|
||||
|
||||
@@ -69,6 +69,9 @@ func workspaceCmd() *cobra.Command {
|
||||
membersCmd(),
|
||||
inviteCmd(),
|
||||
joinCmd(),
|
||||
myInvitationsCmd(),
|
||||
acceptInvitationCmd(),
|
||||
declineInvitationCmd(),
|
||||
workspaceDeletedCmd(),
|
||||
workspaceRestoreCmd(),
|
||||
storageCmd(),
|
||||
|
||||
+2
-2
File diff suppressed because one or more lines are too long
+2
-2
File diff suppressed because one or more lines are too long
@@ -2495,3 +2495,48 @@ func (c *Client) AckReminder(wsSlug, reminderID string) (*models.Reminder, error
|
||||
func (c *Client) DeleteReminder(wsSlug, reminderID string) error {
|
||||
return c.delete("/workspaces/" + wsSlug + "/reminders/" + reminderID)
|
||||
}
|
||||
|
||||
// MyInvitation is one pending invitation addressed to the signed-in account
|
||||
// (GET /api/v1/me/invitations, BUG-2136).
|
||||
type MyInvitation struct {
|
||||
ID string `json:"id"`
|
||||
Role string `json:"role"`
|
||||
WorkspaceSlug string `json:"workspace_slug"`
|
||||
WorkspaceName string `json:"workspace_name"`
|
||||
WorkspaceOwnerUsername string `json:"workspace_owner_username"`
|
||||
InvitedByName string `json:"invited_by_name"`
|
||||
CreatedAt time.Time `json:"created_at"`
|
||||
ExpiresAt *time.Time `json:"expires_at,omitempty"`
|
||||
}
|
||||
|
||||
// ListMyInvitations returns the pending invitations addressed to the caller,
|
||||
// and whether the caller's email is verified (only a verified email is
|
||||
// matched).
|
||||
func (c *Client) ListMyInvitations() ([]MyInvitation, bool, error) {
|
||||
var body struct {
|
||||
Invitations []MyInvitation `json:"invitations"`
|
||||
EmailVerified bool `json:"email_verified"`
|
||||
}
|
||||
if err := c.get("/me/invitations", &body); err != nil {
|
||||
return nil, false, err
|
||||
}
|
||||
return body.Invitations, body.EmailVerified, nil
|
||||
}
|
||||
|
||||
// AcceptMyInvitation accepts a pending invitation by id and returns the role
|
||||
// held and the workspace slug.
|
||||
func (c *Client) AcceptMyInvitation(id string) (role, workspaceSlug string, err error) {
|
||||
var body struct {
|
||||
Role string `json:"role"`
|
||||
WorkspaceSlug string `json:"workspace_slug"`
|
||||
}
|
||||
if err := c.post("/me/invitations/"+url.PathEscape(id)+"/accept", nil, &body); err != nil {
|
||||
return "", "", err
|
||||
}
|
||||
return body.Role, body.WorkspaceSlug, nil
|
||||
}
|
||||
|
||||
// DeclineMyInvitation declines (deletes) a pending invitation by id.
|
||||
func (c *Client) DeclineMyInvitation(id string) error {
|
||||
return c.post("/me/invitations/"+url.PathEscape(id)+"/decline", nil, nil)
|
||||
}
|
||||
|
||||
@@ -6,7 +6,7 @@ Pad is a project tracker for developers and AI agents — issues (TASK, BUG), pl
|
||||
|
||||
If the user is asking general code questions with no project-management thread, you don't need this server.
|
||||
|
||||
## Tool surface (v0.65)
|
||||
## Tool surface (v0.66)
|
||||
|
||||
Ten resource × action tools, plus `pad_set_workspace`. Eleven tools total.
|
||||
|
||||
|
||||
+14
-1
@@ -1738,6 +1738,19 @@ const CmdhelpVersion = "0.1"
|
||||
// caller cannot read embeds nothing; a failed fetch stays non-fatal,
|
||||
// as on stdio. Stdio is unchanged.
|
||||
//
|
||||
// 0.66 — BUG-2136 (lead ruling, day 87). BEHAVIOR bump on the v0.52 /
|
||||
// v0.43 grounds: `pad_workspace.action=invite` no longer ADDS an existing
|
||||
// account to the workspace. Every invite now creates a pending
|
||||
// invitation that the invitee accepts or declines (in the web app, or
|
||||
// with `pad workspace invitations` / `accept` / `decline`, CLI-only by
|
||||
// ruling), on both transports. The result loses the direct-add shape
|
||||
// (`added`, `user_id`, `name`) and is always `{invited, code, email,
|
||||
// role, join_url?}`, so it also stops saying whether the address has an
|
||||
// account. A re-invite of an address with a pending invitation replaces
|
||||
// it (a fresh code, role and 14-day expiry; the old code stops working).
|
||||
// An agent that invited a user and then acted on them as a member must
|
||||
// now wait for their acceptance. No name, enum or param moved.
|
||||
//
|
||||
// 0.65 — TASK-3413 (SPEC-6 U9c). ADDITIVE bump on the v0.53 grounds:
|
||||
// an item an installed APP created, a comment an app wrote and a version
|
||||
// row an app's write made gain `omitempty` `via_app` (the install id) and
|
||||
@@ -1779,7 +1792,7 @@ const CmdhelpVersion = "0.1"
|
||||
// this surface can receive it; the entry exists so a future action does
|
||||
// not collapse it to permission_denied. When an action that can reach
|
||||
// it is added, that addition is the contract change and owns the bump.
|
||||
const ToolSurfaceVersion = "0.65"
|
||||
const ToolSurfaceVersion = "0.66"
|
||||
|
||||
// MetaVersionURI is the canonical URI of the queryable version document.
|
||||
// Lives outside the pad://workspace/{ws}/... namespace because it's a
|
||||
|
||||
+22
-19
@@ -12,25 +12,28 @@ var ValidActions = []string{
|
||||
|
||||
// Audit action constants for auth/admin events
|
||||
const (
|
||||
ActionLogin = "login"
|
||||
ActionLoginFailed = "login_failed"
|
||||
ActionLogout = "logout"
|
||||
ActionBootstrap = "bootstrap"
|
||||
ActionRegister = "register"
|
||||
ActionPasswordChanged = "password_changed"
|
||||
ActionPasswordReset = "password_reset"
|
||||
ActionTokenCreated = "token_created"
|
||||
ActionTokenRevoked = "token_revoked"
|
||||
ActionTokenRotated = "token_rotated"
|
||||
ActionTOTPEnabled = "totp_enabled"
|
||||
ActionTOTPDisabled = "totp_disabled"
|
||||
ActionMemberInvited = "member_invited"
|
||||
ActionMemberRemoved = "member_removed"
|
||||
ActionRoleChanged = "role_changed"
|
||||
ActionSettingsChanged = "settings_changed"
|
||||
ActionOAuthLogin = "oauth_login"
|
||||
ActionOAuthLoginFailed = "oauth_login_failed"
|
||||
ActionPlanChanged = "plan_changed"
|
||||
ActionLogin = "login"
|
||||
ActionLoginFailed = "login_failed"
|
||||
ActionLogout = "logout"
|
||||
ActionBootstrap = "bootstrap"
|
||||
ActionRegister = "register"
|
||||
ActionPasswordChanged = "password_changed"
|
||||
ActionPasswordReset = "password_reset"
|
||||
ActionTokenCreated = "token_created"
|
||||
ActionTokenRevoked = "token_revoked"
|
||||
ActionTokenRotated = "token_rotated"
|
||||
ActionTOTPEnabled = "totp_enabled"
|
||||
ActionTOTPDisabled = "totp_disabled"
|
||||
ActionMemberInvited = "member_invited"
|
||||
// ActionMemberInviteDeclined: the invitee declined a pending invitation
|
||||
// (BUG-2136). The invitation row is deleted; this is its trail.
|
||||
ActionMemberInviteDeclined = "member_invite_declined"
|
||||
ActionMemberRemoved = "member_removed"
|
||||
ActionRoleChanged = "role_changed"
|
||||
ActionSettingsChanged = "settings_changed"
|
||||
ActionOAuthLogin = "oauth_login"
|
||||
ActionOAuthLoginFailed = "oauth_login_failed"
|
||||
ActionPlanChanged = "plan_changed"
|
||||
// ActionPlanOverridesChanged is logged when an admin updates a
|
||||
// user's plan_overrides JSON via the admin user-detail page.
|
||||
// Surfaces per-user storage / workspace / API-token quota
|
||||
|
||||
@@ -0,0 +1,261 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
"github.com/PerpetualSoftware/pad/internal/store"
|
||||
)
|
||||
|
||||
// BUG-2136 (lead ruling, day 87): an existing user invited to a workspace is
|
||||
// asked, not added. Every invite creates a pending invitation the invitee can
|
||||
// accept or decline; a re-invite replaces the pending one; decline deletes it
|
||||
// and leaves an audit event.
|
||||
|
||||
type inviteBody struct {
|
||||
Added bool `json:"added"`
|
||||
Invited bool `json:"invited"`
|
||||
Code string `json:"code"`
|
||||
UserID string `json:"user_id"`
|
||||
Name string `json:"name"`
|
||||
}
|
||||
|
||||
func myInvitationIDs(t *testing.T, f *accessFixture, tok string) []string {
|
||||
t.Helper()
|
||||
rr := f.do("GET", "/api/v1/me/invitations", tok, nil)
|
||||
f.must(rr, http.StatusOK, "list my invitations")
|
||||
var body struct {
|
||||
Invitations []struct {
|
||||
ID string `json:"id"`
|
||||
} `json:"invitations"`
|
||||
}
|
||||
parseJSON(t, rr, &body)
|
||||
ids := []string{}
|
||||
for _, inv := range body.Invitations {
|
||||
ids = append(ids, inv.ID)
|
||||
}
|
||||
return ids
|
||||
}
|
||||
|
||||
func TestBUG2136_InviteAsksAVerifiedExistingUser(t *testing.T) {
|
||||
forEachDialect(t, func(t *testing.T, d store.DriverType) {
|
||||
f := newAccessFixture(t, d)
|
||||
invitee := mkUser(t, f.srv, "existing@example.com")
|
||||
|
||||
rr := f.do("POST", "/api/v1/workspaces/"+f.wsSlug+"/members/invite", f.ownerTok,
|
||||
map[string]any{"email": "existing@example.com", "role": "editor"})
|
||||
f.must(rr, http.StatusCreated, "invite existing user")
|
||||
var body inviteBody
|
||||
parseJSON(t, rr, &body)
|
||||
if body.Added || !body.Invited || body.Code == "" {
|
||||
t.Fatalf("invite of an existing user: got %s, want a pending invitation", rr.Body.String())
|
||||
}
|
||||
// The response has the invitation's shape, with no direct-add keys.
|
||||
var keys map[string]any
|
||||
parseJSON(t, rr, &keys)
|
||||
for _, k := range []string{"added", "user_id", "name"} {
|
||||
if _, ok := keys[k]; ok {
|
||||
t.Errorf("invite response carries the direct-add key %q: %s", k, rr.Body.String())
|
||||
}
|
||||
}
|
||||
if member, err := f.srv.store.IsWorkspaceMember(f.wsID, invitee.ID); err != nil || member {
|
||||
t.Fatalf("the invitee became a member without accepting (member=%v err=%v)", member, err)
|
||||
}
|
||||
ids := myInvitationIDs(t, f, f.token(invitee))
|
||||
if len(ids) != 1 {
|
||||
t.Fatalf("the invitee sees %d pending invitations, want 1", len(ids))
|
||||
}
|
||||
// Accepting is what joins.
|
||||
rr = f.do("POST", "/api/v1/me/invitations/"+ids[0]+"/accept", f.token(invitee), nil)
|
||||
f.must(rr, http.StatusOK, "accept")
|
||||
if member, _ := f.srv.store.IsWorkspaceMember(f.wsID, invitee.ID); !member {
|
||||
t.Fatalf("accept did not add the member")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestBUG2136_ReinviteReplacesThePendingInvitation(t *testing.T) {
|
||||
forEachDialect(t, func(t *testing.T, d store.DriverType) {
|
||||
f := newAccessFixture(t, d)
|
||||
invitee := mkUser(t, f.srv, "again@example.com")
|
||||
invite := func(role string) inviteBody {
|
||||
t.Helper()
|
||||
rr := f.do("POST", "/api/v1/workspaces/"+f.wsSlug+"/members/invite", f.ownerTok,
|
||||
map[string]any{"email": "again@example.com", "role": role})
|
||||
f.must(rr, http.StatusCreated, "invite")
|
||||
var b inviteBody
|
||||
parseJSON(t, rr, &b)
|
||||
return b
|
||||
}
|
||||
first := invite("viewer")
|
||||
second := invite("editor")
|
||||
pending, err := f.srv.store.ListWorkspaceInvitations(f.wsID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
n := 0
|
||||
for _, inv := range pending {
|
||||
if inv.Email == "again@example.com" {
|
||||
n++
|
||||
if inv.Role != "editor" {
|
||||
t.Errorf("the surviving invitation has role %q, want the re-invite's editor", inv.Role)
|
||||
}
|
||||
}
|
||||
}
|
||||
if n != 1 {
|
||||
t.Fatalf("%d pending invitations for one address after a re-invite, want 1", n)
|
||||
}
|
||||
if rr := f.do("POST", "/api/v1/invitations/"+first.Code+"/accept", f.token(invitee), nil); rr.Code != http.StatusNotFound {
|
||||
t.Errorf("the replaced invitation's code still works: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
rr := f.do("POST", "/api/v1/invitations/"+second.Code+"/accept", f.token(invitee), nil)
|
||||
f.must(rr, http.StatusOK, "accept the re-invite")
|
||||
})
|
||||
}
|
||||
|
||||
func TestBUG2136_DeclineByID(t *testing.T) {
|
||||
forEachDialect(t, func(t *testing.T, d store.DriverType) {
|
||||
f := newAccessFixture(t, d)
|
||||
invitee := mkUser(t, f.srv, "decliner@example.com")
|
||||
other := mkUser(t, f.srv, "someone-else@example.com")
|
||||
rr := f.do("POST", "/api/v1/workspaces/"+f.wsSlug+"/members/invite", f.ownerTok,
|
||||
map[string]any{"email": "decliner@example.com"})
|
||||
f.must(rr, http.StatusCreated, "invite")
|
||||
ids := myInvitationIDs(t, f, f.token(invitee))
|
||||
if len(ids) != 1 {
|
||||
t.Fatalf("want 1 pending invitation, got %d", len(ids))
|
||||
}
|
||||
// Another account cannot decline it, and is told nothing exists.
|
||||
unknown := f.do("POST", "/api/v1/me/invitations/no-such-id/decline", f.token(other), nil)
|
||||
f.must(unknown, http.StatusNotFound, "decline unknown id")
|
||||
if rr := f.do("POST", "/api/v1/me/invitations/"+ids[0]+"/decline", f.token(other), nil); rr.Code != http.StatusNotFound || rr.Body.String() != unknown.Body.String() {
|
||||
t.Fatalf("another account's decline: %d %s, want the unknown-id 404", rr.Code, rr.Body.String())
|
||||
}
|
||||
|
||||
rr = f.do("POST", "/api/v1/me/invitations/"+ids[0]+"/decline", f.token(invitee), nil)
|
||||
if rr.Code != http.StatusNoContent && rr.Code != http.StatusOK {
|
||||
t.Fatalf("decline: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if left := myInvitationIDs(t, f, f.token(invitee)); len(left) != 0 {
|
||||
t.Errorf("a declined invitation is still listed: %v", left)
|
||||
}
|
||||
if pending, _ := f.srv.store.ListWorkspaceInvitations(f.wsID); len(pending) != 0 {
|
||||
t.Errorf("the owner still sees the declined invitation as pending: %d", len(pending))
|
||||
}
|
||||
if rr := f.do("POST", "/api/v1/me/invitations/"+ids[0]+"/accept", f.token(invitee), nil); rr.Code != http.StatusNotFound {
|
||||
t.Errorf("a declined invitation can still be accepted: %d", rr.Code)
|
||||
}
|
||||
if member, _ := f.srv.store.IsWorkspaceMember(f.wsID, invitee.ID); member {
|
||||
t.Errorf("declining made the invitee a member")
|
||||
}
|
||||
acts, err := f.srv.store.ListWorkspaceActivity(f.wsID, models.ActivityListParams{Limit: 50})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
found := false
|
||||
for _, a := range acts {
|
||||
found = found || a.Action == models.ActionMemberInviteDeclined
|
||||
}
|
||||
if !found {
|
||||
t.Errorf("no %s audit event", models.ActionMemberInviteDeclined)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestBUG2136_DeclineByCode(t *testing.T) {
|
||||
forEachDialect(t, func(t *testing.T, d store.DriverType) {
|
||||
f := newAccessFixture(t, d)
|
||||
invitee := mkUser(t, f.srv, "code-decliner@example.com")
|
||||
other := mkUser(t, f.srv, "not-the-invitee@example.com")
|
||||
rr := f.do("POST", "/api/v1/workspaces/"+f.wsSlug+"/members/invite", f.ownerTok,
|
||||
map[string]any{"email": "code-decliner@example.com"})
|
||||
f.must(rr, http.StatusCreated, "invite")
|
||||
var b inviteBody
|
||||
parseJSON(t, rr, &b)
|
||||
|
||||
if rr := f.do("POST", "/api/v1/invitations/"+b.Code+"/decline", f.token(other), nil); rr.Code != http.StatusForbidden {
|
||||
t.Fatalf("a different account declined by code: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
rr = f.do("POST", "/api/v1/invitations/"+b.Code+"/decline", f.token(invitee), nil)
|
||||
if rr.Code != http.StatusNoContent && rr.Code != http.StatusOK {
|
||||
t.Fatalf("decline by code: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if rr := f.do("POST", "/api/v1/invitations/"+b.Code+"/accept", f.token(invitee), nil); rr.Code != http.StatusNotFound {
|
||||
t.Errorf("a declined invitation's code still accepts: %d", rr.Code)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// Two sibling invitations for one address (a concurrent re-invite can leave
|
||||
// both on Postgres without the lock): accepting both is one membership and no
|
||||
// error, because a second accept by a member is idempotent (BUG-3281).
|
||||
func TestBUG2136_SiblingInvitationsAcceptCleanly(t *testing.T) {
|
||||
forEachDialect(t, func(t *testing.T, d store.DriverType) {
|
||||
f := newAccessFixture(t, d)
|
||||
invitee := mkUser(t, f.srv, "twin@example.com")
|
||||
a, err := f.srv.store.CreateInvitation(f.wsID, "twin@example.com", "editor", f.owner.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// Insert the sibling directly, bypassing the replace, to model the race.
|
||||
b := insertRawInvitation(t, f, d, "twin@example.com", "viewer")
|
||||
for _, id := range []string{a.ID, b} {
|
||||
rr := f.do("POST", "/api/v1/me/invitations/"+id+"/accept", f.token(invitee), nil)
|
||||
if rr.Code != http.StatusOK && rr.Code != http.StatusNotFound {
|
||||
t.Fatalf("accepting a sibling invitation: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
}
|
||||
members, err := f.srv.store.ListWorkspaceMembers(f.wsID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
n := 0
|
||||
for _, m := range members {
|
||||
if m.UserID == invitee.ID {
|
||||
n++
|
||||
}
|
||||
}
|
||||
if n != 1 {
|
||||
t.Fatalf("%d memberships after accepting two sibling invitations, want 1", n)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// Inviting an existing member is still refused.
|
||||
func TestBUG2136_InvitingAMemberIsRefused(t *testing.T) {
|
||||
forEachDialect(t, func(t *testing.T, d store.DriverType) {
|
||||
f := newAccessFixture(t, d)
|
||||
m := mkUser(t, f.srv, "already@example.com")
|
||||
if err := f.srv.store.AddWorkspaceMember(f.wsID, m.ID, "editor"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
rr := f.do("POST", "/api/v1/workspaces/"+f.wsSlug+"/members/invite", f.ownerTok,
|
||||
map[string]any{"email": "already@example.com"})
|
||||
f.must(rr, http.StatusConflict, "invite a member")
|
||||
})
|
||||
}
|
||||
|
||||
var rawInvitationSeq atomic.Int64
|
||||
|
||||
// insertRawInvitation inserts a pending invitation row directly, bypassing
|
||||
// CreateInvitation's replace, to model two invites that raced past it.
|
||||
func insertRawInvitation(t *testing.T, f *accessFixture, d store.DriverType, email, role string) string {
|
||||
t.Helper()
|
||||
id := fmt.Sprintf("raw-%d-%s", rawInvitationSeq.Add(1), email)
|
||||
q := `INSERT INTO workspace_invitations (id, workspace_id, email, role, invited_by, code, code_hash, created_at, expires_at)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`
|
||||
if d == store.DriverPostgres {
|
||||
q = `INSERT INTO workspace_invitations (id, workspace_id, email, role, invited_by, code, code_hash, created_at, expires_at)
|
||||
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`
|
||||
}
|
||||
ts := time.Now().UTC().Format(time.RFC3339)
|
||||
exp := time.Now().UTC().Add(time.Hour).Format(time.RFC3339)
|
||||
if _, err := f.srv.store.DB().Exec(q, id, f.wsID, email, role, f.owner.ID, id, "hash-"+id, ts, exp); err != nil {
|
||||
t.Fatalf("insert raw invitation: %v", err)
|
||||
}
|
||||
return id
|
||||
}
|
||||
@@ -44,15 +44,24 @@ func TestBUG3348_InviteDoesNotDirectAddUnverifiedAccount(t *testing.T) {
|
||||
t.Fatalf("unverified squatter read the workspace: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
|
||||
// Control: a verified account is still added directly.
|
||||
mkUser(t, f.srv, "real@example.com")
|
||||
// Control: a verified account gets the same pending invitation (since
|
||||
// BUG-2136 no account is added directly), and as the owner of the
|
||||
// address it can accept it from its own in-app list, which the
|
||||
// squatter cannot.
|
||||
real := mkUser(t, f.srv, "real@example.com")
|
||||
rr = f.do("POST", "/api/v1/workspaces/"+f.wsSlug+"/members/invite", f.ownerTok,
|
||||
map[string]any{"email": "real@example.com", "role": "editor"})
|
||||
f.must(rr, http.StatusCreated, "invite verified")
|
||||
body.Added, body.Invited = false, false
|
||||
parseJSON(t, rr, &body)
|
||||
if !body.Added {
|
||||
t.Fatalf("verified account was not added directly: %s", rr.Body.String())
|
||||
if body.Added || !body.Invited {
|
||||
t.Fatalf("verified account: got %s, want a pending invitation (BUG-2136)", rr.Body.String())
|
||||
}
|
||||
ids := myInvitationIDs(t, f, f.token(real))
|
||||
if len(ids) != 1 {
|
||||
t.Fatalf("the verified invitee lists %d invitations, want 1", len(ids))
|
||||
}
|
||||
f.must(f.do("POST", "/api/v1/me/invitations/"+ids[0]+"/accept", f.token(real), nil), http.StatusOK, "verified accept")
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -144,38 +144,27 @@ func (s *Server) handleInviteMember(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
// BUG-3348: only an account that has PROVEN this address is added
|
||||
// directly. Anyone can register an address on a self-serve instance, so
|
||||
// an unverified account matching the invite is not evidence of who will
|
||||
// read the workspace; it gets the ordinary invitation below, whose code
|
||||
// goes to the address itself. Accepting it verifies the account.
|
||||
if existingUser != nil && existingUser.IsEmailVerified() {
|
||||
// User exists and owns the address — add them directly
|
||||
alreadyMember, _ := s.store.IsWorkspaceMember(workspaceID, existingUser.ID)
|
||||
// BUG-2136 (lead ruling, day 87): an existing account is ASKED, never
|
||||
// added. Every invite is a pending invitation the invitee accepts or
|
||||
// declines, in-app (GET /me/invitations) or from the email when one is
|
||||
// sent. The response has the same SHAPE whether or not the address has
|
||||
// an account. It is not an account-existence guarantee: a code holder,
|
||||
// the inviter included, can still read has_account from the public
|
||||
// invitation preview, which the join page needs (pre-existing). BUG-3348's
|
||||
// direct add of a verified account is gone with it.
|
||||
if existingUser != nil {
|
||||
alreadyMember, merr := s.store.IsWorkspaceMember(workspaceID, existingUser.ID)
|
||||
if merr != nil {
|
||||
writeInternalError(w, merr)
|
||||
return
|
||||
}
|
||||
if alreadyMember {
|
||||
writeError(w, http.StatusConflict, "conflict", "User is already a member of this workspace")
|
||||
return
|
||||
}
|
||||
if err := s.store.AddWorkspaceMember(workspaceID, existingUser.ID, input.Role, s.workspaceLimitMintOpts()...); err != nil {
|
||||
if writeStorePlanLimitError(w, r, err, "") {
|
||||
return
|
||||
}
|
||||
writeInternalError(w, err)
|
||||
return
|
||||
}
|
||||
s.logWorkspaceAuditEvent(workspaceID, models.ActionMemberInvited, r, auditMeta(map[string]string{"email": existingUser.Email, "role": input.Role, "added_directly": "true"}))
|
||||
s.publishWorkspaceAccessChangedFromRequest(r, workspaceID, watchevents.AccessGained, existingUser.ID)
|
||||
writeJSON(w, http.StatusCreated, map[string]interface{}{
|
||||
"added": true,
|
||||
"user_id": existingUser.ID,
|
||||
"email": existingUser.Email,
|
||||
"name": existingUser.Name,
|
||||
"role": input.Role,
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
// User doesn't exist — create an invitation
|
||||
// Create (or, for a re-invite, replace) the pending invitation.
|
||||
inv, err := s.store.CreateInvitation(workspaceID, input.Email, input.Role, inviterID)
|
||||
if writeAccountDisabledIf(w, err) {
|
||||
return
|
||||
@@ -465,6 +454,51 @@ func (s *Server) handleAcceptInvitation(w http.ResponseWriter, r *http.Request)
|
||||
s.writeInvitationAccepted(w, inv, role)
|
||||
}
|
||||
|
||||
// handleDeclineInvitation answers POST /api/v1/invitations/{code}/decline
|
||||
// (BUG-2136): declining from the join link. The caller must be signed in as
|
||||
// the invited address, exactly as accepting by code requires, so a forwarded
|
||||
// code cannot be used to decline on someone else's behalf.
|
||||
func (s *Server) handleDeclineInvitation(w http.ResponseWriter, r *http.Request) {
|
||||
inv, err := s.store.GetInvitationByCode(chi.URLParam(r, "code"))
|
||||
if err != nil {
|
||||
writeInternalError(w, err)
|
||||
return
|
||||
}
|
||||
if inv == nil {
|
||||
writeError(w, http.StatusNotFound, "not_found", "Invitation not found or already accepted")
|
||||
return
|
||||
}
|
||||
user := currentUser(r)
|
||||
if user == nil || user.IsApp() {
|
||||
writeError(w, http.StatusUnauthorized, "unauthorized", "You must be logged in to decline an invitation")
|
||||
return
|
||||
}
|
||||
if !invitationEmailMatches(user.Email, inv.Email) {
|
||||
writeError(w, http.StatusForbidden, "invitation_email_mismatch",
|
||||
"This invitation was sent to a different email address. Sign in with that account to decline.")
|
||||
return
|
||||
}
|
||||
s.declineInvitation(w, r, inv)
|
||||
}
|
||||
|
||||
// declineInvitation is what declining means behind both doors: the pending
|
||||
// invitation is deleted and a member_invite_declined audit event records it.
|
||||
// A row gone in between (accepted, cancelled or declined concurrently) is the
|
||||
// same 404 an unknown invitation gets.
|
||||
func (s *Server) declineInvitation(w http.ResponseWriter, r *http.Request, inv *models.WorkspaceInvitation) {
|
||||
if err := s.store.DeleteInvitationAdmin(inv.ID); err != nil {
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
writeError(w, http.StatusNotFound, "not_found", "Invitation not found or already accepted")
|
||||
return
|
||||
}
|
||||
writeInternalError(w, err)
|
||||
return
|
||||
}
|
||||
s.logWorkspaceAuditEvent(inv.WorkspaceID, models.ActionMemberInviteDeclined, r,
|
||||
auditMeta(map[string]string{"email": inv.Email, "role": inv.Role}))
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
|
||||
// acceptInvitationCore is the one accept path behind both invitation doors:
|
||||
// POST /invitations/{code}/accept and POST /me/invitations/{id}/accept
|
||||
// (TASK-3277). Each door decides first whether the caller may accept inv;
|
||||
@@ -484,6 +518,13 @@ func (s *Server) acceptInvitationCore(w http.ResponseWriter, r *http.Request, in
|
||||
// key direct adds take. A refusal rolls the transaction back, so the
|
||||
// invitation stays pending and can be accepted once there is room.
|
||||
added, role, err := s.store.AcceptWorkspaceInvitation(inv.ID, inv.WorkspaceID, user.ID, inv.Role, s.workspaceLimitMintOpts()...)
|
||||
// Declined, replaced or cancelled after the door read it (BUG-2136): the
|
||||
// store rolled the accept back, so this is the not-found every door gives
|
||||
// an invitation that is gone. No reconcile: nothing was written.
|
||||
if errors.Is(err, store.ErrInvitationGone) {
|
||||
writeError(w, http.StatusNotFound, "not_found", "Invitation not found or already accepted")
|
||||
return "", false
|
||||
}
|
||||
if err != nil {
|
||||
// RECONCILE BEFORE REFUSING (BUG-3026). The store returns the raw
|
||||
// commit error, so a lost acknowledgement lands the membership and the
|
||||
|
||||
@@ -88,3 +88,33 @@ func (s *Server) handleAcceptMyInvitation(w http.ResponseWriter, r *http.Request
|
||||
}
|
||||
s.writeInvitationAccepted(w, inv, role)
|
||||
}
|
||||
|
||||
// handleDeclineMyInvitation answers POST /api/v1/me/invitations/{id}/decline
|
||||
// (BUG-2136): the invitee says no from the in-app list. Same admission as the
|
||||
// accept beside it: a signed-in person with a VERIFIED email, and an id that is
|
||||
// not a pending invitation addressed to them answers 404 identical to an
|
||||
// unknown id. Declining deletes the invitation, as an owner's cancel does, so
|
||||
// every pending list drops it and its code stops working; the audit event is
|
||||
// its trail. An expired invitation may be declined too, which only tidies it.
|
||||
func (s *Server) handleDeclineMyInvitation(w http.ResponseWriter, r *http.Request) {
|
||||
user := currentUser(r)
|
||||
if user == nil || user.IsApp() {
|
||||
writeError(w, http.StatusUnauthorized, "unauthorized", "You must be logged in to decline an invitation")
|
||||
return
|
||||
}
|
||||
if !user.IsEmailVerified() {
|
||||
writeError(w, http.StatusForbidden, "email_not_verified",
|
||||
"Verify your email address to decline invitations from this list, or use the link in the invitation email.")
|
||||
return
|
||||
}
|
||||
inv, err := s.store.GetPendingInvitationForEmail(chi.URLParam(r, "id"), user.Email)
|
||||
if err != nil {
|
||||
writeInternalError(w, err)
|
||||
return
|
||||
}
|
||||
if inv == nil {
|
||||
writeError(w, http.StatusNotFound, "not_found", "Invitation not found or already accepted")
|
||||
return
|
||||
}
|
||||
s.declineInvitation(w, r, inv)
|
||||
}
|
||||
|
||||
@@ -491,15 +491,6 @@ func (e *planLimitEnv) collectionID(t *testing.T, slug string) string {
|
||||
return coll.ID
|
||||
}
|
||||
|
||||
func (e *planLimitEnv) newUser(t *testing.T, email string) *models.User {
|
||||
t.Helper()
|
||||
u, err := e.srv.store.CreateUser(models.UserCreate{Email: email, Name: email, Password: "pw-limit-12345"})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateUser(%s): %v", email, err)
|
||||
}
|
||||
return u
|
||||
}
|
||||
|
||||
func (e *planLimitEnv) itemRace(t *testing.T, door func(t *testing.T) *httptest.ResponseRecorder) workspaceRace {
|
||||
tasks := e.collectionID(t, "tasks")
|
||||
return workspaceRace{
|
||||
@@ -542,27 +533,14 @@ func (e *planLimitEnv) itemImportRace(t *testing.T) workspaceRace {
|
||||
})
|
||||
}
|
||||
|
||||
// W3: the invite door's direct add of an existing user.
|
||||
func (e *planLimitEnv) memberAddRace(t *testing.T) workspaceRace {
|
||||
invitee := e.newUser(t, "invitee@example.com")
|
||||
rival := e.newUser(t, "rival@example.com")
|
||||
return workspaceRace{
|
||||
feature: "members_per_workspace",
|
||||
count: func(t *testing.T) int {
|
||||
return e.countIn(t, `SELECT COUNT(*) FROM workspace_members WHERE workspace_id = ?`)
|
||||
},
|
||||
compete: func(t *testing.T) {
|
||||
if err := e.srv.store.AddWorkspaceMember(e.home.ID, rival.ID, "editor"); err != nil {
|
||||
t.Errorf("competing AddWorkspaceMember: %v", err)
|
||||
}
|
||||
},
|
||||
door: func(t *testing.T) *httptest.ResponseRecorder {
|
||||
body, _ := json.Marshal(map[string]any{"email": invitee.Email, "role": "editor"})
|
||||
return e.do("POST", "/api/v1/workspaces/"+e.home.Slug+"/members/invite", "application/json", body, "")
|
||||
},
|
||||
admitted: http.StatusCreated,
|
||||
}
|
||||
}
|
||||
// The members door is gone from this harness (BUG-2136). The invite still runs
|
||||
// its members_per_workspace pre-check (refusing an invitation at the cap, as it
|
||||
// always did for code invitations), but it no longer creates a membership, so
|
||||
// this harness's member count cannot measure it. The door that creates the
|
||||
// membership is the accept, whose cap the store decides alone; it is covered
|
||||
// by handlers_invitation_accept_limit_test.go (BUG-3098), including the
|
||||
// register race and the self-hosted leg. The invite's refusal at the cap is
|
||||
// handlers_workspace_cap_test.go.
|
||||
|
||||
// W4: the webhook create door. Literal IPs keep ValidateWebhookURL off DNS.
|
||||
func (e *planLimitEnv) webhookRace(t *testing.T) workspaceRace {
|
||||
@@ -604,16 +582,6 @@ func TestPlanLimitRace_ItemsImport_NoCompetitor_Admitted(t *testing.T) {
|
||||
e.runWorkspaceRace(t, e.itemImportRace(t), false)
|
||||
}
|
||||
|
||||
func TestPlanLimitRace_Members_CompetingAddInWindow_Refused(t *testing.T) {
|
||||
e := newPlanLimitEnv(t)
|
||||
e.runWorkspaceRace(t, e.memberAddRace(t), true)
|
||||
}
|
||||
|
||||
func TestPlanLimitRace_Members_NoCompetitor_Admitted(t *testing.T) {
|
||||
e := newPlanLimitEnv(t)
|
||||
e.runWorkspaceRace(t, e.memberAddRace(t), false)
|
||||
}
|
||||
|
||||
func TestPlanLimitRace_Webhooks_CompetingCreateInWindow_Refused(t *testing.T) {
|
||||
e := newPlanLimitEnv(t)
|
||||
e.runWorkspaceRace(t, e.webhookRace(t), true)
|
||||
@@ -653,7 +621,6 @@ func TestPlanLimit_SelfHosted_NotEnforced(t *testing.T) {
|
||||
}{
|
||||
{"Items", (*planLimitEnv).itemCreateRace},
|
||||
{"ItemsImport", (*planLimitEnv).itemImportRace},
|
||||
{"Members", (*planLimitEnv).memberAddRace},
|
||||
{"Webhooks", (*planLimitEnv).webhookRace},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
|
||||
@@ -242,7 +242,9 @@ func TestPlanLimitError_ResponseShape(t *testing.T) {
|
||||
wsSlug := wsResp.Slug
|
||||
|
||||
// Add existing users until we're at the member cap (3 for free tier).
|
||||
// The owner themselves count as 1, so invite 2 more real users.
|
||||
// The owner themselves count as 1, so add 2 more. Added through the store:
|
||||
// since BUG-2136 an invite only asks, and this test's subject is the
|
||||
// invite door's refusal AT the cap.
|
||||
for i := 2; i <= 3; i++ {
|
||||
member, err := srv.store.CreateUser(models.UserCreate{
|
||||
Email: fmt.Sprintf("member%d@test.com", i),
|
||||
@@ -253,10 +255,12 @@ func TestPlanLimitError_ResponseShape(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("CreateUser(member%d): %v", i, err)
|
||||
}
|
||||
invRR := doRequestWithCookie(srv, "POST", "/api/v1/workspaces/"+wsSlug+"/members/invite",
|
||||
map[string]string{"email": member.Email, "role": "editor"}, ownerToken)
|
||||
if invRR.Code != http.StatusCreated {
|
||||
t.Fatalf("invite member %d: expected 201, got %d: %s", i, invRR.Code, invRR.Body.String())
|
||||
ws, err := srv.store.GetWorkspaceBySlug(wsSlug)
|
||||
if err != nil || ws == nil {
|
||||
t.Fatalf("GetWorkspaceBySlug: %v", err)
|
||||
}
|
||||
if err := srv.store.AddWorkspaceMember(ws.ID, member.ID, "editor"); err != nil {
|
||||
t.Fatalf("AddWorkspaceMember(member%d): %v", i, err)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -80,11 +80,17 @@ func isMutatingMethod(method string) bool {
|
||||
// callers have no currentUser, so emailUnverifiedBlocked already
|
||||
// returned false.
|
||||
func verifiedEmailExemptPath(path string) bool {
|
||||
// Invitation accept — matched structurally so any {code} passes,
|
||||
// while /invitations/{code}/preview (GET, already public) and any
|
||||
// other /invitations subpath are unaffected.
|
||||
if strings.HasPrefix(path, "/api/v1/invitations/") && strings.HasSuffix(path, "/accept") {
|
||||
return true
|
||||
// Invitation accept and decline (BUG-2136): the two answers to one
|
||||
// invitation, matched structurally so any {code} passes, while
|
||||
// /invitations/{code}/preview (GET, already public) and any other
|
||||
// /invitations subpath are unaffected.
|
||||
// Exactly two segments after the prefix (codex r2), so no nested path
|
||||
// that happens to end in /accept or /decline is exempted.
|
||||
if rest, ok := strings.CutPrefix(path, "/api/v1/invitations/"); ok {
|
||||
if code, action, found := strings.Cut(rest, "/"); found && code != "" &&
|
||||
(action == "accept" || action == "decline") {
|
||||
return true
|
||||
}
|
||||
}
|
||||
switch path {
|
||||
case "/api/v1/auth/logout",
|
||||
|
||||
@@ -499,3 +499,42 @@ func TestVerifiedEmail_OAuthAuthorize_BlocksUnverified(t *testing.T) {
|
||||
rr := doAuthedRequest(srv, "GET", "/oauth/authorize?"+q.Encode(), nil, sessionToken)
|
||||
assertBlocked(t, rr, "unverified oauth authorize render")
|
||||
}
|
||||
|
||||
// BUG-2136, codex r1: declining by code is the other answer to the same
|
||||
// invitation, so it passes the gate exactly as accepting does.
|
||||
func TestVerifiedEmail_InvitationDecline_CarveOut(t *testing.T) {
|
||||
f := newVerifiedEmailFixture(t, true)
|
||||
admin, err := f.srv.store.GetUserByEmail("admin@ve.test")
|
||||
if err != nil || admin == nil {
|
||||
t.Fatalf("GetUserByEmail admin: %v", err)
|
||||
}
|
||||
otherWS, err := f.srv.store.CreateWorkspace(models.WorkspaceCreate{Name: "Decline Target", OwnerID: admin.ID})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateWorkspace: %v", err)
|
||||
}
|
||||
inv, err := f.srv.store.CreateInvitation(otherWS.ID, f.unv.email, "editor", admin.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("CreateInvitation: %v", err)
|
||||
}
|
||||
rr := doRequestWithCookie(f.srv, "POST", "/api/v1/invitations/"+inv.Code+"/decline", nil, f.unv.session)
|
||||
assertNotEmailBlocked(t, rr, "unverified invitation-decline")
|
||||
if rr.Code != http.StatusNoContent {
|
||||
t.Fatalf("unverified invitation-decline: expected 204, got %d (body: %s)", rr.Code, rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestVerifiedEmailExemptPath_InvitationAnswersOnly(t *testing.T) {
|
||||
for path, want := range map[string]bool{
|
||||
"/api/v1/invitations/abc/accept": true,
|
||||
"/api/v1/invitations/abc/decline": true,
|
||||
"/api/v1/invitations/abc/preview": false,
|
||||
"/api/v1/invitations/abc/nested/accept": false,
|
||||
"/api/v1/invitations/x/y/decline": false,
|
||||
"/api/v1/invitations//accept": false,
|
||||
"/api/v1/invitations/abc": false,
|
||||
} {
|
||||
if got := verifiedEmailExemptPath(path); got != want {
|
||||
t.Errorf("verifiedEmailExemptPath(%q) = %v, want %v", path, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -94,13 +94,17 @@ func TestMyInvitations_ListFilters(t *testing.T) {
|
||||
f.owner.Username = "ownerhandle"
|
||||
|
||||
mine := f.invite("me@example.com", "editor")
|
||||
// Case differs only in how the inviter typed it; stored lowercased.
|
||||
// Case differs only in how the inviter typed it; stored lowercased,
|
||||
// so this is a RE-INVITE of the same address and replaces `mine`
|
||||
// (BUG-2136: one live invitation per workspace and address).
|
||||
mineUpper := f.invite(" ME@Example.com ", "viewer")
|
||||
f.invite("other@example.com", "editor")
|
||||
expired := f.invite("me@example.com", "editor")
|
||||
expireInvitation(t, f.srv, expired.ID)
|
||||
accepted := f.invite("me@example.com", "editor")
|
||||
if err := f.srv.store.AcceptInvitation(accepted.ID); err != nil {
|
||||
// The expired and accepted rows are inserted directly: CreateInvitation
|
||||
// would replace the live one, and these legs test the LIST's filters.
|
||||
expired := insertRawInvitation(t, f, d, "me@example.com", "editor")
|
||||
expireInvitation(t, f.srv, expired)
|
||||
accepted := insertRawInvitation(t, f, d, "me@example.com", "editor")
|
||||
if err := f.srv.store.AcceptInvitation(accepted); err != nil {
|
||||
t.Fatalf("AcceptInvitation: %v", err)
|
||||
}
|
||||
|
||||
@@ -108,9 +112,9 @@ func TestMyInvitations_ListFilters(t *testing.T) {
|
||||
if !got.EmailVerified {
|
||||
t.Fatalf("verified caller reported email_verified=false")
|
||||
}
|
||||
if want := sortedIDs(mine.ID, mineUpper.ID); !reflect.DeepEqual(invitationIDs(got.Invitations), want) {
|
||||
t.Fatalf("listed %v, want %v (not expired %s, not accepted %s, not other's)",
|
||||
invitationIDs(got.Invitations), want, expired.ID, accepted.ID)
|
||||
if want := sortedIDs(mineUpper.ID); !reflect.DeepEqual(invitationIDs(got.Invitations), want) {
|
||||
t.Fatalf("listed %v, want %v (not the replaced %s, not expired %s, not accepted %s, not other's)",
|
||||
invitationIDs(got.Invitations), want, mine.ID, expired, accepted)
|
||||
}
|
||||
for _, inv := range got.Invitations {
|
||||
if inv.WorkspaceSlug != f.wsSlug || inv.WorkspaceName != "Access WS" || inv.WorkspaceOwnerUsername != f.owner.Username {
|
||||
|
||||
@@ -1915,6 +1915,7 @@ func (s *Server) setupRouter() {
|
||||
|
||||
// Invitations (outside workspace scope)
|
||||
r.Post("/invitations/{code}/accept", s.handleAcceptInvitation)
|
||||
r.Post("/invitations/{code}/decline", s.handleDeclineInvitation)
|
||||
|
||||
// Non-consuming invitation preview (BUG-1934). Public/pre-auth
|
||||
// (exempted in isPublicAPIPath) so the logged-out /join page can
|
||||
@@ -1975,6 +1976,7 @@ func (s *Server) setupRouter() {
|
||||
// client only.
|
||||
r.Get("/me/invitations", s.handleListMyInvitations)
|
||||
r.Post("/me/invitations/{id}/accept", s.handleAcceptMyInvitation)
|
||||
r.Post("/me/invitations/{id}/decline", s.handleDeclineMyInvitation)
|
||||
|
||||
// Workspaces
|
||||
r.Route("/workspaces", func(r chi.Router) {
|
||||
|
||||
@@ -203,10 +203,18 @@ func TestWorkspaceAccessChanged_ImportJSONAndBundle(t *testing.T) {
|
||||
func TestWorkspaceAccessChanged_InviteExistingUser(t *testing.T) {
|
||||
forEachDialect(t, func(t *testing.T, d store.DriverType) {
|
||||
f := newAccessFixture(t, d)
|
||||
mkUser(t, f.srv, "invitee@example.com")
|
||||
invitee := mkUser(t, f.srv, "invitee@example.com")
|
||||
since := f.mark()
|
||||
f.must(f.do("POST", "/api/v1/workspaces/"+f.wsSlug+"/members/invite", f.ownerTok,
|
||||
map[string]any{"email": "invitee@example.com", "role": "editor"}), http.StatusCreated, "invite")
|
||||
// BUG-2136: inviting an existing account asks it; nothing is gained
|
||||
// until it accepts.
|
||||
f.expect(since)
|
||||
ids := myInvitationIDs(t, f, f.token(invitee))
|
||||
if len(ids) != 1 {
|
||||
t.Fatalf("the invitee lists %d invitations, want 1", len(ids))
|
||||
}
|
||||
f.must(f.do("POST", "/api/v1/me/invitations/"+ids[0]+"/accept", f.token(invitee), nil), http.StatusOK, "accept")
|
||||
f.expect(since, f.line("gained", "invitee@example.com"))
|
||||
})
|
||||
}
|
||||
@@ -455,6 +463,12 @@ func TestWorkspaceAccessChanged_StreamOptInAndAddressing(t *testing.T) {
|
||||
|
||||
f.must(f.do("POST", "/api/v1/workspaces/"+f.wsSlug+"/members/invite", f.ownerTok,
|
||||
map[string]any{"email": "target@example.com", "role": "viewer"}), http.StatusCreated, "invite target")
|
||||
// BUG-2136: the gain happens on accept, not on invite.
|
||||
tids := myInvitationIDs(t, f, ttok)
|
||||
if len(tids) != 1 {
|
||||
t.Fatalf("the target lists %d invitations, want 1", len(tids))
|
||||
}
|
||||
f.must(f.do("POST", "/api/v1/me/invitations/"+tids[0]+"/accept", ttok, nil), http.StatusOK, "target accepts")
|
||||
ev := waitForWatchEvent(t, targetAccess, 3*time.Second)
|
||||
var p watchEventPayload
|
||||
if err := json.Unmarshal([]byte(ev.Data), &p); err != nil {
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"sync"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// BUG-2136: a re-invite replaces the pending invitation for its (workspace,
|
||||
// address). Concurrent invites of one address must still leave exactly one
|
||||
// live invitation: CreateInvitation takes the workspace lock before its
|
||||
// delete-then-insert, so two of them cannot both see "nothing pending" and
|
||||
// both insert. On SQLite the write lock serializes them anyway; the leg that
|
||||
// can fail without the workspace lock is Postgres (make test-pg).
|
||||
func TestCreateInvitation_ConcurrentReinvitesLeaveOnePending(t *testing.T) {
|
||||
s := testStore(t)
|
||||
owner := createTestUser(t, s, "owner-2136@example.com", "Owner", "password123")
|
||||
ws := createTestWorkspace(t, s, "Reinvite race")
|
||||
if err := s.AddWorkspaceMember(ws.ID, owner.ID, "owner"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
const n = 8
|
||||
var wg sync.WaitGroup
|
||||
errs := make(chan error, n)
|
||||
start := make(chan struct{})
|
||||
for i := 0; i < n; i++ {
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
<-start
|
||||
if _, err := s.CreateInvitation(ws.ID, "raced@example.com", "editor", owner.ID); err != nil {
|
||||
errs <- err
|
||||
}
|
||||
}()
|
||||
}
|
||||
close(start)
|
||||
wg.Wait()
|
||||
close(errs)
|
||||
for err := range errs {
|
||||
t.Errorf("CreateInvitation: %v", err)
|
||||
}
|
||||
|
||||
pending, err := s.ListWorkspaceInvitations(ws.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(pending) != 1 {
|
||||
t.Fatalf("%d pending invitations after %d concurrent invites of one address, want exactly 1", len(pending), n)
|
||||
}
|
||||
}
|
||||
|
||||
// Codex r1: accept must not land after the invitation was declined (or
|
||||
// replaced). The handler reads the invitation before the store transaction,
|
||||
// so a decline can delete the row in between; the accept must then add no
|
||||
// membership and report the invitation gone. Modelled by deleting the row
|
||||
// between the read and AcceptWorkspaceInvitation.
|
||||
func TestAcceptWorkspaceInvitation_RowGoneAddsNoMembership(t *testing.T) {
|
||||
s := testStore(t)
|
||||
owner := createTestUser(t, s, "owner-2136b@example.com", "Owner", "password123")
|
||||
invitee := createTestUser(t, s, "late@example.com", "Late", "password123")
|
||||
ws := createTestWorkspace(t, s, "Accept after decline")
|
||||
if err := s.AddWorkspaceMember(ws.ID, owner.ID, "owner"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
inv, err := s.CreateInvitation(ws.ID, "late@example.com", "editor", owner.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// Control: a live invitation accepts.
|
||||
ctl, err := s.CreateInvitation(ws.ID, "control@example.com", "editor", owner.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
ctlUser := createTestUser(t, s, "control@example.com", "Control", "password123")
|
||||
if _, _, err := s.AcceptWorkspaceInvitation(ctl.ID, ws.ID, ctlUser.ID, "editor"); err != nil {
|
||||
t.Fatalf("control accept: %v", err)
|
||||
}
|
||||
|
||||
if err := s.DeleteInvitationAdmin(inv.ID); err != nil { // the decline lands first
|
||||
t.Fatal(err)
|
||||
}
|
||||
_, _, err = s.AcceptWorkspaceInvitation(inv.ID, ws.ID, invitee.ID, "editor")
|
||||
if !errors.Is(err, ErrInvitationGone) {
|
||||
t.Fatalf("accept of a declined invitation: err = %v, want ErrInvitationGone", err)
|
||||
}
|
||||
if member, _ := s.IsWorkspaceMember(ws.ID, invitee.ID); member {
|
||||
t.Errorf("an accept that lost to a decline still added the member")
|
||||
}
|
||||
// A second accept of an ALREADY ACCEPTED invitation stays idempotent
|
||||
// (BUG-3281): the loser of two concurrent accepts.
|
||||
if _, _, err := s.AcceptWorkspaceInvitation(ctl.ID, ws.ID, ctlUser.ID, "editor"); err != nil {
|
||||
t.Errorf("re-accepting an accepted invitation must stay idempotent, got %v", err)
|
||||
}
|
||||
}
|
||||
@@ -145,6 +145,11 @@ func (s *Store) commitWorkspaceMemberTx(tx *sql.Tx) error {
|
||||
return tx.Commit()
|
||||
}
|
||||
|
||||
// ErrInvitationGone: the invitation an accept was admitted on no longer
|
||||
// exists, because it was declined, replaced or cancelled after the caller read
|
||||
// it (BUG-2136). The accept is rolled back.
|
||||
var ErrInvitationGone = errors.New("invitation no longer pending")
|
||||
|
||||
// AcceptWorkspaceInvitation accepts an invitation in one transaction: the
|
||||
// membership through addWorkspaceMemberTx, then the invitation's accepted_at.
|
||||
// An existing membership is an idempotent success (BUG-3281, lead ruling):
|
||||
@@ -170,9 +175,25 @@ func (s *Store) AcceptWorkspaceInvitation(invitationID, workspaceID, userID, rol
|
||||
return false, "", err
|
||||
}
|
||||
// The first accept's timestamp stands; a concurrent loser leaves it alone.
|
||||
if _, err := tx.Exec(s.q(`UPDATE workspace_invitations SET accepted_at = ? WHERE id = ? AND accepted_at IS NULL`), now(), invitationID); err != nil {
|
||||
res, err := tx.Exec(s.q(`UPDATE workspace_invitations SET accepted_at = ? WHERE id = ? AND accepted_at IS NULL`), now(), invitationID)
|
||||
if err != nil {
|
||||
return false, "", fmt.Errorf("accept workspace invitation: %w", err)
|
||||
}
|
||||
// Nothing updated is one of two things (BUG-2136). The row is still there,
|
||||
// accepted: a concurrent accept won, and this one is the idempotent loser
|
||||
// (BUG-3281). The row is GONE: it was declined, replaced or cancelled after
|
||||
// the caller read it, and this accept must not land, so the membership
|
||||
// written above is rolled back. On Postgres the UPDATE holds the row's lock
|
||||
// until commit, so a decline racing it waits and then finds nothing pending.
|
||||
if n, rerr := res.RowsAffected(); rerr == nil && n == 0 {
|
||||
var one int
|
||||
switch qerr := tx.QueryRow(s.q(`SELECT 1 FROM workspace_invitations WHERE id = ?`), invitationID).Scan(&one); {
|
||||
case errors.Is(qerr, sql.ErrNoRows):
|
||||
return false, "", ErrInvitationGone
|
||||
case qerr != nil:
|
||||
return false, "", fmt.Errorf("accept workspace invitation: recheck: %w", qerr)
|
||||
}
|
||||
}
|
||||
if err := s.commitWorkspaceMemberTx(tx); err != nil {
|
||||
// added and effectiveRole describe what this transaction wrote, so they
|
||||
// are returned with a commit error: if the caller finds the commit
|
||||
@@ -963,11 +984,23 @@ func (s *Store) CreateInvitation(workspaceID, email, role, invitedBy string) (*m
|
||||
return nil, fmt.Errorf("insert invitation: begin: %w", err)
|
||||
}
|
||||
defer func() { _ = tx.Rollback() }()
|
||||
// One live invitation per (workspace, address) (BUG-2136): a re-invite
|
||||
// REPLACES the pending one, so a fresh code, role and expiry win and the
|
||||
// old code stops working. The workspace lock serializes two concurrent
|
||||
// invites of one address on Postgres (SQLite's write lock already does);
|
||||
// it is taken first, the order the other workspace writers use.
|
||||
if err := s.acquireWorkspaceSeqLock(tx, workspaceID); err != nil {
|
||||
return nil, fmt.Errorf("insert invitation: lock: %w", err)
|
||||
}
|
||||
if invitedBy != "" {
|
||||
if err := s.requireActiveUserTx(tx, invitedBy); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
if _, err := tx.Exec(s.q(`DELETE FROM workspace_invitations WHERE workspace_id = ? AND email = ? AND accepted_at IS NULL`),
|
||||
workspaceID, strings.ToLower(strings.TrimSpace(email))); err != nil {
|
||||
return nil, fmt.Errorf("insert invitation: replace pending: %w", err)
|
||||
}
|
||||
if _, err := tx.Exec(s.q(`
|
||||
INSERT INTO workspace_invitations (id, workspace_id, email, role, invited_by, code, code_hash, created_at, expires_at, proof_hash)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
|
||||
@@ -151,6 +151,24 @@ async function createItems(
|
||||
return items;
|
||||
}
|
||||
|
||||
/**
|
||||
* Accept, as the invitee, the pending invitation to workspace `ws` (BUG-2136:
|
||||
* an invite no longer adds an existing account, it asks).
|
||||
*/
|
||||
async function acceptInvitationTo(baseURL: string, token: string, ws: string, key: string): Promise<void> {
|
||||
const invitee = await bearer(baseURL, token);
|
||||
try {
|
||||
const list = (await (
|
||||
await expectOk(await invitee.get('/api/v1/me/invitations'), `list invitations for ${key}`)
|
||||
).json()) as { invitations: { id: string; workspace_slug: string }[] };
|
||||
const inv = list.invitations.find((i) => i.workspace_slug === ws);
|
||||
if (!inv) throw new Error(`no pending invitation to ${ws} for ${key}`);
|
||||
await expectOk(await invitee.post(`/api/v1/me/invitations/${inv.id}/accept`), `accept for ${key}`);
|
||||
} finally {
|
||||
await invitee.dispose();
|
||||
}
|
||||
}
|
||||
|
||||
/** Seed one walk world. Each call builds a fresh, independent one. */
|
||||
export async function seedPermissionWalk(): Promise<PermissionWalk> {
|
||||
const { baseURL, adminSessionToken } = suiteFixture();
|
||||
@@ -195,6 +213,8 @@ export async function seedPermissionWalk(): Promise<PermissionWalk> {
|
||||
}),
|
||||
`invite ${key}`
|
||||
);
|
||||
// BUG-2136: an invite asks; the account joins when it accepts.
|
||||
await acceptInvitationTo(baseURL, accounts[key].token, ws, key);
|
||||
}
|
||||
|
||||
const tasksColl = (await (
|
||||
|
||||
@@ -112,6 +112,13 @@ async function seed(fixture: SuiteFixture): Promise<World> {
|
||||
await owner.api.post(`/api/v1/workspaces/${shared}/members/invite`, { data: { email: member.email, role: 'editor' } }),
|
||||
'invite'
|
||||
);
|
||||
// BUG-2136: an invite asks; the member joins when they accept.
|
||||
const list = (await (await ok(await member.api.get('/api/v1/me/invitations'), 'list invitations')).json()) as {
|
||||
invitations: { id: string; workspace_slug: string }[];
|
||||
};
|
||||
const inv = list.invitations.find((i) => i.workspace_slug === shared);
|
||||
if (!inv) throw new Error(`no pending invitation to ${shared}`);
|
||||
await ok(await member.api.post(`/api/v1/me/invitations/${inv.id}/accept`), 'accept');
|
||||
await setOpenSet(member, [own, shared]);
|
||||
return { owner, member, own, shared };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user