Skip to content

Commit

Permalink
some fixes based on comment
Browse files Browse the repository at this point in the history
  • Loading branch information
Junjiequan committed Jul 23, 2024
1 parent 865cca4 commit f705241
Show file tree
Hide file tree
Showing 9 changed files with 40 additions and 42 deletions.
1 change: 0 additions & 1 deletion .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ LOGBOOK_BASE_URL="http://localhost:3030/scichatapi"
METADATA_KEYS_RETURN_LIMIT=100
METADATA_PARENT_INSTANCES_RETURN_LIMIT=100
MONGODB_URI="mongodb://<USERNAME>:<PASSWORD>@<HOST>:27017/<DB_NAME>"
MONGDB_TEST_NAME="scicat-test"
OAI_PROVIDER_ROUTE="<OAI_PROVIDER_ROUTE>"
PID_PREFIX="<PID_PREFIX>"
PUBLIC_URL_PREFIX="https://doi.esss.se/detail/"
Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,7 @@ jobs:
- name: API tests
env:
ELASTICSEARCH_ENABLED: ${{ matrix.elasticsearch_enabled }}
MONGODB_URI: mongodb://localhost:27017/scicat
MONGODB_TEST: scicat
MONGODB_URI: mongodb://localhost:27017/scicat-test-db
EXPRESS_SESSION_SECRET: a_scicat_secret
JWT_SECRET: a_scicat_secret
PORT: 3000
Expand Down
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ Valid environment variables for the .env file. See [.env.example](/.env.example)
| `METADATA_KEYS_RETURN_LIMIT` | number | Yes | The return limit for the `/Datasets/metadataKeys` endpoint. | |
| `METADATA_PARENT_INSTANCES_RETURN_LIMIT` | number | Yes | The return limit of Datasets to extract metadata keys from for the `/Datasets/metadataKeys` endpoint. | |
| `MONGODB_URI` | string | | The URI for your MongoDB instance. | |
| `MONGDB_TEST_NAME` | string | | The DB name for your MongoDB instance that runs on the local testing environment. |"scicat-test"|
| `OAI_PROVIDER_ROUTE` | string | Yes | URI to OAI provider, used for the `/publisheddata/:id/resync` endpoint. | |
| `PID_PREFIX` | string | | The facility PID prefix, with trailing slash. | |
| `PUBLIC_URL_PREFIX` | string | | The base URL to the facility Landing Page. | |
Expand Down
6 changes: 3 additions & 3 deletions src/proposals/proposals.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -549,12 +549,12 @@ export class ProposalsController {
return proposal;
}

