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

Support app-defined ACLs via user-data table and whoAmI RPC #590

Merged
merged 22 commits into from
Nov 26, 2019

Conversation

eddyashton
Copy link
Member

This is primarily completing #431 with some minor additions. To get there, I also needed the caller lookup RPCs mentioned in #575, and to demonstrate this functionality I updated the txregulator sample app.

So 3 things:

  • A user_data field in the UserInfo object, writable by members via consensus. This can store arbitrary json objects, so its up to the app instance to determine how permissions are encoded.
  • New whoAmI and whoIs RPCs, so that an external user can find out their CCF User or Member ID.
  • Changes to the txregulator sample app to check user_data permissions for registering regulators and banks. Most of the changes in the client are about drawing a clear distinction between the local name for a user (used as the X in userX_privk.pem for our Python infrastructure) and their CCF ID.

@eddyashton eddyashton requested a review from a team as a code owner November 26, 2019 09:57
@cimetrics
Copy link

images

@@ -172,8 +172,8 @@ def start_and_join(self, args):
cmd = ["rm", "-f"] + glob("member*.pem")
infra.proc.ccall(*cmd)

self.consortium = infra.consortium.Consortium([1, 2, 3])
self.initial_users = [1, 2, 3]
self.consortium = infra.consortium.Consortium([0, 1, 2])
Copy link
Member Author

Choose a reason for hiding this comment

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

Previously we created a member1_cert.pem and member1_privk.pem, and these were associated with the member with ID 0 inside CCF. This changes the default member (and user) names used locally to match their eventual CCF IDs. This results in all of the -1 changes in memberclient.py. In general we should probably try to break the assumption that these IDs are in-sync to avoid domino bugs - maybe the default users should be alice, bob, and charlie?

@cimetrics
Copy link

images

src/node/rpc/frontend.h Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Nov 26, 2019

Codecov Report

Merging #590 into master will increase coverage by 0.04%.
The diff coverage is 70.15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #590      +/-   ##
==========================================
+ Coverage    78.2%   78.24%   +0.04%     
==========================================
  Files         143      143              
  Lines       10897    10964      +67     
==========================================
+ Hits         8521     8578      +57     
- Misses       2376     2386      +10
Flag Coverage Δ
#e2e_BFT 50.49% <13.64%> (+0.05%) ⬆️
#e2e_CFT 72.76% <40.91%> (-0.04%) ⬇️
#unit_BFT 64.08% <ø> (ø) ⬆️
#unit_CFT 71.29% <65.67%> (-0.05%) ⬇️
Impacted Files Coverage Δ
src/node/proposals.h 100% <ø> (ø) ⬆️
src/luainterp/txscriptrunner.h 95.88% <100%> (+0.27%) ⬆️
src/apps/luageneric/luageneric_test.cpp 100% <100%> (ø) ⬆️
src/node/rpc/frontend.h 73.49% <33.33%> (-2.3%) ⬇️
src/node/rpc/memberfrontend.h 85.2% <81.82%> (-0.1%) ⬇️
src/host/tcp.h 74.04% <0%> (+0.85%) ⬆️
src/consensus/raft/raft.h 83.86% <0%> (+1.27%) ⬆️

src/node/rpc/calltypes.h Outdated Show resolved Hide resolved
@jumaffre
Copy link
Contributor

Should this new feature be briefly documented in https://microsoft.github.io/CCF/developers/index.html?

@cimetrics
Copy link

images

@eddyashton
Copy link
Member Author

Should this new feature be briefly documented in https://microsoft.github.io/CCF/developers/index.html?

I've added this to the member docs - a section on "Adding users" under "Common governance operations". I don't think there's a clean spot to put it in the developers documentation at the moment. If it proves generally useful, we can add a few example uses in time.

@cimetrics
Copy link

images

@jumaffre
Copy link
Contributor

Should this new feature be briefly documented in https://microsoft.github.io/CCF/developers/index.html?

I've added this to the member docs - a section on "Adding users" under "Common governance operations". I don't think there's a clean spot to put it in the developers documentation at the moment. If it proves generally useful, we can add a few example uses in time.

There's already a "Adding Users" section described when the network is opened (https://microsoft.github.io/CCF/members/open_network.html#adding-users). Should this two be merged somehow?

@eddyashton
Copy link
Member Author

Should this new feature be briefly documented in https://microsoft.github.io/CCF/developers/index.html?

I've added this to the member docs - a section on "Adding users" under "Common governance operations". I don't think there's a clean spot to put it in the developers documentation at the moment. If it proves generally useful, we can add a few example uses in time.

There's already a "Adding Users" section described when the network is opened (https://microsoft.github.io/CCF/members/open_network.html#adding-users). Should this two be merged somehow?

Good spot! I've moved the new info under that existing section.

@cimetrics
Copy link

images

@eddyashton eddyashton merged commit 14167d3 into microsoft:master Nov 26, 2019
@eddyashton eddyashton deleted the whoami branch November 26, 2019 17:16
eddyashton added a commit to eddyashton/CCF that referenced this pull request Mar 24, 2020
…t#590)

* Add whoAmI and whoIs RPCs

* Name cert files [user|member]0-2, not 1-3

* Remove manual offsetting from txregulatorclient

* Separate local name from CCF-retrieved ID

* Remove manual offset from demo script too

* Auto stringify

* Add user_data field in UserInfo, settable by members

* USERS table should be whitelisted

* Trying to log most types from lua is an error

* Add privilege model to txregulator app - WIP

* Semantic indentation

* Format

* Add permissions for demo script

* who_is should return a WhoIs::Out

* Schema for new RPCs

* Fix IDs in memberclient

* Address PR comments

* Document adding users + user-data

* Move new info to existing Adding Users section
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.

4 participants