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

Change the way of integrating RBAC API and Management API to a one Enforcer #56

Closed
sagilio opened this issue Jul 23, 2020 · 4 comments · Fixed by #135
Closed

Change the way of integrating RBAC API and Management API to a one Enforcer #56

sagilio opened this issue Jul 23, 2020 · 4 comments · Fixed by #135
Assignees
Labels
breaking change This issue will incude some breaking changes enhancement Enhancement the exist feature
Milestone

Comments

@sagilio
Copy link
Member

sagilio commented Jul 23, 2020

I think the best way to integrate IEnforcer (rbac api) and ManagementEnforcer (management api) to a one IEnforcer is use Extension Method to implement it. Here are some reasons:

  1. IEnforcer can be a simple and clean interface.
  2. We can maintain the RBAC and Management API at different class, and we can easy to add new helper type API.
  3. Follow the CARP desgin.

It will become a break change, we should change the protect field to Read-only property (also GetModel method can do it) and delete the InternalEnforcer, ManagementEnforcer and CoreEnforcer and convert them to a Enforcer class which only includes the API in InternalEnforcer and CoreEnforcer in now version).

Relate comment:
#55 (comment)

@sagilio sagilio added the enhancement Enhancement the exist feature label Jul 23, 2020
@sagilio sagilio self-assigned this Jul 23, 2020
@sagilio sagilio changed the title Change the way of integrating to a one IEnforcer Change the way of integrating RBAC API and Management API to a one Enforcer Jul 23, 2020
@hsluoyz
Copy link
Member

hsluoyz commented Jul 23, 2020

Why does it have breaking change?

@sagilio
Copy link
Member Author

sagilio commented Jul 23, 2020

Why does it have breaking change?

Beacause the InternalEnforcer, ManagementEnforcer and CoreEnforcer class are public. If someone use them in the code, it will can not pass the compiling.

@huazhikui
Copy link
Member

Agree with you, I think that the IManagementEnforcer don't need inherit ICoreEnforcer, it only need to include management api. through Extension Method integrate rbac api and management api. We can also add some decorator class to implement it.

@hsluoyz
Copy link
Member

hsluoyz commented Jul 25, 2020

Good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change This issue will incude some breaking changes enhancement Enhancement the exist feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants