Re: change the default action context to omit api key

2019-03-12 Thread Chetan Mehrotra
We may need such a feature flag so created a PR 4334 for it. Chetan Mehrotra On Wed, Mar 6, 2019 at 10:47 PM Rodric Rabbah wrote: > There is a new comment on the issue related to this patch [1] which > suggests a deployment time parameter for enabling the feature that > restricts whisk API

Re: change the default action context to omit api key

2019-03-06 Thread Rodric Rabbah
There is a new comment on the issue related to this patch [1] which suggests a deployment time parameter for enabling the feature that restricts whisk API keys being available to all actions. I looked at the PR and think it can be done with some changes in the controller and the tests. We already

Re: change the default action context to omit api key

2019-02-26 Thread Rodric Rabbah
I have addressed all the feedback to date for this pull request: https://github.com/apache/incubator-openwhisk/pull/4284 As a reminder, this patch adds an annotation such that its presence with an appropriate value will cause the API key to be added to the activation context. The absence of the

Re: change the default action context to omit api key

2019-02-20 Thread Rodric Rabbah
> Existing demo apps which a new user might deploy and uses the SDK > won't work. Should we think about client side tooling to help? For js functions and zip files we can scan for occurrence of require(openwhisk) for example and generate a warning if the annotation is missing. This can lead

Re: change the default action context to omit api key

2019-02-20 Thread James Thomas
This makes sense from a security POV. Given the potential for breaking user applications[1] - we should try to document this as widely as possible. It could probably do with a blog post. I've opened an issue to add this to the JS SDK itself -

Re: change the default action context to omit api key

2019-02-19 Thread Rodric Rabbah
Thanks to all the input here and on the PR - I think we ended up somewhere positive. Here's a summary: 1. for pre-existing actions that are already deployed, they're grandfathered in and will continue to behave in a way where they receive the api key on activation. This is done by detecting the

Re: change the default action context to omit api key

2019-02-14 Thread Rodric Rabbah
I've implemented changes to the PR https://github.com/apache/incubator-openwhisk/pull/4284 for backward compatibility --- such that, actions which do not have the annotation will still get the api key injected. The annotation is added by the controller when an action is created or updated unless

Re: change the default action context to omit api key

2019-02-14 Thread Olivier Tardieu
update -a ...) or redeployed. We already have async users today in prod. Olivier Rodric Rabbah wrote on 02/14/2019 10:26:08 AM: > From: Rodric Rabbah > To: dev@openwhisk.apache.org > Date: 02/14/2019 10:35 AM > Subject: Re: change the default action context to omit api key > &

Re: change the default action context to omit api key

2019-02-14 Thread Rodric Rabbah
> If we end up making a backward incompatible change, we need to stage it: if we do it in a way that's backward compatible for existing actions, does that alleviate the problem for composer? -r

Re: change the default action context to omit api key

2019-02-14 Thread Olivier Tardieu
Composer relies on the api key to implement asynchronous and parallel invocations. Omitting the api key by default would imply that a new composer release is required whenever the change is made. If we end up making a backward incompatible change, we need to stage it: - stage 1: fix the

Re: change the default action context to omit api key

2019-02-14 Thread Rodric Rabbah
> Probably the ideal would be to replace the API key with a key with an expiry time, that can be used only within the lifespan of the action to invoke other actions. Yes - a short term key with specific rights (read, write or execute) would be superior to the current approach, for sure. -r

Re: change the default action context to omit api key

2019-02-14 Thread Rodric Rabbah
Markus posted a suggestion to the PR to make the change backward compatible: 1. treat a missing annotation as truthy (key is injected) 2. on new action create or action update, unless the annotation is already present, add the new annotation with a false value This would leave existing actions in

Re: change the default action context to omit api key

2019-02-14 Thread Dominic Kim
Regarding OpenWhisk SDK, do we have any way to selectively include API Key if an action uses the OW SDK? I think it is a useful feature to be able to omit explicit API key configuration if the SDK is used in the context of OpenWhisk and it is already widely used in my company. Is there any way

Re: change the default action context to omit api key

2019-02-14 Thread Michele Sciabarra
My concern is that if you do not pass the API key, all the actions that invoke other actions must be marked explictly as requiring another API key. From one side I understand the fact the security risk that an action can be fooled to leak the authorization key, from the other side I think

Re: change the default action context to omit api key

2019-02-13 Thread Christian Bickel
Hi Rodric, I agree, that the key should not be passed to the action, if it is not required. But in my opinion, existing actions should continue to work without any update. But I'm OK, if all newly created actions have the default, that they don't have an API-key. Greetings Christian Am Mi., 13.

Re: change the default action context to omit api key

2019-02-13 Thread Rodric Rabbah
Thanks for the quick initial feedback. I've opened a PR that excludes just the API key. https://github.com/apache/incubator-openwhisk/pull/4284 This will be a breaking change - actions that are deployed already and which need the key will need to be updated. I added the annotation `-a

Re: change the default action context to omit api key

2019-02-13 Thread Tyson Norris
I agree the api_key is bad, when not using e.g. OW npm within the action. +1 for using an annotation to enable this. activation_id is required to do the right thing for logging with concurrency enabled - but I'm also not sure what risk it is to include that? It will be in the response header