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

Feature/json validation #1700

Merged
merged 11 commits into from
Feb 12, 2025
14 changes: 14 additions & 0 deletions Server/db/models/Monitor.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,20 @@ const MonitorSchema = mongoose.Schema(
"distributed_http",
],
},
jsonPath: {
type: String,
},
expectedValue: {
type: String,
},
matchMethod: {
type: String,
enum: [
"equal",
"include",
"regex",
],
},
url: {
type: String,
required: true,
Expand Down
8 changes: 7 additions & 1 deletion Server/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -144,5 +144,11 @@
"monitorResume": "Monitor resumed successfully",
"statusPageDelete": "Status page deleted successfully",
"statusPageUpdate": "Status page updated successfully",
"statusPageByTeamId": "Got status pages by team id successfully"
"statusPageByTeamId": "Got status pages by team id successfully",
"httpNetworkError": "Network error",
"httpNotJson": "Response data is not json",
"httpJsonPathError": "Failed to parse json data",
"httpEmptyResult": "Result is empty",
"httpMatchSuccess": "Response data match successfully",
"httpMatchFail": "Failed to match response data"
}
10 changes: 10 additions & 0 deletions Server/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"handlebars": "^4.7.8",
"helmet": "^8.0.0",
"ioredis": "^5.4.2",
"jmespath": "^0.16.0",
"joi": "^17.13.1",
"jsonwebtoken": "9.0.2",
"mailersend": "^2.2.0",
Expand Down
68 changes: 59 additions & 9 deletions Server/service/networkService.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import jmespath from 'jmespath';
const SERVICE_NAME = "NetworkService";
const UPROCK_ENDPOINT = "https://api.uprock.com/checkmate/push";

