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

[request] ACL #225

Closed
tamizhgeek opened this issue May 12, 2015 · 51 comments
Closed

[request] ACL #225

tamizhgeek opened this issue May 12, 2015 · 51 comments
Assignees
Labels
idea/new plugin [legacy] those issues belong to Kong Nation, since GitHub issues are reserved for bug reports.
Milestone

Comments

@tamizhgeek
Copy link
Contributor

Currently, we can create a list of APIs and a list of Consumers. Any consumer has access to any API. Instead we would like to create a to be API accessible only to a set of consumers. Something like a Premium plan to access a API. Do we have something already available? Or some way to achieve this?

We can do this adding the list of consumers who have access to the API in the /apis resource.
Or else, have a separate private-apis plugin and maintain the mapping of APIs and Consumers in a separate column family.

Please let us know your suggestions. We would like send a PR for the change.

@thibaultcha
Copy link
Member

This is something we talked about with @montanaflynn and @thefosk. A blacklist/whitelist feature for plugins. Not only the authentication ones. We did encounter implementation issues when trying to modelling it but we did not look into it more than that since we've been busy with our current roadmap.

As a side note, please do not jump into implementing this as it would imply some remodelling of the database schema, and also because Kong still has no plugins infrastructure but this should be implemented with this consideration in mind because we don't want to grow our technical debt.

@tamizhgeek
Copy link
Contributor Author

@thibaultcha I'm not sure if I understand you correctly. But this isn't similar to blacklist/whitelist plugins. This is more like a ACL for the APIs. An ACL which maintains 'who(consumer) has access to what(API)' This can be implemented pretty much outside the existing schema, with a new column family and new api resource - which both can be achieved by writing a new plugin. Am i missing something here?

I'm sorry, but I also don't understand when you say, 'Kong still has no plugins infrastructure'.

@thibaultcha
Copy link
Member

To distinguish consumers you need to use one of the authentication plugins. Keyauth, basicauth, or any future one. There is no way around it. If the user is not authenticated then you don't know who is consuming the API. So I actually might not understand your question. You have an API, everybody can access it, yes. You want only Consumers A and B to be able to access it? Fine, put one of the authentication plugins on top of them.

So I actually remember now our discussion was for plugins triggered after the authentication step. Once we know who the consumer is, we can apply plugin X or Y. For example ratelimiting for Consumer A. But not for Consumer B, who has unlimited access.

As for the plugins architecture: for now all plugins are bundled into the core (this repo), instead of being separate repos and installable from the CLI: kong install keyauth. This has a lot of downsides and is restraining us from doing a lot of things. Only Mashape can accept/merge plugins, updates etc... The core (this repo) is bloated, not correctly unit tested and the coverage is low because of them polluting this repo. Plugins should be installable, have their own dependencies, migrations files for the DB schema, etc etc. Related issue is #93. We have big plans for them.

@tamizhgeek
Copy link
Contributor Author

@thibaultcha Thanks for your detailed explanation.

  1. Well, the scenario we need is much like - we want consumer A and B to access an API api-1, but disallow consumer C from accessing it. But consumer C should be able to access api-2.
  2. Enabling rate-limiting only for a single consumer is already available using a consumer-id. I was under the assumption, we can use the same mechanics to write a ACL plugin also. Instead of rate-limiting, the plugin will just say whether this API is accessible to this consumer.
  3. Thanks, the plan sounds great and makes sense. Any rough timelines for this happen?

