Skip to content

Commit 1137877

Browse files
committed
Do not evict cookies on suspend.
Cookies can't be used by suspended accounts for refreshing access tokens (see last commit), so this is safe.
1 parent 80aefe4 commit 1137877

4 files changed

Lines changed: 13 additions & 32 deletions

File tree

services/brig/src/Brig/API/Auth.hs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ import Wire.EmailSubsystem (EmailSubsystem)
6161
import Wire.Error (HttpError (..))
6262
import Wire.Events (Events)
6363
import Wire.GalleyAPIAccess
64-
import Wire.Sem.Concurrency
6564
import Wire.Sem.Metrics (Metrics)
6665
import Wire.Sem.Now (Now)
6766
import Wire.Sem.Random (Random)
@@ -82,7 +81,6 @@ accessH ::
8281
Member (Embed IO) r,
8382
Member Metrics r,
8483
Member SessionStore r,
85-
Member (Concurrency Unsafe) r,
8684
Member CryptoSign r,
8785
Member Now r,
8886
Member AuthenticationSubsystem r,
@@ -111,7 +109,6 @@ access ::
111109
Member (Embed IO) r,
112110
Member Metrics r,
113111
Member SessionStore r,
114-
Member (Concurrency Unsafe) r,
115112
Member CryptoSign r,
116113
Member Now r,
117114
Member AuthenticationSubsystem r,
@@ -142,7 +139,6 @@ login ::
142139
Member ActivationCodeStore r,
143140
Member AuthenticationSubsystem r,
144141
Member (Input AuthenticationSubsystemConfig) r,
145-
Member (Concurrency Unsafe) r,
146142
Member Now r,
147143
Member CryptoSign r,
148144
Member Random r
@@ -246,7 +242,6 @@ legalHoldLogin ::
246242
Member Events r,
247243
Member AuthenticationSubsystem r,
248244
Member (Input AuthenticationSubsystemConfig) r,
249-
Member (Concurrency Unsafe) r,
250245
Member Now r,
251246
Member CryptoSign r,
252247
Member Random r,
@@ -265,7 +260,6 @@ ssoLogin ::
265260
Member UserSubsystem r,
266261
Member Events r,
267262
Member (Input AuthenticationSubsystemConfig) r,
268-
Member (Concurrency Unsafe) r,
269263
Member Now r,
270264
Member CryptoSign r,
271265
Member Random r,

services/brig/src/Brig/API/Internal.hs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,6 @@ accountAPI ::
254254
Member RateLimit r,
255255
Member SparAPIAccess r,
256256
Member EnterpriseLoginSubsystem r,
257-
Member (Concurrency Unsafe) r,
258257
Member ClientStore r,
259258
Member ClientSubsystem r
260259
) =>
@@ -316,8 +315,7 @@ teamsAPI ::
316315
Member (Polysemy.Error UserSubsystemError) r,
317316
Member Events r,
318317
Member (Input (Local ())) r,
319-
Member IndexedUserStore r,
320-
Member AuthenticationSubsystem r
318+
Member IndexedUserStore r
321319
) =>
322320
ServerT BrigIRoutes.TeamsAPI (Handler r)
323321
teamsAPI =
@@ -347,7 +345,6 @@ authAPI ::
347345
Member UserSubsystem r,
348346
Member AuthenticationSubsystem r,
349347
Member (Input AuthenticationSubsystemConfig) r,
350-
Member (Concurrency Unsafe) r,
351348
Member Now r,
352349
Member CryptoSign r,
353350
Member Random r,
@@ -785,8 +782,6 @@ getPasswordResetCode email =
785782
changeAccountStatusH ::
786783
( Member UserSubsystem r,
787784
Member Events r,
788-
Member (Concurrency Unsafe) r,
789-
Member AuthenticationSubsystem r,
790785
Member UserStore r
791786
) =>
792787
UserId ->

