POC: unified OAuth flow via ActivityHandler #35
Conversation
There was a problem hiding this comment.
Although I like the idea of subclassing ActivityHandler for oauth, I'm wondering how to reconcile this change when using the TeamsActivityHandler.
This might be a good case to split OAuth flows, as discussed in https://github.com/microsoft/copilot-sdk-for-js/discussions/182
| @@ -46,6 +48,7 @@ public CloudAdapter( | |||
| { | |||
| _activityTaskQueue = activityTaskQueue ?? throw new ArgumentNullException(nameof(activityTaskQueue)); | |||
| _async = async; | |||
| _logger = logger ?? NullLogger.Instance; | |||
There was a problem hiding this comment.
This line fixes a bug I'm investigating, so I think this change belongs to another PR
There was a problem hiding this comment.
Matt has another fix related to this as well in another PR. Also, the OnTurnError is broken because for auth issues, the Body on the ErrorResponseException is null. Which just causes the original exception to be lost because of a null access.
tracyboehrer
changed the title
Me too. I'd rather have something that can be "mixed id" (so to speak). We already have a problem with ActivityHandlers because of single inheritance. I was rather hoping this would spur discussion on how blocks of functionality (API + Activity handling) can be added to a bot. One option would be to put this into ActivityHandler. The default behavior would be as it was before. But if the bot supplies IStorage and OAuthSettings, it would be enabled. Though I'd want to use ConversationState. This was a quick-and-dirty effort and I just used existing sample code. |
POC for consolidating OAuth flow on the bot side... ActivityHandler. This POC subclasses ActivityHandler, and handles the state management and Activities until the user is signed in.
Compare the AuthBot to the original. This also includes the Teams and SharePoint SSO Middleware functionality so that the bot dev does not have to add that middleware to the Adapter.
Possible next steps if we want to go this direction:
We could go further... Make State load/save automatic too. This avoids the bot writer from having to override OnTurnAsync and saving state. It would just happen.