search

microsoft / BotFramework-Composer

Dialog creation and management for Microsoft Bot Framework Applications
https://docs.microsoft.com/en-us/composer/
MIT License
869 stars 372 forks source link

AllowedCallers should show up as a warning in the warning panel when it is blank for the root and skills #7255

Open lauren-mills opened 3 years ago

lauren-mills commented 3 years ago

Describe the bug

When I have skills added to my project, allowedCallers being blank should show as a warning in the error panel because skill communication will fail with 403 unauthorized errors if not properly configured and its not clear why.

This shows in the Skill Configuration: image

But nothing shows here: image

Version

Version: 1.4.0-nightly.237625.d1378c6 Electron: 8.2.4 Chrome: 80.0.3987.165 NodeJS: 12.13.0 V8: 8.0.426.27-electron.0

Browser

OS

To Reproduce

Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Expected behavior

Screenshots

Additional context

lauren-mills commented 3 years ago

@darrenj https://github.com/microsoft/botframework-components/issues/960

lauren-mills commented 3 years ago

@darrenj - I'm not sure who to assign this to from the Composer side, but this is painful for the experience. If the user doesn't know to configure the allowedCallers via docs or composer, they will hit this 403 error and have no idea what to do next. We shouldn't set a default value for security, so Composer should at least surface this as a warning

luhan2017 commented 3 years ago

@lauren-mills, when adding a remote skill ,we will automatically added the appid to the allowCaller list of the root bot, so this should not happen except there is a bug. Could you please add more details here? did you add the skill by adding the manifest url through UX?

lauren-mills commented 3 years ago

This is from the enterprise template, which has two skills included out of the box. So it is not a case of going through the add skill flow. The skills are added at template creation. They are local skills

mewa1024 commented 3 years ago

Makes sense to me that this should appear as a warning. Can we use the same text that appears in the validation message that appears on the settings page?

image