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

feat: Implementing encrypted local storage for user sessions with tests #1211

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

rommansabbir
Copy link
Member

@rommansabbir rommansabbir commented Aug 15, 2024

New Pull Request Checklist

Issue Description

From an existing PR:


Currently the SDK saved cached user session as clear text files.
Feature / Enhancement Description

Encrypting cached user sessions using Jetpack security features to prevent session copy on rooted devices.
Alternatives / Workarounds

No workarounds at the moment.
3rd Party References

I found this [gist](https://github.com/appmattus/layercache/tree/main/testutils/src/main/kotlin/com/appmattus/layercache/keystore) which provides a good way for testing encryption methods on Robolectric.

Closes: #1192

  • Implemented encrypted local storage for user sessions with tests.
  • It was an existing PR.

Approach

  • Added EncryptedFileObjectStore to enable encryption and support CRUD operations.
  • Enabled migration in ParseCorePlugins when getCurrentUserController() is invoked by ParseObjectStoreMigrator.
  • Updated ParseFileUtils to support CRUD operations for encrypted objects.

Testing:

  • Added tests for the new functionality and updates.

TODOs before merging

  • Add tests - gradle clean test
  • Add changes to documentation (guides, repository pages, in-code descriptions)

Copy link

parse-github-assistant bot commented Aug 15, 2024

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

@rommansabbir
Copy link
Member Author

@mtrezza

Comment on lines +24 to +49
return store.getAsync()
.continueWithTask(
new Continuation<T, Task<T>>() {
@Override
public Task<T> then(Task<T> task) throws Exception {
if (task.getResult() != null) return task;
return legacy.getAsync()
.continueWithTask(
new Continuation<T, Task<T>>() {
@Override
public Task<T> then(Task<T> task)
throws Exception {
T object = task.getResult();
if (object == null) return task;
return legacy.deleteAsync()
.continueWith(
task1 ->
ParseTaskUtils.wait(
store
.setAsync(
object)))
.onSuccess(task1 -> object);
}
});
}
});
Copy link
Member

Choose a reason for hiding this comment

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

Is there a better way to structure this code?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will look into it.

@@ -46,7 +46,9 @@ public boolean isCancellationRequested() {
}
}

/** @return the token that can be passed to asynchronous method to control cancellation. */
Copy link
Member

@mtrezza mtrezza Oct 5, 2024

Choose a reason for hiding this comment

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

Please revert these unrelated changes in bolts, twitter and wherever you find these comment-only refactors.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay.

@mtrezza
Copy link
Member

mtrezza commented Oct 5, 2024

As this PR is a continuation of #1191, did you consider the PR review comments and discussion feedback there when creating this PR?

@rommansabbir
Copy link
Member Author

As this PR is a continuation of #1191, did you consider the PR review comments and discussion feedback there when creating this PR?

Yes, I did. Did i missed something during the discussion feedback? I will again look into the discussion. Thanks for the attention.

@mtrezza
Copy link
Member

mtrezza commented Oct 6, 2024

Did i missed something during the discussion feedback?

Noting specific, I didn't check the discussion there, only noticed that there has been extensive discussion.

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.

Preventing Session Copy On Rooted Devices By Encrypting Current Session Files
3 participants