MESH-Research / Pilcrow

A web application for Collaborative Community Review
https://pilcrow.meshresearch.dev/
GNU Lesser General Public License v3.0
4 stars 3 forks source link

Resolve All Outstanding Dependency Vulnerabilities #260

Closed gmeben closed 3 years ago

gmeben commented 3 years ago

Problem/Why

There are currently 15 outstanding dependency vulnerability fix branches created by Dependabot. Some are older (22 days) and some are newer (14 hours).

Goal/What

Implement all dependency vulnerability fixes as reported by Dependabot into the master branch.

Solution/How

Merge all dependency fix branches into one, rebuild locally, run tests to confirm functionality, and approve merges of all individual branches into the development branch. Then merge into the master branch.

To Do / Acceptance Criteria

The development branch will need to be merged into the master branch soon to formally resolve all the Dependabot alerts.

Review Checklist

gmeben commented 3 years ago

I created a new branch here: https://github.com/MESH-Research/CCR/compare/all-dep-fixes?expand=1

All merge conflicts have been resolved but there's still accessibility errors that resulted upon updating axe-core that will need to be resolved: https://github.com/MESH-Research/CCR/pull/239/checks?check_run_id=2445011700

gmeben commented 3 years ago

Here are the changes I've made so far:

Accessibility Testing

I'm still trying to figure out where these violations are taking place specifically before I can resolve them.

gmeben commented 3 years ago

It appears the default QTabs component is found to violate a11y according to axe-core library version 4.2. Cypress axe reports nested-interactive as the error message.

Here are the existing issue reports on Quasar's GitHub repo that pertain to tabs: https://github.com/quasarframework/quasar/issues?q=is%3Aissue+is%3Aopen+tab

gmeben commented 3 years ago

The a11y issues with nested-interactive have been resolved by removing the placeholder tabs.

I discovered today that aria-label is apparently not allowed as an attribute for inputs with a password type attribute. I was able to mitigate this simply for the My Account page, but the Login and Registration pages introduce additional complexity to mitigate as they have Vue property infrastructure tied to this attribute.

gmeben commented 3 years ago

It should be noted that Quasar's demo of password inputs only uses hints, not labels: https://quasar.dev/vue-components/input#input-types

gmeben commented 3 years ago

I was able to mitigate the a11y aria-label error fairly easily by refactoring the password inputs so that they use hints instead of labels. However, this has inadvertently disrupted the way that hint message updating works and now hints for passwords are not updated, which breaks a separate assertion in the Register Cypress spec that expects to see the message "is required" when a password input is submitted empty. I'm still trying to figure this one out.

gmeben commented 3 years ago

I figured it out. It looks like there was some missing code in client/src/components/forms/atoms/QInputPassword.vue and a slot property in client/src/components/forms/NewPasswordInput.vue that needed to be adjusted.