diff --git a/client/coral-framework/actions/user.js b/client/coral-framework/actions/user.js index 69f4882bb..0ee8659d8 100644 --- a/client/coral-framework/actions/user.js +++ b/client/coral-framework/actions/user.js @@ -32,7 +32,7 @@ export const saveBio = (user_id, formData) => dispatch => { export const fetchCommentsByUserId = userId => { return (dispatch) => { dispatch({type: actions.COMMENTS_BY_USER_REQUEST}); - return coralApi(`/comments?user_id${userId}`) + return coralApi(`/comments?user_id=${userId}`) .then(({comments, assets}) => { comments.forEach(comment => dispatch(addItem(comment, 'comments'))); diff --git a/client/coral-framework/reducers/user.js b/client/coral-framework/reducers/user.js index 6e9f3529c..bd5f78e87 100644 --- a/client/coral-framework/reducers/user.js +++ b/client/coral-framework/reducers/user.js @@ -1,4 +1,4 @@ -import {Map, fromJS} from 'immutable'; +import {Map} from 'immutable'; import * as authActions from '../constants/auth'; import * as actions from '../constants/user'; import * as assetActions from '../constants/assets'; @@ -34,9 +34,9 @@ export default function user (state = initialState, action) { return state .set('settings', action.settings); case actions.COMMENTS_BY_USER_SUCCESS: - return state.set('myComments', fromJS(action.comments)); + return state.set('myComments', action.comments); case assetActions.MULTIPLE_ASSETS_SUCCESS: - return state.set('myAssets', fromJS(action.assets)); + return state.set('myAssets', action.assets); default : return state; } diff --git a/client/coral-plugin-history/Comment.js b/client/coral-plugin-history/Comment.js index c54a0feef..37a93df13 100644 --- a/client/coral-plugin-history/Comment.js +++ b/client/coral-plugin-history/Comment.js @@ -6,7 +6,7 @@ const Comment = props => { return (

- {props.asset.url} + {`${props.asset.url}#${props.comment.id}`}

{props.comment.body}

diff --git a/routes/api/comments/index.js b/routes/api/comments/index.js index 8f21a9488..90b8b40f2 100644 --- a/routes/api/comments/index.js +++ b/routes/api/comments/index.js @@ -9,7 +9,7 @@ const _ = require('lodash'); const router = express.Router(); -router.get('/', authorization.needed('admin'), (req, res, next) => { +router.get('/', (req, res, next) => { const { status = null, @@ -18,6 +18,18 @@ router.get('/', authorization.needed('admin'), (req, res, next) => { user_id = null } = req.query; + // everything on this route requires admin privileges besides listing comments for owner of said comments + if (!authorization.has(req.user, 'admin') && !user_id) { + next(authorization.ErrNotAuthorized); + return; + } + + // if the user is not an admin, only return comment list for the owner of the comments + if (req.user.id !== user_id && !authorization.has(req.user, 'admin')) { + next(authorization.ErrNotAuthorized); + return; + } + /** * This adds the asset_id requirement to the query if the asset_id is defined. */ @@ -31,10 +43,13 @@ router.get('/', authorization.needed('admin'), (req, res, next) => { let query; - if (status) { - query = assetIDWrap(Comment.findByStatus(status === 'new' ? null : status)); - } else if (user_id) { + // the check for user_id MUST be first here. + // otherwise this will be a vulnerability if you pass user_id and something else, + // the app will return admin-level data without the proper checks + if (user_id) { query = Comment.findByUserId(user_id); + } else if (status) { + query = assetIDWrap(Comment.findByStatus(status === 'new' ? null : status)); } else if (action_type) { query = Comment .findIdsByActionType(action_type)