OfficeDev / TeamsFx

Developer tools for building Teams apps
Other
427 stars 164 forks source link

Notification bot sso option #9493

Closed msdev512 closed 6 months ago

msdev512 commented 9 months ago

can we do SSO with notification bot?

Like on the following notification bot code can I get the logged in user details like username and email.

server.post( "/api/notification", restify.plugins.queryParser(), restify.plugins.bodyParser(), ,async (req, res) => {})

ghost commented 9 months ago

Thank you for contacting us! Any issue or feedback from you is quite important to us. We will do our best to fully respond to your issue as soon as possible. Sometimes additional investigations may be needed, we will usually get back to you within 2 days by adding comments to this issue. Please stay tuned.

microsoft-github-policy-service[bot] commented 9 months ago

Thank you for contacting us! Any issue or feedback from you is quite important to us. We will do our best to fully respond to your issue as soon as possible. Sometimes additional investigations may be needed, we will usually get back to you within 2 days by adding comments to this issue. Please stay tuned.

swatDong commented 9 months ago

@msdev512 - you can add sso to any bot. See this doc for adding sso to your existing app, or this sample with sso enabled bot command.

However, the entry point should be /api/messages, not /api/notification.

By the way, sso is to get access token on behalf of the user. To get user's basic information, you can just try this without sso:

var members = target.getPagedMembers();
for (const member of members) {
    member.account; // basic information in member.account
}
swatDong commented 9 months ago

Adding @KennethBWSong for sso-specific question. Is it possible to get Bot SSO work with proactive messaging? Such as:

adapter.continueConversationAsync(id, conversationRef, async(ctx) =>{
    // bot sso here?
})
msdev512 commented 9 months ago
var members = target.getPagedMembers();
for (const member of members) {
    member.account; // basic information in member.account
}

but this is giving following error

target.getPagedMembers is not a fu…ions (node:internal/process/task_queues:95:5)', message: 'target.getPagedMembers is not a function

swatDong commented 9 months ago

What version of teamsfx sdk are you using? There's a deprecated TeamsBotInstallation which only has members() method. And another BotBuilderCloudAdapter.TeamsBotInstallation which has getPagedMembers() method.

msdev512 commented 9 months ago

Teamsfx "version": "2.2.1",

var members = target.getPagedMembers(); for (const member of members) { member.account; // basic information in member.account } this is not available in my version i have following:

can i also use the following code on notification if yes then how to get this "tokenResponse.ssoToken" on notification call

server.post( "/api/notification", restify.plugins.queryParser(), restify.plugins.bodyParser(), // Add more parsers if needed async (req, res) => {

const appCredential = new AppCredential(appAuthConfig); const scope = "User.Read"; const client = createMicrosoftGraphClientWithCredential(appCredential, ["User.Read"]); const profile = await client.api("/me").get();

 const oboCredential = new OnBehalfOfUserCredential(tokenResponse.ssoToken, oboAuthConfig);
  const graphClient = createMicrosoftGraphClientWithCredential(oboCredential, ["User.Read"]);
    const me = await graphClient.api("/me").get();  

});

swatDong commented 9 months ago

SSO needs to work in bot context, so at least in /api/notification you will need:

await target.adapter.continueConversationAsync(
    botId,
    target.conversationReference,
    async (context) => {
        // trigger sso
});

For how to trigger sso in proactive message, @blackchoey , any recommendation on directly using TeamsBotSsoPrompt?

blackchoey commented 9 months ago

@msdev512 can you share your scenario that requires SSO? I see you shared some code that uses AppCredential, this should work so you don't need to use SSO in your code. Your sample code need to be updated slightly as below:

const appCredential = new AppCredential(appAuthConfig);
const client = createMicrosoftGraphClientWithCredential(appCredential, ["User.Read.All"]);
const profile = await client.api("/{fill_user_object_id_here}").get();
msdev512 commented 9 months ago

Hi @blackchoey ,

Scenario , "Each Incoming notification Should be validate and Should go to relevant Member Only"

