Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#1631][BotFrameworkAdapter] process_activity returns HTTP 412 error when exchanging a token #1632

Merged
merged 4 commits into from
Apr 12, 2021

Conversation

ceciliaavila
Copy link
Collaborator

Fixes #1631

Description

This PR fixes the issue found when exchanging a token from the Host to the Skill. The response status is 412 and the body's description The bot is unable to exchange token. Proceed with regular login.
The fix evaluates the correct exchange_async method response and returns the actual TokenResponse instance.
There is also a fix for the InvokeResponse result in the process_activity method, which now will be returning the body serialized.
Also, two new unit tests were included to address these fixes.

Specific Changes

  • bot_framework_adapter.py
    • Imported TokenResponse from botframework.connector.token_api.models as ConnectorTokenResponse named class due to existing TokenResponse from bot.builder.schema.
    • Replaced invoke_response.value in process_activity_with_identity with a new instance with the body serialized.
    • Replaced the condition after the exchange_async token is executed in the exchange_token_from_credentials method for evaluating with the ConnectorTokenResponse and returning the TokenResponse instance.
  • test_bot_framework_adapter.py
    • Added the mock _create_token_api_client method in the AdapterUnderTest class to mock the exchange_async method.
    • Added the test test_process_activity_with_identity_token_exchange_invoke_response that will evaluate that the InvokeResponse comes serialized.
    • Added the test test_exchange_token_from_credentials that will evaluate that returns the TokenResponse correctly.

Testing

image
image

@axelsrz axelsrz self-requested a review April 12, 2021 19:24
@axelsrz
Copy link
Member

axelsrz commented Apr 12, 2021

@ceciliaavila this looks great! could you update the branch to merge it?

@ceciliaavila
Copy link
Collaborator Author

@ceciliaavila this looks great! could you update the branch to merge it?

Done @axelsrz! Thanks.

@axelsrz axelsrz merged commit 8e46411 into microsoft:main Apr 12, 2021
@sw-joelmut sw-joelmut deleted the southworks/fix/exchange-token branch December 7, 2021 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BotFrameworkAdapter] Process_activity returns HTTP 412 error when exchanging a token
3 participants