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

Permissions implementation #23

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

Conversation

meejah
Copy link
Member

@meejah meejah commented Jul 22, 2021

WIP: this is a proof-of-concept implementation of the "permissions" spec, implementing the "none" and "hashcash" methods.

Needs:

  • unit-tests
  • cleanup

src/wormhole_mailbox_server/server.py Outdated Show resolved Hide resolved
src/wormhole_mailbox_server/server.py Outdated Show resolved Hide resolved
@vu3rdd
Copy link

vu3rdd commented Aug 17, 2021

We may also need some command line flags to turn on/off the type of methods supported by the server?

@meejah meejah marked this pull request as ready for review July 28, 2022 23:29
@codecov
Copy link

codecov bot commented Aug 22, 2022

Codecov Report

Base: 97.10% // Head: 96.74% // Decreases project coverage by -0.36% ⚠️

Coverage data is based on head (0683574) compared to base (4b35885).
Patch coverage: 93.81% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #23      +/-   ##
==========================================
- Coverage   97.10%   96.74%   -0.36%     
==========================================
  Files           8        9       +1     
  Lines         829      923      +94     
  Branches      154      161       +7     
==========================================
+ Hits          805      893      +88     
- Misses         16       18       +2     
- Partials        8       12       +4     
Impacted Files Coverage Δ
src/wormhole_mailbox_server/server.py 95.70% <66.66%> (-0.68%) ⬇️
src/wormhole_mailbox_server/server_websocket.py 99.03% <91.30%> (-0.97%) ⬇️
src/wormhole_mailbox_server/permission.py 98.27% <98.27%> (ø)
src/wormhole_mailbox_server/server_tap.py 88.73% <100.00%> (+1.04%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@meejah meejah force-pushed the permissions-proof-of-concept branch from 61f6008 to 02707c7 Compare August 22, 2022 20:35
src/wormhole_mailbox_server/server.py Outdated Show resolved Hide resolved
@meejah meejah force-pushed the permissions-proof-of-concept branch from 17bece3 to 3da60ab Compare February 1, 2023 04:06
@meejah
Copy link
Member Author

meejah commented Feb 1, 2023

Locally, there's 1 uncovered line. Don't know what codecov is doing...

@meejah meejah force-pushed the permissions-proof-of-concept branch from 7ac741a to 6617587 Compare February 2, 2023 22:42
@meejah
Copy link
Member Author

meejah commented Feb 3, 2023

Further to the "multiple permissions method" thread and #30 and options-wise, what probably makes sense is:

  • the default is ["none"]
  • if you specify any --permission then the set is only those (e.g. with --permission hashcash the set becomes ["hashcash"], so you'd do --permission none --permission hashcash or similar to enabled both).
  • as already said, this makes way more sense if there's a 3rd (or more) method

Separately, I'd also like to add "external" permissions so operators can play with non-standard ones. That is, "anything that implements IPermission", approximately.

Not sure precisely how to handle merging etc -- and maybe we want at least the options interface to work like #30 says...

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.

None yet

3 participants