Conversation
🦋 Changeset detectedLatest commit: 3c4cfe1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughChangesWhatsApp ID registration linking
Sequence Diagram(s)sequenceDiagram
participant Client
participant RegistrationAPI
participant token.decode
participant createCredential
participant CredentialsDB
Client->>RegistrationAPI: Send WebAuthn registration request with Wa-Id
RegistrationAPI->>token.decode: Decode token with chat secret and wa-link audience
token.decode-->>RegistrationAPI: Return waId
RegistrationAPI->>createCredential: Pass waId with credential data
createCredential->>CredentialsDB: Insert credential and waId
CredentialsDB-->>Client: Return registration response
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to link a WhatsApp ID (waId) to a user's credential during registration. It adds a unique waId column to the credentials database table, validates the encrypted Wa-Id header in the registration endpoints, and implements JWT token utilities for encoding and decoding the WhatsApp ID. The feedback suggests adding a unit test to verify that attempting to register a duplicate waId is properly rejected.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| }); | ||
|
|
||
| it("rejects registration with a wa id token for another audience", async () => { |
There was a problem hiding this comment.
Add a unit test to verify that attempting to register a duplicate waId is rejected with a 400 Bad Request and the correct error code. Note that validation of injected dependencies (like parsing the address) can be omitted for trusted values in tests.
});
it("rejects registration if the wa id is already linked", async () => {
const id1 = "creda1";
const account1 = "0x1234567890123456789012345678901234567812";
vi.spyOn(derive, "default").mockReturnValue(account1);
const token = await encode("5491123456789", chatSecret, "wa-link");
const response1 = await registrationAppClient.index.$post(
{ json: registrationWebauthnAssertion({ id: id1, rawId: id1 }) },
{ headers: { cookie: "session_id=test-session", "Wa-Id": token } },
);
expect(response1.status).toBe(200);
await redis.set("test-session", "test-challenge");
const id2 = "creda2";
const account2 = "0x1234567890123456789012345678901234567813";
vi.spyOn(derive, "default").mockReturnValue(account2);
const response2 = await registrationAppClient.index.$post(
{ json: registrationWebauthnAssertion({ id: id2, rawId: id2 }) },
{ headers: { cookie: "session_id=test-session", "Wa-Id": token } },
);
expect(response2.status).toBe(400);
await expect(response2.json()).resolves.toStrictEqual({ code: "wa already linked" });
});
it("rejects registration with a wa id token for another audience", async () => {References
- In TypeScript tests, validation of injected dependencies (e.g., using
parse(Address, ...)on an injected address) is not mandatory. It can be omitted for trusted values even if other tests include it for consistency.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 79eeb584-b2b5-4ed4-8af5-0d523fd12c66
📒 Files selected for processing (10)
.changeset/quick-otters-link.mdserver/api/auth/registration.tsserver/database/schema.tsserver/script/openapi.tsserver/test/api/auth.test.tsserver/test/utils/token.test.tsserver/utils/chatSecret.tsserver/utils/createCredential.tsserver/utils/token.tsserver/vitest.config.mts
| if (!process.env.CHAT_IDENTITY_KEY) throw new Error("missing chat key"); | ||
|
|
||
| export default process.env.CHAT_IDENTITY_KEY as string; // eslint-disable-line @typescript-eslint/no-unnecessary-type-assertion |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Extract to a constant to avoid the type assertion and lint disable.
By assigning the environment variable to a local constant first, TypeScript will correctly narrow its type after the if check, eliminating the need for as string and the eslint-disable-line comment.
♻️ Proposed refactor
-if (!process.env.CHAT_IDENTITY_KEY) throw new Error("missing chat key");
-
-export default process.env.CHAT_IDENTITY_KEY as string; // eslint-disable-line `@typescript-eslint/no-unnecessary-type-assertion`
+const chatSecret = process.env.CHAT_IDENTITY_KEY;
+if (!chatSecret) throw new Error("missing chat key");
+
+export default chatSecret;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!process.env.CHAT_IDENTITY_KEY) throw new Error("missing chat key"); | |
| export default process.env.CHAT_IDENTITY_KEY as string; // eslint-disable-line @typescript-eslint/no-unnecessary-type-assertion | |
| const chatSecret = process.env.CHAT_IDENTITY_KEY; | |
| if (!chatSecret) throw new Error("missing chat key"); | |
| export default chatSecret; |
Summary by CodeRabbit
New Features
Bug Fixes
Tests