-
Notifications
You must be signed in to change notification settings - Fork 966
refactor(streams): replace STREAMS_SECRET with session-based auth #1360
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,6 @@ | ||
| import { db } from "@superset/db"; | ||
| import { sessions } from "@superset/db/schema/auth"; | ||
| import { and, eq, gt } from "drizzle-orm"; | ||
|
Comment on lines
+1
to
+3
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Locate packages/db directory structure
cd apps/streams || cd . 2>/dev/null
find . -path "*/packages/db*" -type d 2>/dev/null | head -5Repository: superset-sh/superset Length of output: 46 🏁 Script executed: #!/bin/bash
# Search for database client initialization in packages/db
fd -t f '\.(ts|js)$' packages/db --exec head -50 {} \; 2>/dev/null | head -100Repository: superset-sh/superset Length of output: 2334 🏁 Script executed: #!/bin/bash
# Look for DATABASE_URL references in packages/db
rg 'DATABASE_URL' packages/db 2>/dev/null || echo "No matches found"Repository: superset-sh/superset Length of output: 415 🏁 Script executed: #!/bin/bash
# Check the structure of packages/db
ls -la packages/db/ 2>/dev/null || echo "Directory not found at packages/db"Repository: superset-sh/superset Length of output: 484 🏁 Script executed: #!/bin/bash
# Get the full content of env.ts in packages/db
cat packages/db/src/env.tsRepository: superset-sh/superset Length of output: 554 🏁 Script executed: #!/bin/bash
# Check if there's any dotenv config or initialization in packages/db
rg 'config|dotenv' packages/db/src/ -A 2 -B 2Repository: superset-sh/superset Length of output: 1423 Remove redundant dotenv config from client.ts; DATABASE_URL resolution is already handled correctly by env.ts. The 🤖 Prompt for AI Agents |
||
| import { Hono } from "hono"; | ||
| import { cors } from "hono/cors"; | ||
| import { logger } from "hono/logger"; | ||
|
|
@@ -16,15 +19,20 @@ import { | |
| } from "./routes"; | ||
| import type { AIDBProtocolOptions } from "./types"; | ||
|
|
||
| type SessionEnv = { | ||
| Variables: { | ||
| userId: string; | ||
| }; | ||
| }; | ||
|
|
||
| export interface AIDBProxyServerOptions extends AIDBProtocolOptions { | ||
| cors?: boolean; | ||
| logging?: boolean; | ||
| corsOrigins?: string | string[]; | ||
| authToken?: string; | ||
| } | ||
|
|
||
| export function createServer(options: AIDBProxyServerOptions) { | ||
| const app = new Hono(); | ||
| const app = new Hono<SessionEnv>(); | ||
|
|
||
| const protocol = new AIDBSessionProtocol({ | ||
| baseUrl: options.baseUrl, | ||
|
|
@@ -56,16 +64,26 @@ export function createServer(options: AIDBProxyServerOptions) { | |
| app.route("/health", createHealthRoutes()); | ||
|
|
||
| // No auth on health; Bearer token required on /v1/* | ||
| if (options.authToken) { | ||
| const expectedHeader = `Bearer ${options.authToken}`; | ||
| app.use("/v1/*", async (c, next) => { | ||
| const authorization = c.req.header("Authorization"); | ||
| if (authorization !== expectedHeader) { | ||
| return c.json({ error: "Unauthorized" }, 401); | ||
| } | ||
| return next(); | ||
| }); | ||
| } | ||
| app.use("/v1/*", async (c, next) => { | ||
| const authorization = c.req.header("Authorization"); | ||
| if (!authorization?.startsWith("Bearer ")) { | ||
| return c.json({ error: "Unauthorized" }, 401); | ||
| } | ||
|
|
||
| const token = authorization.slice(7); | ||
| const [session] = await db | ||
| .select({ userId: sessions.userId }) | ||
| .from(sessions) | ||
| .where(and(eq(sessions.token, token), gt(sessions.expiresAt, new Date()))) | ||
| .limit(1); | ||
|
|
||
| if (!session) { | ||
| return c.json({ error: "Unauthorized" }, 401); | ||
| } | ||
|
|
||
| c.set("userId", session.userId); | ||
| return next(); | ||
| }); | ||
|
|
||
| const v1 = new Hono(); | ||
| v1.route("/sessions", createSessionRoutes(protocol)); | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quote the variable to prevent word splitting.
Per the static analysis hint (SC2086),
$DATABASE_URLshould be double-quoted to prevent globbing and word splitting, especially since connection strings can contain special characters.Proposed fix
- name: Load database URL run: | source database-status.env - echo "DATABASE_URL=$DATABASE_URL" >> $GITHUB_ENV + echo "DATABASE_URL=${DATABASE_URL}" >> "$GITHUB_ENV"📝 Committable suggestion
🧰 Tools
🪛 actionlint (1.7.10)
[error] 138-138: shellcheck reported issue in this script: SC2086:info:2:38: Double quote to prevent globbing and word splitting
(shellcheck)
🤖 Prompt for AI Agents