Skip to content
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.

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

Fix bug: handle lost after registration of scim-invited users #1303

Merged
merged 4 commits into from
Dec 31, 2020

Conversation

smatting
Copy link
Contributor

@smatting smatting commented Dec 28, 2020

This PR fixes the following bug:

  1. SCIM creates an invitation with a user handle
  2. The registers with the invitation. The registration function ignores the handle value from the invitation and thereby overwrites it with null in the users table. However the user and handle are still contained user_handle which leads to the issue described in https://wearezeta.atlassian.net/browse/SQSERVICES-158

This bug can be verified by an additional check in the integeration test for SCIM-invitations.

@smatting smatting requested a review from fisx December 28, 2020 17:29
Copy link
Contributor

@fisx fisx left a comment

Choose a reason for hiding this comment

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

Nice!

@@ -283,7 +283,7 @@ testCreateUserNoIdP = do
>>= maybe (error "could not find user in brig") pure
liftIO $ accountStatus brigUser `shouldBe` Active
liftIO $ userManagedBy (accountUser brigUser) `shouldBe` ManagedByScim

liftIO $ userHandle (accountUser brigUser) `shouldBe` Just handle
Copy link
Contributor

Choose a reason for hiding this comment

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

for the record, without the fix this fails as expected:

  test-integration/Test/Spar/Scim/UserSpec.hs:286:14: 
  1) Spar.Scim.User, POST /Users, team has no SAML IdP, creates a user with PendingInvitation, and user can follow usual invitation process
       expected: Just (Handle {fromHandle = "scimuser_8444630"})
        but got: Nothing

👍

services/brig/src/Brig/Data/User.hs Show resolved Hide resolved
@fisx
Copy link
Contributor

fisx commented Dec 29, 2020

Integration tests passed locally.

@jschaul jschaul force-pushed the SQSERVICES-158/bug-scim-handle-lost branch from 5962c26 to 5272cee Compare December 30, 2020 19:16
@fisx fisx merged commit 2b90807 into develop Dec 31, 2020
@fisx fisx deleted the SQSERVICES-158/bug-scim-handle-lost branch December 31, 2020 20:45
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.

None yet

3 participants