Skip to content

feat: implement hybrid single-bot architecture and general bug and security fixes#43

Merged
abhinavkrin merged 42 commits intomainfrom
re-arch
Apr 8, 2026
Merged

feat: implement hybrid single-bot architecture and general bug and security fixes#43
abhinavkrin merged 42 commits intomainfrom
re-arch

Conversation

@abhinavkrin
Copy link
Copy Markdown
Member

@abhinavkrin abhinavkrin commented Mar 18, 2026

Description

This PR implements the "Hybrid Single-Bot Architecture", deprecating the previous dummy-user architecture. This massively simplifies account management by routing all fallback messages through the primary App User (microsoftteamsbridge.bot) instead of provisioning individual RC accounts for Teams users.

Along with this architectural shift, this PR includes a major codebase restructuring, security hardening of the OAuth endpoint, and new UI features for managing Teams rooms.

Major Changes

1. Single-Bot Architecture

  • Bridge rooms are now tracked via an isBridged flag on the room model.
  • Teams users send messages to RC via the single app bot using a display name prefix (**Name** _via Teams_).
  • Added /teamsbridge-login-app-user to grant the bot a delegated token for Microsoft Graph interactions.
  • Added RecentActivity tracking to mitigate race conditions and outbound echos.

2. Code Refactoring

  • Persistence: Monolithic PersistHelper.ts split into 13 namespace files in lib/persistence/.
  • Handlers: EventHandler.ts logic moved to 11 isolated files inside lib/handlers/.
  • Graph API: Split into a lib/graph/ directory (28 isolated endpoint calls).
  • Inbound Logic: Split into specific Create/Update/Delete handlers.

3. New Features & UI

  • Live Graph Search: /teamsbridge-add-user now fully paginates and searches Teams users live via Microsoft Graph (local DB cache removed).
  • View Members: Added /teamsbridge-view-members slash command to view Teams thread participants.

4. Security Hardening

  • Added a secure, verified nonce to the OAuth2 state parameter to prevent CSRF attacks.
  • Refactored token renewal and added strict scoping for Bot vs. Normal users.

How to Test

  1. As an admin, run /teamsbridge-login-app-user and complete the OAuth flow.
  2. Add the app bot to an RC room to bridge it.
  3. Send text and file messages from both logged-in and non-logged-in RC users.
  4. Edit and delete messages in both platforms.
  5. Use /teamsbridge-add-user to test the new live search contextual bar.

https://rocketchat.atlassian.net/browse/SUP-959
https://rocketchat.atlassian.net/browse/CORE-1891

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
- handlePreFileUploadAsync: replace findAllDummyUsersInRocketChatUserListAsync
  member scan with isBridgeRoomAsync check; de-indent body, remove dead
  members variable
- handleAddTeamsUserContextualBarSubmitAsync: remove dummy-user lookup and RC
  room member update; iterate directly over teamsUserIdsToSave for Teams
  thread; drop unused modify param from signature and call-site
- Remove stale imports: findAllDummyUsersInRocketChatUserListAsync,
  retrieveDummyUserByTeamsUserIdAsync, getMessageFootPrintExistenceInfo,
  saveLastBridgedMessageFootprint

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
- handlePreMessageOperationPreventAsync: body was a no-op (all branches
  returned false); simplify to a single return false with explanatory
  comment; prefix param with _ to satisfy no-unused-vars
- handlePreFileUploadAsync: replace checkDummyUserByRocketChatUserIdAsync
  sender guard with standard app-user identity check (same pattern as
  handlePostMessageSentAsync)
- Remove stale imports: checkDummyUserByRocketChatUserIdAsync,
  retrieveDummyUserByRocketChatUserIdAsync

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Batch G - lib/InboundNotificationHelper.ts:
- Room creation member loop: drop retrieveDummyUserByTeamsUserIdAsync
  fallback; skip Teams-only members with no RC registration.
- SystemAddMembers handler: same.
- getSenderUser: fall back to getAppUser(appId) for unregistered senders;
  drop persis/modify/http from signature and call-site.
- Remove imports: syncAllTeamsBotUsersAsync, retrieveDummyUserByTeamsUserIdAsync.

Batch H - lib/UserInterfaceHelper.ts:
- Replace findAllDummyUsersInRocketChatUserListAsync filter with
  Promise.all(retrieveUserByRocketChatUserIdAsync) over room members.
- Remove import: findAllDummyUsersInRocketChatUserListAsync.
- Add import: retrieveUserByRocketChatUserIdAsync.

Batch I - slashcommands/AddUserSlashCommand.ts:
- Remove subcommand path guarded on retrieveDummyUserByRocketChatUserIdAsync;
  no dummy users in single-bot arch. All invocations open contextual bar.