@thibaultcha
Copy link
Member

  1. Oh, yes, I see it now. Currently when a consumer has a key, he can access any API-with-keyauth plugin enabled. You want a consumer with a key to only access API-with-keyauth X or Y. So this comes back to my first comment: it is part of what we talked about: whitelist/blacklist per plugin because the same concept can be reused for all plugins. wWe need to figure out a solution taking usability, and maintainability (with the proper DB schema and plugins infrastructure) in mind.
  2. Yes it is, but not as a whitelist/blacklist feature. My example wasn't very good. If you have X consumers and want to execute a plugin for only 10 consumers, or to bypass the plugin for only 10 consumers, that is a different implementation than what ratelimiting currently does. Ratelimiting can be enabled for 1 consumer, but if you want the same setting for 100 consumers, it's pretty annoying. So with a whitelist, you'll set up ratelimiting and apply the rule to X consumers at once. Or, if we talk about blacklist, you can enable ratelimiting on an API, and entirely disable it for X consumers, which is different than the behaviour you currently described. It means the plugin won't even execute, instead of overriding the ratelimiting with another value.
  3. I don't know

@tamizhgeek
Copy link
Contributor Author

@thibaultcha Thanks! We'll probably wait for the proper plugin infrastructure to come in. As a intermediate fix, probably from our side we will maintain a private plugin to achieve what we want. You can close this issue.

@sonicaghi
Copy link
Member

+1 for Access Control for APIs

@DavidTPate
Copy link

Another API gateway that I've been looking at has Granular Access Control which I find to be particularly useful when access to systems are managed within another service. So my service which deals with limiting access to end-points just has to communicate the paths to Tyk and they have them available.

@subnetmarco subnetmarco added this to the 0.3.1 milestone Jun 2, 2015
@sonicaghi sonicaghi added the idea/new plugin [legacy] those issues belong to Kong Nation, since GitHub issues are reserved for bug reports. label Jun 3, 2015
@sonicaghi sonicaghi changed the title [request] Restricting access to an API only for certain consumers [request] ACL Jun 3, 2015
@sonicaghi
Copy link
Member

is this what you're looking for @lucamaraschi? cc @ahmadnassri @thefosk

@lucamaraschi
Copy link

@sinzone 👍 exactly!

@thibaultcha thibaultcha modified the milestones: 0.3.1, 0.4.0 Jun 8, 2015
@shashiranjan84 shashiranjan84 self-assigned this Jun 17, 2015
@shashiranjan84
Copy link
Contributor

So I am thinking to implement something like this
OBJECT : API
USER : CONSUMER
ACCESS RIGHTS : CRUD operation (http methods)

ACL table(primary key cpi, customer_type, rights)

api consumer_type rights
api1 Generic read
api1 Generic write
api1 Pro read
api1 Pro read
api2 Mega read
api2 Mega write
api2 Mega delete

Customer mapping table(primary key customer_id)
———————————————
Customer_id Customer_type
123 Generic
345 Mega

@tamizhgeek & @lucamaraschi is it something what you looking for?
@thibaultcha @thefosk do you see any problem with this apporach?

@tamizhgeek
Copy link
Contributor Author

@shashiranjan84 We already have a custom plugin for this with a much simpler schema :

id uuid,
api_id uuid,
consumer_id uuid,
created_at timestamp,
PRIMARY KEY ((api_id, consumer_id))

I assume the consumer_type here is similar to a Role based ACL. We didn't really need roled based ACL's at this stage. And in our way, the access control can be done very fast with a single cassandra lookup. Now in the above design you mentioned, it might require two queries. Anyway this will be really helpful.

@shashiranjan84
Copy link
Contributor

@tamizhgeek role based ACL will give more granular control otherwise keyauth provide similar access control.

@tamizhgeek
Copy link
Contributor Author

@shashiranjan84 Please read the previous comments, specifically #225 (comment) to understand the subtle difference between keyauth and acl plugins. keyauth can't allow/disallow specific users for a API. once keyauth is enabled on a API, all consumers having keyauth configured will get access to it.

@sonicaghi
Copy link
Member

@tamizhgeek what you built is more close to a simple whitelist/blacklist than a full ACL with deeper control.

@shashiranjan84
Copy link
Contributor

@tamizhgeek What exactly I am missing, As a provider if you don't want C to access Api 1, just dont share the key with them unlike A and B.

@tamizhgeek
Copy link
Contributor Author

