Merge branch 'fix/stripe-key-redaction' into staging
This commit is contained in:
commit
bf60cfd99d
@ -352,7 +352,8 @@ function CreateOrgForm({ onCreated }: { onCreated: (slug: string) => void }) {
|
||||
});
|
||||
if (!res.ok) {
|
||||
const body = await res.text();
|
||||
throw new Error(`${res.status}: ${body}`);
|
||||
console.error(`[orgs] create ${res.status}: ${body}`);
|
||||
throw new Error(`Failed to create organization (${res.status})`);
|
||||
}
|
||||
onCreated(slug);
|
||||
} catch (e) {
|
||||
|
||||
@ -70,7 +70,7 @@ describe("startCheckout", () => {
|
||||
expect(body.cancel_url).toContain("checkout=cancel");
|
||||
});
|
||||
|
||||
it("throws with the body text on non-2xx so the UI can surface it", async () => {
|
||||
it("throws with status code on non-2xx; body is logged not surfaced", async () => {
|
||||
(global.fetch as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
ok: false,
|
||||
status: 402,
|
||||
@ -78,8 +78,11 @@ describe("startCheckout", () => {
|
||||
json: async () => ({}),
|
||||
});
|
||||
|
||||
// Status code must appear so callers know what happened.
|
||||
await expect(startCheckout("starter", "acme")).rejects.toThrow(/402/);
|
||||
await expect(startCheckout("starter", "acme")).rejects.toThrow(/payment required/);
|
||||
// Body text must NOT appear — it may contain Stripe API detail.
|
||||
await expect(startCheckout("starter", "acme")).rejects.toThrow(/checkout failed/);
|
||||
await expect(startCheckout("starter", "acme")).rejects.not.toThrow(/payment required/);
|
||||
});
|
||||
|
||||
it("sends users to /orgs on success, back to current page on cancel", async () => {
|
||||
|
||||
@ -120,8 +120,12 @@ export async function startCheckout(
|
||||
}),
|
||||
});
|
||||
if (!res.ok) {
|
||||
const text = await res.text();
|
||||
throw new Error(`checkout: ${res.status} ${text}`);
|
||||
// Never embed res.text() in the thrown error — the response body
|
||||
// may contain Stripe API error detail (e.g. invalid key, card decline
|
||||
// message, raw Stripe envelope) that should not reach the client.
|
||||
const detail = await res.text();
|
||||
console.error(`[billing] checkout ${res.status}: ${detail}`);
|
||||
throw new Error(`checkout failed (${res.status})`);
|
||||
}
|
||||
return res.json();
|
||||
}
|
||||
@ -141,8 +145,12 @@ export async function openBillingPortal(orgSlug: string): Promise<string> {
|
||||
body: JSON.stringify({ org_slug: orgSlug, return_url: returnUrl }),
|
||||
});
|
||||
if (!res.ok) {
|
||||
const text = await res.text();
|
||||
throw new Error(`portal: ${res.status} ${text}`);
|
||||
// Never embed res.text() in the thrown error — the response body
|
||||
// may contain Stripe API error detail (e.g. invalid key, card decline
|
||||
// message, raw Stripe envelope) that should not reach the client.
|
||||
const detail = await res.text();
|
||||
console.error(`[billing] portal ${res.status}: ${detail}`);
|
||||
throw new Error(`portal failed (${res.status})`);
|
||||
}
|
||||
const data = (await res.json()) as { url: string };
|
||||
return data.url;
|
||||
|
||||
@ -18,10 +18,12 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"crypto/subtle"
|
||||
"database/sql"
|
||||
"log"
|
||||
"net/http"
|
||||
"os"
|
||||
"strings"
|
||||
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/wsauth"
|
||||
@ -55,6 +57,20 @@ func (h *AdminTestTokenHandler) GetTestToken(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// IDOR fix (#112, CRITICAL): when ADMIN_TOKEN is set, require it
|
||||
// explicitly. Org-scoped tokens and session cookies must not grant
|
||||
// access — the original gap was that AdminAuth accepted any bearer
|
||||
// that matched a live org token, allowing cross-org token minting.
|
||||
adminSecret := os.Getenv("ADMIN_TOKEN")
|
||||
if adminSecret != "" {
|
||||
tok := c.GetHeader("Authorization")
|
||||
tok = strings.TrimPrefix(tok, "Bearer ")
|
||||
if tok == "" || subtle.ConstantTimeCompare([]byte(tok), []byte(adminSecret)) != 1 {
|
||||
c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "admin auth required"})
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
workspaceID := c.Param("id")
|
||||
if workspaceID == "" {
|
||||
c.JSON(http.StatusNotFound, gin.H{"error": "not found"})
|
||||
|
||||
@ -105,6 +105,61 @@ type putAllowlistRequest struct {
|
||||
EnabledBy string `json:"enabled_by"` // workspace ID of the admin performing the change
|
||||
}
|
||||
|
||||
// requireCallerOwnsOrg returns the caller's org workspace ID from the
|
||||
// request context, or "" if the caller is not an org-token holder.
|
||||
// Used to enforce org isolation on org-scoped routes — the org_token_id
|
||||
// is the org-scoped token's ID (not the org workspace ID), so we look
|
||||
// it up via the created_by workspace or a direct relationship.
|
||||
//
|
||||
// Returns ("", nil) when the caller is a session/ADMIN_TOKEN user (they
|
||||
// bypass via the session cookie path or ADMIN_TOKEN, not org tokens).
|
||||
func requireCallerOwnsOrg(c *gin.Context) (string, error) {
|
||||
tokenID, ok := c.Get("org_token_id")
|
||||
if !ok {
|
||||
return "", nil // not an org-token caller — caller is session/admin
|
||||
}
|
||||
tokID, ok := tokenID.(string)
|
||||
if !ok || tokID == "" {
|
||||
return "", nil
|
||||
}
|
||||
// Look up the org workspace that owns this token.
|
||||
// org_api_tokens has no org_id column, but we can look up the token's
|
||||
// created_by workspace and treat that as the caller's org anchor.
|
||||
var createdBy string
|
||||
err := db.DB.QueryRowContext(c.Request.Context(),
|
||||
`SELECT created_by FROM org_api_tokens WHERE id = $1`, tokID,
|
||||
).Scan(&createdBy)
|
||||
if err != nil || createdBy == "" {
|
||||
// Token has no created_by (CLI bootstrap path) — treat as unowned;
|
||||
// deny by default to prevent cross-org access.
|
||||
return "", fmt.Errorf("token has no org anchor")
|
||||
}
|
||||
return createdBy, nil
|
||||
}
|
||||
|
||||
// requireOrgOwnership verifies the caller has authority over the target org.
|
||||
// Returns 403 and abandons the request if the caller is an org-token holder
|
||||
// whose org does not match targetOrgID.
|
||||
func requireOrgOwnership(c *gin.Context, targetOrgID string) bool {
|
||||
callerOrg, err := requireCallerOwnsOrg(c)
|
||||
if err != nil {
|
||||
log.Printf("allowlist: requireOrgOwnership: %v", err)
|
||||
c.AbortWithStatusJSON(http.StatusForbidden, gin.H{"error": "org access denied"})
|
||||
return false
|
||||
}
|
||||
// callerOrg "" means session/admin user — they have full access (no
|
||||
// org token → full platform admin via session/ADMIN_TOKEN path).
|
||||
if callerOrg == "" {
|
||||
return true
|
||||
}
|
||||
if callerOrg != targetOrgID {
|
||||
log.Printf("allowlist: org-token org %s tried to access org %s (denied)", callerOrg, targetOrgID)
|
||||
c.AbortWithStatusJSON(http.StatusForbidden, gin.H{"error": "org access denied"})
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// GetAllowlist handles GET /orgs/:id/plugins/allowlist.
|
||||
//
|
||||
// Returns the current allowlist for the org workspace identified by :id.
|
||||
@ -128,6 +183,13 @@ func (h *OrgPluginAllowlistHandler) GetAllowlist(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// IDOR fix (#112, HIGH): org-token holders must only access their own org.
|
||||
// requireOrgOwnership denies cross-org access (403) while letting session
|
||||
// and ADMIN_TOKEN callers through.
|
||||
if !requireOrgOwnership(c, orgID) {
|
||||
return
|
||||
}
|
||||
|
||||
rows, err := db.DB.QueryContext(ctx, `
|
||||
SELECT plugin_name, enabled_by, enabled_at
|
||||
FROM org_plugin_allowlist
|
||||
@ -210,6 +272,11 @@ func (h *OrgPluginAllowlistHandler) PutAllowlist(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// IDOR fix (#112, HIGH): same as GetAllowlist — require org ownership.
|
||||
if !requireOrgOwnership(c, orgID) {
|
||||
return
|
||||
}
|
||||
|
||||
// Replace atomically: delete all current entries, then insert the new set.
|
||||
tx, err := db.DB.BeginTx(ctx, nil)
|
||||
if err != nil {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user