Skip to content
This repository was archived by the owner on Oct 23, 2019. It is now read-only.

Record all signup attempts in DB #418

Closed
wants to merge 11 commits into from
1 change: 1 addition & 0 deletions misc/thentos-install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ set -o errexit
declare -a SOURCES
SUBMODULE_SOURCES=( servant/servant
servant/servant-server
servant/servant-cassava
servant/servant-client
servant/servant-docs
servant/servant-blaze
Expand Down
7 changes: 7 additions & 0 deletions thentos-core/schema/schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,10 @@ CREATE TABLE IF NOT EXISTS captchas (
solution text NOT NULL,
timestamp timestamptz NOT NULL DEFAULT now()
);

CREATE TABLE IF NOT EXISTS signup_attempts (
user_name text NOT NULL,
email text NOT NULL,
captcha_correct bool NOT NULL,
timestamp timestamptz NOT NULL
);
17 changes: 17 additions & 0 deletions thentos-core/src/Data/Csv/Missing.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{-# LANGUAGE OverloadedStrings #-}

{-# OPTIONS_GHC -fno-warn-orphans #-}

module Data.Csv.Missing where

import Data.Csv
import Control.Monad (mzero)

instance ToField Bool where
toField True = "1"
toField False = "0"

instance FromField Bool where
parseField "1" = pure True
parseField "0" = pure False
parseField _ = mzero
Copy link
Collaborator

Choose a reason for hiding this comment

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

16 changes: 13 additions & 3 deletions thentos-core/src/Thentos/Action.hs
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,13 @@ module Thentos.Action
, makeAudioCaptcha
, solveCaptcha
, deleteCaptcha
, getAllSignupAttempts

, collectGarbage
)
where

import Control.Conditional ((<||>), unlessM)
import Control.Conditional ((<||>))
import Control.Lens ((^.))
import Control.Monad (unless, void, when)
import Control.Monad.Except (throwError, catchError)
Expand Down Expand Up @@ -244,8 +245,10 @@ sendUserExistsMail email = do
-- captcha remains in the DB to allow another attempt. See also: 'makeCaptcha', 'confirmNewUser'.
addUnconfirmedUserWithCaptcha :: (Show e, Typeable e) => UserCreationRequest -> Action e s ()
addUnconfirmedUserWithCaptcha ucr = do
unlessM (solveCaptcha (csId $ ucCaptcha ucr) (csSolution $ ucCaptcha ucr)) $
throwError InvalidCaptchaSolution
captchaCorrect <- solveCaptcha (csId $ ucCaptcha ucr) (csSolution $ ucCaptcha ucr)
let user = ucUser ucr
recordSignupAttempt (udName user) (udEmail user) captchaCorrect
unless captchaCorrect $ throwError InvalidCaptchaSolution
addUnconfirmedUser (ucUser ucr)
deleteCaptcha . csId $ ucCaptcha ucr

Expand Down Expand Up @@ -737,6 +740,13 @@ solveCaptcha cid solution = do
deleteCaptcha :: CaptchaId -> Action e s ()
deleteCaptcha = query'P . T.deleteCaptcha

recordSignupAttempt :: UserName -> UserEmail -> Bool -> Action e s ()
recordSignupAttempt name email captchaCorrect =
query'P $ T.recordSignupAttempt name email captchaCorrect

getAllSignupAttempts :: Action e s [SignupAttempt]
getAllSignupAttempts = query'P $ T.getAllSignupAttempts

-- * garbage collection

collectGarbage :: Exception (ActionError e) => Action e s ()
Expand Down
5 changes: 5 additions & 0 deletions thentos-core/src/Thentos/Backend/Api/Docs/Common.hs
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,11 @@ instance ToSample ServiceSessionToken where

instance ToSample ByUserOrServiceId

instance ToSample SignupAttempt where
toSamples _ = Docs.singleSample $ SignupAttempt (UserName "UserName") email False ts
where ts = Timestamp $ read "2015-12-15 07:12:34 CET"
email = (\(Just e) -> e) $ parseUserEmail "alice@example.com"


instance HasDocs sublayout => HasDocs (ThentosAuth :> sublayout) where
docsFor _ dat opts = docsFor (Proxy :: Proxy sublayout) dat opts & Docs.apiIntros %~ (intro:)
Expand Down
3 changes: 3 additions & 0 deletions thentos-core/src/Thentos/Backend/Api/Simple.hs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import Network.Wai (Application)
import Servant.API ((:<|>)((:<|>)), (:>), Get, Post, Delete, Capture, ReqBody, JSON)
import Servant.Server (ServerT, Server, serve, enter)
import Servant.API.ResponseHeaders (Headers, addHeader)
import Servant.CSV.Cassava (CSV', DefaultEncodeOpts)
import System.Log.Logger (Priority(INFO))

import qualified Servant.Docs as Docs
Expand Down Expand Up @@ -87,6 +88,7 @@ type ThentosUser =
:<|> "activate" :> ReqBody '[JSON] (JsonTop ConfirmationToken)
:> Post '[JSON] (JsonTop ThentosSessionToken)
:<|> "login" :> ReqBody '[JSON] LoginFormData :> Post '[JSON] (JsonTop ThentosSessionToken)
:<|> "registration_attempts" :> Get '[JSON, (CSV', DefaultEncodeOpts)] [SignupAttempt]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the path so different from the name of the handler function?

:<|> Capture "uid" UserId :> Delete '[JSON] ()
:<|> Capture "uid" UserId :> "name" :> Get '[JSON] (JsonTop UserName)
:<|> Capture "uid" UserId :> "email" :> Get '[JSON] (JsonTop UserEmail)
Expand All @@ -100,6 +102,7 @@ thentosUser =
:<|> addUnconfirmedUserWithCaptcha
:<|> (JsonTop <$>) . (snd <$>) . confirmNewUser . fromJsonTop
:<|> (\(LoginFormData n p) -> JsonTop . snd <$> startThentosSessionByUserName n p)
:<|> getAllSignupAttempts
:<|> deleteUser
:<|> (JsonTop . ((^. userName) . snd) <$>) . lookupConfirmedUser
:<|> (JsonTop . ((^. userEmail) . snd) <$>) . lookupConfirmedUser
Expand Down
11 changes: 11 additions & 0 deletions thentos-core/src/Thentos/Transaction.hs
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,17 @@ garbageCollectCaptchas timeout = void $ execT
[sql| DELETE FROM captchas WHERE timestamp < now() - ?::interval; |] (Only timeout)


-- | store signup attempt in the db
recordSignupAttempt :: UserName -> UserEmail -> Bool -> ThentosQuery e ()
recordSignupAttempt name email captchaCorrect = void $ execT
[sql| INSERT INTO signup_attempts (user_name, email, captcha_correct, timestamp)
VALUES (?, ?, ?, now()) |] (name, email, captchaCorrect)

getAllSignupAttempts :: ThentosQuery e [SignupAttempt]
getAllSignupAttempts = map (\(n, e, cc, t) -> SignupAttempt n e cc t) <$>
queryT [sql| SELECT user_name, email, captcha_correct, timestamp
FROM signup_attempts |] ()

Copy link
Collaborator

Choose a reason for hiding this comment

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

missing empty line.

-- * helpers

-- | Throw an error from a situation which (we believe) will never arise.
Expand Down
55 changes: 54 additions & 1 deletion thentos-core/src/Thentos/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ module Thentos.Types
, ImageData(..)
, CaptchaId(..)
, CaptchaSolution(..)
, SignupAttempt(..)

, ThentosError(..)

Expand All @@ -84,7 +85,7 @@ where
import Control.Exception (Exception)
import Control.Lens (makeLenses)
import Control.Monad.Except (MonadError, throwError)
import Control.Monad (when, unless)
import Control.Monad (when, unless, mzero)
import Data.Aeson (FromJSON, ToJSON, Value(String), (.=))
import Data.Aeson.Types (Parser)
import Data.Attoparsec.ByteString.Char8 (parseOnly)
Expand All @@ -95,6 +96,8 @@ import Database.PostgreSQL.Simple.TypeInfo.Static (interval)
import Database.PostgreSQL.Simple.TypeInfo (typoid)
import Data.ByteString.Builder (doubleDec)
import Data.ByteString.Conversion (ToByteString)
import Data.Csv ((.:))
import Data.Csv.Missing ()
import Data.Char (isAlpha)
import Data.EitherR (fmapL)
import Data.Function (on)
Expand Down Expand Up @@ -122,6 +125,7 @@ import Web.HttpApiData (parseQueryParam)

import qualified Crypto.Scrypt as Scrypt
import qualified Data.Aeson as Aeson
import qualified Data.Csv as CSV
import qualified Data.HashMap.Strict as H
import qualified Data.ByteString as SBS
import qualified Data.Text as ST
Expand Down Expand Up @@ -187,6 +191,12 @@ newtype UserName = UserName { fromUserName :: ST }
deriving (Eq, Ord, Show, Read, FromJSON, ToJSON, Typeable, Generic, IsString, FromField,
ToField)

instance CSV.ToField UserName where
toField = CSV.toField . fromUserName

instance CSV.FromField UserName where
parseField = pure . UserName . cs

-- | BUG #399: ToJSON instance should go away in order to avoid accidental leakage of cleartext
-- passwords. but for the experimentation phase this is too much of a headache. (Under no
-- circumstances render to something like "[password hidden]". Causes a lot of confusion.)
Expand Down Expand Up @@ -214,6 +224,14 @@ instance FromField UserEmail where
instance ToField UserEmail where
toField = toField . fromUserEmail

instance CSV.ToField UserEmail where
toField = toByteString . userEmailAddress

instance CSV.FromField UserEmail where
parseField s = case parseUserEmail (cs s) of
Just e -> pure e
Nothing -> mzero

parseUserEmail :: ST -> Maybe UserEmail
parseUserEmail t = do
email <- emailAddress (cs t)
Expand Down Expand Up @@ -455,6 +473,12 @@ instance ToField Timestamp where
instance FromField Timestamp where
fromField f dat = Timestamp . toThyme <$> fromField f dat

instance CSV.ToField Timestamp where
toField = cs . timestampToString

instance CSV.FromField Timestamp where
parseField = timestampFromString . cs

newtype Timeout = Timeoutms { toMilliseconds :: Int }
deriving (Eq, Ord, Show)

Expand Down Expand Up @@ -731,6 +755,35 @@ data CaptchaSolution = CaptchaSolution
instance Aeson.FromJSON CaptchaSolution where parseJSON = Aeson.gparseJson
instance Aeson.ToJSON CaptchaSolution where toJSON = Aeson.gtoJson

data SignupAttempt = SignupAttempt UserName UserEmail Bool Timestamp
deriving (Show, Generic)

instance Aeson.ToJSON SignupAttempt where
toJSON = Aeson.gtoJson

instance Aeson.FromJSON SignupAttempt where
parseJSON = Aeson.gparseJson

instance CSV.ToNamedRecord SignupAttempt where
toNamedRecord (SignupAttempt name email captcha ts) =
CSV.namedRecord [
("name", CSV.toField name),
("email", CSV.toField email),
("captcha_solved", CSV.toField captcha),
("timestamp", CSV.toField ts)
]

instance CSV.FromNamedRecord SignupAttempt where
parseNamedRecord v = SignupAttempt <$>
v .: "name" <*>
v .: "email" <*>
v .: "captcha_solved" <*>
v .: "timestamp"

instance CSV.FromRecord SignupAttempt

instance CSV.DefaultOrdered SignupAttempt where
headerOrder _ = CSV.header ["name", "email", "captcha_solved", "timestamp"]

-- * errors

Expand Down
3 changes: 3 additions & 0 deletions thentos-core/thentos-core.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ library
cpp-options: -DDEVELOPMENT
exposed-modules:
Database.PostgreSQL.Simple.Missing
, Data.Csv.Missing
, LIO.Missing
, Paths.TH
, Paths_thentos_core__
Expand Down Expand Up @@ -100,6 +101,7 @@ library
, bytestring >=0.10.6.0 && <0.11
, bytestring-conversion >=0.3.1 && <0.4
, case-insensitive >=1.2.0.4 && <1.3
, cassava >= 0.4.4 && <0.5
, cond >=0.4 && <0.5
, configifier >=0.0.7 && <0.1
, containers >=0.5.6.2 && <0.6
Expand Down Expand Up @@ -146,6 +148,7 @@ library
, sandi >=0.3.5 && <0.4
, scrypt >=0.5.0 && <0.6
, servant ==0.5
, servant-cassava == 0.4.4.2
, servant-blaze ==0.5
, servant-docs ==0.5
, servant-foreign ==0.5
Expand Down
32 changes: 32 additions & 0 deletions thentos-tests/tests/Thentos/ActionSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,38 @@ spec_user = describe "user" $ do
void . runWithoutPrivs sta $ confirmUserEmailChange token
checkEmail uid $ (==) newEmail

describe "addUnconfirmedUserWithCaptcha" $ do
it "records signup attempts in the database" $ \sta -> do
let ActionState (conns, _, _) = sta
name = UserName "someName"
userData = UserFormData name "secret" (forceUserEmail "me@example.com")
cid = "myCaptchaId"
solution = "theSolution"
captchaSolution = CaptchaSolution cid solution
req = UserCreationRequest userData captchaSolution
Right () <- runVoidedQuery conns $ T.storeCaptcha cid solution
Copy link
Collaborator

Choose a reason for hiding this comment

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

you don't need to store the captcha solution: validity is not a requirement for this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I meant to write a test for each case (with and without valid captcha) to make sure we store the attempt either way.

Copy link
Collaborator

@fisx fisx Dec 16, 2015 via email

Choose a reason for hiding this comment

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

void . runPrivs ([] :: [Bool]) sta $ addUnconfirmedUserWithCaptcha req
[(name', captchaCorrect)] <- doQuery conns
[sql| SELECT user_name, captcha_correct
FROM signup_attempts |] ()
name `shouldBe` name'
captchaCorrect `shouldBe` True
Copy link
Collaborator

Choose a reason for hiding this comment

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

as above: different test?


it "records signup attempts in the database if the captcha is wrong" $ \sta -> do
let ActionState (conns, _, _) = sta
name = UserName "someName"
userData = UserFormData name "secret" (forceUserEmail "me@example.com")
cid = "myCaptchaId"
captchaSolution = CaptchaSolution cid "wrong answer"
req = UserCreationRequest userData captchaSolution
Right () <- runVoidedQuery conns $ T.storeCaptcha cid "right answer"
Left _ <- runPrivsE ([] :: [Bool]) sta $ addUnconfirmedUserWithCaptcha req
[(name', captchaCorrect)] <- doQuery conns
[sql| SELECT user_name, captcha_correct
FROM signup_attempts |] ()
name `shouldBe` name'
captchaCorrect `shouldBe` False

spec_service :: SpecWith ActionState
spec_service = describe "service" $ do
describe "addService, lookupService, deleteService" $ do
Expand Down
30 changes: 30 additions & 0 deletions thentos-tests/tests/Thentos/Backend/Api/SimpleSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ import Test.Hspec.Wai (shouldRespondWith, WaiSession, with, request, matchStatus

import qualified Data.Aeson as Aeson
import qualified Data.ByteString.Lazy as LBS
import qualified Data.Csv as CSV
import qualified Data.Text as ST
import qualified Data.Vector as V

import Thentos.Action.Core
import Thentos.Backend.Api.Simple (serveApi)
Expand Down Expand Up @@ -312,6 +314,31 @@ specRest = do
lrsp <- request "POST" "/user/login" jsonHeader $ Aeson.encode loginData
liftIO $ statusCode (simpleStatus lrsp) `shouldBe` 401

describe "registration_attempts" $ do
it "returns a json list of all registration attempts" $ do
(cid, solution) <- getCaptchaAndSolution
_ <- registerUserAndGetConfirmationToken (cid, solution)
rsp <- request "GET" "/user/registration_attempts" jsonHeader ""
liftIO $ statusCode (simpleStatus rsp) `shouldBe` 200
let body = simpleBody rsp
Just (signupAttempts :: [SignupAttempt]) = Aeson.decode body
[SignupAttempt name email captchaCorrect _] = signupAttempts
liftIO $ name `shouldBe` udName defaultUserData
liftIO $ email `shouldBe` udEmail defaultUserData
liftIO $ captchaCorrect `shouldBe` True

it "returns a csv list of all registration attempts" $ do
(cid, solution) <- getCaptchaAndSolution
_ <- registerUserAndGetConfirmationToken (cid, solution)
rsp <- request "GET" "/user/registration_attempts" csvHeader ""
liftIO $ statusCode (simpleStatus rsp) `shouldBe` 200
let body = simpleBody rsp
Right signupAttempts = CSV.decode CSV.HasHeader body
SignupAttempt name email captchaCorrect _ = V.head signupAttempts
liftIO $ name `shouldBe` udName defaultUserData
liftIO $ email `shouldBe` udEmail defaultUserData
liftIO $ captchaCorrect `shouldBe` True


describe "thentos_session" $ do
describe "ReqBody '[JSON] ThentosSessionToken :> Get Bool" $ do
Expand Down Expand Up @@ -344,6 +371,9 @@ postDefaultUser = do
jsonHeader :: [Header]
jsonHeader = [("Content-Type", "application/json")]

csvHeader :: [Header]
csvHeader = [("Content-Type", "application/json"), ("Accept", "text/csv")]

-- | God Headers plus content-type = json
ctHeader :: IO [Header]
ctHeader = (("Content-Type", "application/json") :) <$> readIORef godHeaders
Expand Down
2 changes: 2 additions & 0 deletions thentos-tests/thentos-tests.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ test-suite tests
, blaze-html
, bytestring
, case-insensitive >=1
, cassava >=0.4.4.0 && <0.5
, configifier >=0.0.7 && <0.1
, containers >=0.5.6.2 && <0.6
, cookie >=0.4.1.6 && <4.2
Expand Down Expand Up @@ -135,6 +136,7 @@ test-suite tests
, thyme >=0.3.5.5 && <0.4
, transformers >=0.4.2.0 && <0.5
, vault >=0.3.0.4 && <3.1
, vector >=0.10 && <0.11
, wai >=3.0.3.0 && <3.1
, wai-extra >=3.0.10 && <3.1
, wai-session >=0.3.2 && <0.4
Expand Down