@sinzone Right. Because, that was actually our requirement. But I'd be happy to see a full blown ACL support inside kong.

@shashiranjan84 Lets say there is API 1 and API 2. I want C to access API2 but not API1, I want A and B to access API1 and API2. Now I'll enable keyauth for API1 and API2. All A,B and C are now enabled with their own keys for key_auth. Now, how do I restrict C from accessing API1 ?

@subnetmarco
Copy link
Member

Me and @shashiranjan84 were talking about this earlier.

Basically we want to introduce a new entity called roles, along with apis, consumers and plugins_configurations. A role could be a table like:

CREATE TABLE IF NOT EXISTS roles(
  id uuid,
  api_id uuid,
  consumer_id uuid,
  permission int,
  created_at timestamp,
  PRIMARY KEY (id)
);

When a consumer makes a request, the system will try to find a role for that (consumer_id + api_id), and if it exists it will allow or deny the request based on the permission level.

In my example the permission is a numeric value (that can be mapped to constants, like "1 = can consume", "2 = cannot consume", "3 = can only execute GET").

To implement some sort of whitelisting or blacklisting we could use the same table in the following way:

[Whitelisting] To block the API for every user, but allow it only to some users

By default the API blocks every user:

-- Inserting a role without a consumer_id. The permission = 2, which means "cannot consume"
INSERT INTO roles (id, api_id, consumer_id, permission, created_at) VALUES (.., "some-id", "", 2, ..)

And only allows some users:

-- Inserting a role with a consumer_id. The permission = 1, which means "can consume"
INSERT INTO roles (id, api_id, consumer_id, permission, created_at) VALUES (.., "some-id", "some-id", 1, ..)

[Blacklisting] Block the API only to some users

By default the API allows every user:

-- Inserting a role without a consumer_id. The permission = 2, which means "cannot consume"
INSERT INTO roles (id, api_id, consumer_id, permission, created_at) VALUES (.., "some-id", "", 1, ..)

And only blocks some users:

-- Inserting a role with a consumer_id. The permission = 1, which means "can consume"
INSERT INTO roles (id, api_id, consumer_id, permission, created_at) VALUES (.., "some-id", "some-id", 2, ..)

By having a separate table, the ACL functionality is also easy to extend in the future.

@shashiranjan84 did I put properly down in words your suggestion?

Feedback?

@shashiranjan84
Copy link
Contributor

@tamizhgeek you are right, key is linked to customer not api so same key is used for all the api.

@thefosk I was thinking of little different approach

CREATE TABLE IF NOT EXISTS ACL(
  id uuid,
  api_id uuid,
  consumer_role text,
  permission int,
  created_at timestamp,
  PRIMARY KEY (id, api_id, consumer_roles, permission)
);

so ACL table will have all the roles with there capabilities defined in ACL table. Once you add customer, assign a role to it. it will help us define different level of access control.

@subnetmarco
Copy link
Member

What is consumer_role ?

@dvh
Copy link

dvh commented Jul 28, 2015

+1! What's the status on this? Any progress?

@ahmadnassri
Copy link
Contributor

@dvh it is in progress, you can see more details about the roadmap here: https://github.com/mashape/kong/wiki

@nijikokun
Copy link
Contributor

@shashiranjan84 has one of the best approaches, but needs a slight change.

  1. Access Group table for managing access rights
  2. Users table for association to access group rights.

Access Group Table

group_id group_name rights
1 Generic read
2 Pro read, write
3 Mega read, write, delete

Consumer Table

consumer_id api_id group_id
1 1 1
2 1 2
3 2 3

In this manner, you're not filling your database with redundant data, and it is an association. Future would be nice to able to associate rights to an even granular approach such as per endpoint

This would still support the whitelist and blacklist ideas, but with a proper data structure.


Single table may work for small number of consumers, but when you grow to a thousand, you really wouldn't want to individually go through each one and change rights, this way you change the right once and it is propagated to them all.

@subnetmarco
Copy link
Member

Should this be in core, or built as a plugin? I think it should be in core, because the interface for adding a plugin requires an api_id and optionally a consumer_id. But the whole point is having ACL groups and roles not bound to a specific API or consumer. Thoughts?

@shashiranjan84
Copy link
Contributor

Both actually, update the api and customer table to support roles and ACL
as part of core setup.It would remove lots of data redundancy.Start it
with all default values. And then plugin to configure roles and assign
roles to cusumer.
On Aug 17, 2015 8:25 PM, "Marco Palladino" notifications@github.com wrote:

Should this be in core, or built as a plugin? I think it should be in
core, because the interface for adding a plugin requires an api_id and
optionally a consumer_id. But the whole point is having ACL groups and
roles not bound to a specific API or consumer. Thoughts?


Reply to this email directly or view it on GitHub
#225 (comment).

@lucamaraschi
Copy link

It really depends on what kind of responsibilities you see in KONG, because my feeling is that adding too many functionalities in the core is going to end-up in a monolithic component.
For this reason I think the ACL should be a plugin in order to keep the main engine of KONG as light as possible.
The group attribute on the consumer I would instead incorporate as core functionality, as it allows a better consumer management.
How would you then handle the dependency with other access/user control mechanism like LDAP?

@sonicaghi
Copy link
Member

The group attribute is de facto "organizations" like everyone else has it.

@lucamaraschi
Copy link

@sinzone I think is more a security group...it might be helpful to use the same forest structure/hierarchy as LDAP

@subnetmarco
Copy link
Member

In order to work as a plugin, adding an ACL on top of an API would mean executing something like:

curl -d "name=acl" \
     -d "api_id=XXX" \ 
     -d "value.groups.basic.rights=read, write" \
     -d "value.groups.admin.rights=read, write, delete" \
     -d "value.default=basic" kong:8001/apis/test.com/plugins/

The command above will never accept consumer_id.

Adding the groups to a consumer could be done like:

curl -d "api_id=XXX" \
     -d "group=basic" kong:8001/consumers/thefosk/groups/

By doing it this way, this also means that different APIs can have different groups.

@subnetmarco subnetmarco added this to the 0.5.0 milestone Aug 18, 2015
@thibaultcha
Copy link
Member

I share @lucamaraschi's point of view too.

@subnetmarco
Copy link
Member

Regarding permissions, we mentioned read, write, etc. These permissions would translate to get, post, put, delete, etc. Should we just use those instead? Like:

curl -d "name=acl" \
     -d "api_id=XXX" \ 
     -d "value.groups.basic.rights=get, post" \
     -d "value.groups.admin.rights=get, post, put, delete" \
     -d "value.default=basic" kong:8001/apis/test.com/plugins/

@dvh
Copy link

dvh commented Aug 21, 2015

Doesn't that perhaps leave little flexibility? REST is already starting to lose support for it's pure principles. If I want to write something using get it might not be RESTful, but that's not the concern of Kong. If you use read and write instead you don't have this problem. Just a thought ;-)

@thibaultcha
Copy link
Member

My point of view too. Without even mentioning custom HTTP methods, once again.

@ahmadnassri
Copy link
Contributor

agreed, as much as I love following REST principles, they are at the end of the day all optional and people will have different approaches to how they expect their API to work.

@subnetmarco
Copy link
Member

When the ACL plugin detects an incoming request from a user with, let's say, read permission, what would the check be? How can ACL know if the user is trying to do a read or write besides filtering the request by its HTTP method which, given REST semantics, identify a read (GET) or write (POST) action?

The only thing that the plugin will see is an incoming HTTP request, and that's it. It knows nothing about the action being invoked by the HTTP request.

@thibaultcha in my example custom headers are not a problem. Just add the custom header to the list of permissions: value.groups.basic.rights=get, post, hellomethod, anothermethod, supmethod

@ahmadnassri
Copy link
Contributor