// GET /proposals/:pid/access
// GET /proposals/:pid/authorization
@UseGuards(PoliciesGuard)
@CheckPolicies((ability: AppAbility) =>
ability.can(Action.ProposalsRead, ProposalClass),
)
@Get("/:pid/access")
@Get("/:pid/authorization")
@ApiOperation({
summary: "Check user access to a specific proposal.",
description:
Expand Down Expand Up @@ -585,7 +585,7 @@ export class ProposalsController {
proposal,
request,
);
return { canAccess: canAccess };
return { canAccess };
}

// PATCH /proposals/:pid
Expand Down
4 changes: 2 additions & 2 deletions src/samples/samples.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -554,12 +554,12 @@ export class SamplesController {
return sample;
}

// GET /samples/:id/access
// GET /samples/:id/authorization
@UseGuards(PoliciesGuard)
@CheckPolicies((ability: AppAbility) =>
ability.can(Action.SampleRead, SampleClass),
)
@Get("/:id/access")
@Get("/:id/authorization")
@ApiOperation({
summary: "Check user access to a specific sample.",
description:
Expand Down
26 changes: 13 additions & 13 deletions test/ProposalAuthorization.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ describe("1400: ProposalAuthorization: Test access to proposal", () => {
done();
});

it("0000: adds proposal 1", async () => {
it("0010: adds proposal 1", async () => {
return request(appUrl)
.post("/api/v3/proposals")
.send(proposal1)
Expand All @@ -130,7 +130,7 @@ describe("1400: ProposalAuthorization: Test access to proposal", () => {
});
});

it("0010: adds proposal 2", async () => {
it("0020: adds proposal 2", async () => {
return request(appUrl)
.post("/api/v3/proposals")
.send(proposal2)
Expand All @@ -146,7 +146,7 @@ describe("1400: ProposalAuthorization: Test access to proposal", () => {
});
});

it("0020: adds proposal 3", async () => {
it("0030: adds proposal 3", async () => {
return request(appUrl)
.post("/api/v3/proposals")
.send(proposal3)
Expand All @@ -162,7 +162,7 @@ describe("1400: ProposalAuthorization: Test access to proposal", () => {
});
});

// it("0030: adds proposal 10", async () => {
// it("0035: adds proposal 10", async () => {
// return request(appUrl)
// .post("/api/v3/proposals")
// .send(proposal10)
Expand Down Expand Up @@ -220,7 +220,7 @@ describe("1400: ProposalAuthorization: Test access to proposal", () => {

it("0061: check admin access to proposal 1 should return true", async () => {
return request(appUrl)
.get("/api/v3/proposals/" + encodedProposalPid1 + "/access")
.get("/api/v3/proposals/" + encodedProposalPid1 + "/authorization")
.set("Accept", "application/json")
.set({ Authorization: `Bearer ${accessTokenAdminIngestor}` })
.expect(TestData.SuccessfulGetStatusCode)
Expand Down Expand Up @@ -256,7 +256,7 @@ describe("1400: ProposalAuthorization: Test access to proposal", () => {

it("0081: check admin access to proposal 2 should return true", async () => {
return request(appUrl)
.get("/api/v3/proposals/" + encodedProposalPid2 + "/access")
.get("/api/v3/proposals/" + encodedProposalPid2 + "/authorization")
.set("Accept", "application/json")
.set({ Authorization: `Bearer ${accessTokenAdminIngestor}` })
.expect(TestData.SuccessfulGetStatusCode)
Expand All @@ -280,7 +280,7 @@ describe("1400: ProposalAuthorization: Test access to proposal", () => {

it("0091: check admin access to proposal 3 should return true", async () => {
return request(appUrl)
.get("/api/v3/proposals/" + encodedProposalPid3 + "/access")
.get("/api/v3/proposals/" + encodedProposalPid3 + "/authorization")
.set("Accept", "application/json")
.set({ Authorization: `Bearer ${accessTokenAdminIngestor}` })
.expect(TestData.SuccessfulGetStatusCode)
Expand Down Expand Up @@ -314,7 +314,7 @@ describe("1400: ProposalAuthorization: Test access to proposal", () => {

it("0111: check user 1 access to proposal 1 should return false", async () => {
return request(appUrl)
.get("/api/v3/proposals/" + encodedProposalPid1 + "/access")
.get("/api/v3/proposals/" + encodedProposalPid1 + "/authorization")
.set("Accept", "application/json")
.set({ Authorization: `Bearer ${accessTokenUser1}` })
.expect(TestData.SuccessfulGetStatusCode)
Expand All @@ -338,7 +338,7 @@ describe("1400: ProposalAuthorization: Test access to proposal", () => {

it("0121: check user 1 access to proposal 2 should return true", async () => {
return request(appUrl)
.get("/api/v3/proposals/" + encodedProposalPid2 + "/access")
.get("/api/v3/proposals/" + encodedProposalPid2 + "/authorization")
.set("Accept", "application/json")
.set({ Authorization: `Bearer ${accessTokenUser1}` })
.expect(TestData.SuccessfulGetStatusCode)
Expand All @@ -359,7 +359,7 @@ describe("1400: ProposalAuthorization: Test access to proposal", () => {

it("0131: check user 1 access to proposal 3 should return false", async () => {
return request(appUrl)
.get("/api/v3/proposals/" + encodedProposalPid3 + "/access")
.get("/api/v3/proposals/" + encodedProposalPid3 + "/authorization")
.set("Accept", "application/json")
.set({ Authorization: `Bearer ${accessTokenUser1}` })
.expect(TestData.SuccessfulGetStatusCode)
Expand Down Expand Up @@ -406,7 +406,7 @@ describe("1400: ProposalAuthorization: Test access to proposal", () => {

it("0161: check user 2 access to proposal 1 should return false", async () => {
return request(appUrl)
.get("/api/v3/proposals/" + encodedProposalPid1 + "/access")
.get("/api/v3/proposals/" + encodedProposalPid1 + "/authorization")
.set("Accept", "application/json")
.set({ Authorization: `Bearer ${accessTokenUser2}` })
.expect("Content-Type", /json/)
Expand All @@ -430,7 +430,7 @@ describe("1400: ProposalAuthorization: Test access to proposal", () => {

it("0166: check user 2 access to proposal 2 should return true", async () => {
return request(appUrl)
.get("/api/v3/proposals/" + encodedProposalPid2 + "/access")
.get("/api/v3/proposals/" + encodedProposalPid2 + "/authorization")
.set("Accept", "application/json")
.set({ Authorization: `Bearer ${accessTokenUser2}` })
.expect("Content-Type", /json/)
Expand All @@ -454,7 +454,7 @@ describe("1400: ProposalAuthorization: Test access to proposal", () => {

it("0171: check user 2 access to proposal 3 should return true", async () => {
return request(appUrl)
.get("/api/v3/proposals/" + encodedProposalPid3 + "/access")
.get("/api/v3/proposals/" + encodedProposalPid3 + "/authorization")
.set("Accept", "application/json")
.set({ Authorization: `Bearer ${accessTokenUser2}` })
.expect("Content-Type", /json/)
Expand Down
38 changes: 19 additions & 19 deletions test/SampleAuthorization.js
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ describe("2250: Sample Authorization", () => {
.post("/api/v3/Samples")
.send(sample)
.set("Accept", "application/json")
.expect(TestData.UnauthorizedStatusCode);
.expect(TestData.CreationUnauthorizedStatusCode);
});

it("0180: adds a new sample as an unauthenticated user with owner group as sampleingestor, which should fail", async () => {
Expand All @@ -498,7 +498,7 @@ describe("2250: Sample Authorization", () => {
.post("/api/v3/Samples")
.send(sample)
.set("Accept", "application/json")
.expect(TestData.UnauthorizedStatusCode);
.expect(TestData.CreationUnauthorizedStatusCode);
});

it("0190: adds a new sample as an unauthenticated user with owner group as user1, which should fail", async () => {
Expand All @@ -511,7 +511,7 @@ describe("2250: Sample Authorization", () => {
.post("/api/v3/Samples")
.send(sample)
.set("Accept", "application/json")
.expect(TestData.UnauthorizedStatusCode);
.expect(TestData.CreationUnauthorizedStatusCode);
});

it("0200: adds a new sample as an unauthenticated user with its owner group, which should fail", async () => {
Expand All @@ -524,7 +524,7 @@ describe("2250: Sample Authorization", () => {
.post("/api/v3/Samples")
.send(sample)
.set("Accept", "application/json")
.expect(TestData.UnauthorizedStatusCode);
.expect(TestData.CreationUnauthorizedStatusCode);
});

it("0210: adds a new sample as Archive Manager with owner group as adminingestor, which should fail", async () => {
Expand Down Expand Up @@ -977,31 +977,31 @@ describe("2250: Sample Authorization", () => {
.post("/api/v3/Samples/" + sampleId1 + "/attachments")
.send(TestData.AttachmentCorrect)
.set("Accept", "application/json")
.expect(TestData.UnauthorizedStatusCode);
.expect(TestData.CreationUnauthorizedStatusCode);
});

it("0420: adds an attachment as an unauthenticated user to sample 2, which should fail", async () => {
return request(appUrl)
.post("/api/v3/Samples/" + sampleId2 + "/attachments")
.send(TestData.AttachmentCorrect)
.set("Accept", "application/json")
.expect(TestData.UnauthorizedStatusCode);
.expect(TestData.CreationUnauthorizedStatusCode);
});

it("0430: adds an attachment as an unauthenticated user to sample 3, which should fail", async () => {
return request(appUrl)
.post("/api/v3/Samples/" + sampleId3 + "/attachments")
.send(TestData.AttachmentCorrect)
.set("Accept", "application/json")
.expect(TestData.UnauthorizedStatusCode);
.expect(TestData.CreationUnauthorizedStatusCode);
});

it("0440: adds an attachment as an unauthenticated user to sample 4, which should fail", async () => {
return request(appUrl)
.post("/api/v3/Samples/" + sampleId4 + "/attachments")
.send(TestData.AttachmentCorrect)
.set("Accept", "application/json")
.expect(TestData.UnauthorizedStatusCode);
.expect(TestData.CreationUnauthorizedStatusCode);
});

it("0450: adds an attachment as Archive Manager to sample 1, which should fail", async () => {
Expand Down Expand Up @@ -1351,7 +1351,7 @@ describe("2250: Sample Authorization", () => {

it("0641: check Admin Ingestor access to public sample 1 should return true", async () => {
return request(appUrl)
.get("/api/v3/Samples/" + sampleId1 + "/access")
.get("/api/v3/Samples/" + sampleId1 + "/authorization")
.set("Accept", "application/json")
.set({ Authorization: `Bearer ${accessTokenAdminIngestor}` })
.expect(TestData.SuccessfulGetStatusCode)
Expand Down Expand Up @@ -1599,7 +1599,7 @@ describe("2250: Sample Authorization", () => {

it("0731: check Sample Ingestor access to sample 1 should return false", async () => {
return request(appUrl)
.get("/api/v3/Samples/" + sampleId1 + "/access")
.get("/api/v3/Samples/" + sampleId1 + "/authorization")
.set("Accept", "application/json")
.set({ Authorization: `Bearer ${accessTokenSampleIngestor}` })
.expect(TestData.SuccessfulGetStatusCode)
Expand Down Expand Up @@ -1631,7 +1631,7 @@ describe("2250: Sample Authorization", () => {

it("0741: check Sample Ingestor access to sample 2 should return true", async () => {
return request(appUrl)
.get("/api/v3/Samples/" + sampleId2 + "/access")
.get("/api/v3/Samples/" + sampleId2 + "/authorization")
.set("Accept", "application/json")
.set({ Authorization: `Bearer ${accessTokenSampleIngestor}` })
.expect(TestData.SuccessfulGetStatusCode)
Expand Down Expand Up @@ -1807,7 +1807,7 @@ describe("2250: Sample Authorization", () => {

it("0831: check User 1 access to sample 2 should return false", async () => {
return request(appUrl)
.get("/api/v3/Samples/" + sampleId1 + "/access")
.get("/api/v3/Samples/" + sampleId1 + "/authorization")
.set("Accept", "application/json")
.set({ Authorization: `Bearer ${accessTokenUser1}` })
.expect(TestData.SuccessfulGetStatusCode)
Expand Down Expand Up @@ -1855,7 +1855,7 @@ describe("2250: Sample Authorization", () => {

it("0851: check User 1 access to sample 3 should return true", async () => {
return request(appUrl)
.get("/api/v3/Samples/" + sampleId3 + "/access")
.get("/api/v3/Samples/" + sampleId3 + "/authorization")
.set("Accept", "application/json")
.set({ Authorization: `Bearer ${accessTokenUser1}` })
.expect(TestData.SuccessfulGetStatusCode)
Expand Down Expand Up @@ -2798,7 +2798,7 @@ describe("2250: Sample Authorization", () => {

it("1331: check unauthenticated user access to sample 1 should return false", async () => {
return request(appUrl)
.get("/api/v3/Samples/" + sampleId1 + "/access")
.get("/api/v3/Samples/" + sampleId1 + "/authorization")
.set("Accept", "application/json")
.expect(TestData.SuccessfulGetStatusCode)
.expect("Content-Type", /json/)
Expand Down Expand Up @@ -2939,7 +2939,7 @@ describe("2250: Sample Authorization", () => {

it("1421: check unauthenticated user access to public sample 10 should return true", async () => {
return request(appUrl)
.get("/api/v3/Samples/" + sampleId10 + "/access")
.get("/api/v3/Samples/" + sampleId10 + "/authorization")
.set("Accept", "application/json")
.expect(TestData.SuccessfulGetStatusCode)
.expect("Content-Type", /json/)
Expand Down Expand Up @@ -3658,7 +3658,7 @@ describe("2250: Sample Authorization", () => {
.patch("/api/v3/Samples/" + sampleId1)
.send({ sampleCharacteristics: characteristics })
.set("Accept", "application/json")
.expect(TestData.UnauthorizedStatusCode);
.expect(TestData.CreationUnauthorizedStatusCode);
});

it("2280: update sample characteristic for public sample 10 as Unauthenticated User, which should fail", async () => {
Expand All @@ -3674,15 +3674,15 @@ describe("2250: Sample Authorization", () => {
.patch("/api/v3/Samples/" + sampleId10)
.send({ sampleCharacteristics: characteristics })
.set("Accept", "application/json")
.expect(TestData.UnauthorizedStatusCode);
.expect(TestData.CreationUnauthorizedStatusCode);
});

// delete sample attachment
it("4000: delete attachment 1 from sample 1 as Unauthenticated User, which should fail", async () => {
return request(appUrl)
.delete("/api/v3/samples/" + sampleId1 + "/attachments/" + attachmentId1)
.set("Accept", "application/json")
.expect(TestData.UnauthorizedStatusCode);
.expect(TestData.CreationUnauthorizedStatusCode);
});

it("4010: delete attachment 10 from sample 10 as Unauthenticated User, which should fail", async () => {
Expand All @@ -3691,7 +3691,7 @@ describe("2250: Sample Authorization", () => {
"/api/v3/samples/" + sampleId10 + "/attachments/" + attachmentId10,
)
.set("Accept", "application/json")
.expect(TestData.UnauthorizedStatusCode);
.expect(TestData.CreationUnauthorizedStatusCode);
});

it("4020: delete attachment 1 from sample 1 as User 5, which should fail", async () => {
Expand Down
1 change: 1 addition & 0 deletions test/TestData.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const TestData = {
BadRequestStatusCode: 400,
AccessForbiddenStatusCode: 403,
UnauthorizedStatusCode: 401,
CreationUnauthorizedStatusCode: 401,
ConflictStatusCode: 409,
ApplicationErrorStatusCode: 500,
LoginSuccessfulStatusCode: 201,
Expand Down
2 changes: 1 addition & 1 deletion test/config/pretest.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

var chaiHttp = require("chai-http");
const { MongoClient } = require("mongodb");
const testDbName = process.env.MONGODB_TEST || "scicat-test";
const testDbName = "scicat-test-db";
const uri = `mongodb://localhost:27017/${testDbName}`;

const client = new MongoClient(uri);
Expand Down

0 comments on commit f705241

Please sign in to comment.