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

Admin API should have a "provisioning" endpoint #682

Closed ahmadnassri closed 3 years ago

ahmadnassri commented 8 years ago

provisioning endpoint does two things:

e.g.

curl -X POST localhost:8001/apis/{id}/provision/ --data "username=foo" --data "secret=abc"

Which returns something like:

{
  "authentication": "basic", 
  "credential": {
    "id": "id123", 
    "keys": { 
      "username": "foo", 
      "password": "abc" 
    }
  }
}
yoanisgil commented 8 years ago

@ahmadnassri what's the difference between doing this and following the existent workflow where a consumer is created and then access is granted to an API, like it's described here:

https://getkong.org/plugins/basic-authentication/

Maybe I'm missing something obvious here but I'm 100% in favor of:

    Explicit is better than implicit.
    There should be one-- and preferably only one --obvious way to do it.
ahmadnassri commented 8 years ago

@yoanisgil the point here is convenience and expediting adoption, I like the current separation, but it makes it extremely heavy to initiate a system and import an existing set of users / consumers (think thousands of records) this new endpoint would allow mainly for importing of user / consumer records faster.

another possibility here is the endpoint supports a JSON array of objects, which means you can provision your entire user base with one call on initial setup.

There should be one-- and preferably only one --obvious way to do it.

the internals remain the same, using the same internal Lua APIs

yoanisgil commented 8 years ago

@ahmadnassri The same operation could be performed on the /consumers endpoint, i.e it could take a JSON to import any numbers of users in a one shot operation and then all it's left to do is the call to enable the authentication plugin on the corresponding API.

ahmadnassri commented 8 years ago

@yoanisgil sure, how would that look like?

ahmadnassri commented 8 years ago

and then all it's left to do is the call to enable the authentication plugin on the corresponding API.

but you're forgetting provisioning credentials? or are you suggesting that's done on the /consumers endpoint as well?

yoanisgil commented 8 years ago

For instance we could do a POST http://kong:8001/consumers/provision which takes a JSON file. I'm not sure how to answer your question on provisioning credentials. What uses cases do we want to support? If it's always basic auth (which might not be a fit for all use cases) then I guess it could be done on the /consumers endpoint.

ahmadnassri commented 8 years ago

provisioning credentials:

all enabled auth plugins per specific APIs (hence why I suggested the end point lives on the API path)

e.g. /api/xyz has basic auth and oauth2 enabled.

the /provision takes a list of user objects (names / custom id and secret)

yoanisgil commented 8 years ago

Ok. I see your point. Yes, it looks more convenient to place the endpoint on /apis. It stills look a bit smelly to me but I cannot think of something better right now.

ahmadnassri commented 8 years ago

It stills look a bit smelly to me but I cannot think of something better right now.

same here ... happy to get more ideas from the community

yoanisgil commented 8 years ago

How about an endpoint which does not involve /apis/{api} not /consumers? Maybe if you provide some concrete use cases we could elaborate on this. @JnMik what do you think?

ahmadnassri commented 8 years ago

like a root endpoint?

POST /provision

{
  "api_id": "abc",
  "consumers": [ ... ]
}
yoanisgil commented 8 years ago

Yes that makes me feel better ;). It looks to me if we do it the other way, we're just contaminating the existing endpoints which look pretty neat to me. Btw, provision looks too generic to me but I'm not good at names.

ahmadnassri commented 8 years ago

use cases:

yoanisgil commented 8 years ago

I have not played around with all existing authentication plugins but will you support all of them on the /provision endpoint? Like for instance, how does /provision works for JWT or API Key authentication?

The first use case seems 100% valid to me. As to the second one, it seems to me Gelato could just follow the standard procedure ;) but that's just me.

ahmadnassri commented 8 years ago

the idea is to always provide a username (or custom_id) + secret

the latter will generically provide: password, key, client_secret, signature, etc ... per authentication method ...

ahmadnassri commented 8 years ago

it seems to me Gelato could just follow the standard procedure ;) but that's just me.

I'm in agreement on that, but figured there is good overlap here so could be beneficial too.

yoanisgil commented 8 years ago

Like I said I'm a huge fan of There should be one-- and preferably only one --obvious way to do it. so if it were me I would strive to make clear this endpoints are as different as possible, from the input they take, the output they produce and documentation.

