search

apache / openwhisk-package-pushnotifications

OpenWhisk Package for Bluemix Push Notifications Service
Apache License 2.0
10 stars 17 forks source link

OpenWhisk Mobile push package binding (Bluemix Dashboard) have some issues #55

Closed AnanthaKrish closed 6 years ago

AnanthaKrish commented 7 years ago

The Openwhisk mobile push dashboard binding in Bluemix dashboard have couple of issues.

  1. Both AppGUID and AppSecret should be given at the time of Binding. Now only appSecret value is there

  2. The input to the sendMessage action should not contain appSecret and AppId. It is given already at the time of binding the push service.

rabbah commented 7 years ago

I removed your images since they contained credentials. Please make sure next time you redact these. If these were valid credentials I suggest you take action immediately to revoke them.

rabbah commented 7 years ago

@jasonpet who do you suggest can handle this?

AnanthaKrish commented 7 years ago

@rabbah No It is not valid credentials :)

csantanapr commented 7 years ago

Can someone repost the picture? Can you do a package refresh and a package get on the new binding to see if the values show they the CLI. Just want to compare output from CLI vs UI

@ibm-speeter should look into this

nwspeete-ibm commented 7 years ago

The fix for 1. is the following: -a parameters '[ {"name":"appId", "required":true, "description":"Bluemix application GUID"}, {"name":"appSecret", "required":true, "bindTime":true, "type":"password", "description":"Bluemix Push Service Secret"}]' \ ->appId in three places in the pushPackage needs to have bindTime:true like appSecret

For 2. This is currently by design in the UI, Anantha if you think it looks cleaner we can take out appSecret and AppId.

nwspeete-ibm commented 7 years ago

https://github.com/openwhisk/openwhisk-package-pushnotifications/pull/56 has the suggested fix, note these might fail in local travis if the old credentials issue still exists (then pass in travis once pulled)

csantanapr commented 6 years ago

This is fixed now with #56