Skip to content

Commit 190c968

Browse files
suibianwanwanclaude
andcommitted
fix(auth): make oauthLoginParam opt-in so PAT/password logins stop hitting portal 5014
loginWithRetry unconditionally injected oauthLoginParam into every login. Portals that don't implement OAuth authorization-code login (e.g. ap-southeast-1-aws.api.singdata.com) reject any credential login carrying it with business code 5014, breaking pure username/password profiles. oauthLoginParam/PKCE are now gated behind an opt-in `oauth` flag (default false) on loginWithPat/loginWithPassword; the authorizationCode exchange is guarded by `pkce && data.authorizationCode`. Only loginWithBrowser mints refresh tokens; runtime refresh stays a plain credential exchange. Keeps the four login methods (PAT, password, browser OAuth, cookie) independent. Rewrites token-store/token-refresh/login-oauth tests to seed persisted OAuth tokens via TokenStore instead of assuming a password login returns an OAuth token. Translates stray Chinese comments in pkce test, task.ts, poll.ts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 628af47 commit 190c968

8 files changed

Lines changed: 191 additions & 162 deletions

File tree

packages/clickzetta-sdk/src/auth/login.ts

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -95,22 +95,32 @@ async function loginWithRetry(
9595
baseUrl: string,
9696
body: Record<string, unknown>,
9797
instance: string,
98+
oauth: boolean,
9899
): Promise<AuthToken> {
99100
let lastError: Error | undefined
100101
// client.py:305 — one request id for the entire login attempt sequence
101102
const requestId = generateRequestId()
102103

104+
// The three login methods (PAT, password, browser OAuth) are independent.
105+
// PAT and password logins are plain credential exchanges and must NOT carry
106+
// `oauthLoginParam` — portals that don't implement OAuth authorization-code
107+
// login reject the whole request (business code 5014, "missing required
108+
// parameter or otherwise malformed"). `oauthLoginParam` is attached ONLY when
109+
// the caller explicitly opts into the OAuth upgrade (`oauth === true`).
110+
//
103111
// PKCE is generated once per login sequence; codeVerifier stays in memory
104112
// only and is never logged. codeChallenge is sent to the portal so the
105113
// gateway can later validate the matching verifier at /oauth2/token.
106-
const pkce = generatePkce()
107-
const loginBody = {
108-
...body,
109-
oauthLoginParam: buildOauthLoginParam({
110-
redirectUri: OAUTH_REDIRECT_URI,
111-
codeChallenge: pkce.codeChallenge,
112-
}),
113-
}
114+
const pkce = oauth ? generatePkce() : undefined
115+
const loginBody = pkce
116+
? {
117+
...body,
118+
oauthLoginParam: buildOauthLoginParam({
119+
redirectUri: OAUTH_REDIRECT_URI,
120+
codeChallenge: pkce.codeChallenge,
121+
}),
122+
}
123+
: body
114124

115125
for (let attempt = 0; attempt <= LOGIN_MAX_RETRIES; attempt++) {
116126
try {
@@ -128,9 +138,11 @@ async function loginWithRetry(
128138
} else {
129139
const data = resp.data
130140
// OAuth path: a non-empty authorizationCode means the portal opted
131-
// into the code exchange. Swap the legacy token for the OAuth tokens
132-
// while keeping the portal-issued instanceId/userId.
133-
if (data.authorizationCode) {
141+
// into the code exchange. Only reachable when we sent PKCE (oauth ===
142+
// true); the guard keeps this correct even if a portal echoes a code
143+
// for a plain credential login. Swap the legacy token for the OAuth
144+
// tokens while keeping the portal-issued instanceId/userId.
145+
if (pkce && data.authorizationCode) {
134146
const oauth = await exchangeAuthorizationCode(baseUrl, data.authorizationCode, pkce.codeVerifier, OAUTH_REDIRECT_URI)
135147
return {
136148
token: oauth.accessToken,
@@ -174,20 +186,23 @@ export async function loginWithPat(
174186
baseUrl: string,
175187
pat: string,
176188
instanceName: string,
189+
oauth = false,
177190
): Promise<AuthToken> {
178-
return loginWithRetry(baseUrl, { accessToken: pat, instanceName }, instanceName)
191+
return loginWithRetry(baseUrl, { accessToken: pat, instanceName }, instanceName, oauth)
179192
}
180193

181194
export async function loginWithPassword(
182195
baseUrl: string,
183196
username: string,
184197
password: string,
185198
instanceName: string,
199+
oauth = false,
186200
): Promise<AuthToken> {
187201
return loginWithRetry(
188202
baseUrl,
189203
{ username, password, instanceName },
190204
instanceName,
205+
oauth,
191206
)
192207
}
193208

packages/clickzetta-sdk/src/sql/poll.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ export function coerceValue(value: string | null, typeCategory: string, timezone
260260
return value.toLowerCase() === "true"
261261
}
262262

263-
// 时间类型
263+
// Temporal types
264264
// DATE: TEXT format is already "YYYY-MM-DD", return as ISO string.
265265
if (upper === "DATE") {
266266
return value.trim()
@@ -275,7 +275,7 @@ export function coerceValue(value: string | null, typeCategory: string, timezone
275275
return normaliseTimestampText(value, upper)
276276
}
277277

278-
// 二进制
278+
// Binary types
279279
if (upper === "BINARY" || upper === "VARBINARY") {
280280
return hexToBytes(value)
281281
}

packages/clickzetta-sdk/src/studio/task.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ export interface SaveCdcTaskParams {
319319

320320
export interface CdcStartupPositionConfig {
321321
datasourceId: number | string
322-
startupMode: number // 2=指定时间, 3=指定文件
322+
startupMode: number // 2=specific time, 3=specific file
323323
startTimestamp?: string // unix ms string, for startupMode=2
324324
file?: string // binlog file, for startupMode=3
325325
pos?: string // binlog offset, for startupMode=3
@@ -329,7 +329,7 @@ export interface CdcTaskStartParams {
329329
fileId: number
330330
updateBy: string
331331
workspace: string
332-
startupMode?: number // 0=无状态启动, 1=从上次保存状态恢复, 4=自定义起始位置
332+
startupMode?: number // 0=stateless start, 1=resume from last saved state, 4=custom start position
333333
engineType?: number // 5=default
334334
snapshotTaskSwitch?: number // 0=off, 1=on
335335
snapshotTaskPoolSize?: number // snapshot concurrency (default 1, only when snapshotTaskSwitch=1)

packages/clickzetta-sdk/src/types/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ export interface AuthToken {
5353
userId: number
5454
expireTimeMs: number
5555
obtainedAt: number
56-
refreshToken?: string // OAuth refresh token;传统登录模式下为 undefined
56+
refreshToken?: string // OAuth refresh token; undefined for legacy (PAT/password) logins
5757
// OAuth issuer host (no protocol, e.g. "api.clickzetta.com") — the OIDC
5858
// authorization server that issued this token. OAuth `/oauth2/token` is ONLY
5959
// served by the issuer, NOT the region business host in `config.service`

packages/clickzetta-sdk/test/login-oauth.test.ts

Lines changed: 58 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { afterEach, describe, expect, test } from "bun:test"
22
import { createHash } from "node:crypto"
33

4-
import { loginWithPassword } from "../src/auth/login.js"
4+
import { loginWithPassword, loginWithPat } from "../src/auth/login.js"
55

66
function base64Url(input: Buffer): string {
77
return input.toString("base64").replace(/\+/g, "-").replace(/\//g, "_").replace(/=+$/g, "")
@@ -13,8 +13,58 @@ afterEach(() => {
1313
globalThis.fetch = originalFetch
1414
})
1515

16-
describe("OAuth login", () => {
17-
test("loginWithPassword sends oauthLoginParam and exchanges authorizationCode", async () => {
16+
describe("login OAuth opt-in", () => {
17+
test("password login stays a plain credential exchange by default (no oauthLoginParam)", async () => {
18+
let loginPayload: Record<string, unknown> | undefined
19+
let tokenExchangeCalls = 0
20+
globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => {
21+
const url = new URL(String(input))
22+
if (url.pathname === "/clickzetta-portal/user/loginSingle" && init?.method === "POST") {
23+
loginPayload = JSON.parse(String(init.body)) as Record<string, unknown>
24+
return new Response(JSON.stringify({
25+
code: 0,
26+
data: { token: "legacy-token", userId: 7, instanceId: 9, expireTime: 123 },
27+
}), { status: 200, headers: { "content-type": "application/json" } })
28+
}
29+
if (url.pathname === "/clickzetta-hornhub/oauth2/token") tokenExchangeCalls += 1
30+
return new Response("not found", { status: 404 })
31+
}) as typeof fetch
32+
33+
const token = await loginWithPassword("https://service.example.com", "user", "pass", "inst")
34+
35+
// The whole payload is just credentials — no OAuth upgrade fields leak in.
36+
expect(loginPayload?.username).toBe("user")
37+
expect(loginPayload?.password).toBe("pass")
38+
expect(loginPayload?.instanceName).toBe("inst")
39+
expect(loginPayload?.oauthLoginParam).toBeUndefined()
40+
expect(token.token).toBe("legacy-token")
41+
expect(token.refreshToken).toBeUndefined()
42+
expect(tokenExchangeCalls).toBe(0)
43+
})
44+
45+
test("PAT login stays a plain credential exchange by default (no oauthLoginParam)", async () => {
46+
let loginPayload: Record<string, unknown> | undefined
47+
globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => {
48+
const url = new URL(String(input))
49+
if (url.pathname === "/clickzetta-portal/user/loginSingle" && init?.method === "POST") {
50+
loginPayload = JSON.parse(String(init.body)) as Record<string, unknown>
51+
return new Response(JSON.stringify({
52+
code: 0,
53+
data: { token: "legacy-token", userId: 7, instanceId: 9, expireTime: 123 },
54+
}), { status: 200, headers: { "content-type": "application/json" } })
55+
}
56+
return new Response("not found", { status: 404 })
57+
}) as typeof fetch
58+
59+
const token = await loginWithPat("https://service.example.com", "my-pat", "inst")
60+
61+
expect(loginPayload?.accessToken).toBe("my-pat")
62+
expect(loginPayload?.instanceName).toBe("inst")
63+
expect(loginPayload?.oauthLoginParam).toBeUndefined()
64+
expect(token.token).toBe("legacy-token")
65+
})
66+
67+
test("explicit oauth opt-in sends oauthLoginParam and exchanges authorizationCode", async () => {
1868
let loginPayload: Record<string, unknown> | undefined
1969
let tokenPayload: URLSearchParams | undefined
2070
globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => {
@@ -44,12 +94,9 @@ describe("OAuth login", () => {
4494
return new Response("not found", { status: 404 })
4595
}) as typeof fetch
4696

47-
const token = await loginWithPassword("https://service.example.com", "user", "pass", "inst")
97+
const token = await loginWithPassword("https://service.example.com", "user", "pass", "inst", true)
4898

4999
const oauthLoginParam = loginPayload?.oauthLoginParam as Record<string, unknown>
50-
expect(loginPayload?.username).toBe("user")
51-
expect(loginPayload?.password).toBe("pass")
52-
expect(loginPayload?.instanceName).toBe("inst")
53100
expect(oauthLoginParam.oauthLogin).toBe(true)
54101
expect(oauthLoginParam.clientId).toBe("official-cli")
55102
expect(oauthLoginParam.redirectUri).toBe("http://127.0.0.1/callback")
@@ -67,28 +114,21 @@ describe("OAuth login", () => {
67114
expect(token.expireTimeMs).toBe(900_000)
68115
})
69116

70-
test("loginWithPassword keeps legacy token when authorizationCode is absent", async () => {
117+
test("oauth opt-in without a returned authorizationCode keeps the legacy token", async () => {
71118
let tokenExchangeCalls = 0
72119
globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => {
73120
const url = new URL(String(input))
74121
if (url.pathname === "/clickzetta-portal/user/loginSingle" && init?.method === "POST") {
75122
return new Response(JSON.stringify({
76123
code: 0,
77-
data: {
78-
token: "legacy-token",
79-
userId: 7,
80-
instanceId: 9,
81-
expireTime: 123,
82-
},
124+
data: { token: "legacy-token", userId: 7, instanceId: 9, expireTime: 123 },
83125
}), { status: 200, headers: { "content-type": "application/json" } })
84126
}
85-
if (url.pathname === "/clickzetta-hornhub/oauth2/token") {
86-
tokenExchangeCalls += 1
87-
}
127+
if (url.pathname === "/clickzetta-hornhub/oauth2/token") tokenExchangeCalls += 1
88128
return new Response("not found", { status: 404 })
89129
}) as typeof fetch
90130

91-
const token = await loginWithPassword("https://service.example.com", "user", "pass", "inst")
131+
const token = await loginWithPassword("https://service.example.com", "user", "pass", "inst", true)
92132

93133
expect(token.token).toBe("legacy-token")
94134
expect(tokenExchangeCalls).toBe(0)

packages/clickzetta-sdk/test/pkce.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ function base64Url(input: Buffer): string {
1111
const UNRESERVED = /^[A-Za-z0-9\-._~]+$/
1212

1313
describe("generatePkce", () => {
14-
// Property 1: PKCE 一致性 — codeChallenge == base64url(sha256(codeVerifier)),
15-
// codeVerifier 长度 ∈ [43,128] 且仅含 unreserved 字符。
14+
// Property 1: PKCE consistency — codeChallenge == base64url(sha256(codeVerifier)),
15+
// codeVerifier length ∈ [43,128] and uses only unreserved characters.
1616
// Validates: Requirements 2.1, 2.2
1717
test("codeChallenge equals base64url(sha256(codeVerifier)) with no padding", () => {
1818
for (let i = 0; i < 100; i++) {
@@ -25,7 +25,7 @@ describe("generatePkce", () => {
2525
}
2626
})
2727

28-
// Property 1: codeVerifier 长度 ∈ [43,128] 且仅含 RFC 7636 unreserved 字符。
28+
// Property 1: codeVerifier length ∈ [43,128] and uses only RFC 7636 unreserved characters.
2929
// Validates: Requirements 2.1
3030
test("codeVerifier length is within [43,128] and uses only unreserved characters", () => {
3131
for (let i = 0; i < 100; i++) {
@@ -36,7 +36,7 @@ describe("generatePkce", () => {
3636
}
3737
})
3838

39-
// Property 2: PKCE 唯一性连续多次生成的 codeVerifier 互不相同。
39+
// Property 2: PKCE uniquenessconsecutive generations produce distinct codeVerifier values.
4040
// Validates: Requirements 2.3
4141
test("multiple calls produce distinct codeVerifier values", () => {
4242
const verifiers = Array.from({ length: 100 }, () => generatePkce().codeVerifier)

0 commit comments

Comments
 (0)