Incoming notification has email so if i can get logged in user email then i can check, if he should get this bot notification or not.

With the Following suggested code I am getting following Error:

Code: const appCredential = new AppCredential(appAuthConfig); const client = createMicrosoftGraphClientWithCredential(appCredential, ["User.Read.All"]); const profile = await client.api("/{fill_user_object_id_here}").get();

Error:

'Get M365 tenant credential failed with error: invalid_scope: 1002012 - [2023-08-05 14:01:20Z]: AADSTS1002012: The provided value for scope User.Read.All is not valid. Client credential flows must have a scope value with /.default suffixed to the resource identifier (application ID URI).

Please Help and if you working sample then please share me.

blackchoey commented 9 months ago

Incoming notification has email so if i can get logged in user email then i can check, if he should get this bot notification or not.

I'm not sure whether you mean to SSO every user when a notification comes, in order to get each user's email and compare with the payload (email) in the notification request. If yes, this is not a recommended practice and SSO requires a user to be online (opened Teams client). @swatDong do you have any suggestions to this case?

With the Following suggested code I am getting following Error:

Sorry I made a mistake in the updated code. You can refer https://github.com/OfficeDev/TeamsFx/tree/dev/packages/sdk#invoke-graph-api-without-user-application-identity for full example code. The code requires to add Application permission for your app's AAD app and grant admin consent.

swatDong commented 9 months ago

@msdev512 to get user's email, you can call members() on each target (based on your sdk version)

var members = target.members();
for (const member of members) {
    member.account; // basic information in member.account, including email
}
msdev512 commented 9 months ago
var members = target.members();
for (const member of members) {
    member.account; // basic information in member.account, including email
}

I am using Following Code and getting member email successfully but the problem is "When all member are logged in. They are receiving the notification of other members "

"But if i could get the email from logged in user this problem could be avoided"

var useremail = req.body.email;

for (const target of await bot.notification.installations()) {

      const member = await bot.notification.findMember(
        async (m) => m.account.email === useremail
      );

}

swatDong commented 9 months ago

Could you please provide more details about "logged in member"? When your app is added to a team, members() returns all members in that team. (What does "logged in" mean here in your scenario?)

Or, you can call member.sendAdaptiveCard(...) to directly notify the member you found.

msdev512 commented 9 months ago

Hi, Logged in member = the Teams user who has logged in to team and install this bot App.

" member.sendAdaptiveCard(...)" is working but its sending notification adaptive card twice.

msdev512 commented 9 months ago

Incoming notification has email so if i can get logged in user email then i can check, if he should get this bot notification or not.

I'm not sure whether you mean to SSO every user when a notification comes, in order to get each user's email and compare with the payload (email) in the notification request. If yes, this is not a recommended practice and SSO requires a user to be online (opened Teams client). @swatDong do you have any suggestions to this case?

With the Following suggested code I am getting following Error:

Sorry I made a mistake in the updated code. You can refer https://github.com/OfficeDev/TeamsFx/tree/dev/packages/sdk#invoke-graph-api-without-user-application-identity for full example code. The code requires to add Application permission for your app's AAD app and grant admin consent.

Hi,

i mean if you install a bot app in teams then you need sso for token and to fetch user info.

yes, its illogical to do SSO on each notification. Is there any way if sso done once on app load then on any new notification.

1)we can fetch SSO Token 2)if its expire we can refresh it 3) call relevant graph api

Thanks for your Support.

blackchoey commented 9 months ago

@msdev512 to get user's email, you can call members() on each target (based on your sdk version)

var members = target.members();
for (const member of members) {
    member.account; // basic information in member.account, including email
}

Please refer @swatDong 's above reply. I feel the direction should be consuming the email properties in member.acount. If you can't find email in the properties, let us know.

microsoft-github-policy-service[bot] commented 6 months ago

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

microsoft-github-policy-service[bot] commented 6 months ago

Due to lack of details for further investigation, we will archive the issue for now. In case you still have following-up questions on this issue, please always feel free to reopen the issue by clicking ‘reopen issue’ button below the comment box. We will get back to you as soon as possible.