Skip to content

Commit

Permalink
chore: update code as per PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
kamaksheeAtl committed Jan 3, 2024
1 parent 64b1def commit 32b233a
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 28 deletions.
14 changes: 7 additions & 7 deletions src/rest/rest-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,23 @@ import { RestErrorHandler } from "./middleware/error";
import { JiraAdminEnforceMiddleware } from "./middleware/jira-admin/jira-admin-check";
import { AnalyticsProxyHandler } from "./routes/analytics-proxy";
import { SubscriptionsRouter } from "./routes/subscriptions";
import { gheServerRouter, deleteEnterpriseAppHandler } from "./routes/enterprise";
import { DeferredRouter } from "./routes/deferred";
import { deleteEnterpriseAppHandler, deleteEnterpriseServerHandler } from "./routes/enterprise";


export const RestRouter = Router({ mergeParams: true });

const subRouter = Router({ mergeParams: true });
const gheServerRouter = Router({ mergeParams: true });

/**
* Separate route which returns the list of both cloud and server subscriptions
*/
RestRouter.use("/subscriptions", JwtHandler, JiraAdminEnforceMiddleware, SubscriptionsRouter);

RestRouter.use("/ghes-servers/:serverUrl",JwtHandler, JiraAdminEnforceMiddleware, gheServerRouter);
gheServerRouter.delete("/", deleteEnterpriseServerHandler);
/**
* Separate route which deletes the GHE server for given serverUrl
*/
RestRouter.use("/ghes-servers/:serverUrl", JwtHandler, JiraAdminEnforceMiddleware, gheServerRouter);

