Skip to content

Commit b467685

Browse files
committed
fix(auth): fail closed when a KNOWN org's session policy cannot be read
Review catch, and the reviewer is right that the previous comment described behavior the code no longer had. `getSessionPolicy` now throws, so the create hook's catch covered two different failures and answered both the same way. They deserve opposite answers: - Org KNOWN — membership resolved, only the policy read failed. This user is definitely governed by an org, so the "would break sign-in for everyone" rationale simply does not apply. Fail closed. Minting a full-length session for a member whose policy could not be read is the exact outcome the feature exists to prevent, and the blast radius is org members during a window where the `organization` read fails while the `member` read just succeeded. - Org UNKNOWN — both membership reads failed, so a governed member is indistinguishable from the majority who belong to no org. Failing closed here would take authentication down for everyone to protect a policy most of them do not have. Allow, and let the update hook clamp retroactively. Throwing an APIError from this hook is the established pattern in this file — the blocked-email check directly above does the same.
1 parent 8b928f5 commit b467685

1 file changed

Lines changed: 30 additions & 18 deletions

File tree

apps/sim/lib/auth/auth.ts

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -697,30 +697,42 @@ export const auth = betterAuth({
697697
},
698698
}
699699
} catch (error) {
700-
// DELIBERATE TRADE, not an oversight. Reaching here means both the
701-
// direct membership read and the cached fallback failed, so the
702-
// governing org — and therefore the policy — is unknown. The
703-
// alternative, refusing the sign-in, would turn a `member` read
704-
// failure into a total authentication outage for EVERY user,
705-
// including the overwhelming majority who belong to no org and have
706-
// no policy to enforce. That is a worse outcome than a bounded
707-
// enforcement delay.
700+
// Two different failures land here, and they get opposite answers.
708701
//
709-
// The delay is bounded: this session takes the DB path at its next
710-
// cookie-cache expiry (24h) and the update hook clamps it then —
711-
// retroactively, since the max-lifetime bound is measured from
712-
// `createdAt`, so an over-long session expires immediately on that
713-
// refresh. Any admin action (policy save, sign-out-all) bumps the
702+
// Org KNOWN: membership resolved, so this user is definitely
703+
// governed by an org — only the policy read failed. Fail closed.
704+
// The blast radius is org members during a window where the
705+
// `organization` read fails but the `member` read just succeeded,
706+
// which is narrow, loud, and self-clearing. Minting a full-length
707+
// session for a member whose policy we could not read is the one
708+
// outcome this feature exists to prevent.
709+
if (membershipOrgId) {
710+
logger.error('Refusing session: org session policy could not be resolved', {
711+
error,
712+
userId: session.userId,
713+
organizationId: membershipOrgId,
714+
})
715+
throw new APIError('INTERNAL_SERVER_ERROR', {
716+
message: 'Could not verify your organization session policy. Please try again.',
717+
})
718+
}
719+
720+
// Org UNKNOWN: both the direct membership read and the cached
721+
// fallback failed, so we cannot tell a governed member from the
722+
// overwhelming majority of users who belong to no org at all.
723+
// Failing closed here would turn a `member` read failure into a
724+
// total authentication outage for everyone, to protect a policy
725+
// that most of them do not have. Allow, and let it self-correct:
726+
// the session takes the DB path at its next cookie-cache expiry
727+
// and the update hook clamps it retroactively (the max-lifetime
728+
// bound is measured from `createdAt`, so an over-long session
729+
// expires immediately on that refresh). Any admin action bumps the
714730
// security-policy version and closes the window at once.
715731
logger.error('Error clamping new session to org policy; session not clamped', {
716732
error,
717733
userId: session.userId,
718734
})
719-
return {
720-
data: membershipOrgId
721-
? { ...session, activeOrganizationId: membershipOrgId }
722-
: session,
723-
}
735+
return { data: session }
724736
}
725737
},
726738
},

0 commit comments

Comments
 (0)