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

Check validity of notification IDs #3550

Merged
merged 4 commits into from
Sep 4, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions integration/test/SetupHelpers.hs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import Data.Aeson.Types qualified as Aeson
import Data.Default
import Data.Function
import Data.List qualified as List
import Data.UUID.V1 (nextUUID)
import Data.UUID.V4 (nextRandom)
import GHC.Stack
import Testlib.Prelude
Expand Down Expand Up @@ -83,8 +84,10 @@ resetFedConns owndom = do
Internal.deleteFedConn' owndom `mapM_` rdoms

randomId :: HasCallStack => App String
randomId = do
liftIO (show <$> nextRandom)
randomId = liftIO (show <$> nextRandom)

randomUUIDv1 :: HasCallStack => App String
randomUUIDv1 = liftIO (show . fromJust <$> nextUUID)

randomUserId :: (HasCallStack, MakesValue domain) => domain -> App Value
randomUserId domain = do
Expand Down
19 changes: 19 additions & 0 deletions integration/test/Test/Notifications.hs
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,22 @@ testLastNotification = do

lastNotif <- getLastNotification user "c" >>= getJSON 200
lastNotif %. "payload" `shouldMatch` [object ["client" .= "c"]]

testInvalidNotification :: HasCallStack => App ()
testInvalidNotification = do
user <- randomUserId OwnDomain
let client = "deadbeef"

-- test uuid v4 as "since"
do
notifId <- randomId
void $
getNotifications user client def {since = Just notifId}
>>= getJSON 400

-- test arbitrary uuid v1 as "since"
do
notifId <- randomUUIDv1
void $
getNotifications user client def {since = Just notifId}
>>= getJSON 404
9 changes: 9 additions & 0 deletions libs/wire-api/src/Wire/API/Notification.hs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

module Wire.API.Notification
( NotificationId,
isValidNotificationId,
RawNotificationId (..),
Event,

Expand All @@ -41,6 +42,7 @@ import Control.Lens (makeLenses, (.~))
import Control.Lens.Operators ((?~))
import Data.Aeson (FromJSON (..), ToJSON (..))
import Data.Aeson.Types qualified as Aeson
import Data.Bits
import Data.HashMap.Strict.InsOrd qualified as InsOrdHashMap
import Data.Id
import Data.Json.Util
Expand All @@ -50,6 +52,7 @@ import Data.Schema
import Data.Swagger (ToParamSchema (..))
import Data.Swagger qualified as S
import Data.Time.Clock (UTCTime)
import Data.UUID qualified as UUID
import Imports
import Servant
import Wire.API.Routes.MultiVerb
Expand Down Expand Up @@ -80,6 +83,12 @@ eventSchema = mkSchema sdoc Aeson.parseJSON (Just . Aeson.toJSON)
)
]

isValidNotificationId :: NotificationId -> Bool
isValidNotificationId (Id uuid) =
-- check that the version bits are set to 1
case UUID.toWords uuid of
(_, w, _, _) -> (w `shiftR` 12) .&. 0xf == 1

--------------------------------------------------------------------------------
-- QueuedNotification

Expand Down
6 changes: 3 additions & 3 deletions services/gundeck/src/Gundeck/Monad.hs
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ where
import Bilge hiding (Request, header, options, statusCode)
import Bilge.RPC
import Cassandra
import Control.Error hiding (err)
import Control.Error
import Control.Exception (throwIO)
import Control.Lens hiding ((.=))
import Control.Lens
import Control.Monad.Catch hiding (tryJust)
import Data.Aeson (FromJSON)
import Data.Default (def)
Expand All @@ -61,7 +61,7 @@ import Network.Wai
import Network.Wai.Utilities
import System.Logger qualified as Log
import System.Logger qualified as Logger
import System.Logger.Class hiding (Error, info)
import System.Logger.Class
import UnliftIO (async)

-- | TODO: 'Client' already has an 'Env'. Why do we need two? How does this even work? We should
Expand Down
11 changes: 11 additions & 0 deletions services/gundeck/src/Gundeck/Notification.hs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import Bilge.IO hiding (options)
import Bilge.Request
import Bilge.Response
import Control.Lens (view)
import Control.Monad.Catch
import Control.Monad.Except
import Data.ByteString.Conversion
import Data.Id
import Data.Misc (Milliseconds (..))
Expand All @@ -35,10 +37,13 @@ import Gundeck.Monad
import Gundeck.Notification.Data qualified as Data
import Gundeck.Options hiding (host, port)
import Imports hiding (getLast)
import Network.HTTP.Types hiding (statusCode)
import Network.Wai.Utilities.Error
import System.Logger.Class
import System.Logger.Class qualified as Log
import Util.Options hiding (host, port)
import Wire.API.Internal.Notification
import Wire.API.Notification

data PaginateResult = PaginateResult
{ paginateResultGap :: Bool,
Expand All @@ -47,6 +52,7 @@ data PaginateResult = PaginateResult

paginate :: UserId -> Maybe NotificationId -> Maybe ClientId -> Range 100 10000 Int32 -> Gundeck PaginateResult
paginate uid since mclt size = do
traverse_ validateNotificationId since
for_ mclt $ \clt -> updateActivity uid clt

time <- posixTime
Expand All @@ -60,6 +66,11 @@ paginate uid since mclt size = do
(Just (millisToUTC time))
millisToUTC = posixSecondsToUTCTime . fromIntegral . (`div` 1000) . ms

validateNotificationId :: NotificationId -> Gundeck ()
validateNotificationId n =
unless (isValidNotificationId n) $
throwM (mkError status400 "bad-request" "Invalid Notification ID")

-- | Update last_active property of the given client by making a request to brig.
updateActivity :: UserId -> ClientId -> Gundeck ()
updateActivity uid clt = do
Expand Down
Loading