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

Remove the code access to old column #1638

Merged
merged 1 commit into from
Oct 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions src/jira/verify-installation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ describe("verify-installation", () => {
sharedSecret: "new-encrypted-shared-secret",
clientKey: "client-key"
});
//doing bellow so that the sharedSecret is "shared-secret",
//while the encryptedSharedSecret will be "new-encrypted-shared-secret"
//so that we can test the FF
installation.sharedSecret = "shared-secret";
});

function mockJiraResponse(status: number) {
Expand Down
2 changes: 1 addition & 1 deletion src/middleware/jira-jwt-middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ describe("#verifyJiraMiddleware", () => {

});

describe("decyrpting installation sharedSecret", ()=>{
describe("decyrpting installation encryptedSharedSecret", ()=>{
let installation: Installation;
beforeEach(()=>{
installation = {
Expand Down
34 changes: 3 additions & 31 deletions src/models/installation.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,12 @@
import { BOOLEAN, DataTypes, DATE } from "sequelize";
import { Subscription } from "./subscription";
import { encrypted, getHashedKey, sequelize } from "models/sequelize";
import { getHashedKey, sequelize } from "models/sequelize";
import { EncryptedModel } from "models/encrypted-model";
import { EncryptionSecretKeyEnum } from "utils/encryption-client";
import { getLogger } from "config/logger";

// TODO: this should not be there. Should only check once a function is called
if (!process.env.STORAGE_SECRET) {
throw new Error("STORAGE_SECRET is not defined.");
}

const logger = getLogger("model-installations");

export class Installation extends EncryptedModel {
id: number;
jiraHost: string;
secrets: string;
sharedSecret: string;
encryptedSharedSecret: string;
clientKey: string;
updatedAt: Date;
Expand Down Expand Up @@ -126,11 +116,6 @@ Installation.init({
autoIncrement: true
},
jiraHost: DataTypes.STRING,
secrets: encrypted.vault("secrets"),
sharedSecret: encrypted.field("sharedSecret", {
type: DataTypes.STRING,
allowNull: true
}),
encryptedSharedSecret: {
type: DataTypes.TEXT,
allowNull: true
Expand All @@ -146,24 +131,12 @@ Installation.init({
hooks: {
beforeSave: async (instance: Installation, opts) => {
if (!opts.fields) return;
try {
await instance.encryptChangedSecretFields(opts.fields);
} catch (_) {
//Just catch and swallow the error, don't want to fail installations right now if cryptor fail for whatever reason
//TODO: monitor the prod behaviour and remove this catch along with other migration rollout in another PR.
logger.error(`Fail encrypting sharedSecret using cryptor`);
}
await instance.encryptChangedSecretFields(opts.fields);
},
beforeBulkCreate: async (instances: Installation[], opts) => {
for (const instance of instances) {
if (!opts.fields) return;
try {
await instance.encryptChangedSecretFields(opts.fields);
} catch (_) {
//Just catch and swallow the error, don't want to fail installations right now if cryptor fail for whatever reason
//TODO: monitor the prod behaviour and remove this catch along with other migration rollout in another PR.
logger.error(`Fail encrypting sharedSecret using cryptor`);
}
await instance.encryptChangedSecretFields(opts.fields);
}
}
},
Expand All @@ -173,6 +146,5 @@ Installation.init({
export interface InstallationPayload {
host: string;
clientKey: string;
// secret: string;
sharedSecret: string;
}
3 changes: 0 additions & 3 deletions src/routes/github/setup/github-setup-router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,6 @@ describe("Github Setup", () => {
it("should return a 200 with the redirect url to the app if a valid domain is given and an installation already exists", async () => {
await Installation.create({
jiraHost,
//TODO: why? Comment this out make test works?
//setting both fields make sequelize confused as it internally storage is just the "secrets"
//secrets: "secret",
encryptedSharedSecret: "sharedSecret",
clientKey: "clientKey"
});
Expand Down
2 changes: 0 additions & 2 deletions src/routes/jira/events/jira-events-install-post.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ describe("Webhook: /events/installed", () => {
jiraHost: body.baseUrl,
clientKey: body.clientKey,
enabled: true,
secrets: "def234",
sharedSecret: body.sharedSecret,
subscriptions: jest.fn().mockResolvedValue([])
};

Expand Down
2 changes: 0 additions & 2 deletions src/routes/jira/events/jira-events-uninstall-post.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ describe("Webhook: /events/uninstalled", () => {
jiraHost: "https://test-host.jira.com",
clientKey: getHashedKey("abc123"),
enabled: true,
secrets: "def234",
sharedSecret: "ghi345",
uninstall: jest
.fn()
.mockName("uninstall")
Expand Down