When the ACL plugin detects an incoming request from a user with, let's say, read permission, what would the check be? How can ACL know if the user is trying to do a read or write besides filtering the request by its HTTP method which, given REST semantics, identify a read (GET) or write (POST) action?

its possible to use any combination of the HTTP protocol, possible combinations:

  • query strings
  • http methods
  • uri paths
  • request objects in body (complex, better deffer)

however, v1.0.0 of this should be only users and groups, and per API acces, the ACL info can also be passed to the API server to help do business logic on the backend.

v2.0.0 can dive a bit more detail as mentioned, using verbs, query strings, etc ..

@subnetmarco
Copy link
Member

its possible to use any combination of the HTTP protocol, possible combinations [..]

This is an interesting problem, because in a way this is going to overlap with #160. And because #160 is also a very requested feature, I am worried that we came up with a solution that needs to be rebuilt later on.

I also think we are getting farther from the original issue, which is simply to specify which APIs some consumers can use (because right now every consumer can access any API), and the implementation solutions we came up with are vague.

We need to come up with a spec for v1 of the ACL feature, which for sure needs to include:

  • Being able to specify if a consumer can, or cannot, consume an API at all. Regardless of the HTTP method, endpoints, or request body.

If #160 will be implemented supporting HTTP methods and paths (as discussed in the comment thread), then the capability of allowing/blocking a consumer per HTTP method, path, etc will be provided not by the plugin itself, but by Kong core when resolving which plugin configuration to load at runtime.

This means that ACL simply allows or blocks a consumer, while any specific configuration will be handled by the plugin loader engine.

@ahmadnassri
Copy link
Contributor

@thefosk yep, you nailed it, I forgot about #160 👍

this also rings my bell to remind us of #295 because of having to potentially add many different paths / rules to what is essentially the same API, with different ACL rules ...

@subnetmarco
Copy link
Member

Since we would rely on #160 for the fine tuning, should this plugin just be:

curl -d "name=acl" \
     -d "api_id=XXX" \
     -d "value.whitelist=CONSUMER_ID1, CONSUMER_ID2" \
     -d "value.blacklist=CONSUMER_ID3, CONSUMER_ID4" kong:8001/apis/test.com/plugins/

Similar at how the IP Restriction Plugins works. The fine tuning will allow to set this rule to a specific HTTP method, maybe endpoint, etc.

The only problem with this is that we don't really have groups, and if #160 is being implemented we don't need to have groups in this plugin.

I think the combination of this plugin and #160 make this thing a little bit messy and it's a design problem. If #160 is being implemented, then groups should exist in the Plugin Configuration scope, not ACL necessarily: they would become "groups" of configuration options that every plugin could leverage maybe? 😕

When building this plugin I would also try to consider the future scenarios to avoid having big migrations happening in the future. Thoughts?

@thibaultcha
Copy link
Member

I think it is better to have groups at the ACL level and not rely on 160, or it would be pretty messy and the configuration would be confusing.

@dvh
Copy link

dvh commented Aug 25, 2015

Agreed.

@subnetmarco
Copy link
Member

Consider that a group at the ACL level won't be effective once #160 has been done, effectively voiding any ACL group the user has created, because #160 has precedence over whatever group ACL may have defined (since #160 won't even load the plugin if it doesn't meet the criteria).

#160 and ACL groups are conflicting each other.

PS: #160 has been closed in favor of #505.

@subnetmarco
Copy link
Member

Available with 0.5.0 at https://getkong.org/plugins/acl/

@sonicaghi sonicaghi mentioned this issue Dec 8, 2015
hutchic added a commit that referenced this issue Jun 10, 2022
…them (#225)

* chore(ci) run the kong tests

* fix(tests) adjust the kong postgres container auth permissions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idea/new plugin [legacy] those issues belong to Kong Nation, since GitHub issues are reserved for bug reports.
Projects
None yet
Development

No branches or pull requests

10 participants