Kong / kong

🦍 The Cloud-Native API Gateway and AI Gateway.
https://konghq.com/install/#kong-community
Apache License 2.0
38.78k stars 4.77k forks source link

[request] ACL #225

Closed tamizhgeek closed 8 years ago

tamizhgeek commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago
  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 commented 9 years ago

@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 commented 9 years ago

+1 for Access Control for APIs

DavidTPate commented 9 years ago

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.

sonicaghi commented 9 years ago

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

lucamaraschi commented 9 years ago

@sinzone :+1: exactly!

shashiranjan84 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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

tamizhgeek commented 9 years ago

@shashiranjan84 Please read the previous comments, specifically https://github.com/Mashape/kong/issues/225#issuecomment-101284901 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 commented 9 years ago

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

shashiranjan84 commented 9 years ago

@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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

What is consumer_role ?

shashiranjan84 commented 9 years ago

consumer_role would be roles consumer will have, let say we have following state of ACL table

ACL table(primary key cpi, customer_type, rights)

api consumer_role permission
api1 Generic read
api1 Generic write
api1 Pro read
api1 Pro read
api2 Mega read
api2 Mega write
api2 Mega delete

And we add a customer A with role Generic, it will allow user to access API1 for read and write but not other http operations.

Usually this ACL list would be very small can be put into memory.

subnetmarco commented 9 years ago

Ok, so the consumer_role is more of a group name that can be assigned to consumers.

Like, it's just a tag/label?

shashiranjan84 commented 9 years ago

yes which will define there power/capabilities.

thibaultcha commented 9 years ago

If you wait for #346, you can define slightly improved schemas that will save you some troubles:

-- one row per api_id, with all the possible roles in it, for more granular control like you suggested
CREATE TABLE acl(
  api_id uuid,
  role_name text,
  permission text, -- maybe just define an enum here, nicer than integers ("read", "write" etc...)
  created_at timestamp, -- not necessary anymore after 346, so up to you if you still want it
  PRIMARY KEY ((api_id, role_name)) -- composite partition key for a couple api/role
);

-- one row per (api_id, consumer_id)
CREATE TABLE roles(
  api_id uuid,
  consumer_id uuid,
  role_name text, -- foreign to acl
  created_at timestamp, -- not required anymore too
  PRIMARY KEY ((api_id, consumer_id)) -- composite partition key for a couple api/consumer
);

Note about this schema: when doing this you will have to manually check if a role_name exists for a given API in acl before inserting a row in roles.

The new API routes could be:

/apis/{api}/acl
/consumers/{consumer}/roles (with the `api_id` in the body). So you give a role to a consumer for a particular API.

But maybe this approach gives providers too much configuration hassle and a simpler, 1 table schema like described by @thefosk (here, the roles table with permission instead of role_name) would be preferable...

shashiranjan84 commented 9 years ago

@thefosk approach may leads to redundancy as its linked to customer id, for new permission we would need to add a new row and things will get worse as number of consumer increases. Also we will have two tables customer and roles maintaining customer id.

sonicaghi commented 9 years ago

@lucamaraschi do you have any feedback on this?

lucamaraschi commented 9 years ago

I do agree with @thefosk 's approach...but I still think is missing some fine grain access to different endpoints per API.

sonicaghi commented 9 years ago

ah right...ACL x endpoints too

dvh commented 9 years ago

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

ahmadnassri commented 9 years ago

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

nijikokun commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

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 https://github.com/Mashape/kong/issues/225#issuecomment-132056945.

lucamaraschi commented 9 years ago

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 commented 9 years ago

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

lucamaraschi commented 9 years ago

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

subnetmarco commented 9 years ago

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.

thibaultcha commented 9 years ago

I share @lucamaraschi's point of view too.

subnetmarco commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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

ahmadnassri commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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:

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 commented 9 years ago

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:

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 commented 9 years ago

@thefosk yep, you nailed it, I forgot about #160 :+1:

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 commented 9 years ago

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? :confused:

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

thibaultcha commented 9 years ago

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 commented 9 years ago

Agreed.

subnetmarco commented 9 years ago

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.