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

[WPB-11122] Disallow searching user by old email #4260

Merged
merged 17 commits into from
Sep 23, 2024

Conversation

fisx
Copy link
Contributor

@fisx fisx commented Sep 20, 2024

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

Checklist

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

@echoes-hq echoes-hq bot added echoes: technical-roadmap/security More specific category, to highlight task that tackle security requirements. echoes: technical-roadmap/technical-debt More specific category, to highlight Technical Debt being tackled. labels Sep 20, 2024
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Sep 20, 2024
@fisx
Copy link
Contributor Author

fisx commented Sep 20, 2024

some tracing through relevant code:

$ make ci-safe package=integration TEST_INCLUDE=testDeleteEmail
[...]
[brig@example.com] **************************================================== 4b38d835-f651-4177-95eb-7120ba8ea9b0 
[brig@example.com] IndexUser {userId = 4b38d835-f651-4177-95eb-7120ba8ea9b0, teamId = Just (WithWriteTime {value = b83c2a0f-85bf-4b63-96c1-a34a407f2e14, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:00.065 UTC}}), name = WithWriteTime {value = Name {fromName = "y6sZcF9wbi@example.com"}, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:00.065 UTC}}, accountStatus = Just (WithWriteTime {value = Active, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:00.065 UTC}}), handle = Nothing, email = Just (WithWriteTime {value = "y6sZcF9wbi@example.com", writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:00.099 UTC}}), colourId = WithWriteTime {value = ColourId {fromColourId = 0}, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:00.065 UTC}}, activated = WithWriteTime {value = True, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:00.099 UTC}}, serviceId = Nothing, managedBy = Just (WithWriteTime {value = ManagedByWire, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:00.065 UTC}}), ssoId = Nothing, unverifiedEmail = Nothing}
[brig@example.com] ExternalGT (ExternalDocVersion (DocVersion {docVersionNumber = 1726831140099000}))
[brig@example.com] **************************================================== 2829d0ab-0343-48a6-ae1b-58b6365b190d 
[brig@example.com] IndexUser {userId = 2829d0ab-0343-48a6-ae1b-58b6365b190d, teamId = Just (WithWriteTime {value = b83c2a0f-85bf-4b63-96c1-a34a407f2e14, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.026 UTC}}), name = WithWriteTime {value = Name {fromName = "NGA9ZxUbbKmSL@example.com"}, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.026 UTC}}, accountStatus = Just (WithWriteTime {value = Active, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.026 UTC}}), handle = Nothing, email = Just (WithWriteTime {value = "NGA9ZxUbbKmSL@example.com", writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.047 UTC}}), colourId = WithWriteTime {value = ColourId {fromColourId = 0}, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.026 UTC}}, activated = WithWriteTime {value = True, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.047 UTC}}, serviceId = Nothing, managedBy = Just (WithWriteTime {value = ManagedByWire, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.026 UTC}}), ssoId = Nothing, unverifiedEmail = Nothing}
[brig@example.com] ExternalGT (ExternalDocVersion (DocVersion {docVersionNumber = 1726831141047000}))
[brig@example.com] **************************================================== 2829d0ab-0343-48a6-ae1b-58b6365b190d 
[brig@example.com] IndexUser {userId = 2829d0ab-0343-48a6-ae1b-58b6365b190d, teamId = Just (WithWriteTime {value = b83c2a0f-85bf-4b63-96c1-a34a407f2e14, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.026 UTC}}), name = WithWriteTime {value = Name {fromName = "NGA9ZxUbbKmSL@example.com"}, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.026 UTC}}, accountStatus = Just (WithWriteTime {value = Active, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.026 UTC}}), handle = Just (WithWriteTime {value = Handle {fromHandle = "2829d0ab-0343-48a6-ae1b-58b6365b190d"}, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.549001 UTC}}), email = Just (WithWriteTime {value = "NGA9ZxUbbKmSL@example.com", writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.047 UTC}}), colourId = WithWriteTime {value = ColourId {fromColourId = 0}, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.026 UTC}}, activated = WithWriteTime {value = True, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.047 UTC}}, serviceId = Nothing, managedBy = Just (WithWriteTime {value = ManagedByWire, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.026 UTC}}), ssoId = Nothing, unverifiedEmail = Nothing}
[brig@example.com] ExternalGT (ExternalDocVersion (DocVersion {docVersionNumber = 1726831141549001}))
[brig@example.com] **************************================================== 2829d0ab-0343-48a6-ae1b-58b6365b190d 
[brig@example.com] IndexUser {userId = 2829d0ab-0343-48a6-ae1b-58b6365b190d, teamId = Just (WithWriteTime {value = b83c2a0f-85bf-4b63-96c1-a34a407f2e14, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.026 UTC}}), name = WithWriteTime {value = Name {fromName = "NGA9ZxUbbKmSL@example.com"}, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.026 UTC}}, accountStatus = Just (WithWriteTime {value = Active, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.026 UTC}}), handle = Just (WithWriteTime {value = Handle {fromHandle = "2829d0ab-0343-48a6-ae1b-58b6365b190d"}, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.549001 UTC}}), email = Just (WithWriteTime {value = "NGA9ZxUbbKmSL@example.com", writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.047 UTC}}), colourId = WithWriteTime {value = ColourId {fromColourId = 0}, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.026 UTC}}, activated = WithWriteTime {value = True, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.047 UTC}}, serviceId = Nothing, managedBy = Just (WithWriteTime {value = ManagedByScim, writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.585 UTC}}), ssoId = Just (WithWriteTime {value = UserSSOId (UserRef {_uidTenant = Issuer {_fromIssuer = URI {uriScheme = Scheme {schemeBS = "https"}, uriAuthority = Just (Authority {authorityUserInfo = Nothing, authorityHost = Host {hostBS = "issuer.net"}, authorityPort = Nothing}), uriPath = "/_3031f321-9829-44ca-b3c1-44e633f9e623", uriQuery = Query {queryPairs = []}, uriFragment = Nothing}}, _uidSubject = NameID {_nameID = UNameIDEmail (Email {fromEmail = "NGA9ZxUbbKmSL@example.com"}), _nameIDNameQ = Nothing, _nameIDSPNameQ = Nothing, _nameIDSPProvidedID = Nothing}}), writetime = Writetime {writetimeToUTC = 2024-09-20 11:19:01.592 UTC}}), unverifiedEmail = Nothing}
[brig@example.com] ExternalGT (ExternalDocVersion (DocVersion {docVersionNumber = 1726831141592000}))
----- Brig.testDeleteEmail FAIL (1.66 s) -----
assertion failure:
Actual:
1
Expected:
0

