Skip to content
This repository was archived by the owner on Sep 30, 2023. It is now read-only.

feat: add firebase-access-controller #44

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vasa-develop
Copy link

Added a firebase-based access controller.

License: MIT
Signed-off-by: Vaibhav Saini vasa@dappkit.io

return resolve(verifiedIdentity);
} else {
// No user is signed in.
return resolve(false);
Copy link
Author

Choose a reason for hiding this comment

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

I did resolve(false) instead of reject(false) as this would need the developer to catch the error when canAppend is rejected, which is not good in terms of developer experience.

@vasa-develop
Copy link
Author

I'll add some more comments in the AccessController so that it's easy to understand how to use it.

License: MIT
Signed-off-by: Vaibhav Saini <vasa@dappkit.io>
@haadcode
Copy link
Member

Amazing @vasa-develop! 🎉 Thank you for the PR 🙏

The code looks good to me, but would love second pair of eyes on it to confirm.

What we should have is a set of tests for the AC, similar to https://github.com/orbitdb/orbit-db-access-controllers/blob/master/test/contract-access-controller.test.js and https://github.com/orbitdb/orbit-db-access-controllers/blob/master/test/contract-access-controller-integration.test.js, in order to make sure this works today and our future selves can modify things fearlessly :)

@vasa-develop could you add some basic set of tests for this PR?

@vasa-develop
Copy link
Author

@haadcode sure thing! I'll add the tests ASAP.

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants