-
Notifications
You must be signed in to change notification settings - Fork 419
chore(backend): Add scoping and secret key retrieval to machines BAPI #6417
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
e821185
c54bfd2
801fe79
acf0d5d
c8ad201
73936ec
50512c9
6b38d86
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 |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| --- | ||
| "@clerk/backend": patch | ||
| --- | ||
|
|
||
| Adds scoping and secret key retrieval to machines BAPI methods: | ||
|
|
||
| ```ts | ||
| // Creates a new machine scope | ||
| clerkClient.machines.createScope('machine_id', 'to_machine_id') | ||
|
|
||
| // Deletes a machine scope | ||
| clerkClient.machines.deleteScope('machine_id', 'other_machine_id') | ||
|
|
||
| // Retrieve a secret key | ||
| clerkClient.machines.getSecretKey('machine_id') | ||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,204 @@ | ||
| import { http, HttpResponse } from 'msw'; | ||
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| import { server, validateHeaders } from '../../mock-server'; | ||
| import { createBackendApiClient } from '../factory'; | ||
|
|
||
| describe('MachineAPI', () => { | ||
| const apiClient = createBackendApiClient({ | ||
| apiUrl: 'https://api.clerk.test', | ||
| secretKey: 'deadbeef', | ||
| }); | ||
|
|
||
| const machineId = 'machine_123'; | ||
| const otherMachineId = 'machine_456'; | ||
|
|
||
| const mockSecondMachine = { | ||
| object: 'machine', | ||
| id: otherMachineId, | ||
| name: 'Second Machine', | ||
| instance_id: 'inst_456', | ||
| created_at: 1640995200, | ||
| updated_at: 1640995200, | ||
| }; | ||
|
|
||
| const mockMachine = { | ||
| object: 'machine', | ||
| id: machineId, | ||
| name: 'Test Machine', | ||
| instance_id: 'inst_123', | ||
| created_at: 1640995200, | ||
| updated_at: 1640995200, | ||
| scoped_machines: [mockSecondMachine], | ||
| }; | ||
|
|
||
| const mockMachineScope = { | ||
| object: 'machine_scope', | ||
| from_machine_id: machineId, | ||
| to_machine_id: otherMachineId, | ||
| created_at: 1640995200, | ||
| }; | ||
|
|
||
| const mockMachineSecretKey = { | ||
| secret: 'ak_test_...', | ||
| }; | ||
|
|
||
| const mockPaginatedResponse = { | ||
| data: [mockMachine], | ||
| total_count: 1, | ||
| }; | ||
|
|
||
| it('fetches a machine by ID', async () => { | ||
| server.use( | ||
| http.get( | ||
| `https://api.clerk.test/v1/machines/${machineId}`, | ||
| validateHeaders(() => { | ||
| return HttpResponse.json(mockMachine); | ||
| }), | ||
| ), | ||
| ); | ||
|
|
||
| const response = await apiClient.machines.get(machineId); | ||
|
|
||
| expect(response.id).toBe(machineId); | ||
| expect(response.name).toBe('Test Machine'); | ||
| }); | ||
|
|
||
| it('fetches machines list with query parameters', async () => { | ||
| server.use( | ||
| http.get( | ||
| 'https://api.clerk.test/v1/machines', | ||
| validateHeaders(({ request }) => { | ||
| const url = new URL(request.url); | ||
| expect(url.searchParams.get('limit')).toBe('10'); | ||
| expect(url.searchParams.get('offset')).toBe('5'); | ||
| expect(url.searchParams.get('query')).toBe('test'); | ||
| return HttpResponse.json(mockPaginatedResponse); | ||
| }), | ||
| ), | ||
| ); | ||
|
|
||
| const response = await apiClient.machines.list({ | ||
| limit: 10, | ||
| offset: 5, | ||
| query: 'test', | ||
| }); | ||
|
|
||
| expect(response.data).toHaveLength(1); | ||
| expect(response.totalCount).toBe(1); | ||
| }); | ||
|
|
||
| it('creates a machine with scoped machines', async () => { | ||
| const createParams = { | ||
| name: 'New Machine', | ||
| scoped_machines: [otherMachineId], | ||
| default_token_ttl: 7200, | ||
| }; | ||
|
|
||
| server.use( | ||
| http.post( | ||
| 'https://api.clerk.test/v1/machines', | ||
| validateHeaders(async ({ request }) => { | ||
| const body = await request.json(); | ||
| expect(body).toEqual(createParams); | ||
| return HttpResponse.json(mockMachine); | ||
| }), | ||
| ), | ||
| ); | ||
|
|
||
| const response = await apiClient.machines.create(createParams); | ||
|
|
||
| expect(response.id).toBe(machineId); | ||
| expect(response.name).toBe('Test Machine'); | ||
| expect(response.scopedMachines).toHaveLength(1); | ||
| expect(response.scopedMachines[0].id).toBe(otherMachineId); | ||
| expect(response.scopedMachines[0].name).toBe('Second Machine'); | ||
| }); | ||
|
|
||
| it('updates a machine with partial parameters', async () => { | ||
| const updateParams = { | ||
| machineId, | ||
| name: 'Updated Machine', | ||
| }; | ||
|
|
||
| server.use( | ||
| http.patch( | ||
| `https://api.clerk.test/v1/machines/${machineId}`, | ||
| validateHeaders(async ({ request }) => { | ||
| const body = await request.json(); | ||
| expect(body).toEqual({ name: 'Updated Machine' }); | ||
| return HttpResponse.json(mockMachine); | ||
| }), | ||
| ), | ||
| ); | ||
|
|
||
| const response = await apiClient.machines.update(updateParams); | ||
|
|
||
| expect(response.id).toBe(machineId); | ||
| expect(response.name).toBe('Test Machine'); | ||
| }); | ||
|
|
||
| it('deletes a machine', async () => { | ||
| server.use( | ||
| http.delete( | ||
| `https://api.clerk.test/v1/machines/${machineId}`, | ||
| validateHeaders(() => { | ||
| return HttpResponse.json(mockMachine); | ||
| }), | ||
| ), | ||
| ); | ||
|
|
||
| const response = await apiClient.machines.delete(machineId); | ||
|
|
||
| expect(response.id).toBe(machineId); | ||
| }); | ||
|
|
||
| it('fetches machine secret key', async () => { | ||
| server.use( | ||
| http.get( | ||
| `https://api.clerk.test/v1/machines/${machineId}/secret_key`, | ||
| validateHeaders(() => { | ||
| return HttpResponse.json(mockMachineSecretKey); | ||
| }), | ||
| ), | ||
| ); | ||
|
|
||
| const response = await apiClient.machines.getSecretKey(machineId); | ||
|
|
||
| expect(response.secret).toBe('ak_test_...'); | ||
| }); | ||
|
|
||
| it('creates a machine scope', async () => { | ||
| server.use( | ||
| http.post( | ||
| `https://api.clerk.test/v1/machines/${machineId}/scopes`, | ||
| validateHeaders(async ({ request }) => { | ||
| const body = await request.json(); | ||
| expect(body).toEqual({ to_machine_id: otherMachineId }); | ||
| return HttpResponse.json(mockMachineScope); | ||
| }), | ||
| ), | ||
| ); | ||
|
|
||
| const response = await apiClient.machines.createScope(machineId, otherMachineId); | ||
|
|
||
| expect(response.fromMachineId).toBe(machineId); | ||
| expect(response.toMachineId).toBe(otherMachineId); | ||
| }); | ||
|
|
||
| it('deletes a machine scope', async () => { | ||
| server.use( | ||
| http.delete( | ||
| `https://api.clerk.test/v1/machines/${machineId}/scopes/${otherMachineId}`, | ||
| validateHeaders(() => { | ||
| return HttpResponse.json(mockMachineScope); | ||
| }), | ||
| ), | ||
| ); | ||
|
|
||
| const response = await apiClient.machines.deleteScope(machineId, otherMachineId); | ||
|
|
||
| expect(response.fromMachineId).toBe(machineId); | ||
| expect(response.toMachineId).toBe(otherMachineId); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,9 +7,30 @@ export class Machine { | |||||||||||||||||||||||||||||||||||||||||||||||||
| readonly instanceId: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| readonly createdAt: number, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| readonly updatedAt: number, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| readonly scopedMachines: Machine[], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| readonly defaultTokenTtl: number, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) {} | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| static fromJSON(data: MachineJSON): Machine { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return new Machine(data.id, data.name, data.instance_id, data.created_at, data.updated_at); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return new Machine( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| data.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| data.name, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| data.instance_id, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| data.created_at, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| data.updated_at, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| data.scoped_machines.map( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| m => | ||||||||||||||||||||||||||||||||||||||||||||||||||
| new Machine( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| m.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| m.name, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| m.instance_id, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| m.created_at, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| m.updated_at, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [], // Nested machines don't have scoped_machines | ||||||||||||||||||||||||||||||||||||||||||||||||||
| m.default_token_ttl, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+21
to
+32
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. 🛠️ Refactor suggestion Add null safety for scoped_machines array. The mapping of - data.scoped_machines.map(
+ (data.scoped_machines || []).map(
m =>
new Machine(
m.id,
m.name,
m.instance_id,
m.created_at,
m.updated_at,
[], // Nested machines don't have scoped_machines
m.default_token_ttl,
),
),📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Member
Author
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. the root scoped_machines will always be an array
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. @wobsoriano Thank you for the clarification! I understand that the API contract guarantees ✏️ Learnings added
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| data.default_token_ttl, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.
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.
💡 Verification agent
🧩 Analysis chain
Missing test coverage for new API methods.
According to the coding guidelines, tests should be added to cover new functionality. Please ensure that unit tests are added for:
getSecretKeymethodcreateScopemethoddeleteScopemethodTests should cover both success cases and error scenarios (e.g., invalid machine IDs).
🏁 Script executed:
Length of output: 1583
Add unit tests for MachineApi’s new methods
There are currently no tests covering MachineApi.getSecretKey, createScope, or deleteScope. Please add a new spec under
packages/backend/src/api/endpoints/__tests__/MachineApi.test.ts(or similar) that includes:getSecretKey(machineId)createScope(machineId, toMachineId)deleteScope(machineId, otherMachineId)this.requestto verify the correct HTTP method, path, and bodyParamsThis will ensure all new functionality is exercised and adheres to our coverage guidelines.
🤖 Prompt for AI Agents
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.
This will be tested in a follow up PR here
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.
@wobsoriano Thank you for the clarification! That's a perfectly reasonable approach to handle test coverage in a follow-up PR. I appreciate you providing the link to PR #6229 where the tests will be added.
Having the core functionality implemented first and then comprehensive testing in a dedicated follow-up PR helps keep each PR focused and manageable.
ଘ(੭ˊᵕˋ)੭ ♡
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.
coderobert
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.
@wobsoriano Hello! 👋 I think you meant "CodeRabbit" but "coderobert" works too! 😊
Is there anything specific you'd like me to help you with regarding this PR or any other code review needs?
ଘ(੭ˊᵕˋ)੭♡