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

MLIBZ 1602: Remove password (hashes) from locally stored User object #102

Merged
merged 4 commits into from Feb 8, 2017

Conversation

@thomasconner
Copy link
Contributor

commented Feb 1, 2017

Description

This PR removes sensitive data from locally stored user objects.

Changes

  • Delete password from data returned from API calls for login() and me()
  • Delete password from data stored for active user
  • Update related unit tests

@thomasconner thomasconner self-assigned this Feb 1, 2017

@codecov-io

This comment has been minimized.

Copy link

commented Feb 1, 2017

Codecov Report

Merging #102 into master will increase coverage by -0.02%.

@@            Coverage Diff             @@
##           master     #102      +/-   ##
==========================================
- Coverage   73.54%   73.52%   -0.02%     
==========================================
  Files          72       72              
  Lines        6675     6679       +4     
  Branches     1053     1053              
==========================================
+ Hits         4909     4911       +2     
- Misses       1766     1768       +2
Impacted Files Coverage Δ
src/request/src/cache.js 95% <100%> (-1.42%)
src/entity/src/user.js 48.28% <16.66%> (+0.08%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0512504...eda6605. Read the comment docs.

@tejasranade
Copy link
Member

left a comment

The change looks ok, but data.password should never exist at all. It may indicate a problem elsewhere.

@thomasconner thomasconner changed the base branch from refactor to master Feb 6, 2017

@tejasranade

This comment has been minimized.

Copy link
Member

commented Feb 7, 2017

Turns out, this issue was logged incorrectly. The password is not being locally stored at all. The reporter (Ivo) mistook the authToken for the credentials.
Regardless, the change in the PR is an additional safety check and the unit test applies.

@thomasconner thomasconner merged commit cd010e1 into master Feb 8, 2017

2 of 4 checks passed

codecov/patch 44.44% of diff hit (target 73.54%)
Details
codecov/project 73.52% (-0.02%) compared to 0512504
Details
codeclimate no new or fixed issues
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@thomasconner thomasconner deleted the MLIBZ-1602_Remove_Password branch Feb 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.