Skip to content

Commit

Permalink
Make Akismet and Google Safe Browsing work
Browse files Browse the repository at this point in the history
And fix db ins race that can cause tx rollbacks.
Disable Akismet registr spam.
  • Loading branch information
kajmagnus committed May 4, 2019
1 parent 579ae61 commit 3e19981
Show file tree
Hide file tree
Showing 43 changed files with 1,996 additions and 447 deletions.
10 changes: 10 additions & 0 deletions app/controllers/DebugTestController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,16 @@ class DebugTestController @Inject()(cc: ControllerComponents, edContext: EdConte
}


def getE2eTestCounters: Action[Unit] = GetAction { _ =>
throwForbiddenIf(globals.isProd, "TyE0MAGIC", "Show me magic and I'll give you the numbers")
val responseJson = Json.obj(
"numReportedSpamFalsePositives" -> globals.e2eTestCounters.numReportedSpamFalsePositives,
"numReportedSpamFalseNegatives" -> globals.e2eTestCounters.numReportedSpamFalseNegatives,
)
Ok(responseJson.toString) as JSON
}


/** For performance tests. */
def pingExceptionAction: Action[Unit] = ExceptionAction(cc.parsers.empty) { _ =>
Ok("exception-action-pong")
Expand Down
16 changes: 13 additions & 3 deletions app/controllers/LoginAsGuestController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,19 @@ class LoginAsGuestController @Inject()(cc: ControllerComponents, edContext: EdCo
val theBrowserId: String = request.theBrowserIdData.idCookie getOrElse throwForbidden(
"TyE0BRIDGST", "Browser id missing when logging in as guest")

globals.spamChecker.detectRegistrationSpam(request, name = name, email = email) map {
isSpamReason =>
SpamChecker.throwForbiddenIfSpam(isSpamReason, "EdE5KJU3_")
val spamCheckTask = SpamCheckTask(
createdAt = globals.now(),
siteId = request.siteId,
postToSpamCheck = None,
who = request.whoOrUnknown,
requestStuff = request.spamRelatedStuff.copy(
userName = Some(name).noneIfBlank,
userEmail = Some(email).noneIfBlank,
userTrustLevel = None))

globals.spamChecker.detectRegistrationSpam(spamCheckTask) map {
spamCheckResults: SpamCheckResults =>
SpamChecker.throwForbiddenIfSpam(spamCheckResults, "EdE5KJU3_")

val loginAttempt = GuestLoginAttempt(
ip = request.ip,
Expand Down
18 changes: 14 additions & 4 deletions app/controllers/LoginWithOpenAuthController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class LoginWithOpenAuthController @Inject()(cc: ControllerComponents, edContext:
def conf: Configuration = globals.rawConf

private val cache = caffeine.cache.Caffeine.newBuilder()
.maximumSize(20*1000) // change to config value, e.g. 1e9 = 1GB mem cache. Default to 50M?
.maximumSize(20*1000) // change to config value, e.g. 1e9 = 1GB mem cache. Default to 50M? [ADJMEMUSG]
// Don't expire too quickly — the user needs time to choose & typ a username.
// SECURITY COULD expire sooner (say 10 seconds) if just logging in, because then
// the user need not think or type anything.
Expand Down Expand Up @@ -620,9 +620,19 @@ class LoginWithOpenAuthController @Inject()(cc: ControllerComponents, edContext:
if (ed.server.security.ReservedNames.isUsernameReserved(username)) // [5LKKWA10]
throwForbidden("EdE4SWWB9", s"Username is reserved: '$username'; choose another username")

globals.spamChecker.detectRegistrationSpam(request, name = username, email = emailAddress) map {
isSpamReason =>
SpamChecker.throwForbiddenIfSpam(isSpamReason, "EdE2KP89")
val spamCheckTask = SpamCheckTask(
createdAt = globals.now(),
siteId = request.siteId,
postToSpamCheck = None,
who = request.whoOrUnknown,
requestStuff = request.spamRelatedStuff.copy(
userName = Some(username),
userEmail = Some(emailAddress),
userTrustLevel = Some(TrustLevel.NewMember)))

globals.spamChecker.detectRegistrationSpam(spamCheckTask) map {
spamCheckResults: SpamCheckResults =>
SpamChecker.throwForbiddenIfSpam(spamCheckResults, "TyE2AKF067")

val becomeOwner = LoginController.shallBecomeOwner(request, emailAddress)

Expand Down
16 changes: 13 additions & 3 deletions app/controllers/LoginWithPasswordController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,19 @@ class LoginWithPasswordController @Inject()(cc: ControllerComponents, edContext:
if (ed.server.security.ReservedNames.isUsernameReserved(username)) // [5LKKWA10]
throwForbidden("EdE5PKW01", s"Username is reserved: '$username'; choose another username")

globals.spamChecker.detectRegistrationSpam(request, name = username, email = emailAddress) map {
isSpamReason =>
SpamChecker.throwForbiddenIfSpam(isSpamReason, "EdE7KVF2_")
val spamCheckTask = SpamCheckTask(
createdAt = globals.now(),
siteId = request.siteId,
postToSpamCheck = None,
who = request.whoOrUnknown,
requestStuff = request.spamRelatedStuff.copy(
userName = Some(username),
userEmail = Some(emailAddress),
userTrustLevel = Some(TrustLevel.NewMember)))

globals.spamChecker.detectRegistrationSpam(spamCheckTask) map {
spamCheckResults: SpamCheckResults =>
SpamChecker.throwForbiddenIfSpam(spamCheckResults, "TyEPWREGSPM_")

// Password strength tested in createPasswordUserCheckPasswordStrong() below.

Expand Down
16 changes: 14 additions & 2 deletions app/debiki/Globals.scala
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ object Globals {
}


class E2eTestCounters {
@volatile var numReportedSpamFalsePositives: Int = 0
@volatile var numReportedSpamFalseNegatives: Int = 0
}


class Globals(
private val appLoaderContext: p.ApplicationLoader.Context,
Expand All @@ -113,6 +118,8 @@ class Globals(

var edContext: EdContext = _

val e2eTestCounters = new E2eTestCounters

private implicit def execCtc: ExecutionContext = executionContext

val conf: p.Configuration = appLoaderContext.initialConfiguration
Expand Down Expand Up @@ -572,6 +579,9 @@ class Globals(

val anyPublicUploadsDir: Option[String] = anyUploadsDir.map(_ + "public/")

def originOfSiteId(siteId: SiteId): Option[String] =
systemDao.getSite(siteId).flatMap(_.canonicalHostname.map(originOf))

def originOf(site: Site): Option[String] = site.canonicalHostname.map(originOf)
def originOf(host: Hostname): String = originOf(host.hostname)
def originOf(hostOrHostname: String): String = {
Expand Down Expand Up @@ -1031,7 +1041,8 @@ class Globals(
elasticSearchClient, actorSystem, systemDao))

def spamCheckBatchSize: Int = conf.getInt("talkyard.spamcheck.batchSize") getOrElse 20
def spamCheckIntervalSeconds: Int = conf.getInt("talkyard.spamcheck.intervalSeconds") getOrElse 1
def spamCheckIntervalSeconds: Int = conf.getInt("talkyard.spamcheck.intervalSeconds").getOrElse(
if (isOrWasTest) 1 else 3)

val spamCheckActorRef: Option[ActorRef] =
if (isTestDisableBackgroundJobs) None
Expand All @@ -1046,8 +1057,9 @@ class Globals(
RenderContentService.startNewActor(outer, edContext.nashorn)

val spamChecker = new SpamChecker(
isDevTest = isOrWasTest, originOfSiteId,
executionContext, appLoaderContext.initialConfiguration, wsClient,
applicationVersion, new TextAndHtmlMaker("dummysiteid", edContext.nashorn))
new TextAndHtmlMaker("dummysiteid", edContext.nashorn))

spamChecker.start()

Expand Down
14 changes: 7 additions & 7 deletions app/debiki/TextAndHtml.scala
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ sealed trait TextAndHtml {
/** Raw ip addresses (ipv4 or 6) of any links that use raw ip addresses rather than
* domain names. If there is any, the post should probably be blocked as spam?
*/
def linkAddresses: immutable.Seq[String]
def linkIpAddresses: immutable.Seq[String]

def isTitle: Boolean

Expand Down Expand Up @@ -83,7 +83,7 @@ class TextAndHtmlMaker(pubSiteId: PublSiteId, nashorn: Nashorn) {
val usernameMentions: Set[String],
val links: immutable.Seq[String],
val linkDomains: immutable.Set[String],
val linkAddresses: immutable.Seq[String],
val linkIpAddresses: immutable.Seq[String],
val isTitle: Boolean,
val followLinks: Boolean,
val allowClassIdDataAttrs: Boolean) extends TextAndHtml {
Expand All @@ -102,7 +102,7 @@ class TextAndHtmlMaker(pubSiteId: PublSiteId, nashorn: Nashorn) {
usernameMentions = usernameMentions ++ more.usernameMentions,
(links.toSet ++ more.links.toSet).to[immutable.Seq],
linkDomains ++ more.linkDomains,
(linkAddresses.toSet ++ more.linkAddresses.toSet).to[immutable.Seq],
(linkIpAddresses.toSet ++ more.linkIpAddresses.toSet).to[immutable.Seq],
isTitle = isTitle && more.isTitle,
followLinks = followLinks,
allowClassIdDataAttrs = allowClassIdDataAttrs)
Expand Down Expand Up @@ -154,7 +154,7 @@ class TextAndHtmlMaker(pubSiteId: PublSiteId, nashorn: Nashorn) {
val safeHtml = nashorn.sanitizeHtml(text, followLinks = false)
new TextAndHtmlImpl(text, safeHtml, links = Nil, usernameMentions = Set.empty,
linkDomains = Set.empty,
linkAddresses = Nil, isTitle = true, followLinks = followLinks,
linkIpAddresses = Nil, isTitle = true, followLinks = followLinks,
allowClassIdDataAttrs = allowClassIdDataAttrs)
}
else {
Expand Down Expand Up @@ -192,7 +192,7 @@ class TextAndHtmlMaker(pubSiteId: PublSiteId, nashorn: Nashorn) {
}
new TextAndHtmlImpl(text, renderResult.safeHtml, usernameMentions = renderResult.mentions,
links = links, linkDomains = linkDomains,
linkAddresses = linkAddresses, isTitle = false, followLinks = followLinks,
linkIpAddresses = linkAddresses, isTitle = false, followLinks = followLinks,
allowClassIdDataAttrs = allowClassIdDataAttrs)
}
}
Expand All @@ -205,7 +205,7 @@ class TextAndHtmlMaker(pubSiteId: PublSiteId, nashorn: Nashorn) {
def test(text: String, isTitle: Boolean): TextAndHtml = {
dieIf(Globals.isProd, "EsE7GPM2")
new TextAndHtmlImpl(text, text, links = Nil, usernameMentions = Set.empty,
linkDomains = Set.empty, linkAddresses = Nil, isTitle = isTitle, followLinks = false,
linkDomains = Set.empty, linkIpAddresses = Nil, isTitle = isTitle, followLinks = false,
allowClassIdDataAttrs = false)
}

Expand All @@ -215,7 +215,7 @@ class TextAndHtmlMaker(pubSiteId: PublSiteId, nashorn: Nashorn) {
def wrapInParagraphNoMentionsOrLinks(text: String, isTitle: Boolean): TextAndHtml = {
new TextAndHtmlImpl(text, s"<p>$text</p>", usernameMentions = Set.empty,
links = Nil, linkDomains = Set.empty,
linkAddresses = Nil, isTitle = isTitle, followLinks = false,
linkIpAddresses = Nil, isTitle = isTitle, followLinks = false,
allowClassIdDataAttrs = false)
}

Expand Down
54 changes: 40 additions & 14 deletions app/debiki/dao/PagesDao.scala
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ trait PagesDao {
val groupIds = tx.loadGroupIdsMemberIdFirst(author)
val permissions = tx.loadPermsOnPages()
val authzCtx = ForumAuthzContext(Some(author), groupIds, permissions)
val settings = loadWholeSiteSettings(tx)

dieOrThrowNoUnless(Authz.mayCreatePage( // REFACTOR COULD pass a pageAuthzCtx instead [5FLK02]
authorAndLevels, groupIds,
Expand Down Expand Up @@ -250,17 +251,41 @@ trait PagesDao {
categoryId = anyCategoryId, embeddingUrl = None, publishDirectly = true,
hidden = approvedById.isEmpty) // [7AWU2R0]

val reviewTask = if (reviewReasons.isEmpty) None
else Some(ReviewTask(
id = tx.nextReviewTaskId(),
reasons = reviewReasons.to[immutable.Seq],
createdById = SystemUserId,
createdAt = now.toJavaDate,
createdAtRevNr = Some(bodyPost.currentRevisionNr),
maybeBadUserId = authorId,
pageId = Some(pageId),
postId = Some(bodyPost.id),
postNr = Some(bodyPost.nr)))
val anyReviewTask =
if (reviewReasons.isEmpty) None
else Some(ReviewTask(
id = tx.nextReviewTaskId(),
reasons = reviewReasons.to[immutable.Seq],
createdById = SystemUserId,
createdAt = now.toJavaDate,
createdAtRevNr = Some(bodyPost.currentRevisionNr),
maybeBadUserId = authorId,
pageId = Some(pageId),
postId = Some(bodyPost.id),
postNr = Some(bodyPost.nr)))

val anySpamCheckTask =
if (spamRelReqStuff.isEmpty || !globals.spamChecker.spamChecksEnabled) None
else {
// The uri is now sth like /-/create-page. Change to the path to the page
// we're creating.
val spamStuffPageUri = spamRelReqStuff.getOrDie("TyE2045MEQf").copy(uri = pagePath.value)
Some(
SpamCheckTask(
createdAt = globals.now(),
siteId = siteId,
postToSpamCheck = Some(PostToSpamCheck(
postId = bodyPost.id,
postNr = bodyPost.nr,
postRevNr = bodyPost.currentRevisionNr,
pageId = pageMeta.pageId,
pageType = pageMeta.pageType,
pagePublishedAt = When.fromDate(pageMeta.publishedAt getOrElse pageMeta.createdAt),
textToSpamCheck = bodyHtmlSanitized,
language = settings.languageCode)),
who = byWho,
requestStuff = spamStuffPageUri))
}

val auditLogEntry = AuditLogEntry(
siteId = siteId,
Expand Down Expand Up @@ -311,19 +336,20 @@ trait PagesDao {
}

// COULD generate notifications from here — currently done in the callers though.
reviewTask.foreach(tx.upsertReviewTask)

anyReviewTask.foreach(tx.upsertReviewTask)
anySpamCheckTask.foreach(tx.insertSpamCheckTask)
insertAuditLogEntry(auditLogEntry, tx)

tx.indexPostsSoon(titlePost, bodyPost)
spamRelReqStuff.foreach(tx.spamCheckPostsSoon(byWho, _, titlePost, bodyPost))

// Don't start rendering html for this page in the background. [5KWC58]
// (Instead, when the user requests the page, we'll render it directly in
// the request thread. Otherwise either 1) the request thread would have to wait
// for the background thread (which is too complicated) or 2) we'd generate
// the page twice, both in the request thread and in a background thread.)

(pagePath, bodyPost, reviewTask)
(pagePath, bodyPost, anyReviewTask)
}


Expand Down
Loading

0 comments on commit 3e19981

Please sign in to comment.