Skip to content

fix: restrict admin credit grants to internal API #2113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jonit-dev wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: restrict admin credit grants to internal API #2113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 4 additions & 3 deletions messages/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,23 +180,24 @@
"admin-credits-col-notes": "Notes",
"admin-credits-col-org": "Organization",
"admin-credits-current-balance": "Current Balance",
"admin-credits-description": "Grant credits to customer organizations. All grants are logged with admin user ID.",
"admin-credits-description": "Review customer credit balances, grant history, and credit usage analytics.",
"admin-credits-expires": "Next expiration",
"admin-credits-expires-months": "Expires in (months)",
"admin-credits-grant-button": "Grant {amount} Credits",
"admin-credits-grant-error": "Failed to grant credits. Please try again.",
"admin-credits-grant-success": "Successfully granted {amount} credits to {org}",
"admin-credits-grant-title": "Grant Credits to Organization",
"admin-credits-lookup-title": "Organization Credit Lookup",
"admin-credits-grants-load-error": "Failed to load grant history. Please try again.",
"admin-credits-no-balance": "No credits yet",
"admin-credits-no-grants": "No admin grants yet",
"admin-credits-notes-label": "Notes (optional)",
"admin-credits-notes-placeholder": "Reason for granting credits...",
"admin-credits-recent-grants": "Recent Admin Grants",
"admin-credits-readonly-note": "Credit grants are handled through internal operations.",
"admin-credits-search-error": "Failed to search organizations. Please try again.",
"admin-credits-search-placeholder": "Search by name, email, or org ID...",
"admin-credits-select-org": "Select Organization",
"admin-credits-title": "Grant Credits",
"admin-credits-title": "Credits",
"admin-dashboard": "Admin Dashboard",
"admin-users-email-type-breakdown": "Email Type Breakdown",
"admin-users-email-type-breakdown-description": "Registration mix in the selected period, split between professional, personal, and disposable email domains.",
Expand Down
129 changes: 5 additions & 124 deletions src/pages/admin/dashboard/credits.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ meta:
</route>

<script setup lang="ts">
import { FormKit } from '@formkit/vue'
import dayjs from 'dayjs'
import { computed, onMounted, onUnmounted, ref, watch } from 'vue'
import { useI18n } from 'vue-i18n'
Expand Down Expand Up @@ -74,21 +73,6 @@ const selectedOrg = ref<OrgSearchResult | null>(null)
const orgBalance = ref<OrgBalance | null>(null)
const isLoadingBalance = ref(false)

const creditAmountStr = ref('100')
const creditNotes = ref('')
const expiresInMonthsStr = ref('12')
const isGranting = ref(false)

const creditAmount = computed(() => {
const parsed = Number.parseInt(creditAmountStr.value, 10)
return Number.isNaN(parsed) ? 0 : parsed
})

const expiresInMonths = computed(() => {
const parsed = Number.parseInt(expiresInMonthsStr.value, 10)
return Number.isNaN(parsed) ? 12 : parsed
})

const recentGrants = ref<AdminGrant[]>([])
const isLoadingGrants = ref(false)
const globalStatsTrendData = ref<GlobalStatsTrendRow[]>([])
Expand All @@ -98,12 +82,6 @@ let searchDebounce: ReturnType<typeof setTimeout> | null = null
let creditAnalyticsRequestSeq = 0
let currentSearchQuery = '' // Track current query to avoid race conditions

function getExpiresAt() {
if (expiresInMonths.value <= 0)
return null
return dayjs().add(expiresInMonths.value, 'month').toISOString()
}

function formatCredits(value: number) {
return new Intl.NumberFormat(undefined, { minimumFractionDigits: 2, maximumFractionDigits: 2 }).format(value)
}
Expand Down Expand Up @@ -315,59 +293,6 @@ async function loadOrgBalance(orgId: string) {
}
}