- Remove imports: retrieveDummyUserByRocketChatUserIdAsync,
  AddUserNameInvalidHintMessageText.

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
lib/AppUserHelper.ts:
- Rename syncAllTeamsBotUsersAsync -> syncTeamsUserProfilesAsync.
- Strip createAppUserAsync (per-user dummy RC bot creation) and
  persistDummyUserAsync call; now only persists Teams user profiles
  for use by the Add Teams User contextual bar.
- Remove findAllDummyUsersInRocketChatUserListAsync (last caller was
  UserInterfaceHelper.ts, removed in batch H).
- Remove now-unused imports: IModify, IUser, UserType, IBotUser,
  TeamsAppUserNameSurfix, persistDummyUserAsync,
  retrieveDummyUserByRocketChatUserIdAsync, UserModel.

slashcommands/ProvisionTeamsBotUserSlashCommand.ts:
- Import and call syncTeamsUserProfilesAsync instead of the old
  syncAllTeamsBotUsersAsync; drop modify/appId args.
- Remove no-longer-used private appId field and app.getID() init.

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
…efix

Batch K - lib/PersistHelper.ts:
- Remove MiscKeys.DummyUser.
- Remove persistDummyUserAsync (no callers).
- Remove checkDummyUserByRocketChatUserIdAsync (no callers).
- Remove retrieveDummyUserByRocketChatUserIdAsync (no callers).
- Remove retrieveDummyUserByTeamsUserIdAsync (no callers).
- Add retrieveTeamsUserProfileByTeamsUserIdAsync: per-user lookup
  keyed by teamsUserId (USER association + TeamsUserProfile MISC key).

Batch L - lib/InboundNotificationHelper.ts:
- Import retrieveTeamsUserProfileByTeamsUserIdAsync.
- In handleInboundMessageCreatedAsync: when the sender has no RC
  registration (app-bot fallback), prefix the relayed message text
  with the Teams sender display name (**DisplayName:** text) so RC
  users can see who originally sent it.

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
…aces

lib/persistence/ (new directory, 13 entity files + index):
- Each file exports a namespace const object (e.g. MessageMapping,
  Room, UserMapping) with typed async methods scoped to one entity.
- Namespace method names follow the pattern:
    persist / findByX / findAll / delete / isBridged / setBridgeActive
  rather than the old verboseAsync suffix style.
- lib/persistence/index.ts re-exports all 13 namespace objects and
  all model types; no backward-compat flat function aliases.

lib/PersistHelper.ts:
- Thinned to a 5-line re-export: `export * from './persistence'`.
- All prior state (RoomModel.isBridged, MiscKeys, etc.) now lives
  inside the relevant entity files.

lib/PreventRegistry.ts:
- Removed import of MiscKeys from PersistHelper.
- Inlined PREVENT_REGISTRY_KEY constant.

Callers updated (17 files) — imports and call sites replaced with
namespace API throughout:
- lib/EventHandler.ts
- lib/InboundNotificationHelper.ts
- lib/AuthHelper.ts
- lib/AppUserHelper.ts
- lib/UserInterfaceHelper.ts
- lib/TeamsMessageParser.ts
- lib/RocketChatMessageParser.ts
- lib/MessageHelper.ts
- lib/MicrosoftGraphApi.ts
- TeamsBridgeApp.ts
- endpoints/AuthenticationEndpoint.ts
- endpoints/SubscriberEndpoint.ts
- slashcommands/SetupVerificationSlashCommand.ts
- slashcommands/ResubscriptionMessages.ts
- slashcommands/LogoutTeamsSlashCommand.ts

tsc --noEmit passes with zero errors after full migration.

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
…odule

- lib/EventHandler.ts: replaced 1233-line file with 11-line barrel re-export
- lib/handlers/: 11 new files, one per exported handler function;
  private helpers co-located with their handler
- lib/Notifier.ts: new module owning all 4 notification functions
  (notifyRocketChatUserAsync, notifyRocketChatUserInRoomAsync,
  generateHintMessageWithTeamsLoginButton, notifyNotLoggedInUserAsync)
- lib/MessageHelper.ts: removed 3 notification exports + trimmed
  unused imports (INotifier, IMessageAction, MessageActionType, LoginButtonText)
  515 -> 463 lines
- 8 caller files updated to import notifications from lib/Notifier
- tsc --noEmit passes clean
- docs/refactor-progress.md updated
…scheduler

- Remove jobs/InboundNotificationProcessor.ts; inline all three job processors
  as arrow function properties on TeamsBridgeApp, closing over
- lib/InboundNotificationHelper.ts: import type TeamsBridgeApp (breaks CommonJS
  circular dep cycle that caused 'not a constructor' crash)
