Merge pull request #703 from warmbly/feat/session-org-membership

feat: scope every session request to a workspace its user is still a member of, deselect a removed member's workspace on all their sessions, disconnect their realtime sockets on removal, and correct the singular /organization paths in the API docs

docs/content/docs/api/endpoints.mdx

@@ -438,8 +438,8 @@ These changes require the session to have confirmed the account holder within th
 | Create an API key | `POST /api-keys` |
 | Add a passkey | `POST /auth/passkey/register/begin`, `/finish` |
 | Remove a passkey | `DELETE /auth/passkey/credentials/:id` |
-| Transfer a workspace | `POST /organizations/transfer-ownership` |
-| Schedule a workspace or account for deletion | `POST /organizations/current/danger-zone/delete`, `POST /me/danger-zone/delete` |
+| Transfer a workspace | `POST /organization/transfer-ownership` |
+| Schedule a workspace or account for deletion | `POST /organization/current/danger-zone/delete`, `POST /me/danger-zone/delete` |
 | Record, use or remove an admin grant over a whole domain | `POST /emails/grants/google/finish`, `POST /emails/grants/microsoft/finish`, `POST /emails/grants/:id/connect`, `DELETE /emails/grants/:id` |
 
 Each either hands out a credential that outlives the session that created it, cannot be reversed by the person it was done to, or reaches a whole domain's mail. Without a confirmation they answer `403` with code `reauth_required`; confirm with `POST /auth/reauth` (password or a current two-factor code) and retry. See [error codes](/api/error-codes/#confirmation-required).

docs/content/docs/api/error-codes.mdx