call stack: 
1. assertFailure at test/Testlib/Assertions.hs:85
     assertFailure $ (maybe "" (<> "\n") msg) <> "Actual:\n" <> pa <> "\nExpected:\n" <> pb <> diff

2. shouldMatchWithMsg at test/Testlib/Assertions.hs:63
     shouldMatch = shouldMatchWithMsg Nothing

3. shouldMatch at test/Testlib/Assertions.hs:181
     shouldMatchInt = shouldMatch

4. shouldMatchInt at test/Test/Brig.hs:257
     "empty" -> numDocs `shouldMatchInt` 0

5. searchShouldBe at test/Test/Brig.hs:264
     searchShouldBe "empty"



request: 
GET http://127.0.0.1:8082/v7/search/contacts?q=NGA9ZxUbbKmSL%40example.com&domain=example.com
request headers: 
Z-Connection: conn
Z-User: 4b38d835-f651-4177-95eb-7120ba8ea9b0
request body:

response status: 200
response body:
{
    "documents": [
        {
            "accent_id": 0,
            "handle": "2829d0ab-0343-48a6-ae1b-58b6365b190d",
            "id": "2829d0ab-0343-48a6-ae1b-58b6365b190d",
            "name": "NGA9ZxUbbKmSL@example.com",
            "qualified_id": {
                "domain": "example.com",
                "id": "2829d0ab-0343-48a6-ae1b-58b6365b190d"
            },
            "team": "b83c2a0f-85bf-4b63-96c1-a34a407f2e14"
        }
    ],
    "found": 1,
    "returned": 1,
    "search_policy": "full_search",
    "took": 2
}

@fisx
Copy link
Contributor Author

fisx commented Sep 20, 2024

yeah, this is the problem:

cqlsh> select id, email, writetime(email) from brig_test.user where id=83c14585-61f9-4f61-bbfc-5dc3a6f466c8;

 id                                   | email                | writetime(email)
--------------------------------------+----------------------+------------------
 83c14585-61f9-4f61-bbfc-5dc3a6f466c8 | tAkYzM3U@example.com | 1726840408509000

(1 rows)
cqlsh> select id, email, writetime(email) from brig_test.user where id=83c14585-61f9-4f61-bbfc-5dc3a6f466c8;

 id                                   | email                | writetime(email)
--------------------------------------+----------------------+------------------
 83c14585-61f9-4f61-bbfc-5dc3a6f466c8 | tAkYzM3U@example.com | 1726840408509000

(1 rows)
cqlsh> update brig_test.user set email = NULL where id=83c14585-61f9-4f61-bbfc-5dc3a6f466c8;
cqlsh> select id, email, writetime(email) from brig_test.user where id=83c14585-61f9-4f61-bbfc-5dc3a6f466c8;

 id                                   | email | writetime(email)
--------------------------------------+-------+------------------
 83c14585-61f9-4f61-bbfc-5dc3a6f466c8 |  null |             null

(1 rows)

this affects fields email, email_unvalidated, but others may need to be added later; sso_id is a good candidate, even though you can't set this field to NULL as of today.

what about adding an integer field serialno :: int that is bumped every time we NULL a field and want re-index to trigger? this would only affect the indexing code, and the broken update code (two places right now).

@battermann battermann marked this pull request as ready for review September 20, 2024 15:55
@fisx fisx merged commit beefca5 into develop Sep 23, 2024
10 checks passed
@fisx fisx deleted the WPB-11122-disallow-searching-user-by-old-email branch September 23, 2024 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
echoes: technical-roadmap/security More specific category, to highlight task that tackle security requirements. echoes: technical-roadmap/technical-debt More specific category, to highlight Technical Debt being tackled. 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.

3 participants