services/brig/src/Brig/API/User.hs

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ import Data.Json.Util
9090
import Data.LegalHold (UserLegalHoldStatus (..), defUserLegalHoldStatus)
9191
import Data.List.Extra
9292
import Data.List.NonEmpty (NonEmpty)
93-
import Data.List.NonEmpty qualified as NonEmpty
9493
import Data.Misc
9594
import Data.Qualified
9695
import Data.Range
@@ -627,14 +626,19 @@ changeAccountStatus ::
627626
( Member (Concurrency 'Unsafe) r,
628627
Member UserSubsystem r,
629628
Member Events r,
630-
Member AuthenticationSubsystem r,
631629
Member UserStore r
632630
) =>
633631
NonEmpty UserId ->
634632
AccountStatus ->
635633
ExceptT AccountStatusError (AppT r) ()
636634
changeAccountStatus usrs status = do
637-
ev <- mkUserEvent usrs status
635+
-- It is safe to not revoke any cookies here; if no valid access
636+
-- token is available, cookies are only validated when calling `POST
637+
-- /access`, and access token refresh only works on unsuspended
638+
-- users.
639+
--
640+
-- Evidence: `git grep -Hn --color=never 'UserToken\b' | grep libs/wire-api/src/Wire/API/Routes/Public/`.
641+
ev <- mkUserEvent status
638642
lift $ liftSem $ unsafePooledMapConcurrentlyN_ 16 (update ev) usrs
639643
where
640644
update ::
@@ -649,35 +653,27 @@ changeAccountStatus usrs status = do
649653
changeSingleAccountStatus ::
650654
( Member UserSubsystem r,
651655
Member Events r,
652-
Member (Concurrency Unsafe) r,
653-
Member AuthenticationSubsystem r,
654656
Member UserStore r
655657
) =>
656658
UserId ->
657659
AccountStatus ->
658660
ExceptT AccountStatusError (AppT r) ()
659661
changeSingleAccountStatus uid status = do
660662
unlessM (lift . liftSem $ UserStore.doesUserExist uid) $ throwE AccountNotFound
661-
ev <- mkUserEvent (NonEmpty.singleton uid) status
663+
ev <- mkUserEvent status
662664
lift . liftSem $ do
663665
UserStore.updateAccountStatus uid status
664666
User.internalUpdateSearchIndex uid
665667
Events.generateUserEvent uid Nothing (ev uid)
666668

667669
mkUserEvent ::
668-
( Traversable t,
669-
Member (Concurrency Unsafe) r,
670-
Member AuthenticationSubsystem r
671-
) =>
672-
t UserId ->
670+
(Monad m) =>
673671
AccountStatus ->
674-
ExceptT AccountStatusError (AppT r) (UserId -> UserEvent)
675-
mkUserEvent usrs status =
672+
ExceptT AccountStatusError m (UserId -> UserEvent)
673+
mkUserEvent status =
676674
case status of
677675
Active -> pure UserResumed
678-
Suspended -> do
679-
lift $ liftSem (unsafePooledMapConcurrentlyN_ 16 Auth.revokeAllCookies usrs)
680-
pure UserSuspended
676+
Suspended -> pure UserSuspended
681677
Deleted -> throwE InvalidAccountStatus
682678
Ephemeral -> throwE InvalidAccountStatus
683679
PendingInvitation -> throwE InvalidAccountStatus

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ import Wire.API.Team.Member qualified as Teams
6969
import Wire.API.Team.Permission (Perm (AddTeamMember))
7070
import Wire.API.Team.Size
7171
import Wire.API.User hiding (fromEmail)
72-
import Wire.AuthenticationSubsystem
7372
import Wire.BlockListStore
7473
import Wire.EmailSubsystem.Interpreter (renderInvitationUrl)
7574
import Wire.Error
@@ -373,7 +372,6 @@ suspendTeam ::
373372
Member Events r,
374373
Member TinyLog r,
375374
Member InvitationStore r,
376-
Member AuthenticationSubsystem r,
377375
Member UserStore r
378376
) =>
379377
TeamId ->
@@ -394,7 +392,6 @@ unsuspendTeam ::
394392
Member UserSubsystem r,
395393
Member TeamSubsystem r,
396394
Member Events r,
397-
Member AuthenticationSubsystem r,
398395
Member UserStore r
399396
) =>
400397
TeamId ->
@@ -413,7 +410,6 @@ changeTeamAccountStatuses ::
413410
Member TeamSubsystem r,
414411
Member UserSubsystem r,
415412
Member Events r,
416-
Member AuthenticationSubsystem r,
417413
Member UserStore r
418414
) =>
419415
TeamId ->

0 commit comments

Comments
 (0)