- batch-o: split MicrosoftGraphApi into lib/graph/ (28 function files + barrel)
- docs/refactor-progress.md: updated commit history and TeamsBridgeApp status
…vel token

- Remove bridgeUserRocketChatUserId from RoomModel and Room.persist() (now 3 params)
- Add getAppAccessTokenAsync() to AuthHelper (client_credentials grant)
- handlePreMessageSentPrevent: strip election logic + findOneTeamsLoggedInUsersAsync
- handlePostMessageSent: sender token first, app token fallback; all threads via createChatThreadAsync
- handlePreFileUpload: sender token first, app token fallback
- handlePostMessageUpdated: else branch uses app token
- handleAddTeamsUserContextualBarSubmit: use app token directly
- handlePreRoomUserLeave: leaving user token first, app token fallback
- InboundNotificationHelper: dedup vs appUser.id; Room.persist 4th arg removed
- lib/graph/index.ts: remove createOneOnOneChatThreadAsync export
- docs/refactor-progress.md: record Batch P

tsc --noEmit passes clean.
- Add /teamsbridge-login-app-user slash command (admin-only) to initiate
  OAuth2 login for the app user, storing a delegated token under appUser.id

- Add AppUserLoginNotified persistence entity to rate-limit per-room
  notifications (once per day via isSetToday; cleared on app user login)

- Add notifyRoomMembersAppUserNotLoggedInAsync to Notifier: admins receive
  a login button scoped to appUser.id, non-admins get a plain 'ask admin' msg

- Fix handlePostMessageSent: remove getAppAccessTokenAsync fallback for
  message relay — Graph API POST /chats/{thread}/messages rejects app-level
  tokens; fallback is now the app user's own delegated token

- handlePostMessageSent: eliminate double Room.findByRCRoomId DB call by
  deriving isBridged from the single fetched record

- Notify room members when app user joins a bridged room without a token

