[feature/oauth2] Allow OAuth2 access/refresh tokens to be supplied

[eddmann]

Whilst testing the OAuth2 plugin we noticed that a 500 error status was returned every so often when generating a new access token.
Having a look at the code and also this issue #478, we found that it was most likely due to worker uuid seeds being the same - causing duplicate 'random' access tokens.
This feature request adds the ability to supply the desired access/refresh tokens upon issue.

[thibaultcha]

Thank you @eddmann!

@thefosk wrote the OAuth2 plugin and should be the one reviewing this.

[eddmann]

Great thanks :)

[subnetmarco]

On a side note, while I am reviewing this, it's also worthwhile investigating the "worker uuid seeds being the same" issue since that could happen somewhere else too. Including @Tieske in the conversation since we are using his https://github.com/Tieske/uuid library.

[subnetmarco]

@eddmann correct me if I am wrong - with your code the client would be able to set arbitrary access_token and refresh_token values?

[eddmann]

@thefosk That is correct, so as to allow control of access/refresh tokens generated/used to remain with an existing system i.e legacy app

[subnetmarco]

@eddmann correct me if I am wrong, but if the client can set arbitrary access_token or refresh_token values, wouldn't that be a security issue (as opposed to the server being able to set them)?

On a side note starting from the next release, we fixed the uuid problem (#478).

[eddmann]

@thefosk The intention of adding the ability to set arbitrary tokens is for the trusted 'Webapp Backend' to set them, not proxying values provided by the user, not sure if that is what you meant by client?
This addition allows current token generation methods to be maintained without enforcing a new external means - also providing a graceful migration path in the situation when moving over to Kong and its auto-generated tokens.

[subnetmarco]

@eddmann I was mentioning the client because I got confused by this integration test: https://github.com/Mashape/kong/pull/659/files#diff-0458d9c5302a8f52664b5c7dcc2ad586R366

It seems like the client is setting the access_token without passing any provision_key.

[eddmann]

@thefosk Very good point, just spent sometime updating the pull request.

Think it is probably easiest if I just layout more clearly what this feature is trying to achieve - as I don't think I have done the best of jobs, confusing matters by discussing the UUID bug within the initial description.

This pull adds the ability to optionally supply the access (and in some cases refresh) tokens upon issue, providing that a valid provision key has been supplied.
This addition allows current token generation methods to be maintained without enforcing a new external means.
This in-turn provides a graceful migration path in the situation when moving over to Kong and it's auto-generated tokens.
This pull implements this functionality into the client credentials, password and refresh token grant types.

[subnetmarco]

In order to provide a custom access tokens (with optionally a custom refresh token) I see that your implementation uses the proxy port, and now properly checks the provision_key.

Do you think it makes sense to being able to create/update/delete access tokens using the admin API instead?

Let me explain better: like you already mentioned, I see this feature valuable when an API provider wants to migrate existing access tokens into Kong without any service disruption. But then, maybe we can add an additional endpoint on the admin API to do this and migrate all the access tokens?

We don't need this to happen on the proxy port, because if a client knows the provision_key, then it means that the client is the API provider. As such, we could directly expose this functionality in the admin API, in an endpoint like /oauth2_tokens/ which allows an API provider to execute CRUD operations on access tokens.

Thoughts?

[subnetmarco]

The latter approach will be implemented in #729 - which deprecates this PR.