-
Notifications
You must be signed in to change notification settings - Fork 6
feat: license handshake and x-descope-license header #730
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -115,6 +115,11 @@ const nodeSdk = ({ | |
| ); | ||
| }; | ||
|
|
||
| // Rate limit tier from the license handshake. Populated asynchronously on init | ||
| // and injected into the x-descope-license header on every management request so | ||
| // Cloudflare can apply the correct rate limit bucket per customer tier. | ||
| let rateLimitTier: string | undefined; | ||
|
|
||
| const mgmtSdkConfig = { | ||
| fetch, | ||
| ...config, | ||
|
|
@@ -131,6 +136,13 @@ const nodeSdk = ({ | |
| (requestConfig: RequestConfig) => { | ||
| // eslint-disable-next-line no-param-reassign | ||
| requestConfig.token = managementKey; | ||
| if (rateLimitTier) { | ||
| // eslint-disable-next-line no-param-reassign | ||
| requestConfig.headers = { | ||
| ...requestConfig.headers, | ||
| 'x-descope-license': rateLimitTier, | ||
|
Member
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. just making sure the Descope deployment already allow this header (if not, requests will start to fail) |
||
| }; | ||
| } | ||
| return requestConfig; | ||
| }, | ||
| ].concat(config.hooks?.beforeRequest || []), | ||
|
|
@@ -144,6 +156,22 @@ const nodeSdk = ({ | |
| headers: nodeHeaders, | ||
| }); | ||
|
|
||
| // Fire-and-forget license handshake. Backend skips license-header validation | ||
| // for the GetLicense endpoint itself, so this initial request is safe even | ||
| // before the tier is cached. | ||
| if (managementKey) { | ||
|
Member
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. is this only for mgmt actions? also, don't we have mgmt actions that are not bound to mgmt key? |
||
| management.license | ||
| .get() | ||
| .then((resp) => { | ||
| if (resp.ok && resp.data?.rateLimitTier) { | ||
| rateLimitTier = resp.data.rateLimitTier; | ||
|
Member
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. have we considered to wait with other requests until this one completed? |
||
| } | ||
| }) | ||
| .catch((e) => { | ||
| logger?.warn?.('License handshake failed', e); | ||
| }); | ||
| } | ||
|
orius123 marked this conversation as resolved.
|
||
|
|
||
| const sdk = { | ||
| ...coreSdk, | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ import withInboundApplication from './inboundapplication'; | |
| import withOutboundApplication from './outboundapplication'; | ||
| import withDescoper from './descoper'; | ||
| import withManagementKey from './managementKey'; | ||
| import withLicense from './license'; | ||
| import { FGAConfig } from './types'; | ||
|
|
||
| /** Constructs a higher level Management API that wraps the functions from code-js-sdk */ | ||
|
|
@@ -43,6 +44,7 @@ const withManagement = (client: HttpClient, fgaConfig?: FGAConfig) => ({ | |
| fga: WithFGA(client, fgaConfig), | ||
| descoper: withDescoper(client), | ||
| managementKey: withManagementKey(client), | ||
| license: withLicense(client), | ||
|
Member
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. do we want to expose this API to customers? - imho - no |
||
| }); | ||
|
|
||
| export default withManagement; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| import { SdkResponse } from '@descope/core-js-sdk'; | ||
| import withManagement from '.'; | ||
| import apiPaths from './paths'; | ||
| import { mockHttpClient, resetMockHttpClient } from './testutils'; | ||
| import { License } from './types'; | ||
|
|
||
| const management = withManagement(mockHttpClient); | ||
|
|
||
| const mockLicense: License = { | ||
| rateLimitTier: 'tier4', | ||
| }; | ||
|
|
||
| describe('Management License', () => { | ||
| afterEach(() => { | ||
| jest.clearAllMocks(); | ||
| resetMockHttpClient(); | ||
| }); | ||
|
|
||
| describe('get', () => { | ||
| it('should send the correct request and receive correct response', async () => { | ||
| const httpResponse = { | ||
| ok: true, | ||
| json: () => mockLicense, | ||
| clone: () => ({ | ||
| json: () => Promise.resolve(mockLicense), | ||
| }), | ||
| status: 200, | ||
| }; | ||
| mockHttpClient.get.mockResolvedValue(httpResponse); | ||
|
|
||
| const resp: SdkResponse<License> = await management.license.get(); | ||
|
|
||
| expect(mockHttpClient.get).toHaveBeenCalledWith(apiPaths.license.get); | ||
| expect(resp).toEqual({ | ||
| code: 200, | ||
| data: mockLicense, | ||
| ok: true, | ||
| response: httpResponse, | ||
| }); | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import { SdkResponse, transformResponse, HttpClient } from '@descope/core-js-sdk'; | ||
| import apiPaths from './paths'; | ||
| import { License } from './types'; | ||
|
|
||
| const withLicense = (httpClient: HttpClient) => ({ | ||
| get: (): Promise<SdkResponse<License>> => | ||
| transformResponse<License>(httpClient.get(apiPaths.license.get)), | ||
| }); | ||
|
|
||
| export default withLicense; |
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.
nit, I don't think logs should mention CF specifically, its implementation details
I would also not mention the word "customer" tier but "project's company" tier