Skip to content

Commit

Permalink
Prevent CSP violations by not having script URLs (#4062)
Browse files Browse the repository at this point in the history
* Fix: remove inline script to avoid CSP violation

Closes #4039.

* Restore eslint rule that prevents javascript href attributes.

* Remove all inline script links.
  • Loading branch information
arikfr authored Aug 12, 2019
1 parent 7dd62ef commit 685b536
Show file tree
Hide file tree
Showing 12 changed files with 28 additions and 18 deletions.
1 change: 0 additions & 1 deletion client/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ module.exports = {
"consistent-return": "off",
"no-control-regex": "off",
"no-multiple-empty-lines": "warn",
"no-script-url": "off", // some <a> tags should have href="javascript:void(0)"
"no-only-tests/no-only-tests": "error",
"operator-linebreak": "off",
"react/destructuring-assignment": "off",
Expand Down
6 changes: 5 additions & 1 deletion client/app/assets/less/redash/redash-newstyle.less
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ body {

&:hover, &:focus {
color: @yellow-darker;
cursor: pointer;
}

.fa-star {
Expand Down Expand Up @@ -814,11 +815,14 @@ body {
}

.tags-list {

.badge-light {
background: fade(@redash-gray, 10%);
color: fade(@redash-gray, 75%);
}

a:hover {
cursor: pointer;
}
}

.dropdown-menu--profile {
Expand Down
1 change: 0 additions & 1 deletion client/app/components/FavoritesControl.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ export class FavoritesControl extends React.Component {
const title = item.is_favorite ? 'Remove from favorites' : 'Add to favorites';
return (
<a
href="javascript:void(0)"
title={title}
className="btn-favourite"
onClick={event => this.toggleItem(event, item, onChange)}
Expand Down
2 changes: 1 addition & 1 deletion client/app/components/HelpTrigger.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ export class HelpTrigger extends React.Component {
return (
<React.Fragment>
<Tooltip title={tooltip}>
<a href="javascript: void(0)" onClick={this.openDrawer} className={className}>
<a onClick={this.openDrawer} className={className}>
{this.props.children}
</a>
</Tooltip>
Expand Down
4 changes: 4 additions & 0 deletions client/app/components/HelpTrigger.less
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

.help-trigger {
font-size: 15px;

&:hover {
cursor: pointer;
}
}

.help-drawer {
Expand Down
1 change: 0 additions & 1 deletion client/app/components/QuerySelector.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ export function QuerySelector(props) {
<div className="list-group">
{searchResults.map(q => (
<a
href="javascript:void(0)"
className={cx('query-selector-result', 'list-group-item', { inactive: q.is_draft })}
key={q.id}
onClick={() => selectQuery(q.id)}
Expand Down
1 change: 0 additions & 1 deletion client/app/components/TagsList.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ export class TagsList extends React.Component {
{map(allTags, tag => (
<a
key={tag.name}
href="javascript:void(0)"
className={classNames('list-group-item', 'max-character', { active: selectedTags.has(tag.name) })}
onClick={event => this.toggleTag(event, tag.name)}
>
Expand Down
2 changes: 1 addition & 1 deletion client/app/components/empty-state/EmptyState.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ function Step({ show, completed, text, url, urlText, onClick }) {

return (
<li className={classNames({ done: completed })}>
<a href={url || 'javascript:void(0)'} onClick={onClick}>
<a href={url} onClick={onClick}>
{urlText}
</a>{' '}
{text}
Expand Down
4 changes: 4 additions & 0 deletions client/app/components/empty-state/empty-state.less
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@
margin-bottom: 0;
}

a:hover {
cursor: pointer;
}

@media (max-width: 767px) {
flex-direction: column;

Expand Down
2 changes: 1 addition & 1 deletion client/app/pages/dashboards/dashboard.html
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ <h3>
</button>
<ul class="dropdown-menu pull-right" ng-model="$ctrl.refreshRate" uib-dropdown-menu role="menu" aria-labelledby="split-button">
<li role="menuitem" ng-repeat="refreshRate in $ctrl.refreshRates" ng-class="{disabled: !refreshRate.enabled}">
<a href="javascript:void(0)" ng-click="$ctrl.setRefreshRate(refreshRate)">{{refreshRate.name}}</a>
<a ng-click="$ctrl.setRefreshRate(refreshRate)">{{refreshRate.name}}</a>
</li>
<li role="menuitem" ng-if="$ctrl.refreshRate !== null">
<a href="#" ng-click="$ctrl.setRefreshRate(null)">Stop auto refresh</a>
Expand Down
11 changes: 6 additions & 5 deletions client/app/pages/groups/GroupDataSources.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,13 @@ class GroupDataSources extends React.Component {
{!controller.isLoaded && <LoadingState className="" />}
{controller.isLoaded && controller.isEmpty && (
<div className="text-center">
There are no data sources in this group yet.
<p>
There are no data sources in this group yet.
</p>
{currentUser.isAdmin && (
<div className="m-t-5">
<a href="javascript:void(0)" onClick={this.addDataSources}>Click here</a>
{' '} to add data sources.
</div>
<Button type="primary" onClick={this.addDataSources}>
<i className="fa fa-plus m-r-5" />Add Data Sources
</Button>
)}
</div>
)}
Expand Down
11 changes: 6 additions & 5 deletions client/app/pages/groups/GroupMembers.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,13 @@ class GroupMembers extends React.Component {
{!controller.isLoaded && <LoadingState className="" />}
{controller.isLoaded && controller.isEmpty && (
<div className="text-center">
There are no members in this group yet.
<p>
There are no members in this group yet.
</p>
{currentUser.isAdmin && (
<div className="m-t-5">
<a href="javascript:void(0)" onClick={this.addMembers}>Click here</a>
{' '} to add members.
</div>
<Button type="primary" onClick={this.addMembers}>
<i className="fa fa-plus m-r-5" />Add Members
</Button>
)}
</div>
)}
Expand Down

0 comments on commit 685b536

Please sign in to comment.