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

Encrypted Current User in browser #1036

Merged
merged 7 commits into from
Dec 24, 2019
Merged

Encrypted Current User in browser #1036

merged 7 commits into from
Dec 24, 2019

Conversation

macarthuror
Copy link
Contributor

@macarthuror macarthuror commented Dec 17, 2019

UPDATE 17/12/19

Problem

Sometimes we don't want or we don't need to show the information of the current user in the local storage

Solution

With this implementation the information of the current user will be encrypted in the Local Storage to prevent expose the data.
Made it with the crypto-js package.

To implement it you only need to use the next code

Parse.enableEncryptedUser()
Parse.secret= 'my Secrey Key'

#967

Copy link
Member

@dplewis dplewis left a comment

Choose a reason for hiding this comment

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

This looks good to me. I have a few comments.

package.json Outdated Show resolved Hide resolved
src/ParseUser.js Outdated Show resolved Hide resolved
src/__tests__/ParseUser-test.js Outdated Show resolved Hide resolved
src/__tests__/ParseUser-test.js Outdated Show resolved Hide resolved
@dplewis
Copy link
Member

dplewis commented Dec 18, 2019

I just realized if you remove the check for browser, this would still work for node and react-native no?

@macarthuror
Copy link
Contributor Author

I just realized if you remove the check for browser, this would still work for node and react-native no?

At Node I believe there's no problem just let me check react-native compatibility.

@dplewis
Copy link
Member

dplewis commented Dec 18, 2019

I think it should work sad that package uses vanilla js. Also you have conflict.

I’ll pull this down and use this in production.

@macarthuror
Copy link
Contributor Author

Ok, thanks

@macarthuror
Copy link
Contributor Author

Let me try with this package Forge.

@codecov
Copy link

codecov bot commented Dec 23, 2019

Codecov Report

Merging #1036 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1036      +/-   ##
==========================================
+ Coverage   92.11%   92.19%   +0.07%     
==========================================
  Files          53       54       +1     
  Lines        5114     5152      +38     
  Branches     1142     1145       +3     
==========================================
+ Hits         4711     4750      +39     
+ Misses        403      402       -1
Impacted Files Coverage Δ
src/Parse.js 85.59% <100%> (+2.09%) ⬆️
src/ParseUser.js 83.62% <100%> (+0.59%) ⬆️
src/CoreManager.js 100% <100%> (ø) ⬆️
src/CryptoController.js 100% <100%> (ø)

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 fc840f6...e1d9cd1. Read the comment docs.

@dplewis dplewis requested a review from davimacedo December 23, 2019 22:06
@dplewis
Copy link
Member

dplewis commented Dec 23, 2019

@macarthuror I made a few changes to this PR. There is a CryptoController that allows users to add their own implementation. I also updated the tests.

@davimacedo Anything you want to add?

@dplewis dplewis requested a review from acinader December 23, 2019 22:18
@macarthuror
Copy link
Contributor Author

@dplewis Looks amazing 👌

Copy link
Contributor

@acinader acinader left a comment

Choose a reason for hiding this comment

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

nice

src/CryptoController.js Outdated Show resolved Hide resolved
@dplewis dplewis merged commit 684b9e1 into parse-community:master Dec 24, 2019
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.

3 participants