@@ -108,7 +108,7 @@ Fields are named by their JSON key, with nested fields as a dotted path (`inner.
 | `selection_too_large` | A `"all": true` bulk selection resolved to more than `50,000` rows. Narrow the filter and run it in parts; nothing was changed |
 | `invalid_filter` | A task filter carried an id that is not one: `assigned_to`, `contact_id` and `deal_id` name records, and are matched against id columns. Sent by `POST /crm/tasks/search`, `POST /crm/tasks/summary`, and the `filters` of a `"all": true` bulk selection |
 | `invalid_setting` | `PATCH /outreach/settings` (or a campaign's advanced settings) carried a value outside the documented vocabulary, for example a `reply_intent.crm_task_intents` entry that is not a reply intent, or an `inbox_tagging.questions` entry with no question text, a label that is missing, repeated or built in, or a choice question with fewer than two options, or an `inbox_tagging.languages` entry that is not a supported language code |
-| `invalid_slug` | `PATCH /organizations/current` was given a `slug` that is not 2 to 80 lowercase letters, numbers or dashes starting and ending with a letter or number |
+| `invalid_slug` | `PATCH /organization/current` was given a `slug` that is not 2 to 80 lowercase letters, numbers or dashes starting and ending with a letter or number |
 | `invalid_sync_folder` | `PUT /emails/:id/sync` was given a folder the sync always follows (`INBOX`, or a sent, drafts, spam, trash or archive folder by attribute or name), a name that is empty after trimming, longer than 255 characters or carrying a control character, more than 50 names, or a mailbox that is not IMAP. The `message` names the entry refused |
 | `no_organization` | The request needs a workspace and the caller has none selected. Every entitlement, limit and suppression rule is scoped to a workspace, so a write that would run unscoped is refused rather than run without those checks. API keys always carry their workspace; a dashboard session picks one at sign-in, so this normally means the session predates the workspace being chosen. Select a workspace and retry |
 

docs/content/docs/api/realtime.mdx

@@ -51,6 +51,8 @@ Events arrive as channel messages whose event name is the event type, for exampl
 
 Permission filtering happens per event on the org channel: for example inbox events require `access_unibox`, campaign pulses require `view_campaigns`, placement tests require `view_analytics`, mailbox and mailbox import events require `manage_emails`, member changes require `manage_team`, and billing events require `manage_billing`. A member without the permission simply never receives the event.
 
+Removal from a workspace also applies to open connections: every socket the removed member has open is disconnected. On reconnect each channel's join checks membership again, so channels for workspaces they still belong to come back and the removed workspace's are refused with `not_a_member` (code `4010`).
+
 ## Selecting events with intents
 
 By default an `org:<org_id>` subscriber receives every event the member is permitted to see. To narrow the stream, pass an `intents` array in the `phx_join` payload listing the event families you want:

docs/content/docs/api/reference/account-org.mdx

@@ -998,7 +998,7 @@ The organization is the workspace tenant. These endpoints handle creating and sw
 
 ### Get the workspace sending posture
 
-`GET /organizations/current/risk`
+`GET /organization/current/risk`
 
 Returns the workspace's abuse posture and, when it is restricted, why. Readable by any member: a workspace whose sending is limited should be able to see that it is.
 
@@ -1307,7 +1307,7 @@ The updated member object (same shape as a list-members entry).
 
 `DELETE /organization/members/:id`
 
-Removes a member from the organization.
+Removes a member from the organization. The member's sessions stop selecting it at once, so their next request that needs a workspace answers `400` until they switch to another one.
 
 Auth: Session only (not available to API keys). **Org permission** `manage_team`.
 

docs/content/docs/guides/team-roles.mdx

@@ -37,6 +37,8 @@ Each row shows email, role, and join date, with your own row tagged "you". Open
 
 To remove someone, hover (or tap) their row and click the remove icon. **You cannot remove the owner or yourself.** To leave a workspace you own, transfer ownership first.
 
+Removal takes effect at once, including for someone who is signed in right now. Their open dashboard loses the workspace on its next request and its live updates stop, and they are asked to pick another workspace they belong to. Nothing they created is removed with them: mailboxes, contacts and campaigns belong to the workspace.
+
 ## Roles
 
 Roles are workspace data. Every workspace starts with three seeded roles that are ordinary roles in every respect: rename, recolor, re-permission, or delete them, and add your own. **Owner is a membership status, not a role**, and there is exactly one per workspace.

internal/api/handler/organization.go

@@ -302,6 +302,8 @@ func (h *Handler) RemoveMember(c *gin.Context) {
 		errx.JSON(c, xerr)
 		return
 	}
+	// LeaveOrganization captures its own failures; the auth middleware's membership gate holds regardless.
+	_ = h.TokenService.LeaveOrganization(c.Request.Context(), memberUserID, *orgID)
 
 	h.auditOrg(c, models.AuditActionRemove, models.AuditEntityOrganizationMember, &memberUserID, nil, nil)
 

internal/api/middleware/apikey.go

@@ -182,8 +182,10 @@ func (h *Handler) validateJWT(c *gin.Context, token string) {
 	c.Set(UserIDKey, session.UserID.String())
 	c.Set(SessionKey, session)
 	c.Set(AccessTokenKey, token)
-	if session.CurrentOrganizationID != nil {
-		c.Set(OrganizationIDKey, *session.CurrentOrganizationID)
+	if xerr := h.setSessionOrganization(c, session); xerr != nil {
+		errx.JSON(c, xerr)
+		c.Abort()
+		return
 	}
 	c.Next()
 }
@@ -253,7 +255,7 @@ func (h *Handler) RequireAccess(orgPerm models.OrganizationPermission, apiPerm u
 				c.Abort()
 				return
 			}
-			has, xerr := h.OrganizationService.HasPermission(c.Request.Context(), *orgID, userID, orgPerm)
+			has, xerr := h.memberHasPermission(c, *orgID, userID, orgPerm)
 			if xerr != nil {
 				errx.JSON(c, xerr)
 				c.Abort()
@@ -309,7 +311,7 @@ func (h *Handler) RequireAnyAccess(apiPerm uint64, orgPerms ...models.Organizati
 				return
 			}
 			for _, p := range orgPerms {
-				has, xerr := h.OrganizationService.HasPermission(c.Request.Context(), *orgID, userID, p)
+				has, xerr := h.memberHasPermission(c, *orgID, userID, p)
 				if xerr != nil {
 					errx.JSON(c, xerr)
 					c.Abort()

internal/api/middleware/auth.go

@@ -14,6 +14,8 @@ const (
 	AccessTokenKey    = "access_token"
 	SessionKey        = "session"
 	OrganizationIDKey = "organization_id"
+	// SessionMemberKey holds the caller's membership in the session's workspace.
+	SessionMemberKey = "session_member"
 )
 
 func (h *Handler) AuthMiddleware() gin.HandlerFunc {
@@ -38,15 +40,50 @@ func (h *Handler) AuthMiddleware() gin.HandlerFunc {
 		c.Set(SessionKey, session)
 		c.Set(AccessTokenKey, token)
 
-		// Set organization context if available
-		if session.CurrentOrganizationID != nil {
-			c.Set(OrganizationIDKey, *session.CurrentOrganizationID)
+		if xerr := h.setSessionOrganization(c, session); xerr != nil {
+			errx.JSON(c, xerr)
+			c.Abort()
+			return
 		}
 
 		c.Next()
 	}
 }
 
+// setSessionOrganization puts the session's workspace in the request only
+// while the session's user is a member of it; otherwise the request has none.
+func (h *Handler) setSessionOrganization(c *gin.Context, session *models.Session) *errx.Error {
+	if session.CurrentOrganizationID == nil {
+		return nil
+	}
+	orgID := *session.CurrentOrganizationID
+	if h.OrganizationService == nil {
+		c.Set(OrganizationIDKey, orgID)
+		return nil
+	}
+	member, xerr := h.OrganizationService.GetMembership(c.Request.Context(), orgID, session.UserID)
+	if xerr != nil {
+		return xerr
+	}
+	if member == nil {
+		return nil
+	}
+	c.Set(OrganizationIDKey, orgID)
+	c.Set(SessionMemberKey, member)
+	return nil
+}
+
+// memberHasPermission answers from the membership resolved at authentication
+// when it is for orgID, and from the database otherwise.
+func (h *Handler) memberHasPermission(c *gin.Context, orgID, userID uuid.UUID, perm models.OrganizationPermission) (bool, *errx.Error) {
+	if v, ok := c.Get(SessionMemberKey); ok {
+		if m, ok := v.(*models.OrganizationMember); ok && m.OrganizationID == orgID && m.UserID == userID {
+			return m.HasPermission(perm), nil
+		}
+	}
+	return h.OrganizationService.HasPermission(c.Request.Context(), orgID, userID, perm)
+}
+
 func GetUserID(c *gin.Context) string {
 	return c.GetString(UserIDKey)
 }

internal/api/middleware/organization.go

@@ -192,7 +192,7 @@ func (h *Handler) RequirePermission(perm models.OrganizationPermission) gin.Hand
 			return
 		}
 
-		has, xerr := h.OrganizationService.HasPermission(c.Request.Context(), *orgID, userID, perm)
+		has, xerr := h.memberHasPermission(c, *orgID, userID, perm)
 		if xerr != nil {
 			errx.JSON(c, xerr)
 			c.Abort()

internal/api/middleware/session_org_test.go

@@ -0,0 +1,120 @@
+package middleware
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gin-gonic/gin"
+	"github.com/google/uuid"
+	"github.com/warmbly/warmbly/internal/app/organization"
+	"github.com/warmbly/warmbly/internal/app/token"
+	"github.com/warmbly/warmbly/internal/errx"
+	"github.com/warmbly/warmbly/internal/models"
+)
+
+type sessionTokens struct {
+	token.TokenService
+	session *models.Session
+}
+
+func (f sessionTokens) ValidateAccessToken(context.Context, string) (*models.Session, *errx.Error) {
+	return f.session, nil
+}
+
+type membershipOrgs struct {
+	organization.OrganizationService
+	members        map[uuid.UUID]*models.OrganizationMember
+	permissionHits int
+}
+
+func (f *membershipOrgs) GetMembership(_ context.Context, orgID, userID uuid.UUID) (*models.OrganizationMember, *errx.Error) {
+	m := f.members[orgID]
+	if m == nil || m.UserID != userID {
+		return nil, nil
+	}
+	return m, nil
+}
+
+func (f *membershipOrgs) HasPermission(_ context.Context, orgID, userID uuid.UUID, perm models.OrganizationPermission) (bool, *errx.Error) {
+	f.permissionHits++
+	m, _ := f.GetMembership(context.Background(), orgID, userID)
+	return m != nil && m.HasPermission(perm), nil
+}
+
+func (f *membershipOrgs) Get(_ context.Context, orgID uuid.UUID) (*models.Organization, *errx.Error) {
+	return &models.Organization{ID: orgID}, nil
+}
+
+func serveSession(t *testing.T, h *Handler, auth gin.HandlerFunc, gates ...gin.HandlerFunc) (int, *uuid.UUID) {
+	t.Helper()
+	var seen *uuid.UUID
+	r := gin.New()
+	r.Use(RequestIDMiddleware())
+	chain := append([]gin.HandlerFunc{auth}, gates...)
+	chain = append(chain, func(c *gin.Context) {
+		seen = GetOrganizationID(c)
+		c.Status(http.StatusOK)
+	})
+	r.GET("/x", chain...)
+
+	req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/x", nil)
+	req.Header.Set("Authorization", "Bearer t")
+	rec := httptest.NewRecorder()
+	r.ServeHTTP(rec, req)
+	return rec.Code, seen
+}
+
+func TestSessionOrganizationRequiresMembership(t *testing.T) {
+	userID, orgID := uuid.New(), uuid.New()
+	session := &models.Session{ID: uuid.New(), UserID: userID, CurrentOrganizationID: &orgID}
+
+	member := &models.OrganizationMember{OrganizationID: orgID, UserID: userID, Permissions: models.PermViewContacts}
+	for _, tt := range []struct {
+		name     string
+		members  map[uuid.UUID]*models.OrganizationMember
+		wantCode int
+		wantOrg  bool
+	}{
+		{"member keeps the workspace", map[uuid.UUID]*models.OrganizationMember{orgID: member}, http.StatusOK, true},
+		{"former member has none", map[uuid.UUID]*models.OrganizationMember{}, http.StatusBadRequest, false},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			orgs := &membershipOrgs{members: tt.members}
+			h := &Handler{TokenService: sessionTokens{session: session}, OrganizationService: orgs}
+
+			for name, auth := range map[string]gin.HandlerFunc{
+				"AuthMiddleware":         h.AuthMiddleware(),
+				"CombinedAuthMiddleware": h.CombinedAuthMiddleware(),
+			} {
+				code, seen := serveSession(t, h, auth, h.RequireOrganization())
+				if code != tt.wantCode {
+					t.Fatalf("%s: status = %d, want %d", name, code, tt.wantCode)
+				}
+				if (seen != nil) != tt.wantOrg {
+					t.Fatalf("%s: organization in request = %v, want present=%v", name, seen, tt.wantOrg)
+				}
+			}
+		})
+	}
+}
+
+func TestSessionMemberAnswersPermissionGates(t *testing.T) {
+	userID, orgID := uuid.New(), uuid.New()
+	session := &models.Session{ID: uuid.New(), UserID: userID, CurrentOrganizationID: &orgID}
+	orgs := &membershipOrgs{members: map[uuid.UUID]*models.OrganizationMember{
+		orgID: {OrganizationID: orgID, UserID: userID, Permissions: models.PermViewContacts},
+	}}
+	h := &Handler{TokenService: sessionTokens{session: session}, OrganizationService: orgs}
+
+	if code, _ := serveSession(t, h, h.AuthMiddleware(), h.RequirePermission(models.PermViewContacts)); code != http.StatusOK {
+		t.Fatalf("granted permission: status = %d, want 200", code)
+	}
+	if code, _ := serveSession(t, h, h.AuthMiddleware(), h.RequirePermission(models.PermManageTeam)); code != http.StatusForbidden {
+		t.Fatalf("missing permission: status = %d, want 403", code)
+	}
+	if orgs.permissionHits != 0 {
+		t.Fatalf("permission lookups = %d, want 0 (answered from the session's membership)", orgs.permissionHits)
+	}
+}

internal/app/token/gen.go

@@ -250,6 +250,22 @@ func (s *tokenService) SwitchOrganization(ctx context.Context, sessionID uuid.UU
 	return nil
 }
 
+// LeaveOrganization leaves the session with no workspace rather than moving it
+// to another, so a form left open cannot write into a workspace it was not for.
+func (s *tokenService) LeaveOrganization(ctx context.Context, userID, orgID uuid.UUID) *errx.Error {
+	cleared, xerr := s.tokenRepository.ClearOrganization(ctx, userID, orgID)
+	if xerr != nil {
+		return xerr
+	}
+	var failed *errx.Error
+	for _, id := range cleared {
+		if xerr := s.deleteSession(ctx, id); xerr != nil {
+			failed = xerr
+		}
+	}
+	return failed
+}
+
 // GetCurrentOrganization retrieves the current organization for a session
 func (s *tokenService) GetCurrentOrganization(ctx context.Context, sessionID uuid.UUID) (*uuid.UUID, *errx.Error) {
 	session, err := s.GetSession(ctx, sessionID)

internal/app/token/service.go

@@ -54,6 +54,9 @@ type TokenService interface {
 	// Organization switching
 	SwitchOrganization(ctx context.Context, sessionID uuid.UUID, orgID *uuid.UUID) *errx.Error
 	GetCurrentOrganization(ctx context.Context, sessionID uuid.UUID) (*uuid.UUID, *errx.Error)
+	// LeaveOrganization deselects an organization the user no longer belongs to
+	// on every one of their sessions, so the dashboard asks them to pick again.
+	LeaveOrganization(ctx context.Context, userID, orgID uuid.UUID) *errx.Error
 }
 
 type tokenService struct {

internal/repository/pg_token.go

@@ -30,6 +30,7 @@ type TokenRepository interface {
 	// Organization switching
 	UpdateCurrentOrganization(ctx context.Context, sessionID uuid.UUID, orgID *uuid.UUID) *errx.Error
 	DefaultOrganization(ctx context.Context, userID uuid.UUID) (*uuid.UUID, *errx.Error)
+	ClearOrganization(ctx context.Context, userID, orgID uuid.UUID) ([]uuid.UUID, *errx.Error)
 }
 
 type tokenRepository struct {
@@ -405,6 +406,39 @@ func (r *tokenRepository) UpdateCurrentOrganization(ctx context.Context, session
 	return nil
 }
 
+// ClearOrganization deselects orgID on every live session of userID and
+// returns the sessions it changed.
+func (r *tokenRepository) ClearOrganization(ctx context.Context, userID, orgID uuid.UUID) ([]uuid.UUID, *errx.Error) {
+	const query = `
+		UPDATE sessions
+		SET current_organization_id = NULL
+		WHERE user_id = $1 AND current_organization_id = $2 AND revoked_at IS NULL
+		RETURNING id
+	`
+	params := []any{userID, orgID}
+	rows, err := r.DB.Query(ctx, query, params...)
+	if err != nil {
+		db.CaptureError(err, query, params, "query")
+		return nil, errx.InternalError()
+	}
+	defer rows.Close()
+
+	var ids []uuid.UUID
+	for rows.Next() {
+		var id uuid.UUID
+		if err := rows.Scan(&id); err != nil {
+			db.CaptureError(err, query, params, "scan")
+			return nil, errx.InternalError()
+		}
+		ids = append(ids, id)
+	}
+	if err := rows.Err(); err != nil {
+		db.CaptureError(err, query, params, "rows")
+		return nil, errx.InternalError()
+	}
+	return ids, nil
+}
+
 // DefaultOrganization picks the workspace a new session starts in: the one the
 // user owns, else their earliest-joined membership. A session without one used
 // to reach org-scoped writes with no tenant, which is what let orgless rows be

realtime/lib/realtime/event_broadcaster.ex

@@ -15,6 +15,12 @@ defmodule Realtime.EventBroadcaster do
     user_id = event["user_id"]
     event_type = event["event_type"]
 
+    # A removed member loses every socket at once, whatever it joined; each
+    # channel's rejoin checks membership again, so only that org is lost.
+    if removed = removed_member(event) do
+      RealtimeWeb.Endpoint.broadcast("user_socket:#{removed}", "disconnect", %{})
+    end
+
     if present?(user_id) do
       Phoenix.PubSub.broadcast(Realtime.PubSub, "user:#{user_id}", {:pubsub_event, event})
     end
@@ -57,5 +63,17 @@ defmodule Realtime.EventBroadcaster do
     :ok
   end
 
+  @doc false
+  def removed_member(%{
+        "event_type" => "AUDIT_CREATED",
+        "entity_type" => "organization_member",
+        "action" => "remove",
+        "entity_id" => user_id
+      })
+      when is_binary(user_id) and user_id != "",
+      do: user_id
+
+  def removed_member(_event), do: nil
+
   defp present?(value), do: is_binary(value) and value != ""
 end

realtime/test/realtime/event_broadcaster_test.exs

@@ -0,0 +1,37 @@
+defmodule Realtime.EventBroadcasterTest do
+  use ExUnit.Case, async: true
+
+  alias Realtime.EventBroadcaster
+
+  @removed "7f1c2a4e-0000-4000-8000-000000000001"
+
+  defp removal do
+    %{
+      "event_type" => "AUDIT_CREATED",
+      "entity_type" => "organization_member",
+      "action" => "remove",
+      "entity_id" => @removed
+    }
+  end
+
+  test "a member removal names the removed user" do
+    assert EventBroadcaster.removed_member(removal()) == @removed
+  end
+
+  test "other audits name nobody" do
+    assert EventBroadcaster.removed_member(Map.put(removal(), "action", "update")) == nil
+    assert EventBroadcaster.removed_member(Map.put(removal(), "entity_type", "team")) == nil
+    assert EventBroadcaster.removed_member(Map.put(removal(), "entity_id", "")) == nil
+    assert EventBroadcaster.removed_member(%{"event_type" => "EMAIL_SENT"}) == nil
+  end
+
+  test "a removal disconnects the removed user's sockets" do
+    RealtimeWeb.Endpoint.subscribe("user_socket:#{@removed}")
+    EventBroadcaster.broadcast(removal())
+
+    assert_receive %Phoenix.Socket.Broadcast{
+      event: "disconnect",
+      topic: "user_socket:" <> @removed
+    }
+  end
+end