Skip to content

Refactoring Microsoft Teams code to reliably check for external users. #14755

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Nov 28, 2024
Merged

Refactoring Microsoft Teams code to reliably check for external users. #14755

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5089915
Refactoring Microsoft Teams code to reliably check for external users…
malexanderlim Nov 27, 2024
fb04371
package bump.
malexanderlim Nov 27, 2024
8a1fd96
Linter changes.
malexanderlim Nov 27, 2024
be50bf2
lint edits
malexanderlim Nov 27, 2024
c190826
Updating to use correct query syntax.
malexanderlim Nov 27, 2024
344b2fa
Merge branch 'master' into microsoft-teams-external-user-refactor
malexanderlim Nov 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/microsoft_teams/actions/create-channel/create-channel.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export default {
name: "Create Channel",
description: "Create a new channel in Microsoft Teams. [See the docs here](https://docs.microsoft.com/en-us/graph/api/channel-post?view=graph-rest-1.0&tabs=http)",
type: "action",
version: "0.0.7",
version: "0.0.8",
props: {
microsoftTeams,
teamId: {
Expand Down
2 changes: 1 addition & 1 deletion components/microsoft_teams/actions/list-channels/list-channels.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export default {
name: "List Channels",
description: "Lists all channels in a Microsoft Team. [See the docs here](https://docs.microsoft.com/en-us/graph/api/channel-list?view=graph-rest-1.0&tabs=http)",
type: "action",
version: "0.0.7",
version: "0.0.8",
props: {
microsoftTeams,
teamId: {
Expand Down
2 changes: 1 addition & 1 deletion components/microsoft_teams/actions/list-shifts/list-shifts.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export default {
name: "List Shifts",
description: "Get the list of shift instances for a team. [See the documentation](https://learn.microsoft.com/en-us/graph/api/schedule-list-shifts?view=graph-rest-1.0&tabs=http)",
type: "action",
version: "0.0.4",
version: "0.0.5",
props: {
microsoftTeams,
teamId: {
Expand Down
2 changes: 1 addition & 1 deletion components/microsoft_teams/actions/send-channel-message/send-channel-message.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export default {
name: "Send Channel Message",
description: "Send a message to a team's channel. [See the docs here](https://docs.microsoft.com/en-us/graph/api/channel-post-messages?view=graph-rest-1.0&tabs=http)",
type: "action",
version: "0.0.7",
version: "0.0.8",
props: {
microsoftTeams,
teamId: {
Expand Down
2 changes: 1 addition & 1 deletion components/microsoft_teams/actions/send-chat-message/send-chat-message.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export default {
name: "Send Chat Message",
description: "Send a message to a team's chat. [See the docs here](https://docs.microsoft.com/en-us/graph/api/chat-post-messages?view=graph-rest-1.0&tabs=http)",
type: "action",
version: "0.0.7",
version: "0.0.8",
props: {
microsoftTeams,
chatId: {
Expand Down
128 changes: 61 additions & 67 deletions components/microsoft_teams/microsoft_teams.app.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ export default {
label: "Channel",
description: "Team Channel",
async options({
teamId, prevContext,
teamId,
prevContext,
}) {
const response = prevContext.nextLink
? await this.makeRequest({
Expand All @@ -58,76 +59,77 @@ export default {
chat: {
type: "string",
label: "Chat",
description: "Team Chat (internal and external contacts)",
async options({ prevContext }) {
const response = prevContext.nextLink
description: "Select a chat (type to search by participant names)",
async options({
prevContext, query,
}) {
let path = "/chats?$expand=members";
path += "&$top=20";

if (query) {
path += `&$search="${query}"`;
}

const response = prevContext?.nextLink
? await this.makeRequest({
path: prevContext.nextLink,
})
: await this.listChats();

const myTenantId = await this.getAuthenticatedUserTenant();
const options = [];
: await this.makeRequest({
path,
});

this._userCache = this._userCache || new Map();
const options = [];

for (const chat of response.value) {
const messages = await this.makeRequest({
path: `/chats/${chat.id}/messages?$top=50`,
});

const members = await Promise.all(chat.members.map(async (member) => {
const cacheKey = `user_${member.userId}`;
let displayName = member.displayName || this._userCache.get(cacheKey);
let members = chat.members.map((member) => ({
displayName: member.displayName,
wasNull: !member.displayName,
userId: member.userId,
email: member.email,
}));

if (!displayName) {
try {
if (messages?.value?.length > 0) {
const userMessage = messages.value.find((msg) =>
msg.from?.user?.id === member.userId);
if (userMessage?.from?.user?.displayName) {
displayName = userMessage.from.user.displayName;
}
}
if (members.some((member) => !member.displayName)) {
try {
const messages = await this.makeRequest({
path: `/chats/${chat.id}/messages?$top=10&$orderby=createdDateTime desc`,
});

if (!displayName) {
const userDetails = await this.makeRequest({
path: `/users/${member.userId}`,
});
displayName = userDetails.displayName;
const nameMap = new Map();
messages.value.forEach((msg) => {
if (msg.from?.user?.id && msg.from?.user?.displayName) {
nameMap.set(msg.from.user.id, msg.from.user.displayName);
}
});

this._userCache.set(cacheKey, displayName);
} catch (err) {
if (err.statusCode === 404) {
displayName = "User Not Found";
} else if (err.statusCode === 403) {
displayName = "Access Denied";
} else {
displayName = "Unknown User";
}
console.error(`Failed to fetch user details for ${member.userId}:`, err);
}
members = members.map((member) => ({
...member,
displayName: member.displayName || nameMap.get(member.userId) || member.email || "Unknown User",
}));
} catch (err) {
console.error(`Failed to fetch messages for chat ${chat.id}:`, err);
}
}

const isExternal = member.tenantId !== myTenantId || !member.tenantId;
return isExternal
? `${displayName} (External)`
: displayName;
}));
const memberNames = members.map((member) =>
member.wasNull
? `${member.displayName} (External)`
: member.displayName);
Comment on lines +114 to +117
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

External user detection logic needs revision

The current implementation assumes a user is external when their displayName is missing (!member.displayName). However, this is not a reliable way to detect external users as display names could be missing for other reasons. The code should be updated to use official Microsoft Teams API properties for external user detection.

  • components/microsoft_teams/microsoft_teams.app.mjs: The wasNull property should be based on a more reliable indicator of external user status, such as user type or tenant information from the Microsoft Graph API.
🔗 Analysis chain

Verify external user detection logic.

The external user detection logic assumes that missing display names indicate external users. Let's verify this assumption against Microsoft's documentation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for Microsoft documentation about external user identification in Teams
gh api search/code -f q='filename:README.md path:microsoft-graph-docs "external user" Teams displayName'

Length of output: 303

Script:

#!/bin/bash
# Let's examine the actual implementation to understand the external user detection logic
rg -B 5 -A 5 "wasNull" --type js --type ts

# Also search for any related tests or documentation
fd -e md -e js -e ts -e mjs | xargs rg -l "external.*user|guest.*user"

Length of output: 1709

Script:

#!/bin/bash
# Let's examine the context around this Teams integration
rg -B 10 -A 10 "wasNull.*displayName" components/microsoft_teams/

# Look for any tests or documentation specific to the Teams component
fd . components/microsoft_teams/ -t f -e md -e test.mjs -e spec.mjs | xargs cat

# Check if there's any documentation about the member object structure
ast-grep --pattern 'const members = $_' components/microsoft_teams/

Length of output: 3081


options.push({
label: members.join(", "),
label: memberNames.join(", "),
value: chat.id,
});
}

return {
options,
context: {
nextLink: response["@odata.nextLink"],
},
};
},
useQuery: true,
},
channelDisplayName: {
type: "string",
Expand Down Expand Up @@ -168,7 +170,10 @@ export default {
});
},
async makeRequest({
method, path, params = {}, content,
method,
path,
params = {},
content,
}) {
const api = this.client().api(path);

Expand All @@ -192,22 +197,6 @@ export default {
: reduction;
}, api);
},
async getAuthenticatedUserTenant() {
try {
const { value } = await this.client()
.api("/organization")
.get();

if (!value || value.length === 0) {
throw new Error("No organization found");
}

return value[0].id;
} catch (error) {
console.error("Failed to fetch tenant ID:", error);
throw new Error("Unable to determine tenant ID");
}
},
async authenticatedUserId() {
const { id } = await this.client()
.api("/me")
Expand All @@ -231,7 +220,8 @@ export default {
});
},
async createChannel({
teamId, content,
teamId,
content,
}) {
return this.makeRequest({
method: "post",
Expand All @@ -240,7 +230,9 @@ export default {
});
},
async sendChannelMessage({
teamId, channelId, content,
teamId,
channelId,
content,
}) {
return this.makeRequest({
method: "post",
Expand All @@ -249,7 +241,8 @@ export default {
});
},
async sendChatMessage({
chatId, content,
chatId,
content,
}) {
return this.makeRequest({
method: "post",
Expand Down Expand Up @@ -279,7 +272,8 @@ export default {
.get();
},
async listChannelMessages({
teamId, channelId,
teamId,
channelId,
}) {
return this.makeRequest({
path: `/teams/${teamId}/channels/${channelId}/messages/delta`,
Expand Down
2 changes: 1 addition & 1 deletion components/microsoft_teams/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@pipedream/microsoft_teams",
"version": "0.1.3",
"version": "0.1.4",
"description": "Pipedream Microsoft Teams Components",
"main": "microsoft_teams.app.mjs",
"keywords": [
Expand Down
2 changes: 1 addition & 1 deletion components/microsoft_teams/sources/new-channel-message/new-channel-message.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export default {
key: "microsoft_teams-new-channel-message",
name: "New Channel Message",
description: "Emit new event when a new message is posted in a channel",
version: "0.0.8",
version: "0.0.9",
type: "source",
dedupe: "unique",
props: {
Expand Down
2 changes: 1 addition & 1 deletion components/microsoft_teams/sources/new-channel/new-channel.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export default {
key: "microsoft_teams-new-channel",
name: "New Channel",
description: "Emit new event when a new channel is created within a team",
version: "0.0.8",
version: "0.0.9",
type: "source",
dedupe: "unique",
props: {
Expand Down
2 changes: 1 addition & 1 deletion components/microsoft_teams/sources/new-chat-message/new-chat-message.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export default {
key: "microsoft_teams-new-chat-message",
name: "New Chat Message",
description: "Emit new event when a new message is received in a chat",
version: "0.0.8",
version: "0.0.9",
type: "source",
dedupe: "unique",
props: {
Expand Down
2 changes: 1 addition & 1 deletion components/microsoft_teams/sources/new-chat/new-chat.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export default {
key: "microsoft_teams-new-chat",
name: "New Chat",
description: "Emit new event when a new chat is created",
version: "0.0.8",
version: "0.0.9",
type: "source",
dedupe: "unique",
methods: {
Expand Down
2 changes: 1 addition & 1 deletion components/microsoft_teams/sources/new-team-member/new-team-member.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export default {
key: "microsoft_teams-new-team-member",
name: "New Team Member",
description: "Emit new event when a new member is added to a team",
version: "0.0.8",
version: "0.0.9",
type: "source",
dedupe: "unique",
props: {
Expand Down
2 changes: 1 addition & 1 deletion components/microsoft_teams/sources/new-team/new-team.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export default {
key: "microsoft_teams-new-team",
name: "New Team",
description: "Emit new event when a new team is joined by the authenticated user",
version: "0.0.8",
version: "0.0.9",
type: "source",
dedupe: "unique",
methods: {
Expand Down
Loading