Skip to content

Commit 29f214f

Browse files
authored
Merge pull request #52 from screwdriver-cd/remove
fix(450): Check if repo exists before getting permissions
2 parents e6509e1 + 4338e18 commit 29f214f

File tree

2 files changed

+122
-25
lines changed

2 files changed

+122
-25
lines changed

index.js

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,10 @@ function checkResponseError(response) {
5252
default: JSON.stringify(response.body)
5353
});
5454

55-
throw new Error(`${errorMessage} Reason "${errorReason}"`);
55+
const error = new Error(`${errorMessage} Reason "${errorReason}"`);
56+
57+
error.code = response.statusCode;
58+
throw error;
5659
}
5760

5861
/**
@@ -211,10 +214,10 @@ class BitbucketScm extends Scm {
211214
* is instead updated.
212215
* @method _addWebhook
213216
* @param {Object} config
214-
* @param {String} config.scmUri The SCM URI to add the webhook to
215-
* @param {String} config.token Oauth2 token to authenticate with Bitbucket
216-
* @param {String} config.url The URL to use for webhook notifications
217-
* @return {Promise} Resolves upon success
217+
* @param {String} config.scmUri The SCM URI to add the webhook to
218+
* @param {String} config.token Oauth2 token to authenticate with Bitbucket
219+
@param {String} config.webhookUrl The URL to use for the webhook notifications
220+
* @return {Promise} Resolves upon success
218221
*/
219222
_addWebhook(config) {
220223
const repoInfo = getScmUriParts(config.scmUri);
@@ -223,14 +226,14 @@ class BitbucketScm extends Scm {
223226
page: 1,
224227
repoId: repoInfo.repoId,
225228
token: config.token,
226-
url: config.url
229+
url: config.webhookUrl
227230
})
228231
.then(hookInfo =>
229232
this._createWebhook({
230233
hookInfo,
231234
repoId: repoInfo.repoId,
232235
token: config.token,
233-
url: config.url
236+
url: config.webhookUrl
234237
})
235238
);
236239
}
@@ -549,16 +552,27 @@ class BitbucketScm extends Scm {
549552
* @param {String} config.token The token used to authenticate to the SCM
550553
* @return {Promise} Resolves to permissions object with admin, push, pull
551554
*/
552-
_getPermissions(config) {
555+
async _getPermissions(config) {
553556
const scm = getScmUriParts(config.scmUri);
554-
const getPerm = (repoId, desiredAccess, token) => {
555-
const [owner, uuid] = repoId.split('/');
557+
const [owner, uuid] = scm.repoId.split('/');
558+
559+
// First, check to see if the repository exists
560+
await this.breaker.runCommand({
561+
url: `${API_URL_V2}/repositories/${owner}/${uuid}`,
562+
method: 'GET',
563+
json: true,
564+
auth: {
565+
bearer: config.token
566+
}
567+
}).then(checkResponseError);
568+
569+
const getPerm = async (desiredAccess) => {
556570
const options = {
557571
url: `${API_URL_V2}/repositories/${owner}`,
558572
method: 'GET',
559573
json: true,
560574
auth: {
561-
bearer: token
575+
bearer: config.token
562576
}
563577
};
564578

@@ -582,9 +596,9 @@ class BitbucketScm extends Scm {
582596
};
583597

584598
return Promise.all([
585-
getPerm(scm.repoId, 'admin', config.token),
586-
getPerm(scm.repoId, 'push', config.token),
587-
getPerm(scm.repoId, 'pull', config.token)
599+
getPerm('admin'),
600+
getPerm('push'),
601+
getPerm('pull')
588602
]).then(([admin, push, pull]) => ({
589603
admin,
590604
push,

test/index.test.js

Lines changed: 94 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,49 @@ describe('index', function () {
892892
});
893893

894894
describe('getPermissions', () => {
895+
const repos = [
896+
{
897+
url: `${API_URL_V2}/repositories/repoIdPrefix/repoIdSuffix`,
898+
method: 'GET',
899+
json: true,
900+
auth: {
901+
bearer: token
902+
}
903+
},
904+
{
905+
url: `${API_URL_V2}/repositories/repoIdPrefix/repoIdSuffix1`,
906+
method: 'GET',
907+
json: true,
908+
auth: {
909+
bearer: token
910+
}
911+
},
912+
{
913+
url: `${API_URL_V2}/repositories/repoIdPrefix/repoIdSuffix2`,
914+
method: 'GET',
915+
json: true,
916+
auth: {
917+
bearer: token
918+
}
919+
},
920+
{
921+
url: `${API_URL_V2}/repositories/repoIdPrefix/repoIdSuffix3`,
922+
method: 'GET',
923+
json: true,
924+
auth: {
925+
bearer: token
926+
}
927+
},
928+
{
929+
url: `${API_URL_V2}/repositories/repoIdPrefix/fake`,
930+
method: 'GET',
931+
json: true,
932+
auth: {
933+
bearer: token
934+
}
935+
}
936+
];
937+
895938
const pull = {
896939
url: `${API_URL_V2}/repositories/repoIdPrefix`,
897940
method: 'GET',
@@ -916,6 +959,16 @@ describe('index', function () {
916959
bearer: token
917960
}
918961
};
962+
963+
const repoResponse = {
964+
statusCode: 200,
965+
body: {}
966+
};
967+
const repoNotFoundResponse = {
968+
statusCode: 404,
969+
body: 'Not found'
970+
};
971+
919972
const readResponse = {
920973
statusCode: 200,
921974
body: {
@@ -945,6 +998,13 @@ describe('index', function () {
945998
};
946999

9471000
beforeEach(() => {
1001+
requestMock.withArgs(repos[0]).yieldsAsync(null, repoResponse, repoResponse.body);
1002+
requestMock.withArgs(repos[1]).yieldsAsync(null, repoResponse, repoResponse.body);
1003+
requestMock.withArgs(repos[2]).yieldsAsync(null, repoResponse, repoResponse.body);
1004+
requestMock.withArgs(repos[3]).yieldsAsync(null, repoResponse, repoResponse.body);
1005+
requestMock.withArgs(repos[4]).yieldsAsync(null,
1006+
repoNotFoundResponse, repoNotFoundResponse.body);
1007+
9481008
requestMock.withArgs(pull).yieldsAsync(null, readResponse, readResponse.body);
9491009
requestMock.withArgs(push).yieldsAsync(null, writeResponse, writeResponse.body);
9501010
requestMock.withArgs(admin).yieldsAsync(null, adminResponse, adminResponse.body);
@@ -957,7 +1017,8 @@ describe('index', function () {
9571017
scmUri,
9581018
token
9591019
}).then((permissions) => {
960-
assert.calledThrice(requestMock);
1020+
assert.callCount(requestMock, 4);
1021+
assert.calledWith(requestMock, repos[1]);
9611022
assert.calledWith(requestMock, pull);
9621023
assert.calledWith(requestMock, push);
9631024
assert.calledWith(requestMock, admin);
@@ -976,7 +1037,8 @@ describe('index', function () {
9761037
scmUri,
9771038
token
9781039
}).then((permissions) => {
979-
assert.calledThrice(requestMock);
1040+
assert.callCount(requestMock, 4);
1041+
assert.calledWith(requestMock, repos[2]);
9801042
assert.calledWith(requestMock, pull);
9811043
assert.calledWith(requestMock, push);
9821044
assert.calledWith(requestMock, admin);
@@ -995,6 +1057,11 @@ describe('index', function () {
9951057
scmUri,
9961058
token
9971059
}).then((permissions) => {
1060+
assert.callCount(requestMock, 4);
1061+
assert.calledWith(requestMock, repos[3]);
1062+
assert.calledWith(requestMock, pull);
1063+
assert.calledWith(requestMock, push);
1064+
assert.calledWith(requestMock, admin);
9981065
assert.deepEqual(permissions, {
9991066
admin: false,
10001067
push: false,
@@ -1057,6 +1124,22 @@ describe('index', function () {
10571124
assert.equal(error, err);
10581125
});
10591126
});
1127+
1128+
it('rejects if the repository does not exist', () => {
1129+
const error = new Error('Not found');
1130+
const scmUri = 'hostName:repoIdPrefix/fake:branchName';
1131+
1132+
error.code = 404;
1133+
1134+
return scm.getPermissions({
1135+
scmUri,
1136+
token
1137+
}).then(() => {
1138+
assert.fail('Should not get here');
1139+
}).catch((err) => {
1140+
assert.deepEqual(error, err);
1141+
});
1142+
});
10601143
});
10611144

10621145
describe('updateCommitStatus', () => {
@@ -1249,7 +1332,7 @@ describe('index', function () {
12491332
});
12501333
});
12511334

1252-
describe('_addWebhook', () => {
1335+
describe.only('_addWebhook', () => {
12531336
const oauthToken = 'oauthToken';
12541337
const scmUri = 'hostName:repoId:branchName';
12551338

@@ -1273,7 +1356,7 @@ describe('index', function () {
12731356
/* eslint-enable no-underscore-dangle */
12741357
scmUri,
12751358
token: oauthToken,
1276-
url: 'url'
1359+
webhookUrl: 'url'
12771360
})
12781361
.then(() => {
12791362
assert.calledWith(requestMock, {
@@ -1328,7 +1411,7 @@ describe('index', function () {
13281411
/* eslint-enable no-underscore-dangle */
13291412
scmUri,
13301413
token: oauthToken,
1331-
url: 'url'
1414+
webhookUrl: 'url'
13321415
}).then(() => {
13331416
assert.calledWith(requestMock, {
13341417
json: true,
@@ -1396,7 +1479,7 @@ describe('index', function () {
13961479
/* eslint-enable no-underscore-dangle */
13971480
scmUri,
13981481
token: oauthToken,
1399-
url: 'url'
1482+
webhookUrl: 'url'
14001483
}).then(() => {
14011484
assert.calledWith(requestMock, {
14021485
json: true,
@@ -1455,7 +1538,7 @@ describe('index', function () {
14551538
/* eslint-enable no-underscore-dangle */
14561539
scmUri,
14571540
token,
1458-
url: 'url'
1541+
webhookUrl: 'url'
14591542
}).then(assert.fail, (err) => {
14601543
assert.strictEqual(err.message, expectedMessage);
14611544
});
@@ -1475,7 +1558,7 @@ describe('index', function () {
14751558
/* eslint-enable no-underscore-dangle */
14761559
scmUri,
14771560
token,
1478-
url: 'url'
1561+
webhookUrl: 'url'
14791562
}).then(assert.fail, (err) => {
14801563
assert.strictEqual(err.message, expectedMessage);
14811564
});
@@ -1510,7 +1593,7 @@ describe('index', function () {
15101593
/* eslint-enable no-underscore-dangle */
15111594
scmUri,
15121595
token,
1513-
url: 'url'
1596+
webhookUrl: 'url'
15141597
}).then(assert.fail, (err) => {
15151598
assert.strictEqual(err.message, [
15161599
'Your credentials lack one or more required privilege scopes.',
@@ -1555,7 +1638,7 @@ describe('index', function () {
15551638
/* eslint-enable no-underscore-dangle */
15561639
scmUri,
15571640
token,
1558-
url: 'url'
1641+
webhookUrl: 'url'
15591642
}).then(assert.fail, (err) => {
15601643
assert.strictEqual(err.message, expectedMessage);
15611644
});
@@ -1585,7 +1668,7 @@ describe('index', function () {
15851668
/* eslint-enable no-underscore-dangle */
15861669
scmUri,
15871670
token,
1588-
url: 'url'
1671+
webhookUrl: 'url'
15891672
}).then(assert.fail, (err) => {
15901673
assert.strictEqual(err.message, expectedMessage);
15911674
});

0 commit comments

Comments
 (0)