feat(mcp-oauth): let an account with no company connect and sign up from the OAuth popup (#1814 PR 1) (#1855)

* feat(mcp-oauth): let an account with no company connect and sign up from the OAuth popup

Identity unlock for agent-first onboarding (#1814, shape B+). A person
with no Accounted account can now connect from an MCP client, create the
account inside the Connect popup and finish the OAuth dance.

- authorize/token no longer require a company: consent renders a
  companyless variant and the key is minted with company_id NULL.
- validateApiKey returns companyId string|null and binds an unbound key
  to the user's first company on the first validation after it exists.
- MCP server: company-dependent tools and data resources answer with a
  structured NO_COMPANY_YET error; the company-independent tools still
  run; telemetry skips when there is no company scope.
- /api/events fails closed instead of throwing for an unbound key.
- authorize forces TOTP enrollment (not just verification) for password
  accounts with no factor, since the middleware skips enrollment for
  zero-company users; BankID-linked accounts stay exempt.
- /login forwards next to /register; register, GoogleAuthButton and
  /auth/callback carry it back to the consent page (callback honours
  only /api/mcp-oauth/authorize, via safeReturnTo); /mfa/enroll
  hard-navigates to /api/* destinations like /mfa/verify.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018wCdzRTatKiDByKB8hCNT6

* refactor(company): move getActiveCompanyId out of the next/headers module

lib/auth/api-keys.ts needs the resolver for unbound-key binding, but
lib/company/context.ts imports next/headers for the legacy company cookie
and Turbopack refuses that import on some of api-keys' import paths (the
preview build failed). The resolver and CompanyContextError now live in
lib/company/active-company.ts; context.ts re-exports them so every caller
and test mock is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018wCdzRTatKiDByKB8hCNT6

* fix(mcp-oauth): fail closed on a failed assurance lookup; enroll Back aborts instead of looping

Review findings on #1855: requireAal2 let consent through at AAL1 when
getAuthenticatorAssuranceLevel() returned nothing and a verified factor
existed. Only a positive AAL2 answer passes now; a failed lookup and the
inconsistent verified-factor-at-AAL1 case both step up to /mfa/verify.
Back on /mfa/enroll with the consent page as returnTo went straight back
into the redirect loop; it now aborts to the app.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018wCdzRTatKiDByKB8hCNT6

---------

Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
2026-08-25 11:25:42 +02:00
committed by GitHub
co-authored by Claude Fable 5 Jakob Wennberg
parent 2a33291c18
commit a717f03898
21 changed files with 858 additions and 247 deletions
+4
View File
@@ -1202,3 +1202,7 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
[2026-08-24] fiscal_periods.previous_period_id is adjacency-only: findNextPeriod ignores a chained period that does not start the day after the current one, and SIE import only wires predecessor/successor links between date-adjacent periods (before: nearest period across any gap). A non-adjacent link is what sent a company's opening balances two years forward (feedback seq 249297); 40 such links exist on prod across 39 companies and are neutralized by the read-side guard, not repaired in this change. A gap in the chain means a missing räkenskapsår (BFL 3 kap), which reports should show as missing rather than bridge silently.
[2026-08-24] Pending-operation authorization refusals (401/403 from an executor) release the claim back to 'pending' instead of consuming the op as 'rejected': the refusal happens before any side-effect and reflects the credential, not the booking, so the same op must survive for an authorized approver (/pending UI or a scoped key). Every CommitResult now carries operation_status so agents stop inferring "consumed" from status 'failed'. Deterministic content errors (400) still consume the op: re-staging is the only fix for those.
[2026-08-24] tools/call rejects unknown top-level parameters (VALIDATION_ERROR naming the valid keys) instead of ignoring them: hosts do not reliably enforce inputSchema, and a misspelled key silently widened gnubok_query_journal to the whole journal (feedback seq 261545). company_id stays tolerated on every tool because the routing layer owns it. Chose server-side enforcement over per-tool presence guards: every schema already declares additionalProperties:false, so the contract exists, it just was not enforced.
[2026-08-24] Agent-first onboarding (issue #1814) ships as shape B+ (signup inside the MCP OAuth popup, lazy auth, setup tools token-agnostic) and NOT as pre-identity provisional tenants first: bank and Skatteverket connects need a human browser anyway, so the claim-link shape buys little for a lot of TTL/abuse/RLS work; it stays a later auth swap.
[2026-08-24] Keys minted from the OAuth popup before the first company exists get company_id NULL and are bound lazily in validateApiKey (first validation after a company exists) instead of at company creation: creation happens in a Server Action that knows nothing about keys, and one chokepoint covers every creation path.
[2026-08-24] /api/mcp-oauth/authorize now forces TOTP enrollment (not just verification) for password accounts with no factor: the middleware skips enrollment for zero-company users, so a popup signup would otherwise mint an MFA-exempt key for an account with no second factor. BankID-linked accounts stay exempt.
[2026-08-24] /auth/callback honours next only when it targets /api/mcp-oauth/authorize (via safeReturnTo): consent handles the zero-company state, an arbitrary deep link would not.
@@ -1,5 +1,6 @@
import { describe, it, expect, vi, beforeEach } from 'vitest'
import { NextRequest } from 'next/server'
import { createServerClient } from '@supabase/ssr'
const verifyOtp = vi.fn()
const exchangeCodeForSession = vi.fn()
@@ -151,3 +152,90 @@ describe('GET /auth/callback: admin invite flow (type=invite)', () => {
expect(response.headers.get('set-cookie') ?? '').not.toContain('gnubok-invite-token')
})
})
describe('GET /auth/callback: resuming an MCP OAuth consent flow (issue #1814)', () => {
// A signup that started from an MCP client's Connect popup confirms its
// e-mail (or completes Google OAuth) here. The consent page handles the
// zero-company state itself, so it is the one `next` this callback honours
// for a fresh session; anything else still lands on the dashboard.
const CONSENT = '/api/mcp-oauth/authorize?response_type=code&state=xyz'
function clientWithTeamMembership() {
const chain: Record<string, ReturnType<typeof vi.fn>> = {
select: vi.fn(() => chain),
eq: vi.fn(() => chain),
limit: vi.fn(() => chain),
maybeSingle: vi.fn().mockResolvedValue({ data: { team_id: 'team-1' }, error: null }),
}
return {
auth: {
verifyOtp,
exchangeCodeForSession,
getUser: vi.fn().mockResolvedValue({ data: { user: { id: 'user-1' } } }),
mfa: {
getAuthenticatorAssuranceLevel: vi.fn().mockResolvedValue({ data: null }),
listFactors: vi.fn().mockResolvedValue({ data: null }),
},
},
from: vi.fn(() => chain),
rpc: vi.fn(),
}
}
beforeEach(() => {
vi.clearAllMocks()
vi.mocked(createServerClient).mockImplementation(() => clientWithTeamMembership() as never)
})
it('sends a confirmed signup back to the consent page when next targets it', async () => {
verifyOtp.mockResolvedValue({ error: null })
const request = new NextRequest(
`http://localhost:3000/auth/callback?token_hash=abc&type=signup&next=${encodeURIComponent(CONSENT)}`
)
const response = await GET(request)
expect(response.status).toBe(307)
expect(response.headers.get('location')).toBe(`http://localhost:3000${CONSENT}`)
})
it('carries the consent destination through the MFA verify step', async () => {
verifyOtp.mockResolvedValue({ error: null })
const client = clientWithTeamMembership()
client.auth.mfa.getAuthenticatorAssuranceLevel.mockResolvedValue({
data: { currentLevel: 'aal1', nextLevel: 'aal2' },
})
vi.mocked(createServerClient).mockImplementation(() => client as never)
const request = new NextRequest(
`http://localhost:3000/auth/callback?token_hash=abc&type=signup&next=${encodeURIComponent(CONSENT)}`
)
const response = await GET(request)
const location = new URL(response.headers.get('location')!)
expect(location.pathname).toBe('/mfa/verify')
expect(location.searchParams.get('returnTo')).toBe(CONSENT)
})
it('still lands on the dashboard for any other next', async () => {
verifyOtp.mockResolvedValue({ error: null })
const request = new NextRequest(
'http://localhost:3000/auth/callback?token_hash=abc&type=signup&next=%2Fsettings'
)
const response = await GET(request)
expect(response.headers.get('location')).toBe('http://localhost:3000/')
})
it('ignores an off-origin next that merely contains the consent path', async () => {
verifyOtp.mockResolvedValue({ error: null })
const request = new NextRequest(
`http://localhost:3000/auth/callback?token_hash=abc&type=signup&next=${encodeURIComponent('https://evil.example' + CONSENT)}`
)
const response = await GET(request)
expect(response.headers.get('location')).toBe('http://localhost:3000/')
})
})
+23 -3
View File
@@ -2,6 +2,21 @@ import { createServerClient } from '@supabase/ssr'
import { type NextRequest, NextResponse } from 'next/server'
import { hashInviteToken } from '@/lib/auth/invite-tokens'
import { INVITE_COOKIE_NAME } from '@/lib/auth/consume-invite-cookie'
import { safeReturnTo } from '@/lib/auth/safe-return-to'
/**
* The one `next` destination this callback honours for a fresh session: the
* MCP OAuth consent page. A signup that started from an MCP client's Connect
* popup (issue #1814) confirms its e-mail or completes Google OAuth here, and
* has to land back on consent instead of the dashboard. Consent handles the
* zero-company state itself, which is why this is safe where an arbitrary
* deep link would not be (a brand-new account has no membership to spend a
* deep link on). Same-origin only, via safeReturnTo.
*/
function oauthResumePath(next: string): string | null {
const safe = safeReturnTo(next, '/')
return safe.startsWith('/api/mcp-oauth/authorize?') ? safe : null
}
export async function GET(request: NextRequest) {
const { searchParams, origin } = new URL(request.url)
@@ -9,6 +24,7 @@ export async function GET(request: NextRequest) {
const token_hash = searchParams.get('token_hash')
const type = searchParams.get('type')
const next = searchParams.get('next') ?? '/'
const resumeOAuth = oauthResumePath(next)
// Collect cookies that Supabase sets during auth so we can
// explicitly forward them on the redirect response.
@@ -101,7 +117,9 @@ export async function GET(request: NextRequest) {
// Check MFA status: redirect to verify if factor is enrolled but session is AAL1
const { data: aal } = await supabase.auth.mfa.getAuthenticatorAssuranceLevel()
if (aal?.nextLevel === 'aal2' && aal?.currentLevel === 'aal1') {
const response = NextResponse.redirect(new URL('/mfa/verify', origin))
const verifyUrl = new URL('/mfa/verify', origin)
if (resumeOAuth) verifyUrl.searchParams.set('returnTo', resumeOAuth)
const response = NextResponse.redirect(verifyUrl)
for (const { name, value, options } of pendingCookies) {
response.cookies.set({ name, value, ...options })
}
@@ -220,8 +238,10 @@ export async function GET(request: NextRequest) {
}
}
// Always redirect to dashboard: it handles zero-company and incomplete states
redirectPath = '/'
// Redirect to the dashboard (it handles zero-company and incomplete
// states), unless the session was created to resume an MCP OAuth
// consent flow: that page handles the zero-company state too.
redirectPath = resumeOAuth ?? '/'
}
// Create redirect and explicitly set auth cookies on the response
+7 -2
View File
@@ -93,6 +93,10 @@ export function LoginClient({ initialMethod }: { initialMethod: LoginMethod | nu
// (/login?next=/api/mcp-oauth/authorize?...). Sanitized to a same-origin
// relative path; '/' means no explicit destination.
const nextPath = safeReturnTo(searchParams.get('next'), '/')
// A visitor who arrives here from the MCP consent page and has no account
// yet must be able to sign up without losing that destination (issue
// #1814). The register page re-sanitises it through safeReturnTo.
const registerHref = nextPath === '/' ? '/register' : `/register?next=${encodeURIComponent(nextPath)}`
const supabase = createClient()
const bankIdEnabled = isBankIdEnabled()
const googleAuthEnabled = isGoogleAuthEnabled()
@@ -548,7 +552,7 @@ export function LoginClient({ initialMethod }: { initialMethod: LoginMethod | nu
<p className="mt-1 text-muted-foreground">{tAuth('bankid_no_account_body')}</p>
<p className="mt-1">
<Link
href="/register"
href={registerHref}
className="text-muted-foreground underline underline-offset-2 hover:text-foreground transition-colors"
>
{tAuth('bankid_no_account_create')}
@@ -696,6 +700,7 @@ export function LoginClient({ initialMethod }: { initialMethod: LoginMethod | nu
{googleAuthEnabled && (
<GoogleAuthButton
compact
next={nextPath}
onError={(message) => setFormError({ kind: 'oauth', message })}
/>
)}
@@ -718,7 +723,7 @@ export function LoginClient({ initialMethod }: { initialMethod: LoginMethod | nu
<p className="mt-6 text-center text-[13px] text-muted-foreground">
{tAuth('login_new_here')}{' '}
<Link
href="/register"
href={registerHref}
className="font-medium text-foreground underline underline-offset-2 hover:opacity-80 transition-opacity"
>
{tAuth('no_account')}
+20 -4
View File
@@ -37,6 +37,23 @@ function MfaEnrollContent() {
const returnTo = safeReturnTo(searchParams.get('returnTo'), '/')
// Route-handler destinations (the MCP OAuth consent page sends new
// password accounts here with returnTo=/api/mcp-oauth/authorize…) return
// raw HTML the client router cannot render: hard-navigate, like /mfa/verify.
const leave = () => {
if (returnTo.startsWith('/api/')) {
window.location.assign(returnTo)
return
}
router.push(returnTo)
router.refresh()
}
// Back must not bounce into the consent page: with no factor enrolled it
// redirects straight back here. Abort the connect flow to the app instead.
const abort = () => {
router.push(returnTo.startsWith('/api/') ? '/' : returnTo)
}
// UX defense: middleware already blocks this route for BankID-only users
// without a password, but a stale tab might land here too. Bounce them to
// the set-password flow before they enroll a factor they cannot later
@@ -152,8 +169,7 @@ function MfaEnrollContent() {
description: 'Ditt konto är nu skyddat med 2FA.',
})
router.push(returnTo)
router.refresh()
leave()
} catch {
toast({
title: 'Verifiering misslyckades',
@@ -217,7 +233,7 @@ function MfaEnrollContent() {
<Button
variant="ghost"
className="w-full mt-4 text-muted-foreground"
onClick={() => router.push(returnTo)}
onClick={abort}
>
<ArrowLeft className="mr-2 h-4 w-4" />
Tillbaka
@@ -316,7 +332,7 @@ function MfaEnrollContent() {
<Button
variant="ghost"
className="w-full mt-4 text-muted-foreground"
onClick={() => router.push(returnTo)}
onClick={abort}
>
<ArrowLeft className="mr-2 h-4 w-4" />
Tillbaka
@@ -127,8 +127,13 @@ describe('a signup that did not come from an invitation', () => {
})
describe('the duplicate-email screen', () => {
it('sends the user to plain /login', () => {
expect(duplicateScreen()).toContain('<Link href="/login">')
it('sends the user to /login, carrying only the sanitized destination', () => {
// loginHref is /login, or /login?next=… when the signup started from the
// MCP consent page (issue #1814). Nothing else may ride on that link.
expect(duplicateScreen()).toContain('<Link href={loginHref}>')
expect(CODE).toContain(
"const loginHref = nextPath === '/' ? '/login' : `/login?next=${encodeURIComponent(nextPath)}`",
)
})
it('does not put the address in the URL of a page that never reads it', () => {
@@ -142,12 +147,22 @@ describe('the duplicate-email screen', () => {
})
describe('the post-auth destination parameter', () => {
it('does not read `next`', () => {
// Nothing links to /register with one: bounceToAuth (lib/supabase/
// middleware.ts) targets /login and the two MFA pages, and
// app/invite/[token]/page.tsx sends `?invite=`. An unread parameter cannot
// redirect anyone.
expect(CODE).not.toContain("searchParams.get('next')")
it('reads `next` exactly once, and only through safeReturnTo', () => {
// /login forwards `next` here for a visitor who arrived from the MCP
// consent page without an account (issue #1814). The raw value must never
// be consumed anywhere else on the page: every later use goes through
// nextPath, which safeReturnTo has already vetted.
expect(CODE.match(/searchParams\.get\('next'\)/g)).toHaveLength(1)
expect(CODE).toContain("const nextPath = safeReturnTo(searchParams.get('next'), '/')")
})
it('only ever navigates to the vetted destination, never a raw parameter', () => {
// Every hard navigation on the page targets nextPath or '/'.
const targets = CODE.match(/window\.location\.(?:assign\([^)]*\)|href = [^\n]+)/g) ?? []
expect(targets.length).toBeGreaterThan(0)
for (const target of targets) {
expect(target).toMatch(/^window\.location\.(?:assign\(nextPath\)|href = (?:nextPath|'\/'))$/)
}
})
it('would have to sanitize a destination through safeReturnTo if one is ever added', () => {
+35 -16
View File
@@ -28,6 +28,7 @@ import { GoogleAuthButton } from '@/components/auth/GoogleAuthButton'
import { isGoogleAuthEnabled } from '@/lib/auth/google-oauth'
import { classifyAuthError, type AuthErrorKind } from '@/lib/auth/classify-auth-error'
import { persistLoginMethodHint, type LoginMethod } from '@/lib/auth/login-method'
import { safeReturnTo } from '@/lib/auth/safe-return-to'
import { cn } from '@/lib/utils'
const branding = getBranding()
@@ -46,18 +47,23 @@ export default function RegisterPage() {
}
function RegisterPageContent() {
// `invite` is the only query parameter this page reads. It deliberately does
// NOT read `next`: nothing links here with one (bounceToAuth in
// lib/supabase/middleware.ts targets /login and the two MFA pages only, and
// app/invite/[token]/page.tsx sends `?invite=`), the already-signed-in case
// is handled in the middleware behind safeReturnTo, and neither signup path
// has a destination to spend it on: the password path leaves through the
// confirmation mail and /auth/callback, and the BankID path must land a
// brand-new account on '/' or /select-company rather than a deep link it has
// no membership for. If a destination is ever wanted here it MUST go through
// `invite` and `next` are the only query parameters this page reads.
//
// `next` is the post-signup destination /login forwards when a visitor with
// no account arrives from the MCP OAuth consent page
// (/login?next=/api/mcp-oauth/authorize?…, issue #1814). It goes through
// safeReturnTo (lib/auth/safe-return-to.ts); a hand-rolled check on this
// value is an open redirect.
// value is an open redirect. Without one ('/'), nothing changes: the
// password path leaves through the confirmation mail and /auth/callback,
// and the BankID path lands a brand-new account on /select-company. With
// one, every path resumes it: BankID hard-navigates (the consent page is a
// route handler returning raw HTML), while the confirmation link and Google
// OAuth carry it to /auth/callback, which honours only the consent
// destination. A new account has no membership to spend a deep link on, so
// nothing else may ever be forwarded here.
const searchParams = useSearchParams()
const nextPath = safeReturnTo(searchParams.get('next'), '/')
const loginHref = nextPath === '/' ? '/login' : `/login?next=${encodeURIComponent(nextPath)}`
const [email, setEmail] = useState('')
const [password, setPassword] = useState('')
const [confirmPassword, setConfirmPassword] = useState('')
@@ -247,6 +253,13 @@ function RegisterPageContent() {
return
}
if (nextPath !== '/') {
// Resume the MCP consent flow: the account exists and the consent
// page accepts a user with no company yet.
window.location.assign(nextPath)
return
}
router.push('/select-company')
router.refresh()
} catch (error) {
@@ -306,11 +319,15 @@ function RegisterPageContent() {
setIsLoading(true)
try {
// The confirmation link lands on /auth/callback; carry the consent
// destination along so the confirmed session resumes it.
const confirmationCallback = new URL('/auth/callback', window.location.origin)
if (nextPath !== '/') confirmationCallback.searchParams.set('next', nextPath)
const { data, error } = await supabase.auth.signUp({
email: emailValue,
password: passwordValue,
options: {
emailRedirectTo: `${window.location.origin}/auth/callback`,
emailRedirectTo: confirmationCallback.toString(),
},
})
@@ -364,8 +381,9 @@ function RegisterPageContent() {
}
// Auto-confirmed but no invite or invite failed: go to onboarding
// (invite cookie is preserved so the onboarding fallback can retry)
window.location.href = '/'
// (invite cookie is preserved so the onboarding fallback can retry),
// or resume the MCP consent flow when that is where we came from.
window.location.href = nextPath
return
}
@@ -424,7 +442,7 @@ function RegisterPageContent() {
screen above, so nothing is lost by dropping it.
*/}
<Button className="w-full" asChild>
<Link href="/login">
<Link href={loginHref}>
{t('sign_in')}
</Link>
</Button>
@@ -515,7 +533,7 @@ function RegisterPageContent() {
action={
formError.kind === 'email_exists' ? (
<Link
href="/login"
href={loginHref}
className="font-medium underline underline-offset-2"
>
{t('sign_in')}
@@ -785,6 +803,7 @@ function RegisterPageContent() {
{googleAuthEnabled && (
<GoogleAuthButton
compact
next={nextPath}
onError={(message) => setFormError({ kind: 'oauth', message })}
/>
)}
@@ -807,7 +826,7 @@ function RegisterPageContent() {
<p className="mt-6 text-center text-[13px] text-muted-foreground">
{t('already_have_account')}{' '}
<Link
href="/login"
href={loginHref}
className="font-medium text-foreground underline underline-offset-2 hover:opacity-80 transition-opacity"
>
{t('sign_in')}
+7 -4
View File
@@ -9,7 +9,7 @@ import {
} from '@/lib/auth/api-keys'
import { validateQuery } from '@/lib/api/validate'
import { EventsQuerySchema } from '@/lib/api/schemas'
import { requireCompanyId } from '@/lib/company/context'
import { getActiveCompanyId } from '@/lib/company/context'
import type { SupabaseClient } from '@supabase/supabase-js'
import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-message'
import { errorResponseFromCode } from '@/lib/errors/get-structured-error'
@@ -126,10 +126,13 @@ export async function GET(request: Request) {
}
// Session auth resolves the active company; API-key auth uses the key's bound company.
const companyId = keyCompanyId ?? await requireCompanyId(supabase, userId)
// A key minted before the user's first company exists (companyless OAuth,
// issue #1814) has no bound company either; both cases resolve to null and
// fail closed below rather than throwing.
const companyId = keyCompanyId ?? await getActiveCompanyId(supabase, userId)
// Defense in depth: never run the event_log query with an empty/undefined
// scope. requireCompanyId throws when there is no company, but guard the
// key-bound path too so a malformed binding can't widen the query scope.
// scope. A user with no company resolves to null, and the guard also covers
// a malformed key binding so it can't widen the query scope.
if (!companyId) {
return errorResponseFromCode('FORBIDDEN', log)
}
@@ -4,7 +4,7 @@ import crypto from 'crypto'
const mocks = vi.hoisted(() => ({
createClient: vi.fn(),
isAllowedRedirectUri: vi.fn(),
requireCompanyId: vi.fn(),
getActiveCompanyId: vi.fn(),
getBranding: vi.fn(),
}))
@@ -21,7 +21,7 @@ vi.mock('@/lib/auth/oauth-allowlist', () => ({
}))
vi.mock('@/lib/company/context', () => ({
requireCompanyId: (...args: unknown[]) => mocks.requireCompanyId(...args),
getActiveCompanyId: (...args: unknown[]) => mocks.getActiveCompanyId(...args),
}))
vi.mock('@/lib/branding/service', () => ({
@@ -37,15 +37,25 @@ function buildAuthorizeUrl(params: Record<string, string>): string {
}
function buildSupabase(
user: { id: string } | null,
user: { id: string; email?: string } | null,
companyName = 'Test AB',
aal: { currentLevel: string; nextLevel: string } = { currentLevel: 'aal2', nextLevel: 'aal2' },
verifiedFactors: number = aal.nextLevel === 'aal2' ? 1 : 0,
) {
return {
auth: {
getUser: vi.fn().mockResolvedValue({ data: { user }, error: null }),
mfa: {
getAuthenticatorAssuranceLevel: vi.fn().mockResolvedValue({ data: aal, error: null }),
listFactors: vi.fn().mockResolvedValue({
data: {
totp: Array.from({ length: verifiedFactors }, (_, i) => ({
id: `factor-${i}`,
status: 'verified',
})),
},
error: null,
}),
},
},
from: vi.fn().mockReturnValue({
@@ -67,7 +77,7 @@ describe('GET /api/mcp-oauth/authorize: CSP', () => {
process.env.SUPABASE_SERVICE_ROLE_KEY = 'test-service-key'
mocks.createClient.mockResolvedValue(buildSupabase({ id: 'user-1' }))
mocks.isAllowedRedirectUri.mockResolvedValue(true)
mocks.requireCompanyId.mockResolvedValue('company-1')
mocks.getActiveCompanyId.mockResolvedValue('company-1')
mocks.getBranding.mockReturnValue({ appName: 'gnubok' })
})
@@ -258,7 +268,7 @@ describe('MFA step-up on /api/mcp-oauth/authorize', () => {
vi.stubEnv('NEXT_PUBLIC_REQUIRE_MFA', 'true')
vi.stubEnv('NEXT_PUBLIC_SELF_HOSTED', 'false')
mocks.isAllowedRedirectUri.mockResolvedValue(true)
mocks.requireCompanyId.mockResolvedValue('company-1')
mocks.getActiveCompanyId.mockResolvedValue('company-1')
mocks.getBranding.mockReturnValue({ appName: 'gnubok' })
})
@@ -310,6 +320,63 @@ describe('MFA step-up on /api/mcp-oauth/authorize', () => {
expect(response.status).toBe(200)
})
it('GET fails closed to /mfa/verify when the assurance lookup returns nothing', async () => {
// A transient auth error must never read as "no MFA needed": consent
// here mints a key that bypasses MFA on every later call.
const supabase = buildSupabase({ id: 'user-1' }, 'Test AB', { currentLevel: 'aal1', nextLevel: 'aal1' }, 1)
;(supabase.auth.mfa.getAuthenticatorAssuranceLevel as ReturnType<typeof vi.fn>).mockResolvedValue({
data: null,
error: { message: 'boom' },
})
mocks.createClient.mockResolvedValue(supabase)
const response = await GET(new Request(buildAuthorizeUrl(authorizeParams)))
expect(new URL(response.headers.get('location')!).pathname).toBe('/mfa/verify')
})
it('GET steps up (not enroll) when a verified factor exists despite an AAL1 answer', async () => {
mocks.createClient.mockResolvedValue(
buildSupabase({ id: 'user-1' }, 'Test AB', { currentLevel: 'aal1', nextLevel: 'aal1' }, 1),
)
const response = await GET(new Request(buildAuthorizeUrl(authorizeParams)))
expect(new URL(response.headers.get('location')!).pathname).toBe('/mfa/verify')
})
it('GET sends a password account with no factor to /mfa/enroll with returnTo', async () => {
// A brand-new account created inside the OAuth popup (issue #1814) has no
// company, so the middleware never forced enrollment. Without this leg the
// consent would mint an MFA-exempt key for an account with no second factor.
mocks.createClient.mockResolvedValue(
buildSupabase({ id: 'user-1' }, 'Test AB', { currentLevel: 'aal1', nextLevel: 'aal1' }, 0),
)
const response = await GET(new Request(buildAuthorizeUrl(authorizeParams)))
expect(response.status).toBeGreaterThanOrEqual(300)
expect(response.status).toBeLessThan(400)
const location = new URL(response.headers.get('location')!)
expect(location.pathname).toBe('/mfa/enroll')
const returnTo = new URL(location.searchParams.get('returnTo')!, location.origin)
expect(returnTo.pathname).toBe('/api/mcp-oauth/authorize')
expect(returnTo.searchParams.get('state')).toBe('xyz')
})
it('POST refuses consent from a password account with no factor', async () => {
mocks.createClient.mockResolvedValue(
buildSupabase({ id: 'user-1' }, 'Test AB', { currentLevel: 'aal1', nextLevel: 'aal1' }, 0),
)
const formData = new FormData()
formData.set('consent', 'allow')
const response = await POST(
new Request(buildAuthorizeUrl(authorizeParams), { method: 'POST', body: formData }),
)
expect(new URL(response.headers.get('location')!).pathname).toBe('/mfa/enroll')
expect(response.headers.get('location')).not.toContain('code=')
})
it('GET skips step-up for BankID-linked users (inherently 2FA)', async () => {
const supabase = buildSupabase(
{ id: 'user-1' },
@@ -327,6 +394,66 @@ describe('MFA step-up on /api/mcp-oauth/authorize', () => {
})
})
describe('account with no company yet (issue #1814)', () => {
// Someone who signed up inside the MCP client's OAuth popup has an account
// but no company. Consent must still complete: the key is minted unbound
// and binds itself once the company exists.
const authorizeParams = {
response_type: 'code',
redirect_uri: 'https://claude.ai/api/mcp/auth_callback',
code_challenge: 'abc',
code_challenge_method: 'S256',
scope: 'mcp',
state: 'xyz',
}
function signScope(scopeParam: string): string {
const key = crypto.createHash('sha256').update('oauth-scope:test-service-key').digest()
return crypto.createHmac('sha256', key).update(scopeParam).digest('base64url')
}
beforeEach(() => {
vi.clearAllMocks()
process.env.SUPABASE_SERVICE_ROLE_KEY = 'test-service-key'
mocks.isAllowedRedirectUri.mockResolvedValue(true)
mocks.getActiveCompanyId.mockResolvedValue(null)
mocks.getBranding.mockReturnValue({ appName: 'gnubok' })
})
it('GET renders consent labelled with the account instead of a company', async () => {
const supabase = buildSupabase({ id: 'user-1', email: 'ny@example.se' })
mocks.createClient.mockResolvedValue(supabase)
const response = await GET(new Request(buildAuthorizeUrl(authorizeParams)))
expect(response.status).toBe(200)
const html = await response.text()
expect(html).toContain('ny@example.se')
expect(html).toContain('inget företag')
expect(html).not.toContain('Test AB')
// No company to look up: company_settings is never queried.
expect(supabase.from).not.toHaveBeenCalled()
})
it('POST still issues an authorization code', async () => {
mocks.createClient.mockResolvedValue(buildSupabase({ id: 'user-1', email: 'ny@example.se' }))
const formData = new FormData()
formData.set('consent', 'allow')
formData.set('scope_binding', 'mcp')
formData.set('scope_binding_sig', signScope('mcp'))
const response = await POST(
new Request(buildAuthorizeUrl(authorizeParams), { method: 'POST', body: formData }),
)
expect(response.status).toBe(303)
const location = new URL(response.headers.get('location')!)
expect(location.searchParams.get('code')).toBe('test-auth-code')
expect(location.searchParams.get('state')).toBe('xyz')
})
})
describe('RFC 9207 iss parameter on authorization responses', () => {
const authorizeParams = {
response_type: 'code',
@@ -350,7 +477,7 @@ describe('RFC 9207 iss parameter on authorization responses', () => {
vi.stubEnv('NEXT_PUBLIC_APP_URL', 'https://app.test.example')
mocks.createClient.mockResolvedValue(buildSupabase({ id: 'user-1' }))
mocks.isAllowedRedirectUri.mockResolvedValue(true)
mocks.requireCompanyId.mockResolvedValue('company-1')
mocks.getActiveCompanyId.mockResolvedValue('company-1')
mocks.getBranding.mockReturnValue({ appName: 'gnubok' })
})
+65 -23
View File
@@ -4,7 +4,7 @@ import { NextResponse } from 'next/server'
import type { SupabaseClient, User } from '@supabase/supabase-js'
import { createAuthCode } from '@/lib/auth/oauth-codes'
import { shouldEnforceMfa } from '@/lib/auth/mfa'
import { requireCompanyId } from '@/lib/company/context'
import { getActiveCompanyId } from '@/lib/company/context'
import { getBranding } from '@/lib/branding/service'
import { isAllowedRedirectUri } from '@/lib/auth/oauth-allowlist'
import { resolveDiscoveryBaseUrl } from '@/lib/api/v1/base-url'
@@ -114,7 +114,14 @@ function buildLoginRedirect(request: Request): Response {
* The middleware MFA gate deliberately exempts /api/mcp-oauth/* (the token
* endpoint is Bearer-only), which makes this route responsible for its own
* step-up. Returns null when the session is AAL2 (or MFA isn't required),
* otherwise a redirect to /mfa/verify that returns to this authorize URL.
* otherwise a redirect to /mfa/verify (factor enrolled, session still AAL1)
* or /mfa/enroll (no factor at all) that returns to this authorize URL.
*
* The enrollment leg matters for accounts created inside the OAuth popup
* (issue #1814): the middleware only forces enrollment once a company exists,
* so a brand-new password account would otherwise consent at AAL1 and mint an
* MFA-exempt key for an account with no second factor. BankID-linked accounts
* are exempt via shouldEnforceMfa, same as everywhere else.
*/
async function requireAal2(
supabase: SupabaseClient,
@@ -122,15 +129,28 @@ async function requireAal2(
request: Request,
): Promise<Response | null> {
if (!shouldEnforceMfa(user)) return null
const { data: aal } = await supabase.auth.mfa.getAuthenticatorAssuranceLevel()
if (aal?.nextLevel === 'aal2' && aal?.currentLevel !== 'aal2') {
const url = new URL(request.url)
const returnTo = `${url.pathname}${url.search}`
return NextResponse.redirect(
new URL(`/mfa/verify?returnTo=${encodeURIComponent(returnTo)}`, url.origin),
)
}
return null
const url = new URL(request.url)
const returnTo = `${url.pathname}${url.search}`
const stepUp = (page: '/mfa/verify' | '/mfa/enroll') =>
NextResponse.redirect(new URL(`${page}?returnTo=${encodeURIComponent(returnTo)}`, url.origin))
// Only a positive "this session is AAL2" answer lets consent through. A
// failed or empty assurance lookup is treated as AAL1 (verify page), never
// as "no MFA needed": the alternative would mint an MFA-exempt key on a
// transient auth error.
const { data: aal, error: aalError } = await supabase.auth.mfa.getAuthenticatorAssuranceLevel()
if (aalError || !aal) return stepUp('/mfa/verify')
if (aal.currentLevel === 'aal2') return null
if (aal.nextLevel === 'aal2') return stepUp('/mfa/verify')
// nextLevel below aal2 should mean no verified factor exists. If one does
// exist anyway (inconsistent answer), step up rather than enroll a second
// factor. Otherwise enroll: mirrors the middleware gate (lib/supabase/
// middleware.ts), which skips zero-company users and so never ran for an
// account created inside the popup.
const { data: factors } = await supabase.auth.mfa.listFactors()
const hasVerifiedFactor = factors?.totp?.some((f) => f.status === 'verified') ?? false
return stepUp(hasVerifiedFactor ? '/mfa/verify' : '/mfa/enroll')
}
function errorRedirect(request: Request, redirectUri: string, state: string | null, error: string, desc: string): Response {
@@ -209,19 +229,33 @@ export async function GET(request: Request) {
)
}
const companyId = await requireCompanyId(supabase, user.id)
// null for an account with no company yet (signed up from the OAuth popup,
// issue #1814): consent still goes through, the key is minted unbound and
// binds itself once the company exists. The page says so instead of
// showing a company name.
const companyId = await getActiveCompanyId(supabase, user.id)
// Get company name for the consent page
const { data: settings } = await supabase
.from('company_settings')
.select('company_name')
.eq('company_id', companyId)
.single()
const companyName = settings?.company_name || user.email
let companyName: string | null = null
if (companyId) {
const { data: settings } = await supabase
.from('company_settings')
.select('company_name')
.eq('company_id', companyId)
.single()
companyName = settings?.company_name || user.email || null
}
const appNameLower = escapeHtml(getBranding().appName.toLowerCase())
const accountRowHtml = companyName
? `<span class="account-label">Företag</span>
<span class="account-name">${escapeHtml(companyName)}</span>`
: `<span class="account-label">Konto</span>
<span class="account-name">${escapeHtml(user.email ?? '')}</span>`
const noCompanyNoteHtml = companyId
? ''
: `<p class="note">Du har inget företag i ${appNameLower} ännu. Du kan ansluta ändå: skapa företaget i appen så använder anslutningen det automatiskt, utan att du behöver ansluta på nytt.</p>`
// CSP nonce for the inline consent UI controls. A nonce-bound script-src
// makes the inline block executable while keeping the rest of the page
// immune to script injection: without this the consent page is
@@ -374,6 +408,12 @@ export async function GET(request: Request) {
text-align: right;
word-break: break-word;
}
.note {
font-size: 0.8125rem;
color: var(--fg-muted);
line-height: 1.55;
margin: -1rem 0 1.75rem;
}
.scopes-header {
display: flex;
justify-content: space-between;
@@ -557,9 +597,9 @@ export async function GET(request: Request) {
<p class="lede">En extern applikation begär åtkomst till ditt ${appNameLower}-konto. Välj vilka behörigheter du vill bevilja.</p>
<div class="account">
<span class="account-label">Företag</span>
<span class="account-name">${escapeHtml(companyName)}</span>
${accountRowHtml}
</div>
${noCompanyNoteHtml}
<form method="POST" action="${escapeHtml(url.pathname + url.search)}" id="consent-form">
<input type="hidden" name="scope_binding" value="${escapeHtml(scopeBindingValue)}">
@@ -681,7 +721,9 @@ export async function POST(request: Request) {
)
}
await requireCompanyId(supabase, user.id)
// No company check here: the auth code carries only the user id, and the
// token endpoint resolves (or leaves unbound) the company when it mints the
// key. An account without a company may consent (issue #1814).
// Parse form body
const formData = await request.formData()
@@ -3,6 +3,7 @@ import { createQueuedMockSupabase } from '@/tests/helpers'
const mocks = vi.hoisted(() => ({
supabaseFactory: vi.fn(),
getActiveCompanyId: vi.fn().mockResolvedValue('company-1'),
}))
vi.mock('@/lib/auth/api-keys', async (importOriginal) => {
@@ -20,7 +21,7 @@ vi.mock('@/lib/auth/oauth-codes', () => ({
}))
vi.mock('@/lib/company/context', () => ({
requireCompanyId: vi.fn().mockResolvedValue('company-1'),
getActiveCompanyId: (...args: unknown[]) => mocks.getActiveCompanyId(...args),
}))
import { POST } from '../route'
@@ -93,6 +94,45 @@ describe('POST /api/mcp-oauth/token', () => {
expect(body.expires_in).toBe(3600)
})
it('mints an unbound key (company_id null) when the user has no company yet', async () => {
// Signup inside the OAuth popup (issue #1814): the account exists, the
// company does not. The key is stored unbound and validateApiKey binds
// it on the first call after the company is created.
vi.mocked(decryptAuthCode).mockReturnValue({
userId: 'user-1',
codeChallenge: 'challenge',
redirectUri: 'https://claude.ai/api/cb',
exp: Date.now() + 60_000,
})
vi.mocked(verifyPkce).mockReturnValue(true)
mocks.getActiveCompanyId.mockResolvedValueOnce(null)
const { supabase, enqueueMany, findCall } = createQueuedMockSupabase()
mocks.supabaseFactory.mockReturnValue(supabase)
enqueueMany([
{ data: null, error: null }, // insert into oauth_used_codes
{ data: null, error: null }, // delete expired codes (best-effort)
{ data: null, error: null }, // insert into api_keys
])
const res = await POST(
formRequest({
grant_type: 'authorization_code',
code: 'ciphertext',
code_verifier: 'verifier',
redirect_uri: 'https://claude.ai/api/cb',
})
)
expect(res.status).toBe(200)
const body = await res.json()
expect(body.access_token).toMatch(/^gnubok_sk_/)
const inserted = findCall('api_keys', 'insert')?.[0] as Record<string, unknown>
expect(inserted).toBeDefined()
expect(inserted.user_id).toBe('user-1')
expect(inserted.company_id).toBeNull()
})
it('rejects an already-used auth code (replay)', async () => {
vi.mocked(decryptAuthCode).mockReturnValue({
userId: 'user-1',
+5 -2
View File
@@ -9,7 +9,7 @@ import {
DEFAULT_OAUTH_SCOPES,
type ApiKeyScope,
} from '@/lib/auth/api-keys'
import { requireCompanyId } from '@/lib/company/context'
import { getActiveCompanyId } from '@/lib/company/context'
const ACCESS_TOKEN_TTL_SECONDS = 3600
@@ -125,7 +125,10 @@ async function handleAuthorizationCodeGrant(params: URLSearchParams) {
.lt('created_at', new Date(Date.now() - 10 * 60 * 1000).toISOString())
.then(() => {})
const companyId = await requireCompanyId(supabase, payload.userId)
// null for an account that has no company yet (signed up from the OAuth
// popup, issue #1814). The key is minted unbound; validateApiKey binds it
// to the user's first company on the first call after one exists.
const companyId = await getActiveCompanyId(supabase, payload.userId)
const { key, hash, prefix } = generateApiKey()
const refresh = generateRefreshToken()
+13 -1
View File
@@ -21,6 +21,7 @@ import { GoogleMark } from '@/components/ui/provider-marks'
export function GoogleAuthButton({
onError,
compact = false,
next,
}: {
onError: (message: string) => void
/**
@@ -29,6 +30,14 @@ export function GoogleAuthButton({
* kept as the accessible name.
*/
compact?: boolean
/**
* Post-auth destination, already passed through safeReturnTo by the caller.
* Forwarded to /auth/callback as `next` so an OAuth sign-in or sign-up that
* started from the MCP consent page (/login?next=/api/mcp-oauth/authorize…)
* resumes the consent flow instead of landing on the dashboard. '/' (the
* safeReturnTo fallback) means no destination and is not forwarded.
*/
next?: string
}) {
const [isRedirecting, setIsRedirecting] = useState(false)
const supabase = createClient()
@@ -38,10 +47,13 @@ export function GoogleAuthButton({
const handleClick = async () => {
setIsRedirecting(true)
try {
const callback = new URL('/auth/callback', window.location.origin)
callback.searchParams.set('flow', 'oauth')
if (next && next !== '/') callback.searchParams.set('next', next)
const { error } = await supabase.auth.signInWithOAuth({
provider: 'google',
options: {
redirectTo: `${window.location.origin}/auth/callback?flow=oauth`,
redirectTo: callback.toString(),
},
})
if (error) {
@@ -117,6 +117,22 @@ describe('MCP company routing', () => {
expect(chain.eq).toHaveBeenCalledWith('company_id', DEFAULT_COMPANY_ID)
})
it('refuses company-dependent calls on a key whose user has no company yet', async () => {
// A key minted from the OAuth popup before onboarding (issue #1814) has
// no default company. The refusal is a distinct, actionable code and
// never reaches the membership lookup.
const { client, chain } = membershipClient({ data: null, error: null })
await expect(
resolveMcpCompanyContext({
supabase: client as never,
userId: 'user-1',
defaultCompanyId: null,
})
).rejects.toMatchObject({ code: 'NO_COMPANY_YET' })
expect(chain.maybeSingle).not.toHaveBeenCalled()
})
it('rejects companies without a current non-archived membership', async () => {
const { client } = membershipClient({ data: null, error: null })
@@ -29,12 +29,24 @@ interface ToolSchemaSource {
}
export function codedError(
code: 'VALIDATION_ERROR' | 'NOT_FOUND' | 'FORBIDDEN' | 'INTERNAL_ERROR',
code: 'VALIDATION_ERROR' | 'NOT_FOUND' | 'FORBIDDEN' | 'INTERNAL_ERROR' | 'NO_COMPANY_YET',
message: string
) {
return Object.assign(new Error(message), { code })
}
/**
* Thrown when a company-scoped operation runs on a key whose user has no
* company at all (minted from the OAuth popup before onboarding, issue
* #1814). Maps to the NO_COMPANY_YET structured error with its remediation.
*/
export function noCompanyYetError(): Error {
return codedError(
'NO_COMPANY_YET',
'This account has no company yet. Create the company in the web app, then retry.'
)
}
function isCompanyRole(value: unknown): value is CompanyRole {
return value === 'owner' || value === 'admin' || value === 'member' || value === 'viewer'
}
@@ -86,10 +98,14 @@ export function extractRequestedCompany(
export async function resolveMcpCompanyContext(args: {
supabase: SupabaseClient
userId: string
defaultCompanyId: string
/** null while the key's user has no company (see validateApiKey). */
defaultCompanyId: string | null
requestedCompanyId?: string
}): Promise<McpCompanyContext> {
const companyId = args.requestedCompanyId ?? args.defaultCompanyId
if (!companyId) {
throw noCompanyYetError()
}
const { data: membership, error } = await args.supabase
.from('company_members')
+49 -20
View File
@@ -156,6 +156,7 @@ import {
codedError,
extractRequestedCompany,
isCompanyDependentTool,
noCompanyYetError,
projectToolInputSchema,
resolveMcpCompanyContext,
} from './company-routing'
@@ -17901,8 +17902,12 @@ function emitToolCallTelemetry(payload: {
errorMessage: string | null
requestId: string | number | null
userId: string
companyId: string
// null/empty while the key's user has no company yet (issue #1814): the
// event-log persister requires a company scope, so nothing is emitted.
companyId: string | null
}): void {
if (!payload.companyId) return
const companyId = payload.companyId
emitAfterResponse(() => eventBus
.emit({
type: 'mcp.tool_called',
@@ -17923,7 +17928,7 @@ function emitToolCallTelemetry(payload: {
errorMessage: payload.errorMessage ? payload.errorMessage.slice(0, 500) : null,
requestId: payload.requestId,
userId: payload.userId,
companyId: payload.companyId,
companyId,
sessionId: payload.actor.sessionId ?? null,
client: payload.actor.client ?? null,
},
@@ -17942,8 +17947,10 @@ function emitToolsListTelemetry(payload: {
latencyMs: number
requestId: string | number | null
userId: string
companyId: string
companyId: string | null
}): void {
if (!payload.companyId) return
const companyId = payload.companyId
emitAfterResponse(() => eventBus
.emit({
type: 'mcp.tools_list_called',
@@ -17955,7 +17962,7 @@ function emitToolsListTelemetry(payload: {
latencyMs: payload.latencyMs,
requestId: payload.requestId,
userId: payload.userId,
companyId: payload.companyId,
companyId,
sessionId: payload.actor.sessionId ?? null,
client: payload.actor.client ?? null,
},
@@ -17975,8 +17982,10 @@ function emitResourceReadTelemetry(payload: {
latencyMs: number
requestId: string | number | null
userId: string
companyId: string
companyId: string | null
}): void {
if (!payload.companyId) return
const companyId = payload.companyId
emitAfterResponse(() => eventBus
.emit({
type: 'mcp.resource_read',
@@ -17991,7 +18000,7 @@ function emitResourceReadTelemetry(payload: {
actorLabel: payload.actor.label ?? null,
requestId: payload.requestId,
userId: payload.userId,
companyId: payload.companyId,
companyId,
sessionId: payload.actor.sessionId ?? null,
client: payload.actor.client ?? null,
},
@@ -18038,9 +18047,9 @@ function checkAndEmitNextHintFollowed(
toolName: string,
actor: ActorContext,
userId: string,
companyId: string,
companyId: string | null,
): void {
if (!sessionId) return
if (!sessionId || !companyId) return
const prev = lastResponseHintBySession.get(sessionId)
if (!prev || prev.expiresAt < Date.now() || prev.suggestedTool !== toolName) return
// Consume the hint so we don't double-count if the agent calls the same
@@ -18074,8 +18083,10 @@ function emitSkillLoaded(payload: {
tier: 'workflow' | 'horizontal' | 'vertical' | 'modifier'
actor: ActorContext
userId: string
companyId: string
companyId: string | null
}): void {
if (!payload.companyId) return
const companyId = payload.companyId
emitAfterResponse(() => eventBus
.emit({
type: 'mcp.skill_loaded',
@@ -18087,7 +18098,7 @@ function emitSkillLoaded(payload: {
actorId: payload.actor.id ?? null,
actorLabel: payload.actor.label ?? null,
userId: payload.userId,
companyId: payload.companyId,
companyId,
},
})
.catch((err) => console.error('[mcp] skill_loaded emit failed:', err)))
@@ -18098,8 +18109,10 @@ function emitWorkflowStarted(payload: {
slug: string
actor: ActorContext
userId: string
companyId: string
companyId: string | null
}): void {
if (!payload.companyId) return
const companyId = payload.companyId
emitAfterResponse(() => eventBus
.emit({
type: 'mcp.workflow_started',
@@ -18110,7 +18123,7 @@ function emitWorkflowStarted(payload: {
actorId: payload.actor.id ?? null,
actorLabel: payload.actor.label ?? null,
userId: payload.userId,
companyId: payload.companyId,
companyId,
},
})
.catch((err) => console.error('[mcp] workflow_started emit failed:', err)))
@@ -18318,7 +18331,7 @@ export async function handleMcpRequest(request: Request): Promise<Response> {
'Discovery:',
'• tools/list returns common tool schemas. Call gnubok_search_tools(query="…") for specialized tools: it ranks all capabilities; pass detail="name"|"summary"|"full" to control payload size.',
'• gnubok_get_agent_briefing returns recommended_tools: ordered per-workflow tool loadouts (categorize_month, close_period, invoice_run, vat_declaration, payroll_month). If your harness defers tool loading, batch-load a whole workflow in one call (e.g. Claude Code ToolSearch select:a,b,c) instead of searching cluster by cluster.',
`• This connection can work with every non-archived company the API-key user belongs to. Call gnubok_list_companies to discover company_id values. Omit company_id to use the API key default (${companyId}); when selecting another company, repeat company_id on every company-data call, including approval.`,
`• This connection can work with every non-archived company the API-key user belongs to. Call gnubok_list_companies to discover company_id values. Omit company_id to use the API key default (${companyId ?? 'none yet: this account has no company; it must be created in the web app before company-data tools work'}); when selecting another company, repeat company_id on every company-data call, including approval.`,
'• MCP resources use the API key default company. For a selected non-default company, call gnubok_get_agent_briefing with company_id instead of relying on Accounted://company/current or other company-data resources.',
'• When the user asks "how do I do X" or you\'re unsure of the correct sequence (month-end close, VAT review, year-end, invoicing, payroll), call gnubok_list_skills first: domain workflows are documented as loadable skills with tool references.',
'• When a tool is missing, a description misled you, a result looks wrong, or something worked unusually well, call gnubok_feedback (context + suggestion, optional tool_name). It is read by the product team and has fixed real bugs; include ids and what you expected. Rate-limited 1/min/key, so batch a session\'s findings into one call.',
@@ -18492,7 +18505,10 @@ export async function handleMcpRequest(request: Request): Promise<Response> {
}
let toolArgs: Record<string, unknown>
let effectiveCompanyId = companyId
// null only for the company-independent tools on a key whose user has
// no company yet (issue #1814): resolveMcpCompanyContext refuses every
// company-dependent tool with NO_COMPANY_YET before it gets here.
let effectiveCompanyId: string | null = companyId
const companyRoutingStartedAt = Date.now()
try {
const extracted = extractRequestedCompany(rawToolArgs)
@@ -18548,12 +18564,18 @@ export async function handleMcpRequest(request: Request): Promise<Response> {
)
}
// The McpTool.execute contract takes a string company id. Only the
// company-independent tools (search, skills, list_companies) can reach
// this point unbound, and none of them dereferences the id as a tenant:
// the empty string is a placeholder for their signature, never a scope.
const tenantId: string = effectiveCompanyId ?? ''
// Enforce the capability paywall: the MCP/agent path is a paid chokepoint
// just like the HTTP routes (send_invoice → email_send, the two SKV
// submissions → skatteverket). Fail-closed; self-hosted short-circuits to
// all-on inside hasCapability. Blocks before any pending op is staged.
const requiredCapability = MCP_TOOL_CAPABILITY_MAP[toolName]
if (requiredCapability && !(await hasCapability(supabase, effectiveCompanyId, requiredCapability))) {
if (requiredCapability && !(await hasCapability(supabase, tenantId, requiredCapability))) {
const capError = { error: capabilityBlockedError(requiredCapability) }
const publicCapError = projectMcpPayload(capError, toolNamespace)
emitToolCallTelemetry({
@@ -18633,7 +18655,7 @@ export async function handleMcpRequest(request: Request): Promise<Response> {
// so nothing is ever started for a call that would have been refused.
if (taskCapable && tool.shouldRunAsTask?.(toolArgs)) {
const task = await createMcpTask(supabase, {
companyId: effectiveCompanyId,
companyId: tenantId,
userId,
apiKeyId,
toolName,
@@ -18641,8 +18663,10 @@ export async function handleMcpRequest(request: Request): Promise<Response> {
const taskStartedAt = Date.now()
emitAfterResponse(async () => {
try {
const rawResult = await tool.execute(toolArgs, effectiveCompanyId, userId, supabase, actor)
const canonicalResult = addCompanyToTopLevelNext(rawResult, effectiveCompanyId)
const rawResult = await tool.execute(toolArgs, tenantId, userId, supabase, actor)
const canonicalResult = effectiveCompanyId
? addCompanyToTopLevelNext(rawResult, effectiveCompanyId)
: rawResult
const result = projectMcpPayload(canonicalResult, toolNamespace)
const stored: Record<string, unknown> = {
resultType: 'complete',
@@ -18717,8 +18741,10 @@ export async function handleMcpRequest(request: Request): Promise<Response> {
(toolArgs as Record<string, unknown>).__keyScopes = keyScopes
;(toolArgs as Record<string, unknown>).__toolNamespace = toolNamespace
}
const rawResult = await tool.execute(toolArgs, effectiveCompanyId, userId, supabase, actor)
const canonicalResult = addCompanyToTopLevelNext(rawResult, effectiveCompanyId)
const rawResult = await tool.execute(toolArgs, tenantId, userId, supabase, actor)
const canonicalResult = effectiveCompanyId
? addCompanyToTopLevelNext(rawResult, effectiveCompanyId)
: rawResult
const result = projectMcpPayload(canonicalResult, toolNamespace)
const latencyMs = Date.now() - callStartedAt
const response: Record<string, unknown> = {
@@ -18898,6 +18924,9 @@ export async function handleMcpRequest(request: Request): Promise<Response> {
const dataResource = findResource(uri)
if (dataResource) {
try {
// Data resources are company-scoped; an unbound key (no company yet,
// issue #1814) gets the same NO_COMPANY_YET answer as tools/call.
if (!companyId) throw noCompanyYetError()
const result = await dataResource.read({
supabase,
companyId,
+79
View File
@@ -4,6 +4,14 @@ vi.mock('@supabase/supabase-js', () => ({
createClient: vi.fn(),
}))
const contextMocks = vi.hoisted(() => ({
getActiveCompanyId: vi.fn(),
}))
vi.mock('@/lib/company/active-company', () => ({
getActiveCompanyId: (...args: unknown[]) => contextMocks.getActiveCompanyId(...args),
}))
import {
generateApiKey,
hashApiKey,
@@ -339,4 +347,75 @@ describe('validateApiKey', () => {
mode: 'test',
})
})
describe('unbound keys (minted before the first company existed, issue #1814)', () => {
function setupUnboundKeyClient(rpcRow: Record<string, unknown>) {
const chain = {
update: vi.fn(),
eq: vi.fn(),
is: vi.fn().mockResolvedValue({ data: null, error: null }),
}
chain.update.mockReturnValue(chain)
chain.eq.mockReturnValue(chain)
const from = vi.fn().mockReturnValue(chain)
const rpc = vi.fn().mockResolvedValue({ data: [rpcRow], error: null })
// eslint-disable-next-line @typescript-eslint/no-explicit-any
mockCreateClient.mockReturnValue({ rpc, from } as any)
return { from, chain }
}
const unboundRow = {
user_id: 'user-123',
company_id: null,
api_key_id: 'ak_1',
api_key_name: 'MCP-klient (OAuth)',
scopes: ['transactions:read'],
rate_limited: false,
mode: 'live',
}
it('binds the key to the user\'s company once one exists and heals the row', async () => {
const { from, chain } = setupUnboundKeyClient(unboundRow)
contextMocks.getActiveCompanyId.mockResolvedValue('company-789')
const result = await validateApiKey('gnubok_sk_test-key-value')
expect('companyId' in result && result.companyId).toBe('company-789')
expect(from).toHaveBeenCalledWith('api_keys')
expect(chain.update).toHaveBeenCalledWith({ company_id: 'company-789' })
expect(chain.eq).toHaveBeenCalledWith('id', 'ak_1')
// Only an unbound row is ever rewritten: a concurrent bind must not be clobbered.
expect(chain.is).toHaveBeenCalledWith('company_id', null)
})
it('returns companyId null while the user still has no company', async () => {
const { from } = setupUnboundKeyClient(unboundRow)
contextMocks.getActiveCompanyId.mockResolvedValue(null)
const result = await validateApiKey('gnubok_sk_test-key-value')
expect('companyId' in result && result.companyId).toBeNull()
expect(from).not.toHaveBeenCalled()
})
it('treats a failed company resolution as still unbound rather than failing the key', async () => {
setupUnboundKeyClient(unboundRow)
contextMocks.getActiveCompanyId.mockRejectedValue(new Error('db blip'))
const result = await validateApiKey('gnubok_sk_test-key-value')
expect('companyId' in result && result.companyId).toBeNull()
expect('userId' in result && result.userId).toBe('user-123')
})
it('skips the heal when the RPC predates api_key_id in its return shape', async () => {
const { from } = setupUnboundKeyClient({ ...unboundRow, api_key_id: undefined })
contextMocks.getActiveCompanyId.mockResolvedValue('company-789')
const result = await validateApiKey('gnubok_sk_test-key-value')
expect('companyId' in result && result.companyId).toBe('company-789')
expect(from).not.toHaveBeenCalled()
})
})
})
+47 -2
View File
@@ -1,5 +1,10 @@
import crypto from 'crypto'
import type { SupabaseClient } from '@supabase/supabase-js'
import { createServiceRoleClient } from '@/lib/supabase/service-client'
// Not lib/company/context: that module imports next/headers for the legacy
// company cookie, and this file is reachable from bundles where that import
// is a build error.
import { getActiveCompanyId } from '@/lib/company/active-company'
const KEY_PREFIX = 'gnubok_sk_'
const REFRESH_TOKEN_PREFIX = 'gnubok_rt_'
@@ -473,7 +478,12 @@ export async function validateApiKey(
): Promise<
| {
userId: string
companyId: string
/**
* The key's default company. null only while the key's user has no
* company at all (minted from the OAuth popup before onboarding, issue
* #1814): the first validation after a company exists binds the key.
*/
companyId: string | null
apiKeyId?: string
apiKeyName?: string
scopes: ApiKeyScope[]
@@ -509,9 +519,12 @@ export async function validateApiKey(
return { error: 'Rate limit exceeded', status: 429 }
}
const companyId: string | null =
row.company_id ?? (await bindUnboundKey(supabase, row.user_id, row.api_key_id))
return {
userId: row.user_id,
companyId: row.company_id,
companyId,
apiKeyId: row.api_key_id,
apiKeyName: row.api_key_name,
scopes: validateScopes(row.scopes) ?? DEFAULT_SCOPES,
@@ -522,6 +535,38 @@ export async function validateApiKey(
}
}
/**
* Late binding for keys minted before the user's first company existed.
*
* The OAuth token endpoint stores company_id NULL for such keys. Company
* creation happens in the web app (a Server Action) which knows nothing about
* the user's keys, so the binding is healed here, on the first validation after
* a company exists: one place, regardless of how the company was created.
* Returns null while the user still has no company. The UPDATE is best-effort:
* a failed write only means the next call resolves again.
*/
async function bindUnboundKey(
supabase: SupabaseClient,
userId: string,
apiKeyId: string | undefined
): Promise<string | null> {
let companyId: string | null
try {
companyId = await getActiveCompanyId(supabase, userId)
} catch {
return null
}
if (!companyId) return null
if (apiKeyId) {
await supabase
.from('api_keys')
.update({ company_id: companyId })
.eq('id', apiKeyId)
.is('company_id', null)
}
return companyId
}
/**
* Check if a given scope is allowed by the key's scopes.
*/
+166
View File
@@ -0,0 +1,166 @@
import type { SupabaseClient } from '@supabase/supabase-js'
/**
* Active-company resolution with no Next.js request-scope dependency.
*
* Split out of lib/company/context.ts (which imports `next/headers` for the
* legacy company cookie) so that modules on the API-key path, notably
* lib/auth/api-keys.ts, can resolve a user's company without dragging
* `next/headers` into every bundle that validates a key. context.ts
* re-exports everything here; import from there unless the cookie import is
* the problem.
*/
/**
* Thrown by setActiveCompany so callers can tell a permissions problem
* ('not_member') apart from a failed/unverified database write
* ('persist_failed'), and by getActiveCompanyId when a resolution query
* fails ('resolution_failed': the active company is unknown right now,
* which is NOT the same as the user having no companies).
*/
export class CompanyContextError extends Error {
constructor(
message: string,
readonly code: 'not_member' | 'persist_failed' | 'resolution_failed'
) {
super(message)
this.name = 'CompanyContextError'
}
}
/**
* Get the active company ID for the authenticated user.
*
* Resolution order: user_preferences → first non-archived membership.
*
* `user_preferences.active_company_id` is the authoritative source. The
* cookie `gnubok-company-id` is written as a hint for backwards-compat but
* is no longer READ as a source of truth, because Postgres RLS (via
* `current_active_company_id()`) can only read the database, not cookies.
* Having Next.js and RLS both read from `user_preferences` keeps them
* perfectly in sync.
*
* RPC-first: tries `resolve_active_company()` (one round trip, semantically
* identical to the query path and to `current_active_company_id()`), falling
* back to the original query path when the function is not deployed
* (PGRST202), the caller lacks EXECUTE (42501: service-role clients), or the
* RPC returns zero rows (NULL auth.uid(), also service-role clients).
*
* Returns null only when the user positively has no non-archived companies.
* Throws CompanyContextError('resolution_failed') when a query fails: a
* transient failure must never read as "no companies", because callers
* redirect that state to the onboarding wizard (issue #1053).
*/
export async function getActiveCompanyId(
supabase: SupabaseClient,
userId: string
): Promise<string | null> {
const { data, error } = await supabase.rpc('resolve_active_company')
if (error) {
// PGRST202: function not in the schema cache (self-hosted instance not
// migrated yet, or a deploy racing the branch merge).
// 42501: EXECUTE is granted to `authenticated` only, so a service-role
// client is refused. These fallbacks are LOAD-BEARING, not defensive:
// app/api/mcp-oauth/token/route.ts, app/api/events/route.ts (API-key
// branch) and lib/auth/api-keys.ts call this with
// createServiceClientNoCookies(), and must silently resolve via the query
// path or the OAuth token flow breaks.
if (error.code === 'PGRST202' || error.code === '42501') {
return getActiveCompanyIdViaQueries(supabase, userId)
}
throw new CompanyContextError(
`Active company resolution failed: ${error.message}`,
'resolution_failed'
)
}
const row = (Array.isArray(data) ? data[0] : data) as
| { company_id: string | null; locale: string | null; used_fallback: boolean }
| undefined
| null
if (!row) {
// Zero rows = NULL auth.uid() inside the RPC, i.e. a service-role client
// (same call sites as the 42501 branch above). The query path filters by
// the explicit userId param and still resolves correctly.
return getActiveCompanyIdViaQueries(supabase, userId)
}
return row.company_id ?? null
}
/**
* Query-path resolution: the pre-RPC implementation, kept verbatim as the
* fallback for getActiveCompanyId (see the fallback conditions there).
*/
async function getActiveCompanyIdViaQueries(
supabase: SupabaseClient,
userId: string
): Promise<string | null> {
// user_preferences (authoritative) + first membership, fetched in parallel:
// the fallback query result doubles as validation when the preferred
// company happens to be the first membership, which is the common
// single-company case. Most requests pay one round trip instead of two
// sequential ones. This runs on every withRouteContext API request and
// every dashboard layout render, so the sequential version was pure
// wall-clock cost. Mirrors resolveCompanyForMiddleware, minus the
// write-back (read paths shouldn't write).
const [prefsRes, firstRes] = await Promise.all([
supabase
.from('user_preferences')
.select('active_company_id')
.eq('user_id', userId)
.maybeSingle(),
supabase
.from('company_members')
.select('company_id, companies!inner(archived_at)')
.eq('user_id', userId)
.is('companies.archived_at', null)
.order('created_at', { ascending: true })
.limit(1)
.maybeSingle(),
])
const resolutionError = prefsRes.error ?? firstRes.error
if (resolutionError) {
throw new CompanyContextError(
`Active company resolution failed: ${resolutionError.message}`,
'resolution_failed'
)
}
const prefs = prefsRes.data
const firstCompany = firstRes.data
if (prefs?.active_company_id) {
if (firstCompany && prefs.active_company_id === firstCompany.company_id) {
return firstCompany.company_id
}
// Preference points at a different company than the first membership:
// validate it still resolves to a non-archived company the user is a
// member of before trusting it.
const { data: membership, error: membershipError } = await supabase
.from('company_members')
.select('company_id, companies!inner(archived_at)')
.eq('company_id', prefs.active_company_id)
.eq('user_id', userId)
.is('companies.archived_at', null)
.maybeSingle()
// Falling back to the first membership on a FAILED validation would
// silently switch a multi-company user's active company: fail loudly.
if (membershipError) {
throw new CompanyContextError(
`Active company validation failed: ${membershipError.message}`,
'resolution_failed'
)
}
if (membership) return membership.company_id
}
// Fallback: first non-archived membership by created_at (already fetched)
return firstCompany?.company_id ?? null
}
+6 -153
View File
@@ -2,162 +2,15 @@ import type { SupabaseClient } from '@supabase/supabase-js'
import { fetchAllRows } from '@/lib/supabase/fetch-all'
import { cookies } from 'next/headers'
import type { EntityType } from '@/types'
import { CompanyContextError, getActiveCompanyId } from '@/lib/company/active-company'
// The resolver and its error class live in active-company.ts (no
// `next/headers` there) so the API-key path can use them; re-exported here so
// every existing import site and test mock keeps working.
export { CompanyContextError, getActiveCompanyId }
const COMPANY_COOKIE = 'gnubok-company-id'
/**
* Thrown by setActiveCompany so callers can tell a permissions problem
* ('not_member') apart from a failed/unverified database write
* ('persist_failed'), and by getActiveCompanyId when a resolution query
* fails ('resolution_failed': the active company is unknown right now,
* which is NOT the same as the user having no companies).
*/
export class CompanyContextError extends Error {
constructor(
message: string,
readonly code: 'not_member' | 'persist_failed' | 'resolution_failed'
) {
super(message)
this.name = 'CompanyContextError'
}
}
/**
* Get the active company ID for the authenticated user.
*
* Resolution order: user_preferences → first non-archived membership.
*
* `user_preferences.active_company_id` is the authoritative source. The
* cookie `gnubok-company-id` is written as a hint for backwards-compat but
* is no longer READ as a source of truth, because Postgres RLS (via
* `current_active_company_id()`) can only read the database, not cookies.
* Having Next.js and RLS both read from `user_preferences` keeps them
* perfectly in sync.
*
* RPC-first: tries `resolve_active_company()` (one round trip, semantically
* identical to the query path and to `current_active_company_id()`), falling
* back to the original query path when the function is not deployed
* (PGRST202), the caller lacks EXECUTE (42501: service-role clients), or the
* RPC returns zero rows (NULL auth.uid(), also service-role clients).
*
* Returns null only when the user positively has no non-archived companies.
* Throws CompanyContextError('resolution_failed') when a query fails: a
* transient failure must never read as "no companies", because callers
* redirect that state to the onboarding wizard (issue #1053).
*/
export async function getActiveCompanyId(
supabase: SupabaseClient,
userId: string
): Promise<string | null> {
const { data, error } = await supabase.rpc('resolve_active_company')
if (error) {
// PGRST202: function not in the schema cache (self-hosted instance not
// migrated yet, or a deploy racing the branch merge).
// 42501: EXECUTE is granted to `authenticated` only, so a service-role
// client is refused. These fallbacks are LOAD-BEARING, not defensive:
// app/api/mcp-oauth/token/route.ts and app/api/events/route.ts (API-key
// branch) call requireCompanyId with createServiceClientNoCookies(), and
// must silently resolve via the query path or the OAuth token flow breaks.
if (error.code === 'PGRST202' || error.code === '42501') {
return getActiveCompanyIdViaQueries(supabase, userId)
}
throw new CompanyContextError(
`Active company resolution failed: ${error.message}`,
'resolution_failed'
)
}
const row = (Array.isArray(data) ? data[0] : data) as
| { company_id: string | null; locale: string | null; used_fallback: boolean }
| undefined
| null
if (!row) {
// Zero rows = NULL auth.uid() inside the RPC, i.e. a service-role client
// (same call sites as the 42501 branch above). The query path filters by
// the explicit userId param and still resolves correctly.
return getActiveCompanyIdViaQueries(supabase, userId)
}
return row.company_id ?? null
}
/**
* Query-path resolution: the pre-RPC implementation, kept verbatim as the
* fallback for getActiveCompanyId (see the fallback conditions there).
*/
async function getActiveCompanyIdViaQueries(
supabase: SupabaseClient,
userId: string
): Promise<string | null> {
// user_preferences (authoritative) + first membership, fetched in parallel:
// the fallback query result doubles as validation when the preferred
// company happens to be the first membership, which is the common
// single-company case. Most requests pay one round trip instead of two
// sequential ones. This runs on every withRouteContext API request and
// every dashboard layout render, so the sequential version was pure
// wall-clock cost. Mirrors resolveCompanyForMiddleware, minus the
// write-back (read paths shouldn't write).
const [prefsRes, firstRes] = await Promise.all([
supabase
.from('user_preferences')
.select('active_company_id')
.eq('user_id', userId)
.maybeSingle(),
supabase
.from('company_members')
.select('company_id, companies!inner(archived_at)')
.eq('user_id', userId)
.is('companies.archived_at', null)
.order('created_at', { ascending: true })
.limit(1)
.maybeSingle(),
])
const resolutionError = prefsRes.error ?? firstRes.error
if (resolutionError) {
throw new CompanyContextError(
`Active company resolution failed: ${resolutionError.message}`,
'resolution_failed'
)
}
const prefs = prefsRes.data
const firstCompany = firstRes.data
if (prefs?.active_company_id) {
if (firstCompany && prefs.active_company_id === firstCompany.company_id) {
return firstCompany.company_id
}
// Preference points at a different company than the first membership:
// validate it still resolves to a non-archived company the user is a
// member of before trusting it.
const { data: membership, error: membershipError } = await supabase
.from('company_members')
.select('company_id, companies!inner(archived_at)')
.eq('company_id', prefs.active_company_id)
.eq('user_id', userId)
.is('companies.archived_at', null)
.maybeSingle()
// Falling back to the first membership on a FAILED validation would
// silently switch a multi-company user's active company: fail loudly.
if (membershipError) {
throw new CompanyContextError(
`Active company validation failed: ${membershipError.message}`,
'resolution_failed'
)
}
if (membership) return membership.company_id
}
// Fallback: first non-archived membership by created_at (already fetched)
return firstCompany?.company_id ?? null
}
/**
* Resolve a company's effective entity type.
*
+13
View File
@@ -2858,6 +2858,19 @@ const SALARY: Record<string, StructuredErrorEntry> = {
message_sv: 'Företaget kunde inte hittas.',
message_en: 'Company not found.',
},
// An API key minted for an account that has not created its first company
// yet (signup from the MCP OAuth popup, issue #1814). Not a lookup miss:
// there is nothing to look up until the company exists.
NO_COMPANY_YET: {
httpStatus: 409,
message_sv: 'Kontot har inget företag ännu. Skapa företaget i appen och försök igen.',
message_en: 'This account has no company yet. Create the company in the web app, then retry; the connection picks it up automatically.',
remediation: {
description:
'Ask the user to finish company setup in the Accounted web app (/onboarding). No re-authentication is needed afterwards: the same connection binds to the new company on its next call.',
tool: 'gnubok_list_companies',
},
},
// Phase 5 PR-1 carry-over: distinct error code for the salary-run DELETE
// FK-null guard so an operator seeing this in logs knows a journal entry
// is at risk, not just a status race.