Expand Down Expand Up @@ -122,20 +123,19 @@ class NetworkService {
*/
async requestHttp(job) {
try {
const url = job.data.url;
const { url, secret, _id, name, teamId, type, jsonPath, matchMethod, expectedValue } = job.data;
const config = {};

job.data.secret !== undefined &&
(config.headers = { Authorization: `Bearer ${job.data.secret}` });
secret !== undefined && (config.headers = { Authorization: `Bearer ${secret}` });
Comment on lines +126 to +129
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance secret handling security.

The current secret handling might expose sensitive data in logs. Let's keep that secret sauce under wraps!

-    secret !== undefined && (config.headers = { Authorization: `Bearer ${secret}` });
+    if (secret) {
+        // Avoid using template literals with secrets to prevent accidental logging
+        config.headers = { Authorization: 'Bearer ' + secret };
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const { url, secret, _id, name, teamId, type, jsonPath, matchMethod, expectedValue } = job.data;
const config = {};
job.data.secret !== undefined &&
(config.headers = { Authorization: `Bearer ${job.data.secret}` });
secret !== undefined && (config.headers = { Authorization: `Bearer ${secret}` });
const { url, secret, _id, name, teamId, type, jsonPath, matchMethod, expectedValue } = job.data;
const config = {};
if (secret) {
// Avoid using template literals with secrets to prevent accidental logging
config.headers = { Authorization: 'Bearer ' + secret };
}


const { response, responseTime, error } = await this.timeRequest(() =>
this.axios.get(url, config)
);

const httpResponse = {
monitorId: job.data._id,
teamId: job.data.teamId,
type: job.data.type,
monitorId: _id,
teamId,
type,
responseTime,
payload: response?.data,
};
Expand All @@ -144,12 +144,62 @@ class NetworkService {
const code = error.response?.status || this.NETWORK_ERROR;
httpResponse.code = code;
httpResponse.status = false;
httpResponse.message = this.http.STATUS_CODES[code] || "Network Error";
httpResponse.message = this.http.STATUS_CODES[code] || this.stringService.httpNetworkError;
return httpResponse;
}
httpResponse.status = true;

httpResponse.code = response.status;
httpResponse.message = this.http.STATUS_CODES[response.status];

if (!expectedValue) {
// not configure expected value, return
httpResponse.status = true;
httpResponse.message = this.http.STATUS_CODES[response.status];
return httpResponse;
}

// validate if response data match expected value
let result = response?.data;

this.logger.info({
service: this.SERVICE_NAME,
method: "requestHttp",
message: `Job: [${name}](${_id}) match result with expected value`,
details: { expectedValue, result, jsonPath, matchMethod }
});

if (jsonPath) {
const contentType = response.headers['content-type'];

const isJson = contentType?.includes('application/json');
if (!isJson) {
httpResponse.status = false;
httpResponse.message = this.stringService.httpNotJson;
return httpResponse;
}

try {
result = jmespath.search(result, jsonPath);
} catch (error) {
httpResponse.status = false;
httpResponse.message = this.stringService.httpJsonPathError;
return httpResponse;
}
}

if (result === null || result === undefined) {
httpResponse.status = false;
httpResponse.message = this.stringService.httpEmptyResult;
return httpResponse;
}

let match;
result = typeof result === "object" ? JSON.stringify(result) : result.toString();
if (matchMethod === "include") match = result.includes(expectedValue);
else if (matchMethod === "regex") match = new RegExp(expectedValue).test(result);
else match = result === expectedValue;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks good, this is much easier to read 👍


httpResponse.status = match;
httpResponse.message = match ? this.stringService.httpMatchSuccess : this.stringService.httpMatchFail;
return httpResponse;
} catch (error) {
error.service = this.SERVICE_NAME;
Expand Down
24 changes: 24 additions & 0 deletions Server/service/stringService.js
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,30 @@ class StringService {
return this.translationService.getTranslation('getAppSettings');
}

get httpNetworkError() {
return this.translationService.getTranslation('httpNetworkError');
}

get httpNotJson() {
return this.translationService.getTranslation('httpNotJson');
}

get httpJsonPathError() {
return this.translationService.getTranslation('httpJsonPathError');
}

get httpEmptyResult() {
return this.translationService.getTranslation('httpEmptyResult');
}

get httpMatchSuccess() {
return this.translationService.getTranslation('httpMatchSuccess');
}

get httpMatchFail() {
return this.translationService.getTranslation('httpMatchFail');
}

get updateAppSettings() {
return this.translationService.getTranslation('updateAppSettings');
}
Expand Down
6 changes: 6 additions & 0 deletions Server/validation/joi.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,9 @@ const createMonitorBodyValidation = joi.object({
}),
notifications: joi.array().items(joi.object()),
secret: joi.string(),
jsonPath: joi.string(),
expectedValue: joi.string(),
matchMethod: joi.string(),
Comment on lines +197 to +199
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add enumeration validation for matchMethod in create monitor validation

To ensure only valid match methods are accepted, add enumeration constraints to the matchMethod field.

Apply this diff:

      matchMethod: joi.string(),
+     matchMethod: joi.string().valid('equal', 'include', 'regex'),

Committable suggestion skipped: line range outside the PR's diff.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is worth doing

});

const editMonitorBodyValidation = joi.object({
Expand All @@ -202,6 +205,9 @@ const editMonitorBodyValidation = joi.object({
interval: joi.number(),
notifications: joi.array().items(joi.object()),
secret: joi.string(),
jsonPath: joi.string(),
expectedValue: joi.string(),
matchMethod: joi.string(),
Comment on lines +208 to +210
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add enumeration validation for matchMethod in edit monitor validation

Ensure consistency by adding the same enumeration constraints to matchMethod in the edit validation schema.

Apply this diff:

      matchMethod: joi.string(),
+     matchMethod: joi.string().valid('equal', 'include', 'regex'),

Committable suggestion skipped: line range outside the PR's diff.

Copy link
Collaborator

Choose a reason for hiding this comment

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

same here, worth addding this to the validation

});

const pauseMonitorParamValidation = joi.object({
Expand Down