Skip to content

Commit

Permalink
Merge pull request #69 from logion-network/hotfix/authentication
Browse files Browse the repository at this point in the history 
fix: enforce that loc-related resources are called by node owner.
  • Loading branch information
@benoitdevos
benoitdevos authored Jan 11, 2022
2 parents 03e37d9 + 649e731 commit 47f588f
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 16 deletions.
28 changes: 14 additions & 14 deletions src/logion/controllers/locrequest.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ export class LocRequestController extends ApiController {
async rejectLocRequest(rejectLocRequestView: RejectLocRequestView, requestId: string) {
const request = requireDefined(await this.locRequestRepository.findById(requestId));
this.authenticationService.authenticatedUser(this.request)
.is(request.ownerAddress);
.requireNodeOwner();
request.reject(rejectLocRequestView.rejectReason!, moment());
await this.locRequestRepository.save(request)
}
Expand All @@ -369,7 +369,7 @@ export class LocRequestController extends ApiController {
async acceptLocRequest(_ignoredBody: any, requestId: string) {
const request = requireDefined(await this.locRequestRepository.findById(requestId));
this.authenticationService.authenticatedUser(this.request)
.is(request.ownerAddress);
.requireNodeOwner();
request.accept(moment());
await this.locRequestRepository.save(request)
}
Expand All @@ -387,7 +387,7 @@ export class LocRequestController extends ApiController {
async addFile(addFileView: AddFileView, requestId: string): Promise<AddFileResultView> {
const request = requireDefined(await this.locRequestRepository.findById(requestId));
this.authenticationService.authenticatedUser(this.request)
.is(request.ownerAddress);
.requireNodeOwner();

const files: fileUpload.FileArray = this.request.files;
if(files === undefined || files === null) {
Expand Down Expand Up @@ -434,7 +434,7 @@ export class LocRequestController extends ApiController {
async downloadFile(_body: any, requestId: string, hash: string): Promise<void> {
const request = requireDefined(await this.locRequestRepository.findById(requestId));
this.authenticationService.authenticatedUser(this.request)
.is(request.ownerAddress);
.requireNodeOwner();

const file = request.getFile(hash);
const tempFilePath = "/tmp/download-" + requestId + "-" + hash;
Expand All @@ -461,7 +461,7 @@ export class LocRequestController extends ApiController {
async deleteFile(_body: any, requestId: string, hash: string): Promise<void> {
const request = requireDefined(await this.locRequestRepository.findById(requestId));
this.authenticationService.authenticatedUser(this.request)
.is(request.ownerAddress);
.requireNodeOwner();

const file = request.removeFile(hash);
await this.locRequestRepository.save(request);
Expand All @@ -484,7 +484,7 @@ export class LocRequestController extends ApiController {
async confirmFile(_body: any, requestId: string, hash: string) {
const request = requireDefined(await this.locRequestRepository.findById(requestId));
this.authenticationService.authenticatedUser(this.request)
.is(request.ownerAddress);
.requireNodeOwner();

request.confirmFile(hash);
await this.locRequestRepository.save(request);
Expand All @@ -506,7 +506,7 @@ export class LocRequestController extends ApiController {
async closeLoc(_body: any, requestId: string) {
const request = requireDefined(await this.locRequestRepository.findById(requestId));
this.authenticationService.authenticatedUser(this.request)
.is(request.ownerAddress);
.requireNodeOwner();

request.preClose();
await this.locRequestRepository.save(request);
Expand All @@ -532,7 +532,7 @@ export class LocRequestController extends ApiController {
async voidLoc(body: VoidLocView, requestId: string) {
const request = requireDefined(await this.locRequestRepository.findById(requestId));
this.authenticationService.authenticatedUser(this.request)
.is(request.ownerAddress);
.requireNodeOwner();

request.preVoid(body.reason!);
await this.locRequestRepository.save(request);
Expand All @@ -554,7 +554,7 @@ export class LocRequestController extends ApiController {
async addLink(addLinkView: AddLinkView, requestId: string): Promise<void> {
const request = requireDefined(await this.locRequestRepository.findById(requestId));
this.authenticationService.authenticatedUser(this.request)
.is(request.ownerAddress);
.requireNodeOwner();
const targetRequest = requireDefined(await this.locRequestRepository.findById(addLinkView.target!));
request.addLink({
target: targetRequest.id!,
Expand All @@ -578,7 +578,7 @@ export class LocRequestController extends ApiController {
async deleteLink(_body: any, requestId: string, target: string): Promise<void> {
const request = requireDefined(await this.locRequestRepository.findById(requestId));
this.authenticationService.authenticatedUser(this.request)
.is(request.ownerAddress);
.requireNodeOwner();

request.removeLink(target);
await this.locRequestRepository.save(request);
Expand All @@ -599,7 +599,7 @@ export class LocRequestController extends ApiController {
async confirmLink(_body: any, requestId: string, target: string) {
const request = requireDefined(await this.locRequestRepository.findById(requestId));
this.authenticationService.authenticatedUser(this.request)
.is(request.ownerAddress);
.requireNodeOwner();

request.confirmLink(target);
await this.locRequestRepository.save(request);
Expand All @@ -621,7 +621,7 @@ export class LocRequestController extends ApiController {
async addMetadata(addMetadataView: AddMetadataView, requestId: string): Promise<void> {
const request = requireDefined(await this.locRequestRepository.findById(requestId));
this.authenticationService.authenticatedUser(this.request)
.is(request.ownerAddress);
.requireNodeOwner();
const name = requireLength(addMetadataView, "name", 3, 40)
const value = requireLength(addMetadataView, "value", 1, 4096)
request.addMetadataItem({ name, value })
Expand All @@ -643,7 +643,7 @@ export class LocRequestController extends ApiController {
async deleteMetadata(_body: any, requestId: string, name: string): Promise<void> {
const request = requireDefined(await this.locRequestRepository.findById(requestId));
this.authenticationService.authenticatedUser(this.request)
.is(request.ownerAddress);
.requireNodeOwner();

const decodedName = decodeURIComponent(name);
request.removeMetadataItem(decodedName);
Expand All @@ -665,7 +665,7 @@ export class LocRequestController extends ApiController {
async confirmMetadata(_body: any, requestId: string, name: string) {
const request = requireDefined(await this.locRequestRepository.findById(requestId));
this.authenticationService.authenticatedUser(this.request)
.is(request.ownerAddress);
.requireNodeOwner();

const decodedName = decodeURIComponent(name);
request.confirmMetadataItem(decodedName);
Expand Down
8 changes: 6 additions & 2 deletions test/helpers/testapp.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export function setupApp<T>(
controller: Function & { prototype: T; },
mockBinder: (container: Container) => void,
authSucceed: boolean = true,
isNodeOwner: boolean = false,
isNodeOwner: boolean = true,
): Express {

const app = express();
Expand Down Expand Up @@ -58,7 +58,11 @@ function mockAuthenticationSuccess(isNodeOwner: boolean): AuthenticationService
authenticatedUser.setup(instance => instance.require).returns(() => {});
authenticatedUser.setup(instance => instance.requireIs).returns(() => {});
authenticatedUser.setup(instance => instance.requireLegalOfficer).returns(() => {});
authenticatedUser.setup(instance => instance.requireNodeOwner).returns(() => {});
authenticatedUser.setup(instance => instance.requireNodeOwner).returns(() => {
if (!isNodeOwner) {
throw new UnauthorizedException("")
}
});
authenticatedUser.setup(instance => instance.isNodeOwner).returns(() => isNodeOwner);

const authenticationService = new Mock<AuthenticationService>();
Expand Down
24 changes: 24 additions & 0 deletions test/unit/controllers/locrequest.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ describe('LocRequestController', () => {
LocRequestController,
mockModelForCreation,
true,
false
)
await request(app)
.post('/api/loc-request')
Expand Down Expand Up @@ -190,6 +191,7 @@ describe('LocRequestController', () => {
LocRequestController,
container => mockModelForCreation(container, true),
true,
false
)
await request(app)
.post('/api/loc-request')
Expand Down Expand Up @@ -318,6 +320,19 @@ describe('LocRequestController', () => {
});
})

it('fails to add file to loc when not owner', async () => {
const app = setupApp(LocRequestController, mockModelForAddFile, true, false);
const buffer = Buffer.from(SOME_DATA);
await request(app)
.post(`/api/loc-request/${ REQUEST_ID }/files`)
.field({ nature: "some nature" })
.attach('file', buffer, {
filename: FILE_NAME,
contentType: 'text/plain',
})
.expect(401);
})

it('downloads existing file given its hash', async () => {
const app = setupApp(LocRequestController, mockModelForDownloadFile);
const filePath = "/tmp/download-" + REQUEST_ID + "-" + SOME_DATA_HASH;
Expand Down Expand Up @@ -387,6 +402,15 @@ describe('LocRequestController', () => {
It.Is<MetadataItemDescription>(item => item.name == SOME_DATA_NAME && item.value == SOME_DATA_VALUE)))
})

it('fails to adds a metadata item when not owner', async () => {
const locRequest = mockRequestForMetadata();
const app = setupApp(LocRequestController, (container) => mockModelForAllItems(container, locRequest), true, false)
await request(app)
.post(`/api/loc-request/${ REQUEST_ID }/metadata`)
.send({ name: SOME_DATA_NAME, value: SOME_DATA_VALUE })
.expect(401)
})

it('deletes a metadata item', async () => {
const locRequest = mockRequestForMetadata();
const app = setupApp(LocRequestController, (container) => mockModelForAllItems(container, locRequest))
Expand Down

0 comments on commit 47f588f

Please sign in to comment.