Skip to content
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

feat(3266): add authn for banner list and get endpoints #3278

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
13 changes: 10 additions & 3 deletions features/banner.feature
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Feature: Banner

Scenario: Banner with global scope
When they create new banner with message "Hello World" and "GLOBAL" scope
Then they can see that the banner is created with "GLOBAL" scope
Then they "can" see that the banner is created with "GLOBAL" scope
And banner is "updated" when they update the banner with "message" "Some Random Message"
And banner is "not updated" when they update the banner with "scopeId" "1234"
And banner is "not updated" when they update the banner with "scope" "PIPELINE"
Expand All @@ -29,8 +29,15 @@ Feature: Banner
Given an existing pipeline
And there is no banner associated to that pipeline
When they create new banner with message "Hello World" and "PIPELINE" scope
Then they can see that the banner is created with "PIPELINE" scope
Then they "can" see that the banner is created with "PIPELINE" scope
And they can get the banner associated to that pipeline
And banner is "updated" when they update the banner with "isActive" "false"
And banner is "not updated" when they update the banner with "scope" "GLOBAL"
Then banner is deleted
Then banner is deleted

Scenario: User without authorization should not see the banner
When they create new banner with message "Hello World" and "GLOBAL" scope
Then they "can" see that the banner is created with "GLOBAL" scope
And "calvin" has expired token
Then they "cannot" see that the banner is created with "GLOBAL" scope
And they cannot see any banner
53 changes: 42 additions & 11 deletions features/step_definitions/banner.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,25 +52,36 @@ When(
);

Then(
/^they can see that the banner is created with "(GLOBAL|PIPELINE)" scope$/,
/^they "(can|cannot)" see that the banner is created with "(GLOBAL|PIPELINE)" scope$/,
{ timeout: TIMEOUT },
function step(scope) {
function step(ok, scope) {
return request({
url: `${this.instance}/${this.namespace}/banners/${this.bannerId}`,
method: 'GET',
context: {
token: this.jwt
}
}).then(resp => {
Assert.equal(resp.statusCode, 200);
if (scope === 'PIPELINE') {
Assert.equal(resp.body.scope, scope.toUpperCase());
Assert.equal(resp.body.scopeId, this.pipelineId);
})
.then(resp => {
if (ok === 'can') {
Assert.equal(resp.statusCode, 200);
if (scope === 'PIPELINE') {
Assert.equal(resp.body.scope, scope.toUpperCase());
Assert.equal(resp.body.scopeId, this.pipelineId);

return;
}
Assert.equal(resp.body.scope, 'GLOBAL');
});
return;
}
Assert.equal(resp.body.scope, 'GLOBAL');
} else {
throw new Error('User should not be able to see the banner');
}
})
.catch(err => {
if (ok === 'can') {
throw new Error('User should be able to see the banner');
}
Assert.equal(err.statusCode, 401);
});
}
);

Expand Down Expand Up @@ -148,3 +159,23 @@ Then(/^they can get the banner associated to that pipeline$/, { timeout: TIMEOUT
Assert.equal(resp.body[0].scopeId, this.pipelineId);
});
});

Then(/^"([^"]*)" has expired token$/, { timeout: TIMEOUT }, function step() {
this.jwt = null;
});

Then(/^they cannot see any banner$/, { timeout: TIMEOUT }, function step() {
return request({
url: `${this.instance}/${this.namespace}/banners`,
method: 'GET',
context: {
token: this.jwt
}
})
.then(() => {
throw new Error('User should not be able to see any banners');
})
.catch(err => {
Assert.equal(err.statusCode, 401);
});
});
4 changes: 4 additions & 0 deletions plugins/banners/get.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ module.exports = () => ({
description: 'Get a single banner',
notes: 'Return a banner record',
tags: ['api', 'banners'],
auth: {
strategies: ['token'],
scope: ['user']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Banners with GLOBAL scope are currently being fetched without the user being authenticated.
image

This change makes it backward incompatible. We are unable fetch 'GLOBAL' banners when the user is not authenticated.
image

Auth check may need to be done selectively based on the scope. GLOBAL can be accessed by anyone like before. But, PIPELINE should be accessed by a user with valid token.

},
plugins: {
'hapi-rate-limit': {
enabled: false
Expand Down
4 changes: 4 additions & 0 deletions plugins/banners/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ module.exports = () => ({
description: 'Get banners',
notes: 'Returns all banner records',
tags: ['api', 'banners'],
auth: {
strategies: ['token'],
scope: ['user']
},
plugins: {
'hapi-rate-limit': {
enabled: false
Expand Down