Remove cachekey logic, not needed as only a single bitbucket connection is supported, rework tests and update api report
Signed-off-by: Jake Smith <jakemgsmith@gmail.com>
This commit is contained in:
committed by
Fredrik Adelöw
parent
37fba1d8ec
commit
1956b3f398
@@ -69,16 +69,11 @@ describe('bitbucketCloud core', () => {
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it('uses OAuth Bearer token when clientId and clientSecret provided', async () => {
|
||||
it('handles OAuth token fetch errors', async () => {
|
||||
// Test error handling
|
||||
worker.use(
|
||||
rest.post(BITBUCKET_CLOUD_OAUTH_TOKEN_URL, (_, res, ctx) =>
|
||||
res(
|
||||
ctx.status(200),
|
||||
ctx.json({
|
||||
access_token: 'test-oauth-token',
|
||||
expires_in: 3600,
|
||||
}),
|
||||
),
|
||||
res(ctx.status(401), ctx.json({ error: 'invalid_client' })),
|
||||
),
|
||||
);
|
||||
|
||||
@@ -89,11 +84,13 @@ describe('bitbucketCloud core', () => {
|
||||
clientSecret: 'test-client-secret',
|
||||
};
|
||||
|
||||
const result = await getBitbucketCloudRequestOptions(withOAuth);
|
||||
expect(result.headers.Authorization).toEqual('Bearer test-oauth-token');
|
||||
await expect(getBitbucketCloudRequestOptions(withOAuth)).rejects.toThrow(
|
||||
/Failed to fetch OAuth token/,
|
||||
);
|
||||
});
|
||||
|
||||
it('caches OAuth tokens', async () => {
|
||||
it('uses OAuth Bearer token and caches it', async () => {
|
||||
// Test OAuth + caching
|
||||
let callCount = 0;
|
||||
worker.use(
|
||||
rest.post(BITBUCKET_CLOUD_OAUTH_TOKEN_URL, (_, res, ctx) => {
|
||||
@@ -101,7 +98,7 @@ describe('bitbucketCloud core', () => {
|
||||
return res(
|
||||
ctx.status(200),
|
||||
ctx.json({
|
||||
access_token: 'cached-oauth-token',
|
||||
access_token: 'test-oauth-token',
|
||||
expires_in: 3600,
|
||||
}),
|
||||
);
|
||||
@@ -111,42 +108,19 @@ describe('bitbucketCloud core', () => {
|
||||
const withOAuth: BitbucketCloudIntegrationConfig = {
|
||||
host: BITBUCKET_CLOUD_HOST,
|
||||
apiBaseUrl: BITBUCKET_CLOUD_API_BASE_URL,
|
||||
clientId: 'cache-test-client',
|
||||
clientSecret: 'cache-test-secret',
|
||||
clientId: 'test-client-id',
|
||||
clientSecret: 'test-client-secret',
|
||||
};
|
||||
|
||||
// First call should fetch token
|
||||
const result1 = await getBitbucketCloudRequestOptions(withOAuth);
|
||||
expect(result1.headers.Authorization).toEqual(
|
||||
'Bearer cached-oauth-token',
|
||||
);
|
||||
expect(result1.headers.Authorization).toEqual('Bearer test-oauth-token');
|
||||
expect(callCount).toBe(1);
|
||||
|
||||
// Second call should use cached token
|
||||
// Second call should use cached token (proves caching works)
|
||||
const result2 = await getBitbucketCloudRequestOptions(withOAuth);
|
||||
expect(result2.headers.Authorization).toEqual(
|
||||
'Bearer cached-oauth-token',
|
||||
);
|
||||
expect(callCount).toBe(1); // Should still be 1
|
||||
});
|
||||
|
||||
it('handles OAuth token fetch errors', async () => {
|
||||
worker.use(
|
||||
rest.post(BITBUCKET_CLOUD_OAUTH_TOKEN_URL, (_, res, ctx) =>
|
||||
res(ctx.status(401), ctx.json({ error: 'invalid_client' })),
|
||||
),
|
||||
);
|
||||
|
||||
const withOAuth: BitbucketCloudIntegrationConfig = {
|
||||
host: BITBUCKET_CLOUD_HOST,
|
||||
apiBaseUrl: BITBUCKET_CLOUD_API_BASE_URL,
|
||||
clientId: 'invalid-client',
|
||||
clientSecret: 'invalid-secret',
|
||||
};
|
||||
|
||||
await expect(getBitbucketCloudRequestOptions(withOAuth)).rejects.toThrow(
|
||||
/Failed to fetch OAuth token/,
|
||||
);
|
||||
expect(result2.headers.Authorization).toEqual('Bearer test-oauth-token');
|
||||
expect(callCount).toBe(1); // Still 1, proving cache was used
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -14,7 +14,6 @@
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
import { createHash } from 'crypto';
|
||||
import fetch from 'cross-fetch';
|
||||
import parseGitUrl from 'git-url-parse';
|
||||
import { BitbucketCloudIntegrationConfig } from './config';
|
||||
@@ -25,21 +24,11 @@ type OAuthTokenData = {
|
||||
expiresAt: DateTime;
|
||||
};
|
||||
|
||||
// In-memory token cache keyed by hashed credentials
|
||||
const tokenCache = new Map<string, OAuthTokenData>();
|
||||
// In-memory token cache (single entry since there's only one Bitbucket Cloud integration)
|
||||
let cachedToken: OAuthTokenData | undefined;
|
||||
|
||||
// Track in-flight token refresh requests to prevent concurrent fetches
|
||||
const refreshPromises = new Map<string, Promise<string>>();
|
||||
|
||||
/**
|
||||
* Creates a cache key from OAuth credentials by hashing them.
|
||||
* This prevents storing credentials in memory in plain text.
|
||||
*/
|
||||
function createCacheKey(clientId: string, clientSecret: string): string {
|
||||
return createHash('sha256')
|
||||
.update(`${clientId}:${clientSecret}`)
|
||||
.digest('hex');
|
||||
}
|
||||
// Track in-flight token refresh request to prevent concurrent fetches
|
||||
let refreshPromise: Promise<string> | undefined;
|
||||
|
||||
/**
|
||||
* Fetches an OAuth access token from Bitbucket Cloud using client credentials flow.
|
||||
@@ -54,22 +43,18 @@ export async function getBitbucketCloudOAuthToken(
|
||||
clientId: string,
|
||||
clientSecret: string,
|
||||
): Promise<string> {
|
||||
const cacheKey = createCacheKey(clientId, clientSecret);
|
||||
|
||||
// Check cache
|
||||
const cached = tokenCache.get(cacheKey);
|
||||
if (cached && DateTime.now() < cached.expiresAt) {
|
||||
return cached.token;
|
||||
if (cachedToken && DateTime.now() < cachedToken.expiresAt) {
|
||||
return cachedToken.token;
|
||||
}
|
||||
|
||||
// Check if there's already a refresh in progress
|
||||
const inFlight = refreshPromises.get(cacheKey);
|
||||
if (inFlight) {
|
||||
return inFlight;
|
||||
if (refreshPromise) {
|
||||
return refreshPromise;
|
||||
}
|
||||
|
||||
// Start a new token fetch and track it
|
||||
const refreshPromise = (async () => {
|
||||
refreshPromise = (async () => {
|
||||
try {
|
||||
// Fetch new token
|
||||
const credentials = Buffer.from(
|
||||
@@ -108,10 +93,10 @@ export async function getBitbucketCloudOAuthToken(
|
||||
.minus({ minutes: 10 });
|
||||
|
||||
// Cache the token
|
||||
tokenCache.set(cacheKey, {
|
||||
cachedToken = {
|
||||
token: data.access_token,
|
||||
expiresAt,
|
||||
});
|
||||
};
|
||||
|
||||
return data.access_token;
|
||||
} catch (error) {
|
||||
@@ -120,11 +105,10 @@ export async function getBitbucketCloudOAuthToken(
|
||||
);
|
||||
} finally {
|
||||
// Clean up the in-flight promise tracking
|
||||
refreshPromises.delete(cacheKey);
|
||||
refreshPromise = undefined;
|
||||
}
|
||||
})();
|
||||
|
||||
refreshPromises.set(cacheKey, refreshPromise);
|
||||
return refreshPromise;
|
||||
}
|
||||
|
||||
|
||||
@@ -25,6 +25,12 @@ export const createBitbucketPipelinesRunAction: (options: {
|
||||
| {
|
||||
type?: string | undefined;
|
||||
source?: string | undefined;
|
||||
selector?:
|
||||
| {
|
||||
type: string;
|
||||
pattern: string;
|
||||
}
|
||||
| undefined;
|
||||
pull_request?:
|
||||
| {
|
||||
id: string;
|
||||
@@ -39,12 +45,6 @@ export const createBitbucketPipelinesRunAction: (options: {
|
||||
destination?: string | undefined;
|
||||
ref_name?: string | undefined;
|
||||
ref_type?: string | undefined;
|
||||
selector?:
|
||||
| {
|
||||
type: string;
|
||||
pattern: string;
|
||||
}
|
||||
| undefined;
|
||||
destination_commit?:
|
||||
| {
|
||||
hash: string;
|
||||
|
||||
Reference in New Issue
Block a user