Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

feat(security): implement meta server access controller #655

Merged
merged 22 commits into from
Nov 20, 2020

Conversation

levy5307
Copy link
Contributor

@levy5307 levy5307 commented Nov 2, 2020

+ [security]
+ enable_acl = false
+ super_users = 
+ meta_acl_rpc_allow_list = 

src/meta/meta_service.cpp Outdated Show resolved Hide resolved
src/runtime/security/sasl_wrapper.cpp Show resolved Hide resolved
src/runtime/security/access_controller.h Show resolved Hide resolved
src/runtime/security/access_controller.cpp Show resolved Hide resolved
src/runtime/security/access_controller.h Show resolved Hide resolved
src/runtime/security/meta_access_controller.h Outdated Show resolved Hide resolved
src/runtime/security/meta_access_controller.h Outdated Show resolved Hide resolved
src/runtime/security/meta_access_controller.cpp Outdated Show resolved Hide resolved
src/runtime/security/meta_access_controller.cpp Outdated Show resolved Hide resolved
src/runtime/security/meta_access_controller.h Outdated Show resolved Hide resolved

// _client_username is only valid if it is a server rpc_session.
// it represents the name of the corresponding client
std::string _client_username;
Copy link
Contributor

Choose a reason for hiding this comment

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

Dont couple the username into rpc_session. Place it into negotiation or somewhere. rpc_session should not
be responsible for any authorization stuff. Use interceptor or other means.

Copy link
Contributor Author

@levy5307 levy5307 Nov 9, 2020

Choose a reason for hiding this comment

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

I think it's ok to add a client_username property for rpc_session. It's a stuff of the session itself, but not provided for security. security is just using it.
It will be a little too complex if we put it into negotiation

Copy link
Contributor

Choose a reason for hiding this comment

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

No authorization/negotiation then no username. The RPC module should be pure network RPC implementation that can be used in every place whether or not the auth enabled. We must insist open-close principle that prevents such modification to add responsibility to a class.

I will take some time thinking about how to dispose of this variable, if you got no idea then.

Copy link
Contributor Author

@levy5307 levy5307 Nov 9, 2020

Choose a reason for hiding this comment

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

OK, but I don't suggest add user_name to negotiation, because in this way we should get the corresponding negotiation for each rpc_session, which will acquire a lock. It will produce low efficiency

src/runtime/security/access_controller.h Outdated Show resolved Hide resolved
@levy5307 levy5307 added the type/config-change PR that made modification on configs, which should be noted in release note. label Nov 9, 2020
acelyc111
acelyc111 previously approved these changes Nov 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component/security type/config-change PR that made modification on configs, which should be noted in release note.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants