Adds new review-history endpoints #417
Conversation
| @@ -1,6 +1,7 @@ | |||
| 'use strict'; | |||
| const dbUtils = require('./utils') | |||
| const config = require('../../utils/config.js') | |||
| const Security = require('../utils/accessLevels') | |||
There was a problem hiding this comment.
This require needs to backtrack one more directory level.
'../../utils/accessLevels.js'
The current code prevents the API from starting.
| If rule and asset id provided, return that intersection. | ||
| */ | ||
| exports.getReviewHistoryByCollection = async function (collectionId, startDate, endDate, assetId, ruleId, status, userObject) { | ||
| const level1User = userObject.collectionGrants.find( g => g.collection.collectionId === collectionId && g.collection.lvl1User) |
There was a problem hiding this comment.
No such property g.collection.lvl1User
const level1User = userObject.collectionGrants.find( g => g.collection.collectionId === collectionId && g.accessLevel === 1)
There was a problem hiding this comment.
bizarre, no idea where i got lvl1User from.
Is changing to:
const restrictedUser = userObject.collectionGrants.find( g => g.collection.collectionId === collectionId && g.accessLevel === Security.ACCESS_LEVEL.Restricted)
acceptable?
| Projection: asset - Break out statistics by Asset in the specified collection | ||
| */ | ||
| exports.getReviewHistoryStatsByCollection = async function (collectionId, startDate, endDate, assetId, ruleId, status, projection, userObject) { | ||
| const level1User = userObject.collectionGrants.find( g => g.collection.collectionId === collectionId && g.collection.lvl1User) |
There was a problem hiding this comment.
No such property g.collection.lvl1User
const level1User = userObject.collectionGrants.find( g => g.collection.collectionId === collectionId && g.accessLevel === 1)
There was a problem hiding this comment.
same fix made as above
| for(const row of rows) { | ||
| for(const history of row.history) { | ||
|
|
||
| //Translate the numeric values from the database back to text since we don't have lookup tables for these. | ||
| history.result = Security.getKeyByValue(dbUtils.REVIEW_RESULT_API, history.resultId) | ||
| history.action = Security.getKeyByValue(dbUtils.REVIEW_ACTION_API, history.actionId) | ||
| history.status = Security.getKeyByValue(dbUtils.REVIEW_STATUS_API, history.statusId) | ||
|
|
||
| // Remove the properties which have been translated to text. | ||
| delete history.resultId | ||
| delete history.actionId | ||
| delete history.statusId | ||
| } | ||
| } |
There was a problem hiding this comment.
Lookup tables do exist. Instead of iterating through each row of the result set, please write the query to join to tables result, action and status. Each table has a column api which maps XXXXid values to the API string.
Example:
let sql = `
SELECT a.assetId,
(select coalesce(
(select json_arrayagg(
json_object
(
'ruleId', rv.ruleId,
'ts', rh.ts,
'result', result.api,
'resultComment', rh.resultComment,
'action', action.api,
'actionComment', rh.actionComment,
'autoResult', rh.autoResult = 1,
'status', status.api,
'userId', rh.userId,
'username', ud.username,
'rejectText', rh.rejectText,
'rejectUserId', rh.rejectUserId
)
)
FROM review_history rh
INNER JOIN review rv on rh.reviewId = rv.reviewId
INNER JOIN user_data ud on rh.userId = ud.userId
INNER JOIN result on rh.resultId = result.resultId
INNER JOIN status on rh.statusId = status.statusId
INNER JOIN action on rh.actionId = action.actionId
WHERE rv.assetId = a.assetId`
| if(level1User) { | ||
| binds.level1User = userObject.userId | ||
| sql += ` | ||
| AND a.assetId IN ( | ||
| SELECT sam.assetId | ||
| FROM stig_asset_map sam | ||
| INNER JOIN user_stig_asset_map usam ON sam.saId = usam.saId | ||
| WHERE usam.userId = :level1User | ||
| ) | ||
| ` | ||
| } |
There was a problem hiding this comment.
level1User should also be constrained to only those rules in STIGs for which they have Asset/STIG access. However, before proceeding, let's discuss with @cd-rite if it is critical that this endpoint be available to level1Users
There was a problem hiding this comment.
Yeah, I included the lvl1 users for completeness, but it is not critical that they have access to this endpoint. Go ahead and leave them out (I suppose we will have to return a 403?)
There was a problem hiding this comment.
The level 1 condition has been removed
|
@cd-rite Are we awaiting newman test code for this before merging? Or shall that be a separate issue? |
|
@csmig I'll make a separate issue! Merge away, thank you! |
resolves #331
GET /collections/{collectionId}/review-history
Available to all users with a grant to the collection. For level 1 users, only entries that fall under their Asset-STIG grants should be returned.
Returns block of review history entries that fit criteria. Takes optional:
Start Date
End Date
(If no dates provided, return all history. If only one date, return block from that date to current, or that date to oldest, as appropriate)
Asset ID - only return history for this asset id
Rule ID - only return history for this rule id
status- only return history with this status
If rule and asset id provided, return that intersection.
DELETE /collections/{collectionId}/review-history
Available only to level 3 or 4 users ("Manage" or "Owner")
Returns number of history entries deleted.
RetentionDate - Delete all review history entries prior to the specified date.
Asset Id - if provided, only delete entries for that asset.
GET /collections/{collectionId}/review-history/stats
Available to all users with a grant to the collection. For level 1 users, only entries that fall under their Asset-STIG grants should be returned.
Return some simple stats about the number/properties of history entries.
Uses same params as GET review-history, expecting stats to be scoped to whatever would be returned by that query.
Projection: asset - Break out statistics by Asset in the specified collection