diff --git a/docs/content/docs/api/endpoints.mdx b/docs/content/docs/api/endpoints.mdx index f986b2da5..fb0c6b657 100644 --- a/docs/content/docs/api/endpoints.mdx +++ b/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). diff --git a/docs/content/docs/api/error-codes.mdx b/docs/content/docs/api/error-codes.mdx index 12170f8f4..4ab5ccee3 100644 --- a/docs/content/docs/api/error-codes.mdx +++ b/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 | diff --git a/docs/content/docs/api/realtime.mdx b/docs/content/docs/api/realtime.mdx index 736da6f03..430078837 100644 --- a/docs/content/docs/api/realtime.mdx +++ b/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:` 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: diff --git a/docs/content/docs/api/reference/account-org.mdx b/docs/content/docs/api/reference/account-org.mdx index e0d41a9e5..7352f8d30 100644 --- a/docs/content/docs/api/reference/account-org.mdx +++ b/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`. diff --git a/docs/content/docs/guides/team-roles.mdx b/docs/content/docs/guides/team-roles.mdx index 366dee298..47a10d15e 100644 --- a/docs/content/docs/guides/team-roles.mdx +++ b/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. diff --git a/internal/api/handler/organization.go b/internal/api/handler/organization.go index f7b43cf48..b1f82f33c 100644 --- a/internal/api/handler/organization.go +++ b/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) diff --git a/internal/api/middleware/apikey.go b/internal/api/middleware/apikey.go index 8069c6b24..d88cf295c 100644 --- a/internal/api/middleware/apikey.go +++ b/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() diff --git a/internal/api/middleware/auth.go b/internal/api/middleware/auth.go index 78d5ff038..0b48fd612 100644 --- a/internal/api/middleware/auth.go +++ b/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) } diff --git a/internal/api/middleware/organization.go b/internal/api/middleware/organization.go index 52cafdfee..8d3b93dbf 100644 --- a/internal/api/middleware/organization.go +++ b/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() diff --git a/internal/api/middleware/session_org_test.go b/internal/api/middleware/session_org_test.go new file mode 100644 index 000000000..3fd5736a7 --- /dev/null +++ b/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) + } +} diff --git a/internal/app/token/gen.go b/internal/app/token/gen.go index 84bd51b5c..510fa9c72 100644 --- a/internal/app/token/gen.go +++ b/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) diff --git a/internal/app/token/service.go b/internal/app/token/service.go index d3966bd21..ad1c2975d 100644 --- a/internal/app/token/service.go +++ b/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 { diff --git a/internal/repository/pg_token.go b/internal/repository/pg_token.go index cdec39664..c01dde551 100644 --- a/internal/repository/pg_token.go +++ b/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 diff --git a/realtime/lib/realtime/event_broadcaster.ex b/realtime/lib/realtime/event_broadcaster.ex index 0b70017ef..18d6bb0e4 100644 --- a/realtime/lib/realtime/event_broadcaster.ex +++ b/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 diff --git a/realtime/test/realtime/event_broadcaster_test.exs b/realtime/test/realtime/event_broadcaster_test.exs new file mode 100644 index 000000000..062c43abb --- /dev/null +++ b/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