8000 Allow skills with no appId or password (#1406) · ericmicrofocus/botbuilder-python@a30301b · GitHub
[go: up one dir, main page]
Skip to content

Commit a30301b

Browse files
mdrichardsonMichael Richardson
mdrichardson
and
Michael Richardson
authored
Allow skills with no appId or password (microsoft#1406)
* add support for anon appid * add tests * cleanup * black fix * add skill RoleType * black fix * pylint fix Co-authored-by: Michael Richardson <v-micric@microsoft.com>
1 parent b787506 commit a30301b

File tree

15 files changed

+218
-29
lines changed

15 files changed

+218
-29
lines changed

‎libraries/botbuilder-core/botbuilder/core/channel_service_handler.py

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
ClaimsIdentity,
2222
CredentialProvider,
2323
JwtTokenValidation,
24+
SkillValidation,
2425
)
2526

2627

@@ -469,18 +470,28 @@ async def on_upload_attachment(
469470
raise BotActionNotImplementedError()
470471

471472
async def _authenticate(self, auth_header: str) -> ClaimsIdentity:
473+
"""
474+
Helper to authenticate the header.
475+
476+
This code is very similar to the code in JwtTokenValidation.authenticate_request,
477+
we should move this code somewhere in that library when we refactor auth,
478+
for now we keep it private to avoid adding more public static functions that we will need to deprecate later.
479+
"""
472480
if not auth_header:
473481
is_auth_disabled = (
474482
await self._credential_provider.is_authentication_disabled()
475483
)
476-
if is_auth_disabled:
477-
# In the scenario where Auth is disabled, we still want to have the
478-
# IsAuthenticated flag set in the ClaimsIdentity. To do this requires
479-
# adding in an empty claim.
480-
return ClaimsIdentity({}, True)
484+
if not is_auth_disabled:
485+
# No auth header. Auth is required. Request is not authorized.
486+
raise PermissionError()
481487

482-
raise PermissionError()
488+
# In the scenario where Auth is disabled, we still want to have the
489+
# IsAuthenticated flag set in the ClaimsIdentity. To do this requires
490+
# adding in an empty claim.
491+
# Since ChannelServiceHandler calls are always a skill callback call, we set the skill claim too.
492+
return SkillValidation.create_anonymous_skill_claim()
483493

494+
# Validate the header and extract claims.
484495
return await JwtTokenValidation.validate_auth_header(
485496
auth_header,
486497
self._credential_provider,

‎libraries/botbuilder-core/tests/test_channel_service_handler.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
import aiounittest
5+
from botbuilder.core import ChannelServiceHandler
6+
from botframework.connector.auth import (
7+
AuthenticationConfiguration,
8+
ClaimsIdentity,
9+
SimpleCredentialProvider,
10+
JwtTokenValidation,
11+
AuthenticationConstants,
12+
)
13+
import botbuilder.schema
14+
15+
16+
class TestChannelServiceHandler(ChannelServiceHandler):
17+
def __init__(self):
18+
self.claims_identity = None
19+
ChannelServiceHandler.__init__(
20+
self, SimpleCredentialProvider("", ""), AuthenticationConfiguration()
21+
)
22+
23+
async def on_reply_to_activity(
24+
self,
25+
claims_identity: ClaimsIdentity,
26+
conversation_id: str,
27+
activity_id: str,
28+
activity: botbuilder.schema.Activity,
29+
) -> botbuilder.schema.ResourceResponse:
30+
self.claims_identity = claims_identity
31+
return botbuilder.schema.ResourceResponse()
32+
33+
34+
class ChannelServiceHandlerTests(aiounittest.AsyncTestCase):
35+
async def test_should_authenticate_anonymous_skill_claim(self):
36+
sut = TestChannelServiceHandler()
37+
await sut.handle_reply_to_activity(None, "123", "456", {})
38+
39+
assert (
40+
sut.claims_identity.authentication_type
41+
== AuthenticationConstants.ANONYMOUS_AUTH_TYPE
42+
)
43+
assert (
44+
JwtTokenValidation.get_app_id_from_claims(sut.claims_identity.claims)
45+
== AuthenticationConstants.ANONYMOUS_SKILL_APP_ID
46+
)

‎libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/bot_framework_http_client.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
ConversationReference,
1616
ConversationAccount,
1717
ChannelAccount,
18+
RoleTypes,
1819
)
1920
from botframework.connector.auth import (
2021
ChannelProvider,
@@ -97,7 +98,9 @@ async def post_activity(
9798
activity.conversation.id = conversation_id
9899
activity.service_url = service_url
99100
if not activity.recipient:
100-
activity.recipient = ChannelAccount()
101+
activity.recipient = ChannelAccount(role=RoleTypes.skill)
102+
else:
103+
activity.recipient.role = RoleTypes.skill
101104

102105
status, content = await self._post_content(to_url, token, activity)
103106

‎libraries/botbuilder-integration-aiohttp/tests/test_bot_framework_http_client.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
from unittest.mock import Mock
22

33
import aiounittest
4-
from botbuilder.schema import ConversationAccount, ChannelAccount
4+
from botbuilder.schema import ConversationAccount, ChannelAccount, RoleTypes
55
from botbuilder.integration.aiohttp import BotFrameworkHttpClient
66
from botframework.connector.auth import CredentialProvider, Activity
77

@@ -69,3 +69,4 @@ async def _mock_post_content(
6969
)
7070

7171
assert activity.recipient.id == skill_recipient_id
72+
assert activity.recipient.role is RoleTypes.skill

‎libraries/botbuilder-schema/botbuilder/schema/_connector_client_enums.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ class RoleTypes(str, Enum):
88

99
user = "user"
1010
bot = "bot"
11+
skill = "skill"
1112

1213

1314
class ActivityTypes(str, Enum):

‎libraries/botbuilder-schema/botbuilder/schema/_models_py3.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1307,8 +1307,8 @@ class ConversationAccount(Model):
13071307
:param aad_object_id: This account's object ID within Azure Active
13081308
Directory (AAD)
13091309
:type aad_object_id: str
1310-
:param role: Role of the entity behind the account (Example: User, Bot,
1311-
etc.). Possible values include: 'user', 'bot'
1310+
:param role: Role of the entity behind the account (Example: User, Bot, Skill
1311+
etc.). Possible values include: 'user', 'bot', 'skill'
13121312
:type role: str or ~botframework.connector.models.RoleTypes
13131313
:param tenant_id: This conversation's tenant ID
13141314
:type tenant_id: str

‎libraries/botframework-connector/botframework/connector/auth/app_credentials.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,11 @@ def signed_session(self, session: requests.Session = None) -> requests.Session:
104104
def _should_authorize(
105105
self, session: requests.Session # pylint: disable=unused-argument
106106
) -> bool:
107-
return True
107+
# We don't set the token if the AppId is not set, since it means that we are in an un-authenticated scenario.
108+
return (
109+
self.microsoft_app_id != AuthenticationConstants.ANONYMOUS_SKILL_APP_ID
110+
and self.microsoft_app_id is not None
111+
)
108112

109113
def get_access_token(self, force_refresh: bool = False) -> str:
110114
"""

‎libraries/botframework-connector/botframework/connector/auth/authentication_constants.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,3 +113,9 @@ class AuthenticationConstants(ABC):
113113

114114
# Service URL claim name. As used in Microsoft Bot Framework v3.1 auth.
115115
SERVICE_URL_CLAIM = "serviceurl"
116+
117+
# AppId used for creating skill claims when there is no appId and password configured.
118+
ANONYMOUS_SKILL_APP_ID = "AnonymousSkill"
119+
120+
# Indicates that ClaimsIdentity.authentication_type is anonymous (no app Id and password were provided).
121+
ANONYMOUS_AUTH_TYPE = "anonymous"

‎libraries/botframework-connector/botframework/connector/auth/claims_identity.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,12 @@ def __init__(self, claim_type: str, value):
55

66

77
class ClaimsIdentity:
8-
def __init__(self, claims: dict, is_authenticated: bool):
8+
def __init__(
9+
self, claims: dict, is_authenticated: bool, authentication_type: str = None
10+
):
911
self.claims = claims
1012
self.is_authenticated = is_authenticated
13+
self.authentication_type = authentication_type
1114

1215
def get_claim_value(self, claim_type: str):
1316
return self.claims.get(claim_type)

‎libraries/botframework-connector/botframework/connector/auth/jwt_token_validation.py

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
# Licensed under the MIT License.
33
from typing import Dict, List, Union
44

5-
from botbuilder.schema import Activity
5+
from botbuilder.schema import Activity, RoleTypes
66

7+
from ..channels import Channels
78
from .authentication_configuration import AuthenticationConfiguration
89
from .authentication_constants import AuthenticationConstants
910
from .emulator_validation import EmulatorValidation
@@ -43,14 +44,29 @@ async def authenticate_request(
4344
"""
4445
if not auth_header:
4546
# No auth header was sent. We might be on the anonymous code path.
46-
is_auth_disabled = await credentials.is_authentication_disabled()
47-
if is_auth_disabled:
48-
# We are on the anonymous code path.
49-
return ClaimsIdentity({}, True)
50-
51-
# No Auth Header. Auth is required. Request is not authorized.
52-
raise PermissionError("Unauthorized Access. Request is not authorized")
53-
47+
auth_is_disabled = await credentials.is_authentication_disabled()
48+
if not auth_is_disabled:
49+
# No Auth Header. Auth is required. Request is not authorized.
50+
raise PermissionError("Unauthorized Access. Request is not authorized")
51+
52+
# Check if the activity is for a skill call and is coming from the Emulator.
53+
try:
54+
if (
55+
activity.channel_id == Channels.emulator
56+
and activity.recipient.role == RoleTypes.skill
57+
and activity.relates_to is not None
58+
):
59+
# Return an anonymous claim with an anonymous skill AppId
60+
return SkillValidation.create_anonymous_skill_claim()
61+
except AttributeError:
62+
pass
63+
64+
# In the scenario where Auth is disabled, we still want to have the
65+
# IsAuthenticated flag set in the ClaimsIdentity. To do this requires
66+
# adding in an empty claim.
67+
return ClaimsIdentity({}, True, AuthenticationConstants.ANONYMOUS_AUTH_TYPE)
68+
69+
# Validate the header and extract claims.
5470
claims_identity = await JwtTokenValidation.validate_auth_header(
5571
auth_header,
5672
credentials,

‎libraries/botframework-connector/botframework/connector/auth/skill_validation.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,12 @@ def is_skill_claim(claims: Dict[str, object]) -> bool:
6666
if AuthenticationConstants.VERSION_CLAIM not in claims:
6767
return False
6868

69+
if (
70+
claims.get(AuthenticationConstants.APP_ID_CLAIM, None)
71+
== AuthenticationConstants.ANONYMOUS_SKILL_APP_ID
72+
):
73+
return True
74+
6975
audience = claims.get(AuthenticationConstants.AUDIENCE_CLAIM)
7076

7177
# The audience is https://api.botframework.com and not an appId.
@@ -124,6 +130,20 @@ async def authenticate_channel_token(
124130

125131
return identity
126132

133+
@staticmethod
134+
def create_anonymous_skill_claim():
135+
"""
136+
Creates a ClaimsIdentity for an anonymous (unauthenticated) skill.
137+
:return ClaimsIdentity:
138+
"""
139+
return ClaimsIdentity(
140+
{
141+
AuthenticationConstants.APP_ID_CLAIM: AuthenticationConstants.ANONYMOUS_SKILL_APP_ID
142+
},
143+
True,
144+
AuthenticationConstants.ANONYMOUS_AUTH_TYPE,
145+
)
146+
127147
@staticmethod
128148
async def _validate_identity(
129149
identity: ClaimsIdentity, credentials: CredentialProvider

‎libraries/botframework-connector/tests/test_app_credentials.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
import aiounittest
5+
from botframework.connector.auth import AppCredentials, AuthenticationConstants
6+
7+
8+
class AppCredentialsTests(aiounittest.AsyncTestCase):
9+
@staticmethod
10+
def test_should_not_send_token_for_anonymous():
11+
# AppID is None
12+
app_creds_none = AppCredentials(app_id=None)
13+
assert app_creds_none.signed_session().headers.get("Authorization") is None
14+
15+
# AppID is anonymous skill
16+
app_creds_anon = AppCredentials(
17+
app_id=AuthenticationConstants.ANONYMOUS_SKILL_APP_ID
18+
)
19+
assert app_creds_anon.signed_session().headers.get("Authorization") is None
20+
21+
22+
def test_constructor():
23+
should_default_to_channel_scope = AppCredentials()
24+
assert (
25+
should_default_to_channel_scope.oauth_scope
26+
== AuthenticationConstants.TO_CHANNEL_FROM_BOT_OAUTH_SCOPE
27+
)
28+
29+
should_default_to_custom_scope = AppCredentials(oauth_scope="customScope")
30+
assert should_default_to_custom_scope.oauth_scope == "customScope"

‎libraries/botframework-connector/tests/test_auth.py

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66

77
import pytest
88

9-
from botbuilder.schema import Activity
9+
from botbuilder.schema import Activity, ConversationReference, ChannelAccount, RoleTypes
10+
from botframework.connector import Channels
1011
from botframework.connector.auth import (
1112
AuthenticationConfiguration,
1213
AuthenticationConstants,
@@ -21,6 +22,7 @@
2122
GovernmentChannelValidation,
2223
SimpleChannelProvider,
2324
ChannelProvider,
25+
AppCredentials,
2426
)
2527

2628

@@ -262,7 +264,7 @@ async def test_channel_msa_header_valid_service_url_should_be_trusted(self):
262264

263265
await JwtTokenValidation.authenticate_request(activity, header, credentials)
264266

265-
assert MicrosoftAppCredentials.is_trusted_service(
267+
assert AppCredentials.is_trusted_service(
266268
"https://smba.trafficmanager.net/amer-client-ss.msg/"
267269
)
268270

@@ -289,6 +291,32 @@ async def test_channel_msa_header_invalid_service_url_should_not_be_trusted(self
289291
"https://webchat.botframework.com/"
290292
)
291293

294+
@pytest.mark.asyncio
295+
# Tests with a valid Token and invalid service url and ensures that Service url is NOT added to
296+
# Trusted service url list.
297+
async def test_channel_authentication_disabled_and_skill_should_be_anonymous(self):
298+
activity = Activity(
299+
channel_id=Channels.emulator,
300+
service_url="https://webchat.botframework.com/",
301+
relates_to=ConversationReference(),
302+
recipient=ChannelAccount(role=RoleTypes.skill),
303+
)
304+
header = ""
305+
credentials = SimpleCredentialProvider("", "")
306+
307+
claims_principal = await JwtTokenValidation.authenticate_request(
308+
activity, header, credentials
309+
)
310+
311+
assert (
312+
claims_principal.authentication_type
313+
== AuthenticationConstants.ANONYMOUS_AUTH_TYPE
314+
)
315+
assert (
316+
JwtTokenValidation.get_app_id_from_claims(claims_principal.claims)
317+
== AuthenticationConstants.ANONYMOUS_SKILL_APP_ID
318+
)
319+
292320
@pytest.mark.asyncio
293321
async def test_channel_msa_header_from_user_specified_tenant(self):
294322
activity = Activity(
@@ -318,8 +346,10 @@ async def test_channel_authentication_disabled_should_be_anonymous(self):
318346
activity, header, credentials
319347
)
320348

321-
assert claims_principal.is_authenticated
322-
assert not claims_principal.claims
349+
assert (
350+
claims_principal.authentication_type
351+
== AuthenticationConstants.ANONYMOUS_AUTH_TYPE
352+
)
323353

324354
@pytest.mark.asyncio
325355
# Tests with no authentication header and makes sure the service URL is not added to the trusted list.

0 commit comments

Comments
 (0)
0