Anyhow, how about if we work on a input JSON file(s) for existing API? Say we have API on /apis/mockbin where all existing authentication mechanisms are enable (I don't know if that's possible but if not we could just say that there are N API where N = # of existing authentication mechanism and there is one API for each existing one).

As per the documentation there are 5 authentication mechanisms:

what would the JSON input file will look like for an API with all those methods enabled (or 5 of them :))?

JnMik commented 8 years ago

Do you want to limit your use case as " I want to create my API [...]" ?

What about "I want to import a list of APIs, configure authentication plugins wanted for each of them (possibly multiple per API), then import my database of users [...]

Also, from what I see in the documentation, you guys are not linking a user to an api, but a group of user to an api. So my guess is that notion of groups should be in the JSON File.

What about :

{
    "consumers": [
        {
            "username": "jnmik",
            "auth-method": {
                "basic-auth": {
                    "password": "123"
                },
                "key-auth": {
                    "key": "123abc123"
                }
            }
        },
        {
            "username": "ygil",
            "auth-method": {
                "basic-auth": {
                    "password": "123"
                },
                "key-auth": {
                    "key": "123abc123"
                }
            }
        }
    ],
    "consumer-groups": [
        {
            "name": "users-for-api-one",
            "consumers": [
                "jnmik",
                "ygil"
            ]
        },
        {
            "name": "users-for-api-two",
            "consumers": [
                "jnmik",
                "ygil"
            ]
        }
    ],
    "apis": [
        {
            "name": "api-one",
            "request_host": "mockbin.com",
            "request_path": "/someservice",
            "strip_request_path": false,
            "preserve_host": false,
            "upstream_url": "https://mockbin.com",
            "plugins": [
                {
                    "name": "acl",
                    "config.whitelist": "users-for-api-one"
                }
            ]
        }
    ]
}
yoanisgil commented 8 years ago

@JnMik when I first read the issue it came to my mind that this is the sort of thing it could be handle from a client side, though I grant that for performance reasons an endpoint for bulk imports might be really nice.

JnMik commented 8 years ago

I don't see why we would limit the imports to users only :)

ahmadnassri commented 8 years ago

I don't see why we would limit the imports to users only :)

I'm hesitant to agree ... or disagree.

my worry is that this becomes bloated. Sure enough the original use-case is only discussing consumers, but there's not reason it shouldn't include other aspects.

there could be a point to make about having an all inclusive /import endpoint perhaps with a JSON spec to support all the creations of all objects, but even then it would not be fully compatible with the possible configurations.

e.g. some plugin configurations are consumer specific, so when do you get to make that association if you're also passing in the consumers at the same time?

and the counter point would be to simply state the original use case again:

initial system deployment: a system administrator, I want to create my API, configure authentication plugins wanted (possibly multiple per API), then import my database of users into Kong consumer objects and provide credentials for all the created users.

the point is about importing / syncing / provisioning (whatever terminology makes the most sense) an existing user database into consumer objects and map their credentials.

so perhaps if solve this first, we can proceed to discuss /import separately?

again, not fully advocating one way or the other at this point. there are many improvements to be made to the API in general, and we want to keep it simple and consistent as well.

yoanisgil commented 8 years ago

@ahmadnassri I see your point. We were thinking that there might room for an utility for creating API migrations, something similar to what 's already done for DB. The way I see this, this tool will be able to import a Kong API definition (consumers, plugins, credentials, etc) as well as detecting changes and applying on top of the existing API. I think this is really handy for production systems since it allows for keeping track of API changes. But we could track this under a different issue if this is of some interest to you guys.

ahmadnassri commented 8 years ago

I think this is really handy for production systems since it allows for keeping track of API changes.

agreed.

The way I see this, this tool will be able to import a Kong API definition (consumers, plugins, credentials, etc) as well as detecting changes and applying on top of the existing API

both import & export are certainly needed functionality ... whether a different issue or continue here, both options are good.

The main purpose of opening this issue and starting this discussion, was to find alternative ways to https://github.com/Mashape/kong/pull/685 so perhaps your thoughts on that too would be helpful.

sonicaghi commented 8 years ago

maybe the endpoint should be called "Import" rather than provisioning.