/**
* For cloud flow, the path will be `/rest/app/cloud/XXX`,
Expand All @@ -45,10 +46,9 @@ subRouter.use("/deferred", DeferredRouter);
// have done authentication only)?
subRouter.use(JwtHandler);
subRouter.use(JiraAdminEnforceMiddleware);
// This is to delete GHE server with specific UUID
subRouter.delete("/", deleteEnterpriseAppHandler);

// This is to delete GHE app which is associated with specific server having UUID
// subRouter.delete("/ghe-app", deleteEnterpriseAppHandler);
subRouter.delete("/", deleteEnterpriseAppHandler);

subRouter.post("/analytics-proxy", AnalyticsProxyHandler);

Expand Down
16 changes: 3 additions & 13 deletions src/rest/routes/enterprise/delete-ghe-app.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,9 @@ import { encodeSymmetric } from "atlassian-jwt";
import { GitHubServerApp } from "models/github-server-app";
import { v4 as newUUID } from "uuid";

jest.mock("~/src/sqs/queues");
jest.mock("config/feature-flags");

describe("Checking the sync request parsing route", () => {
let app: Express;
let installation: Installation;
const installationIdForCloud = 1;
const installationIdForServer = 2;
const uuid = newUUID();
let gitHubServerApp: GitHubServerApp;
Expand Down Expand Up @@ -44,12 +40,6 @@ describe("Checking the sync request parsing route", () => {
sharedSecret: testSharedSecret,
clientKey: clientKey
});
await Subscription.install({
installationId: installationIdForCloud,
host: jiraHost,
hashedClientKey: installation.clientKey,
gitHubAppId: undefined
});
gitHubServerApp = await GitHubServerApp.install(
{
uuid: uuid,
Expand All @@ -74,7 +64,7 @@ describe("Checking the sync request parsing route", () => {
app.use(RootRouter);
});

describe("cloud", () => {
describe("GHE server app delete", () => {
it("should throw 401 error when no github token is passed", async () => {
const resp = await supertest(app).delete(
`/rest/app/${gitHubServerApp.uuid}`
Expand All @@ -89,11 +79,11 @@ describe("Checking the sync request parsing route", () => {
expect(resp.status).toEqual(400);
});

it("should return 200 on correct uuid", async () => {
it("should return 204 on correct uuid", async () => {
const resp = await supertest(app)
.delete(`/rest/app/${gitHubServerApp.uuid}`)
.set("authorization", `${getToken()}`);
expect(resp.status).toEqual(200);
expect(resp.status).toEqual(204);
});
});
});
2 changes: 1 addition & 1 deletion src/rest/routes/enterprise/delete-ghe-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const deleteEnterpriseApp = async (
if (!(await isConnected(installation.jiraHost))) {
await saveConfiguredAppProperties(installation.jiraHost, req.log, false);
}
res.status(200).json("Success");
res.sendStatus(204);
req.log.debug("Jira Connect Enterprise App deleted successfully.");
};

Expand Down
94 changes: 94 additions & 0 deletions src/rest/routes/enterprise/delete-ghe-server.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import { getFrontendApp } from "~/src/app";
import { Installation } from "models/installation";
import { Subscription } from "models/subscription";
import express, { Express } from "express";
import { RootRouter } from "routes/router";
import supertest from "supertest";
import { encodeSymmetric } from "atlassian-jwt";
import { GitHubServerApp } from "models/github-server-app";
import { v4 as newUUID } from "uuid";

describe("Checking the sync request parsing route", () => {
let app: Express;
let installation: Installation;
// const installationIdForCloud = 1;
const installationIdForServer = 2;
const uuid = newUUID();
let gitHubServerApp: GitHubServerApp;
const testSharedSecret = "test-secret";
const clientKey = "jira-client-key";
const getToken = ({
secret = testSharedSecret,
iss = clientKey,
exp = Date.now() / 1000 + 10000,
qsh = "context-qsh",
sub = "myAccount"
} = {}): string => {
return encodeSymmetric(
{
qsh,
iss,
exp,
sub
},
secret
);
};
beforeEach(async () => {
app = getFrontendApp();
installation = await Installation.install({
host: jiraHost,
sharedSecret: testSharedSecret,
clientKey: clientKey
});
gitHubServerApp = await GitHubServerApp.install(
{
uuid: uuid,
appId: 123,
gitHubAppName: "My GitHub Server App",
gitHubBaseUrl: gheUrl,
gitHubClientId: "lvl.1234",
gitHubClientSecret: "myghsecret",
webhookSecret: "mywebhooksecret",
privateKey: "myprivatekey",
installationId: installation.id
},
jiraHost
);
await Subscription.install({
installationId: installationIdForServer,
host: jiraHost,
hashedClientKey: installation.clientKey,
gitHubAppId: gitHubServerApp.id
});
app = express();
app.use(RootRouter);
});

describe("GHE server delete", () => {
it("should throw 404 error when api path is not found", async () => {
const resp = await supertest(app).delete(
`/rest/ghes-serverss/${gitHubServerApp.gitHubBaseUrl}`
);
expect(resp.status).toEqual(404);
});
it("should throw 401 error when no github token is passed", async () => {
const encodedGHEBaseUrl = encodeURIComponent(
gitHubServerApp.gitHubBaseUrl
);
const resp = await supertest(app)
.delete(`/rest/ghes-servers/${encodedGHEBaseUrl}`);
expect(resp.status).toEqual(500);
});

it("should return 204 on correct uuid", async () => {
const encodedGHEBaseUrl = encodeURIComponent(
gitHubServerApp.gitHubBaseUrl
);
const resp = await supertest(app)
.delete(`/rest/ghes-servers/${encodedGHEBaseUrl}`)
.set("authorization", `${getToken()}`);
expect(resp.status).toEqual(204);
});
});
});
10 changes: 5 additions & 5 deletions src/rest/routes/enterprise/delete-ghe-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ const deleteEnterpriseServer = async (
res: Response<string, BaseLocals>
): Promise<void> => {
const { installation } = res.locals;

const encodedGHEBaseUrl = req.params.serverUrl;
const gitHubBaseUrl = decodeURIComponent(encodedGHEBaseUrl);
if (!gitHubBaseUrl) {

if (!encodedGHEBaseUrl){
throw new InvalidArgumentError(
"Invalid route, couldn't determine gitHubBaseUrl for enterprise server!"
"Invalid route, couldn't find encodedGHEBaseUrl in rest api req params!"
);
}
const gitHubBaseUrl = decodeURIComponent(encodedGHEBaseUrl);

await GitHubServerApp.uninstallServer(
gitHubBaseUrl,
Expand All @@ -29,7 +29,7 @@ const deleteEnterpriseServer = async (
if (!(await isConnected(installation.jiraHost))) {
await saveConfiguredAppProperties(installation.jiraHost, req.log, false);
}
res.status(200).json("Success");
res.sendStatus(204);
};

export const deleteEnterpriseServerHandler = errorWrapper(
Expand Down
8 changes: 6 additions & 2 deletions src/rest/routes/enterprise/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
export { deleteEnterpriseServerHandler } from "./delete-ghe-server";
export { deleteEnterpriseAppHandler } from "./delete-ghe-app";
import { Router } from "express";
import { deleteEnterpriseServerHandler } from "./delete-ghe-server";
export { deleteEnterpriseAppHandler } from "./delete-ghe-app";
export const gheServerRouter = Router({ mergeParams: true });

gheServerRouter.delete("/", deleteEnterpriseServerHandler);

0 comments on commit 32b233a

Please sign in to comment.