- Clear AppUserLoginNotified flags for all rooms when app user logs in
  (AuthenticationEn
- Add /teamsbridge-login-app-user slash command (admin-only) to initiatpp   OAuth2 login for the app user, storing a delegated token under appUseid
- Add AppUserLoginNotified persistence entity to rate-limit per-room

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
- Add /teamsbridge-login-app-user slash command (admin-only)
- Add AppUserLoginNotified persistence entity (per-room, daily gate)
- Role-aware notifications when app user has no delegated token:
  admins get login button (state=appUser.id), non-admins get plain text
- Remove getAppAccessTokenAsync entirely from AuthHelper and all handlers;
  app-level (client_credentials) tokens are rejected by all Teams Graph
  API endpoints used here (chat messages, member mgmt, subscriptions)
- All token fallbacks replaced with getUserAccessTokenAsync(appUser.id)
- handlePostMessageSent: single Room.findByRCRoomId call (was double read);
  add modify param for notification helper
- handlePostRoomUserJoined: notify room on app user join if token missing;
  add modify param
- AuthenticationEndpoint: clear AppUserLoginNotified flags on app user login
- Update docs/refactor-progress.md

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
… QA fixes

- Split AuthenticationScopes into NormalUser/BotUser/ApplicationAuthenticationScopes
- OAuth2 state is now base64(JSON.stringify({rc_uid, type})); AuthenticationEndpoint decodes it
- getLoginUrl, getUserAccessTokenAsync, renewUserAccessTokenAsync accept userType param
- getAccessTokenForRegistration detects bot vs normal user, passes correct type
- getAppAccessTokenAsync re-added with persistence caching (AppToken.find/persist + expires)
- AppToken.ts: added expires field and find() method; SetupVerificationSlashCommand updated
- Inbound bot-fallback prefix: **Name:** text -> **Name** _via Teams_\ntext
- MessageHelper: forceBridgedMessage only triggers bridged format (removed originalSenderName condition)
- handlePostMessageSent: originalSenderName uses sender.name || sender.username; debug console.logs
- LoginAppUserSlashCommand: getLoginUrl with 'bot' userType
- LoginTeamsSlashCommand: getLoginUrl with 'normal' userType
…t handling

- searchTeamsUsersAsync (renamed from listTeamsUserProfilesAsync): query filter
  and nextLink pagination; SearchTeamsUsersResult interface
- getTeamsUserProfileByIdAsync: new file for single-profile live lookup
- TeamsUserProfile persistence deleted; no more DB cache for Teams profiles
- AppUserHelper.ts deleted; syncTeamsUserProfilesAsync no longer needed
- ProvisionTeamsBotUserSlashCommand.ts deleted (sole caller removed)
- UserInterfaceHelper: live search on bar open; ON_CHARACTER_ENTERED dispatch;
  Load More button with base64 LoadMoreButtonState; encodeUserOptionValue /
  decodeUserOptionValue (base64 JSON [id, displayName] in option values);
  updateAddTeamsUserContextualBarAsync; decodeButtonState exported
- addMemberToChatThreadAsync: AddMemberResult union (added/already_member/failed);
  GET filter pre-check before POST; user@odata.bind uses path syntax; no console.log
- handleAddTeamsUserContextualBarSubmit: decodes option values; buckets results;
  permanent msg for added, ephemeral for already-member/failed; all say on MS Teams
- TeamsBridgeApp: executeBlockActionHandler for SearchInput and LoadMore actions;
  passes http/persistence/app to contextual bar; passes modify to submit handler
- AddUserSlashCommand: constructor takes app; uses getAppUser(); passes http/persis/app
- InboundNotificationHelper: getTeamsUserProfileByIdAsync replaces TeamsUserProfile DB
- docs/refactor-progress.md: Batch R documented
- getTeamsChatMembersAsync: new Graph fn calling GET /chats/{id}/members;
  returns { members, nextLink? }; server-side pagination via @odata.nextLink
  (no $top/$count - not supported by this endpoint)
- ViewTeamsMembersSlashCommand: new /teamsbridge-view-members slash command
- Const: UIActionId.ViewTeamsMembersButtonClicked + ViewMembersLoadMore;
  UIElementId.ViewMembersContextualBarId; 6 new UIElementText constants
- UserInterfaceHelper: openViewTeamsMembersContextualBarAsync;
  updateViewTeamsMembersContextualBarAsync (Load More - appends next page);
  createViewMembersContextualBarBlocks - read-only member list with (N shown)
  header; Load More button when @odata.nextLink present;
  ViewMembersButtonState encode/decode
- TeamsBridgeApp: ViewTeamsMembersButtonClicked room action button registered;
  ViewTeamsMembersSlashCommand registered; executeActionButtonHandler handles
  ViewTeamsMembersButtonClicked; executeBlockActionHandler handles ViewMembersLoadMore
- i18n/en.json: view_teams_members_slash_command_description +
  action_button_label_view_teams_members
- docs/refactor-progress.md: Batch S documented
- CSRF: replace getLoginUrl with getLoginUrlAsync; generates randomBytes(16)
  nonce, persists via OAuthNonce, embeds in state JSON; endpoint verifies and
  consumes nonce on callback (one-time use)
- New lib/persistence/OAuthNonce.ts: persist / findAndDelete / deleteStale
- Stale nonce cleanup: recurring job every 600 s removes nonces older than
  10 minutes (OAuthNonceCleanupJobId, oauthNonceCleanupJob processor)
- Prevent duplicate Teams account binding: UserMapping.findByTeamsUserId check
  before writes; returns HTTP 409 if account already bound to a different RC
  user; distinct message if the existing owner is the app user
- refreshToken guard: explicit throw if token exchange omits refresh_token
- Runtime state schema validation: rc_uid / type checked before any network call
- AAD error logging: error + error_description query params logged on early exit
- Phased error handling: single catch split into three labelled try/catch blocks
  (state/settings · token exchange/profile · persistence/subscription)
- Per-value encodeURIComponent in getUserAccessTokenAsync and
  renewUserAccessTokenAsync (replace encodeURI on whole body)
- notifyNotLoggedInUserAsync gains persistence parameter for nonce storage
- New /teamsbridge-logout-app-user slash command (manage-apps permission):
  best-effort subscription deletion, then clears UserRegistration,
  UserMapping, LoginMessage, and AppUserLoginNotified for the app user.
- LogoutTeamsSlashCommand: removed revokeUserRefreshTokenAsync (not needed);
  always runs cleanup regardless of token presence; notification message
  chosen dynamically based on whether token was present.
- Const.ts: LogoutAppUserNoNeedHintMessageText / LogoutAppUserSuccessHintMessageText
- i18n/en.json: logout_app_user_slash_command_description
- docs/refactor-progress.md: updated commit table
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
…message

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
@abhinavkrin abhinavkrin changed the title Re arch feat: implement hybrid single-bot architecture and general bug and security fixes Mar 18, 2026
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
- Implemented consistent bridged-room check constraints for action buttons and slash commands.
- Simplified bot notification strings to use natural language self-references ('me').
- Intersected the /teamsbridge-setup-verification endpoint with a robust GET /me graph check on the App Bot User's delegated token to definitively verify relay capability.
- Added new utility verifyUserAccessTokenAsync in MicrosoftGraphApi.
- Added /teamsbridge-status command for active bridge state verification.
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
@abhinavkrin abhinavkrin merged commit 3681503 into main Apr 8, 2026
5 checks passed
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.

2 participants