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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions apps/backend/src/policies/bot-policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ describe("BotPolicy", () => {
})
const outsider = makeActor({
id: "00000000-0000-4000-8000-000000000403" as UserId,
organizationId: TEST_ORG_ID,
})

const layer = makePolicyLayer(
Expand All @@ -88,7 +87,7 @@ describe("BotPolicy", () => {
const adminDenied = await runWithActorEither(
BotPolicy.use((policy) => policy.canRead(BOT_ID)),
layer,
makeActor({ ...admin, organizationId: TEST_ORG_ID }),
admin,
)
const outsiderDenied = await runWithActorEither(
BotPolicy.use((policy) => policy.canRead(BOT_ID)),
Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/policies/policy-test-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export const makeActor = (overrides?: Partial<CurrentUser.Schema>): CurrentUser.
role: "member",
isOnboarded: true,
timezone: "UTC",
organizationId: TEST_ORG_ID,
settings: null,
...overrides,
})
Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/routes/api-v1/bot-auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ export const botPrincipal = (bot: {
email: `bot-${bot.name}@hazel.bot`,
isOnboarded: true,
timezone: null,
organizationId: null,
settings: null,
}),
botScopes: bot.scopes === null ? null : bot.scopes.filter(isApiScope),
Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/routes/http-success-encoding.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import { CurrentUser } from "@hazel/domain"
const sessionUser = (id: UserId) =>
new CurrentUser.Schema({
id,
organizationId: null,
role: "member",
email: "caller@example.com",
isOnboarded: true,
Expand Down
28 changes: 4 additions & 24 deletions apps/backend/src/routes/integrations.http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { Cause, Config, DateTime, Effect, Layer, Option, Schedule, Schema, Schem
import * as Duration from "effect/Duration"
import { HazelApi } from "../api"
import { Directory } from "../services/directory"
import { requireMember } from "./integration-resources.http"
import { OAuthProviderRegistry } from "../services/oauth"
import { callObject } from "../worker/objects/calls"
import { orgTarget } from "../worker/objects/kinds"
Expand Down Expand Up @@ -270,14 +271,7 @@ const handleGetOAuthUrl = Effect.fn("integrations.getOAuthUrl")(function* (
const { orgId, provider } = path
const level = query.level ?? "organization"

if (!currentUser.organizationId || currentUser.organizationId !== orgId) {
return yield* Effect.fail(
new UnauthorizedError({
message: "You are not authorized to access this organization",
detail: `organizationId=${orgId}`,
}),
)
}
yield* requireMember(orgId)

// Get the OAuth provider from registry
const registry = yield* OAuthProviderRegistry
Expand Down Expand Up @@ -913,14 +907,7 @@ const handleGetConnectionStatus = Effect.fn("integrations.getConnectionStatus")(
// A T3 Code link is a member's own, with its own routes (`./t3code.http.ts`).
if (provider === "t3code") return yield* Effect.fail(new UnsupportedProviderError({ provider }))

if (!currentUser.organizationId || currentUser.organizationId !== orgId) {
return yield* Effect.fail(
new UnauthorizedError({
message: "You are not authorized to access this organization",
detail: `organizationId=${orgId}`,
}),
)
}
yield* requireMember(orgId)

const connectionOption = Option.fromNullishOr(
yield* callObject(IntegrationConnectionCall, orgTarget(orgId), {
Expand Down Expand Up @@ -967,14 +954,7 @@ const handleDisconnect = Effect.fn("integrations.disconnect")(function* (
// A T3 Code link is a member's own, with its own routes (`./t3code.http.ts`).
if (provider === "t3code") return yield* Effect.fail(new UnsupportedProviderError({ provider }))

if (!currentUser.organizationId || currentUser.organizationId !== orgId) {
return yield* Effect.fail(
new UnauthorizedError({
message: "You are not authorized to access this organization",
detail: `organizationId=${orgId}`,
}),
)
}
yield* requireMember(orgId)

// Tokens, the connection and (for a user link) re-attribution, in the org's object.
const connection = yield* callObject(DisconnectIntegrationCall, orgTarget(orgId), {
Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/routes/object-routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ const invokeAs = (object: TestObject, objectId: string, tag: HazelRpcTag, payloa
principal: yield* encodeInvokePrincipal({
user: new CurrentUser.Schema({
id: userId,
organizationId: null,
role: "member",
email: `${userId}@example.com`,
isOnboarded: true,
Expand Down
14 changes: 3 additions & 11 deletions apps/backend/src/routes/t3code.http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
UpdateT3CodeSharingCall,
} from "../worker/objects/org/t3code"
import { validateBotToken } from "./bot-commands.http"
import { requireMember } from "./integration-resources.http"

/** The seeded T3 Code bot's directory user (`IntegrationBotService`); the only caller of `getT3CodeAccess`. */
export const T3CODE_BOT_EXTERNAL_ID = "internal-bot-t3code"
Expand Down Expand Up @@ -110,17 +111,8 @@

const redirectUriOf = (apiBaseUrl: string) => `${new URL(apiBaseUrl).origin}/integrations/t3code/callback`

const requireOrg = (orgId: OrganizationId) =>
Effect.gen(function* () {
const currentUser = yield* CurrentUser.Context
if (currentUser.organizationId !== orgId) {
return yield* new UnauthorizedError({
message: "You are not authorized to access this organization",
detail: `organizationId=${orgId}`,
})
}
return currentUser
})
/** The caller, once they are an active member of `orgId` (sessions carry no organization). */
const requireOrg = (orgId: OrganizationId) => requireMember(orgId).pipe(Effect.andThen(CurrentUser.Context))

Check warning on line 115 in apps/backend/src/routes/t3code.http.ts

View check run for this annotation

Maple Review Bot / Maple / review

tests: No test pins that `requireOrg` admits a member and refuses an outsider

The behavior this PR fixes — an org route answering a member whose session carries no organization — is exercised by no test. `t3code.http.test.ts` covers only `checkCallback` and `requireT3CodeBot`, so a return to an always-failing org check would pass CI again, as it did before.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

No test pins that requireOrg admits a member and refuses an outsider

F1 · Warning · tests

The behavior this PR fixes — an org route answering a member whose session carries no organization — is exercised by no test. t3code.http.test.ts covers only checkCallback and requireT3CodeBot, so a return to an always-failing org check would pass CI again, as it did before.

Export `requireOrg` and test it against a seeded directory the way `cross-org.test.ts` does for `requireMember`: an active `membership_index` row passes, an outsider fails with `UnauthorizedError`.
🤖 Prompt to fix with an AI agent
In `apps/backend/src/routes/t3code.http.ts:115`: No test pins that `requireOrg` admits a member and refuses an outsider.

The behavior this PR fixes — an org route answering a member whose session carries no organization — is exercised by no test. `t3code.http.test.ts` covers only `checkCallback` and `requireT3CodeBot`, so a return to an always-failing org check would pass CI again, as it did before.

Suggested fix: Export `requireOrg` and test it against a seeded directory the way `cross-org.test.ts` does for `requireMember`: an active `membership_index` row passes, an outsider fails with `UnauthorizedError`.

Verify the problem exists at that location before changing it, and keep the fix to those lines.


const toDateTime = (value: Date | DateTime.Utc) =>
DateTime.isDateTime(value) ? value : DateTime.fromDateUnsafe(value)
Expand Down
12 changes: 4 additions & 8 deletions apps/backend/src/rpc/handlers/bots.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,9 @@ const generateBotToken = Effect.promise(async () => {
return { token, tokenHash: Hex.encode(new Uint8Array(digest)) }
})

/** The org a bot call acts in: the routing hint, else the caller's active org. */
const activeOrganization = (hint: OrganizationId | undefined) =>
/** The org a bot call acts in: the routing hint. */
const activeOrganization = (organizationId: OrganizationId | undefined) =>
Effect.gen(function* () {
const user = yield* CurrentUser.Context
const organizationId = hint ?? user.organizationId ?? undefined
// The router sends calls with an org to its OrgObject; anywhere else there is none.
const object = yield* currentObject
if (organizationId === undefined || Option.isNone(object) || object.value.kind !== "org") {
Expand Down Expand Up @@ -317,21 +315,19 @@ export const BotRpcLive = BotRpcs.toLayer(

"bot.listPublic": ({ search, organizationId }) =>
Effect.gen(function* () {
const user = yield* CurrentUser.Context
const botRepo = yield* BotRepo
const userRepo = yield* UserRepo
const directory = yield* Directory
const publicBots = yield* botRepo.findPublic(search)

// Installed here = the bot user is an active member (`membership_index`).
const orgId = organizationId ?? user.organizationId ?? undefined
const installed = new Set<string>()
if (orgId !== undefined && publicBots.length > 0) {
if (organizationId !== undefined && publicBots.length > 0) {
const rows = yield* directory.db.run(
SQLite.from(schema.membershipIndexTable)
.select("userId")
.where(($) => [
$.organizationId.eq(orgId),
$.organizationId.eq(organizationId),
$.deletedAt.isNull(),
$.userId.in_(...publicBots.map((bot) => bot.userId)),
]),
Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/rpc/middleware/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ const makeCurrentUser = (
role: "member",
isOnboarded: true,
timezone: "UTC",
organizationId: null,
settings: null,
...overrides,
})
Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/rpc/middleware/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ export const AuthMiddlewareLive = Layer.effect(
role: "member",
isOnboarded: true,
timezone: user.timezone,
organizationId: null,
settings: user.settings,
}

Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/rpc/per-request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ const requestScheduler = (label: string): Scheduler.Scheduler => ({

const user = new CurrentUser.Schema({
id: "00000000-0000-4000-8000-000000000001" as UserId,
organizationId: null,
role: "member",
email: "me@example.com",
isOnboarded: true,
Expand Down
10 changes: 4 additions & 6 deletions apps/backend/src/rpc/routing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,11 @@ const orgHint = (method: string) => (payload: { readonly organizationId?: Organi
Effect.flatMap(hint(method, "organizationId", payload.organizationId), org)

/**
* The org a bot call acts in: the payload's `organizationId`, else the caller's active org; with
* neither, the directory path (which answers `ActiveOrganizationRequiredError`, or `[]`).
* The org a bot call acts in: the payload's `organizationId`; without one, the directory path
* (which answers `ActiveOrganizationRequiredError`, or `[]`).
*/
const activeOrg = (payload: { readonly organizationId?: OrganizationId | undefined }, current: User) => {
const organizationId = payload.organizationId ?? current.organizationId ?? undefined
return organizationId === undefined ? directory() : org(organizationId)
}
const activeOrg = (payload: { readonly organizationId?: OrganizationId | undefined }) =>
payload.organizationId === undefined ? directory() : org(payload.organizationId)

const byOrg = (payload: { readonly organizationId: OrganizationId }) => org(payload.organizationId)
const byChannel = (payload: { readonly channelId: ChannelId }) => channel(payload.channelId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ const DISCORD_CHANNEL = "222222222222222222" as ExternalChannelId
const userOf = (id: string) =>
new CurrentUser.Schema({
id: id as UserId,
organizationId: null,
role: "member",
email: `${id}@example.com`,
isOnboarded: true,
Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/services/hazel-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ export class HazelSession extends Context.Service<HazelSession>()("@hazel/backen
new CurrentUser.Schema({
id: user.id,
role: "member",
organizationId: undefined,
avatarUrl: user.avatarUrl ?? undefined,
firstName: user.firstName,
lastName: user.lastName,
Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/services/invitations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ const actor = (userId: UserId) =>
new CurrentUser.Schema({
id: userId,
role: "member",
organizationId: undefined,
avatarUrl: undefined,
firstName: "Test",
lastName: "User",
Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/services/oauth-bearer-auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ export class OAuthBearerAuth extends Context.Service<OAuthBearerAuth>()("@hazel/
currentUser: new CurrentUser.Schema({
id: user.id,
role: "member",
organizationId: undefined,
avatarUrl: user.avatarUrl ?? undefined,
firstName: user.firstName,
lastName: user.lastName,
Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/worker/live/live.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ const run = <A, E>(effect: Effect.Effect<A, E, Scope.Scope>) =>
const principalOf = (id: UserId): InvokePrincipal => ({
user: new CurrentUser.Schema({
id,
organizationId: null,
role: "member",
email: `${id}@example.com`,
isOnboarded: true,
Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/worker/objects/channel-messages.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const uuid = () => crypto.randomUUID()
const userOf = (id: string) =>
new CurrentUser.Schema({
id: id as UserId,
organizationId: null,
role: "member",
email: `${id}@example.com`,
isOnboarded: true,
Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/worker/objects/connect-objects.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ const uuid = () => crypto.randomUUID()
const userOf = (id: string) =>
new CurrentUser.Schema({
id: id as UserId,
organizationId: null,
role: "member",
email: `${id}@example.com`,
isOnboarded: true,
Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/worker/objects/cross-org.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ const uuid = () => crypto.randomUUID()
const userOf = (id: string) =>
new CurrentUser.Schema({
id: id as UserId,
organizationId: null,
role: "member",
email: `${id}@example.com`,
isOnboarded: true,
Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/worker/objects/directory-path.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ const uuid = () => crypto.randomUUID()
const userOf = (id: string, email = `${id}@example.com`) =>
new CurrentUser.Schema({
id: id as UserId,
organizationId: null,
role: "member",
email,
isOnboarded: true,
Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/worker/objects/html-renders.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ const uuid = () => crypto.randomUUID()
const userOf = (id: string) =>
new CurrentUser.Schema({
id: id as UserId,
organizationId: null,
role: "member",
email: `${id}@example.com`,
isOnboarded: true,
Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/worker/objects/message-search.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ const uuid = () => crypto.randomUUID()
const userOf = (id: string) =>
new CurrentUser.Schema({
id: id as UserId,
organizationId: null,
role: "member",
email: `${id}@example.com`,
isOnboarded: true,
Expand Down
1 change: 0 additions & 1 deletion apps/backend/src/worker/objects/objects.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ const uuid = () => crypto.randomUUID()
const userOf = (id: string) =>
new CurrentUser.Schema({
id: id as UserId,
organizationId: null,
role: "member",
email: `${id}@example.com`,
isOnboarded: true,
Expand Down
2 changes: 0 additions & 2 deletions apps/backend/src/worker/objects/org-objects.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ const uuid = () => crypto.randomUUID()
const userOf = (id: string) =>
new CurrentUser.Schema({
id: id as UserId,
organizationId: null,
role: "member",
email: `${id}@example.com`,
isOnboarded: true,
Expand Down Expand Up @@ -578,7 +577,6 @@ describe("OrgObject: channels", () => {
unnamed,
new CurrentUser.Schema({
id: world.owner,
organizationId: null,
role: "member",
email: "owner@example.com",
isOnboarded: true,
Expand Down
11 changes: 4 additions & 7 deletions apps/backend/src/worker/objects/org/organization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* cascade of an organization delete to its members' `membership_index` and `user_organizations`.
*/
import { Database, SQLite, schema } from "@hazel/db"
import { CurrentUser } from "@hazel/domain"
import { type OrganizationId, OrganizationId as OrganizationIdSchema } from "@hazel/schema"
import { Effect, Option } from "effect"
import { currentObject } from "../../../lib/current-object"
Expand Down Expand Up @@ -62,13 +61,11 @@ export const publishOrgRemoval = Effect.fn("OrgObject.publishOrgRemoval")(functi

/**
* The organization a `listByOrganization`-style call is about: the `OrgObject` it landed in (the
* api Worker routed it by its `organizationId` hint), else the session's active organization.
* api Worker routed it by its `organizationId` hint); none elsewhere.
*/
export const requestOrganizationId = Effect.gen(function* () {
const object = yield* currentObject
if (Option.isSome(object) && object.value.kind === "org") {
return Option.some(OrganizationIdSchema.make(object.value.id))
}
const user = yield* CurrentUser.Context
return Option.fromNullishOr(user.organizationId)
return Option.isSome(object) && object.value.kind === "org"
? Option.some(OrganizationIdSchema.make(object.value.id))
: Option.none<OrganizationId>()
})
1 change: 1 addition & 0 deletions apps/web-foldkit/public/images/integrations/t3code.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 0 additions & 1 deletion apps/web-foldkit/src/app/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ const fetchCurrentUser = Effect.gen(function* () {
email: user.email,
avatarUrl: user.avatarUrl ?? null,
isOnboarded: user.isOnboarded,
organizationId: user.organizationId ?? null,
},
})
}).pipe(catchCommon((reason) => Message.FailedFetchCurrentUser({ reason })))
Expand Down
4 changes: 2 additions & 2 deletions apps/web-foldkit/src/core/lib/integrations/__data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,12 +226,12 @@ export const integrations: Integration[] = [
},
{
id: "t3code",
logoSrc: "/images/integrations/t3code.svg",
name: "T3 Code",
description: "Drive T3 Code threads on your own machine from Hazel.",
fullDescription:
"Link your T3 Code environment to start and steer its threads with /t3 from any conversation. Each member links their own machine and chooses who else in the organization may drive it; every action is posted in the Hazel thread.",
logoDomain: "t3.chat",
logoType: "icon",
logoDomain: "t3.codes",
brandColor: "#1C1C1E",
category: "developer-tools",
connectionType: "environment",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export const shared: Shared = {
email: "ada@hazel.sh",
avatarUrl: null,
isOnboarded: true,
organizationId,
},
organization: { id: organizationId, name: "Hazel", slug: "hazel", logoUrl: null },
member: null,
Expand Down
Loading
Loading