diff --git a/client/coral-embed-stream/src/CommentStream.js b/client/coral-embed-stream/src/CommentStream.js
index c052e5aa1..e6cc8fbbc 100644
--- a/client/coral-embed-stream/src/CommentStream.js
+++ b/client/coral-embed-stream/src/CommentStream.js
@@ -61,7 +61,11 @@ class CommentStream extends Component {
// Set up messaging between embedded Iframe an parent component
this.pym = new Pym.Child({polling: 100});
- const path = this.pym.parentUrl.split('#')[0];
+ let path = this.pym.parentUrl.split('#')[0];
+
+ if (!path) {
+ path = window.location.href.split('#')[0];
+ }
this.props.getStream(path || window.location);
this.path = path;
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/constants/assets.js b/client/coral-framework/constants/assets.js
index 0224e0945..3883ee835 100644
--- a/client/coral-framework/constants/assets.js
+++ b/client/coral-framework/constants/assets.js
@@ -1,3 +1,3 @@
export const MULTIPLE_ASSETS_REQUEST = 'MULTIPLE_ASSETS_REQUEST';
export const MULTIPLE_ASSETS_SUCCESS = 'MULTIPLE_ASSETS_SUCCESS';
-export const MULTIPLE_ASSSETS_FAILURE = 'MULTIPLE_ASSSETS_FAILURE';
\ No newline at end of file
+export const MULTIPLE_ASSSETS_FAILURE = 'MULTIPLE_ASSSETS_FAILURE';
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..fd515e628 100644
--- a/client/coral-plugin-history/Comment.js
+++ b/client/coral-plugin-history/Comment.js
@@ -6,7 +6,7 @@ const Comment = props => {
return (
@@ -15,6 +15,7 @@ const Comment = props => {
Comment.propTypes = {
comment: PropTypes.shape({
+ id: PropTypes.string,
body: PropTypes.string
}).isRequired,
asset: PropTypes.shape({
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)
diff --git a/tests/client/coral-plugin-history/Comment.spec.js b/tests/client/coral-plugin-history/Comment.spec.js
index 7d49630d6..2098fa1b3 100644
--- a/tests/client/coral-plugin-history/Comment.spec.js
+++ b/tests/client/coral-plugin-history/Comment.spec.js
@@ -5,7 +5,7 @@ import Comment from '../../../client/coral-plugin-history/Comment';
describe('coral-plugin-history/Comment', () => {
let render;
- const comment = {body: 'this is a comment'};
+ const comment = {body: 'this is a comment', id: '123'};
const asset = {url: 'https://google.com'};
beforeEach(() => {
@@ -21,7 +21,7 @@ describe('coral-plugin-history/Comment', () => {
it('should render the asset url as a link', () => {
const wrapper = mount();
expect(wrapper.find('.myCommentAnchor')).to.have.length(1);
- expect(wrapper.find('.myCommentAnchor').text()).to.equal('https://google.com');
+ expect(wrapper.find('.myCommentAnchor').text()).to.equal('https://google.com#123');
});
it('should render the comment with styles', () => {
diff --git a/tests/routes/api/comments/index.js b/tests/routes/api/comments/index.js
index fdd4e4bbe..1d7ad5321 100644
--- a/tests/routes/api/comments/index.js
+++ b/tests/routes/api/comments/index.js
@@ -85,6 +85,29 @@ describe('/api/v1/comments', () => {
]);
});
+ it('should return only the owner’s comments if the user is not an admin', () => {
+ return chai.request(app)
+ .get('/api/v1/comments?user_id=456')
+ .set(passport.inject({id: '456', roles: []}))
+ .then(res => {
+ expect(res).to.have.status(200);
+ expect(res.body.comments).to.have.length(2);
+ expect(res.body.comments[1]).to.have.property('author_id', '456');
+ });
+ });
+
+ it('should fail if a non-admin requests comments not owned by them', () => {
+ return chai.request(app)
+ .get('/api/v1/comments?user_id=456')
+ .set(passport.inject({id: '123', roles: []}))
+ .then((res) => {
+ expect(res).to.be.empty;
+ })
+ .catch((err) => {
+ expect(err).to.have.property('status', 401);
+ });
+ });
+
it('should return all the comments', () => {
return chai.request(app)
.get('/api/v1/comments')