async function grantCredits() {
if (!selectedOrg.value)
return

if (creditAmount.value < 1) {
toast.error(t('admin-credits-amount-required'))
return
}

isGranting.value = true

try {
const { data } = await supabase.auth.getSession()
const response = await fetch(`${defaultApiHost}/private/admin_credits/grant`, {
method: 'POST',
headers: {
'authorization': `Bearer ${data.session?.access_token}`,
'Content-Type': 'application/json',
},
body: JSON.stringify({
org_id: selectedOrg.value.id,
amount: creditAmount.value,
notes: creditNotes.value || undefined,
expires_at: getExpiresAt(),
}),
})

if (!response.ok) {
const errorData = await response.json() as { message?: string }
throw new Error(errorData.message || 'Grant failed')
}

toast.success(t('admin-credits-grant-success', { amount: creditAmount.value, org: selectedOrg.value.name }))

// Refresh balance and grants
await Promise.all([
loadOrgBalance(selectedOrg.value.id),
loadRecentGrants(),
])

// Reset form
creditAmountStr.value = '100'
creditNotes.value = ''
}
catch (error) {
console.error('Grant error:', error)
toast.error(t('admin-credits-grant-error'))
}
finally {
isGranting.value = false
}
}

async function loadRecentGrants() {
isLoadingGrants.value = true

Expand Down Expand Up @@ -495,10 +420,10 @@ onMounted(async () => {
</ChartCard>
</div>

<!-- Grant Form Card -->
<!-- Credit Lookup Card -->
<div class="p-6 bg-white border rounded-lg shadow-lg border-slate-300 dark:bg-gray-800 dark:border-slate-900">
<h2 class="mb-6 text-lg font-semibold text-gray-900 dark:text-white">
{{ t('admin-credits-grant-title') }}
{{ t('admin-credits-lookup-title') }}
</h2>

<div class="space-y-6">
Expand Down Expand Up @@ -586,53 +511,9 @@ onMounted(async () => {
</div>
</div>

<!-- Grant Form Fields -->
<div v-if="selectedOrg" class="grid gap-6 md:grid-cols-2">
<FormKit
v-model="creditAmountStr"
type="number"
name="creditAmount"
:label="t('admin-credits-amount-label')"
validation="required|min:1"
:min="1"
:step="1"
outer-class="!mb-0"
/>

<FormKit
v-model="expiresInMonthsStr"
type="number"
name="expiresInMonths"
:label="t('admin-credits-expires-months')"
:min="1"
:max="60"
:step="1"
outer-class="!mb-0"
/>
</div>

<FormKit
v-if="selectedOrg"
v-model="creditNotes"
type="textarea"
name="notes"
:label="t('admin-credits-notes-label')"
:placeholder="t('admin-credits-notes-placeholder')"
rows="2"
outer-class="!mb-0"
/>

<!-- Submit Button -->
<button
v-if="selectedOrg"
type="button"
:disabled="isGranting || creditAmount < 1"
class="flex items-center justify-center w-full px-6 py-3 text-white transition-colors bg-blue-600 rounded-lg hover:bg-blue-700 disabled:opacity-50 disabled:cursor-not-allowed"
@click="grantCredits"
>
<Spinner v-if="isGranting" size="w-5 h-5" class="mr-2" color="white" />
{{ t('admin-credits-grant-button', { amount: creditAmount }) }}
</button>
<p v-if="selectedOrg" class="text-sm text-gray-500 dark:text-gray-400">
{{ t('admin-credits-readonly-note') }}
</p>
</div>
</div>

Expand Down
23 changes: 7 additions & 16 deletions supabase/functions/_backend/private/admin_credits.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { Context } from 'hono'
import type { MiddlewareKeyVariables } from '../utils/hono.ts'
import { type } from 'arktype'
import { safeParseSchema } from '../utils/ark_validation.ts'
import { createHono, parseBody, simpleError, useCors } from '../utils/hono.ts'
import { createHono, middlewareAPISecret, parseBody, simpleError, useCors } from '../utils/hono.ts'
import { middlewareV2 } from '../utils/hono_middleware.ts'
import { cloudlog, cloudlogErr } from '../utils/logging.ts'
import { supabaseAdmin, supabaseClient } from '../utils/supabase.ts'
Expand Down Expand Up @@ -53,15 +53,10 @@ export const app = createHono('', version)

app.use('*', useCors)

// Grant credits to an organization (admin only)
app.post('/grant', middlewareV2(['all']), async (c) => {
const { isAdmin, userId } = await verifyAdmin(c)

if (!isAdmin) {
cloudlog({ requestId: c.get('requestId'), message: 'not_admin_grant_attempt', userId })
throw simpleError('not_admin', 'Only admin users can grant credits')
}

// Grant credits to an organization. This is intentionally not exposed to
// platform-admin JWTs: platform-admin user actions must stay read-only except
// impersonation, so credit mutations require the internal API secret.
app.post('/grant', middlewareAPISecret, async (c) => {
const body = await parseBody<GrantRequest>(c)
const parsedBodyResult = safeParseSchema(grantSchema, body)

Expand All @@ -74,7 +69,6 @@ app.post('/grant', middlewareV2(['all']), async (c) => {
cloudlog({
requestId: c.get('requestId'),
message: 'admin_credit_grant_request',
adminUserId: userId,
org_id,
amount,
notes,
Expand All @@ -100,8 +94,7 @@ app.post('/grant', middlewareV2(['all']), async (c) => {
}

const sourceRef = {
admin_user_id: userId,
granted_via: 'admin_ui',
granted_via: 'internal_api',
org_name: org.name,
}

Expand All @@ -111,7 +104,7 @@ app.post('/grant', middlewareV2(['all']), async (c) => {
p_amount: amount,
p_expires_at: expires_at || undefined,
p_source: 'manual',
p_notes: notes || `Admin grant by ${userId}`,
p_notes: notes || 'Internal credit grant',
p_source_ref: sourceRef,
})

Expand All @@ -121,7 +114,6 @@ app.post('/grant', middlewareV2(['all']), async (c) => {
message: 'admin_credit_grant_failed',
org_id,
amount,
adminUserId: userId,
error: rpcError,
})
throw simpleError('grant_failed', 'Failed to grant credits', { error: rpcError })
Expand All @@ -130,7 +122,6 @@ app.post('/grant', middlewareV2(['all']), async (c) => {
cloudlog({
requestId: c.get('requestId'),
message: 'admin_credit_grant_success',
adminUserId: userId,
org_id,
org_name: org.name,
amount,
Expand Down
21 changes: 21 additions & 0 deletions tests/admin-credits-auth-boundary.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { describe, expect, it } from 'vitest'
import { app as adminCreditsApp } from '../supabase/functions/_backend/private/admin_credits.ts'

describe('admin credits auth boundary', () => {
it('does not allow platform-admin JWTs to reach credit grants', async () => {
const response = await adminCreditsApp.request(new Request('http://localhost/grant', {
method: 'POST',
headers: {
'Authorization': 'Bearer test.jwt.value',
'Content-Type': 'application/json',
},
body: JSON.stringify({
org_id: '550e8400-e29b-41d4-a716-446655440000',
amount: 25,
}),
}))

expect(response.status).toBe(400)
await expect(response.text()).resolves.toContain('Cannot find authorization')
})
Comment on lines +5 to +20
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

This doesn't actually prove top_up_usage_credits was unreachable.

The test only checks the rejected response. If the handler ever regressed into calling supabaseAdmin().rpc('top_up_usage_credits', ...) before failing, this would still pass. Please mock/spy the RPC path and assert it is never invoked so the test matches its title and the PR objective.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/admin-credits-auth-boundary.unit.test.ts` around lines 5 - 20, The test
currently only asserts the HTTP response; you must also spy/mock the Supabase
admin RPC to ensure top_up_usage_credits is never invoked: in the test for 'does
not allow platform-admin JWTs to reach credit grants' obtain the mocked supabase
admin client (supabaseAdmin) and attach a spy or mock to its rpc method (the
call name 'top_up_usage_credits'), then after making adminCreditsApp.request
assert that supabaseAdmin().rpc was never called (or rpc was not invoked with
'top_up_usage_credits') so the test proves the handler never reached the RPC
path.

})
Loading
Loading