Skip to content

WPB-18929 fix: SCIM cannot re invite user if initial invitation was revoked or expired via teams UI - #5510

Merged
battermann merged 9 commits into
developfrom
WPB-18929-be-scim-cannot-re-invite-user-if-initial-invitation-was-revoked-or-expired-via-teams-ui
Sep 2, 2026
Merged

WPB-18929 fix: SCIM cannot re invite user if initial invitation was revoked or expired via teams UI#5510
battermann merged 9 commits into
developfrom
WPB-18929-be-scim-cannot-re-invite-user-if-initial-invitation-was-revoked-or-expired-via-teams-ui

Conversation

@battermann

@battermann battermann commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

https://wearezeta.atlassian.net/browse/WPB-18929

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@battermann
battermann marked this pull request as ready for review September 2, 2026 09:59
@battermann
battermann requested review from a team as code owners September 2, 2026 09:59
@battermann
battermann requested a lite review from Copilot September 2, 2026 09:59
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Sep 2, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new internal Spar delete endpoint needs stronger safety guards, and the changelog ticket ID/file naming appears inconsistent with the PR metadata.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR addresses a SCIM re-invitation failure by ensuring that revoking a pending SCIM invitation also removes the associated SCIM external-id/metadata (in Spar) and the pending Brig account synchronously, so a subsequent SCIM retry can succeed.

Changes:

  • Add an internal Spar endpoint to delete SCIM user metadata/external-id mappings for a given team/user.
  • Extend Brig invitation revocation to synchronously clean up pending SCIM users (Spar mapping + Brig account + pending activation state) before removing the invitation.
  • Add integration coverage for SCIM re-invite after revocation and update the subsystem RPC layer for the new Spar internal route.
File summaries
File Description
services/spar/src/Spar/Scim/User.hs Exposes deleteScimUserData helper to remove Spar SCIM metadata/external-id mapping derived from a Brig user.
services/spar/src/Spar/API.hs Adds internal route handler to delete SCIM user metadata via Brig lookup + deleteScimUserData.
libs/wire-api/src/Wire/API/Routes/Internal/Spar.hs Extends internal Spar Servant API with DELETE /i/scim/users/:team/:user.
libs/wire-subsystems/src/Wire/SparAPIAccess.hs Adds DeleteScimUser operation to the Spar RPC effect.
libs/wire-subsystems/src/Wire/SparAPIAccess/Rpc.hs Implements RPC call to Spar internal delete-SCIM-user endpoint.
libs/wire-subsystems/test/unit/Wire/MockInterpreters/SparAPIAccess.hs Updates mock interpreter pattern match to include DeleteScimUser.
services/brig/src/Brig/Team/API.hs Enhances invitation deletion to clean up pending SCIM accounts (including calling Spar internal deletion) before removing the invitation.
integration/test/API/Brig.hs Adds integration helper to call DELETE team invitation endpoint.
integration/test/Test/Spar.hs Adds integration test for SCIM re-invite after revocation.
changelog.d/3-bug-fixes/WPB-23177-scim-reinvite-after-revoke Documents the behavior change in release notes.
Review details
  • Files reviewed: 10/10 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread services/spar/src/Spar/API.hs
Comment thread changelog.d/3-bug-fixes/WPB-18929
Comment thread integration/test/Test/Spar.hs Outdated
Comment thread services/spar/src/Spar/API.hs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new Brig cleanup logic is currently gated on lookupInvitation returning Just, so it may not handle “expired invitation but pending SCIM account still exists” cases, which can still block reinvites.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 10/10 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread services/brig/src/Brig/Team/API.hs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The functional changes are cohesive and covered by a new integration test; the only remaining feedback is a minor comment-clarity nit with an inline fix suggestion.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

services/brig/src/Brig/Team/API.hs:272

  • The comment above API.deleteAccount is misleading: API.deleteAccount already deletes the team invitation (InvitationStore.deleteInvitation tid (userIdToInvitationId uid)), so it’s not “before the invitation is removed” — it’s part of the deletion. Updating the comment avoids confusion for future changes.
  • Files reviewed: 10/10 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@battermann
battermann merged commit ef740c6 into develop Sep 2, 2026
10 checks passed
@battermann
battermann deleted the WPB-18929-be-scim-cannot-re-invite-user-if-initial-invitation-was-revoked-or-expired-via-teams-ui branch September 2, 2026 11:29
battermann added a commit that referenced this pull request Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants