From b2ed4247dfa0b7275878a3e2875c55b888a81443 Mon Sep 17 00:00:00 2001 From: Dominik Picheta Date: Sun, 20 May 2018 17:30:39 +0100 Subject: [PATCH] Implements password resets. --- src/auth.nim | 6 +-- src/email.nim | 12 +++-- src/forum.nim | 94 +++++++++++++++------------------- src/frontend/error.nim | 10 ++++ src/frontend/forum.nim | 37 ++++++++++++- src/frontend/karaxutils.nim | 28 ++++++---- src/frontend/newthread.nim | 2 +- src/frontend/postbutton.nim | 2 +- src/frontend/resetpassword.nim | 65 +++++++++++++++++++++++ src/utils.nim | 4 +- 10 files changed, 184 insertions(+), 76 deletions(-) create mode 100644 src/frontend/resetpassword.nim diff --git a/src/auth.nim b/src/auth.nim index 72df257..21588a4 100644 --- a/src/auth.nim +++ b/src/auth.nim @@ -45,7 +45,7 @@ proc makePassword*(password, salt: string, comparingTo = ""): string = let bcryptSalt = if comparingTo != "": comparingTo else: genSalt(8) result = hash(getMD5(salt & getMD5(password)), bcryptSalt) -proc makeIdentHash*(user, password, epoch, secret: string, +proc makeIdentHash*(user, password: string, epoch: int64, secret: string, comparingTo = ""): string = ## Creates a hash verifying the identity of a user. Used for password reset ## links and email activation links. @@ -53,7 +53,7 @@ proc makeIdentHash*(user, password, epoch, secret: string, ## the link is invalid. ## The ``secret`` is the 'salt' field in the ``person`` table. when defined(windows): - result = getMD5(user & password & epoch & secret) + result = getMD5(user & password & $epoch & secret) else: let bcryptSalt = if comparingTo != "": comparingTo else: genSalt(8) - result = hash(user & password & epoch & secret, bcryptSalt) \ No newline at end of file + result = hash(user & password & $epoch & secret, bcryptSalt) \ No newline at end of file diff --git a/src/email.nim b/src/email.nim index e2e8973..60b4527 100644 --- a/src/email.nim +++ b/src/email.nim @@ -1,4 +1,4 @@ -import asyncdispatch, smtp, strutils, times, cgi, tables +import asyncdispatch, smtp, strutils, times, cgi, tables, logging from jester import Request, makeUri @@ -39,7 +39,7 @@ proc sendMail( raise newForumError(msg) if mailer.config.smtpAddress.len == 0: - echo("[WARNING] Cannot send mail: no smtp server configured (smtpAddress).") + warn("Cannot send mail: no smtp server configured (smtpAddress).") return var client = newAsyncSmtp() @@ -101,7 +101,7 @@ proc sendSecureEmail*( kind: SecureEmailKind, req: Request, name, password, email, salt: string ) {.async.} = - let epoch = $int(epochTime()) + let epoch = int(epochTime()) let path = case kind @@ -114,11 +114,13 @@ proc sendSecureEmail*( [ path, encodeUrl(name), - encodeUrl(epoch), + encodeUrl($epoch), encodeUrl(makeIdentHash(name, password, epoch, salt)) ] ) + debug(url) + let emailSentFut = case kind of ActivateEmail: @@ -127,7 +129,7 @@ proc sendSecureEmail*( sendPassReset(mailer, email, name, url) yield emailSentFut if emailSentFut.failed: - echo("[WARNING] Couldn't send email: ", emailSentFut.error.msg) + warn("Couldn't send email: ", emailSentFut.error.msg) if emailSentFut.error of ForumError: raise emailSentFut.error else: diff --git a/src/forum.nim b/src/forum.nim index 36f83fb..805102f 100644 --- a/src/forum.nim +++ b/src/forum.nim @@ -10,7 +10,7 @@ import os, strutils, times, md5, strtabs, math, db_sqlite, scgi, jester, asyncdispatch, asyncnet, sequtils, parseutils, random, rst, recaptcha, json, re, sugar, - strformat + strformat, logging import cgi except setCookie import options @@ -228,16 +228,21 @@ proc rateLimitCheck(c: TForumData): bool = return false -proc verifyIdentHash(c: TForumData, name, epoch, ident: string): bool = +proc verifyIdentHash( + c: TForumData, name: string, epoch: int64, ident: string +) = const query = sql"select password, salt, strftime('%s', lastOnline) from person where name = ?" var row = getRow(db, query, name) - if row[0] == "": return false + if row[0] == "": + raise newForumError("User doesn't exist.", @["nick"]) let newIdent = makeIdentHash(name, row[0], epoch, row[1], ident) # Check that the user has not been logged in since this ident hash has been # created. Give the timestamp a certain range to prevent false negatives. - if row[2].parseInt > (epoch.parseInt + 60): return false - result = newIdent == ident + if row[2].parseInt > (epoch + 60*3): + raise newForumError("Link expired") + if newIdent != ident: + raise newForumError("Invalid ident hash") proc initialise() = randomize() @@ -247,7 +252,7 @@ proc initialise() = captcha = initReCaptcha(config.recaptchaSecretKey, config.recaptchaSiteKey) else: doAssert config.isDev, "Recaptcha required for production!" - echo("[WARNING] No recaptcha secret key specified.") + warn("No recaptcha secret key specified.") mailer = newMailer(config) @@ -1201,7 +1206,7 @@ routes: let exc = (ref ForumError)(getCurrentException()) resp Http400, $(%exc.data), "application/json" - post "/resetPassword": + post "/sendResetPassword": createTFD() if not c.loggedIn(): let err = PostError( @@ -1219,6 +1224,30 @@ routes: let exc = (ref ForumError)(getCurrentException()) resp Http400, $(%exc.data), "application/json" + post "/resetPassword": + createTFD() + cond(@"nick" != "") + cond(@"epoch" != "") + cond(@"ident" != "") + cond(@"newPassword" != "") + let epoch = getInt64(@"epoch", -1) + try: + verifyIdentHash(c, @"nick", epoch, @"ident") + var salt = makeSalt() + let password = makePassword(@"newPassword", salt) + + exec( + db, + sql""" + update person set password = ?, salt = ?, + lastOnline = DATETIME('now') + where name = ?; + """, + password, salt, @"nick" + ) + resp Http200, "{}", "application/json" + except ForumError as exc: + resp Http400, $(%exc.data),"application/json" get "/t/@id": cond "id" in request.params @@ -1276,53 +1305,10 @@ routes: var epoch: BiggestInt = 0 cond(parseBiggestInt(@"epoch", epoch) > 0) var success = false - if verifyIdentHash(c, @"nick", $epoch, @"ident"): - let ban = parseEnum[Rank](db.getValue(sql"select status from person where name = ?", @"nick")) - # if ban == EmailUnconfirmed: - # success = setStatus(c, @"nick", Moderated, "") - - - get "/emailResetPassword/?": - createTFD() - cond(@"nick" != "") - cond(@"epoch" != "") - cond(@"ident" != "") - var epoch: BiggestInt = 0 - cond(parseBiggestInt(@"epoch", epoch) > 0) - if verifyIdentHash(c, @"nick", $epoch, @"ident"): - let formBody = input(`type`="hidden", name="nick", value = @"nick") & - input(`type`="hidden", name="epoch", value = @"epoch") & - input(`type`="hidden", name="ident", value = @"ident") & - input(`type`="password", name="password") & - "
" & - input(`type`="submit", name="submitBtn", - value="Change my password") - let message = htmlgen.p("Please enter a new password for ", - htmlgen.b(@"nick"), ':') - let content = htmlgen.form(action=c.req.makeUri("/doemailresetpassword"), - `method`="POST", message & formBody) - - # resp genMain(c, content, "Reset password - Nim Forum") - else: - discard - # resp genMain(c, "Invalid ident hash", "Error - Nim Forum") - - post "/doemailresetpassword": - createTFD() - cond(@"nick" != "") - cond(@"epoch" != "") - cond(@"ident" != "") - cond(@"password" != "") - var epoch: BiggestInt = 0 - cond(parseBiggestInt(@"epoch", epoch) > 0) # if verifyIdentHash(c, @"nick", $epoch, @"ident"): - # let res = setPassword(c, @"nick", @"password") - # if res: - # resp genMain(c, "Password reset successfully!", "Nim Forum") - # else: - # resp genMain(c, "Password reset failure", "Nim Forum") - # else: - # resp genMain(c, "Invalid ident hash", "Nim Forum") + # let ban = parseEnum[Rank](db.getValue(sql"select status from person where name = ?", @"nick")) + # # if ban == EmailUnconfirmed: + # # success = setStatus(c, @"nick", Moderated, "") post "/search/?@page?": cond isFTSAvailable @@ -1336,7 +1322,7 @@ routes: elif q[i] == '\'': q[i] = '"' c.search = q.replace("\"",""") if @"page".len > 0: - parseInt(@"page", c.pageNum, 0..1000_000) + parseIntSafe(@"page", c.pageNum) cond(c.pageNum > 0) iterator searchResults(): db_sqlite.Row {.closure, tags: [ReadDbEffect].} = const queryFT = "fts.sql".slurp.sql diff --git a/src/frontend/error.nim b/src/frontend/error.nim index a70708d..575958d 100644 --- a/src/frontend/error.nim +++ b/src/frontend/error.nim @@ -43,6 +43,16 @@ when defined(js): button(class="btn btn-primary"): text "Report issue" + proc renderMessage*(message, submessage, icon: string): VNode = + result = buildHtml(): + tdiv(class="empty error"): + tdiv(class="empty icon"): + italic(class="fas " & icon & " fa-5x") + p(class="empty-title h5"): + text message + p(class="empty-subtitle"): + text submessage + proc genFormField*(error: Option[PostError], name, label, typ: string, isLast: bool): VNode = let hasError = diff --git a/src/frontend/forum.nim b/src/frontend/forum.nim index c8210db..35cdafc 100644 --- a/src/frontend/forum.nim +++ b/src/frontend/forum.nim @@ -5,6 +5,7 @@ include karax/prelude import jester/patterns import threadlist, postlist, header, profile, newthread, error, about +import resetpassword import karaxutils type @@ -13,6 +14,7 @@ type profile: ProfileState newThread: NewThread about: About + resetPassword: ResetPassword proc copyLocation(loc: Location): Location = # TODO: It sucks that I had to do this. We need a nice way to deep copy in JS. @@ -32,7 +34,8 @@ proc newState(): State = url: copyLocation(window.location), profile: newProfileState(), newThread: newNewThread(), - about: newAbout() + about: newAbout(), + resetPassword: newResetPassword() ) var state = newState() @@ -92,6 +95,38 @@ proc render(): VNode = r("/about/?@page?", (params: Params) => (render(state.about, params["page"])) ), + r("/activateEmail/success", + (params: Params) => ( + renderMessage( + "Email activated", + "You can now create new posts!", + "fa-check" + ) + ) + ), + r("/activateEmail/failure", + (params: Params) => ( + renderMessage( + "Email activation failed", + "Couldn't verify the supplied ident", + "fa-exclamation" + ) + ) + ), + r("/resetPassword/success", + (params: Params) => ( + renderMessage( + "Password changed", + "You can now login using your new password!", + "fa-check" + ) + ) + ), + r("/resetPassword", + (params: Params) => ( + render(state.resetPassword) + ) + ), r("/404", (params: Params) => render404() ), diff --git a/src/frontend/karaxutils.nim b/src/frontend/karaxutils.nim index 94372f0..1aa5c97 100644 --- a/src/frontend/karaxutils.nim +++ b/src/frontend/karaxutils.nim @@ -1,22 +1,27 @@ import strutils, options, strformat, parseutils -proc parseInt*(s: string, value: var int, validRange: Slice[int]) {. - noSideEffect.} = +proc parseIntSafe*(s: string, value: var int) {.noSideEffect.} = ## parses `s` into an integer in the range `validRange`. If successful, ## `value` is modified to contain the result. Otherwise no exception is ## raised and `value` is not touched; this way a reasonable default value ## won't be overwritten. - var x = value try: - discard parseutils.parseInt(s, x, 0) + discard parseutils.parseInt(s, value, 0) except OverflowError: discard - if x in validRange: value = x proc getInt*(s: string, default = 0): int = ## Safely parses an int and returns it. result = default - parseInt(s, result, 0..1_000_000_000) + parseIntSafe(s, result) + +proc getInt64*(s: string, default = 0): int64 = + ## Safely parses an int and returns it. + result = default + try: + discard parseutils.parseBiggestInt(s, result, 0) + except OverflowError: + discard when defined(js): include karax/prelude @@ -32,7 +37,8 @@ when defined(js): for class in classes: if class.present: result.add(class.name & " ") - proc makeUri*(relative: string, appName=appName, includeHash=false): string = + proc makeUri*(relative: string, appName=appName, includeHash=false, + search: string=""): string = ## Concatenates ``relative`` to the current URL in a way that is ## (possibly) sane. var relative = relative @@ -43,7 +49,7 @@ when defined(js): $window.location.host & appName & relative & - $window.location.search & + search & (if includeHash: $window.location.hash else: "") proc makeUri*(relative: string, params: varargs[(string, string)], @@ -55,7 +61,11 @@ when defined(js): query.add(param[0] & "=" & param[1]) if query.len > 0: - makeUri(relative & "?" & query, appName) + var search = $window.location.search + if search.len != 0: search.add("&") + search.add(query) + if search[0] != '?': search = "?" & search + makeUri(relative, appName, search=search) else: makeUri(relative, appName) diff --git a/src/frontend/newthread.nim b/src/frontend/newthread.nim index 3f94867..8e701a2 100644 --- a/src/frontend/newthread.nim +++ b/src/frontend/newthread.nim @@ -51,7 +51,7 @@ when defined(js): tdiv(class="content"): input(class="form-input", `type`="text", name="subject", placeholder="Type the title here", - onChange=(e: Event, n: VNode) => onSubjectChange(e, n, state)) + oninput=(e: Event, n: VNode) => onSubjectChange(e, n, state)) if state.error.isSome(): p(class="text-error"): text state.error.get().message diff --git a/src/frontend/postbutton.nim b/src/frontend/postbutton.nim index 292c136..8b927e6 100644 --- a/src/frontend/postbutton.nim +++ b/src/frontend/postbutton.nim @@ -31,7 +31,7 @@ when defined(js): var formData = newFormData() formData.append("email", email) result = newPostButton( - makeUri("/resetPassword"), + makeUri("/sendResetPassword"), formData, "Send password reset email", "fas fa-envelope", diff --git a/src/frontend/resetpassword.nim b/src/frontend/resetpassword.nim new file mode 100644 index 0000000..2b98674 --- /dev/null +++ b/src/frontend/resetpassword.nim @@ -0,0 +1,65 @@ +when defined(js): + import sugar, httpcore, options, json + import dom except Event + + include karax/prelude + import karax / [kajax, kdom] + + import error, replybox, threadlist, post + import karaxutils + + type + ResetPassword* = ref object + loading: bool + status: HttpCode + error: Option[PostError] + newPassword: kstring + + proc newResetPassword*(): ResetPassword = + ResetPassword( + status: Http200, + newPassword: "" + ) + + proc onPassChange(e: Event, n: VNode, state: ResetPassword) = + state.newPassword = n.value + + proc onPost(httpStatus: int, response: kstring, state: ResetPassword) = + postFinished: + navigateTo(makeUri("/resetPassword/success")) + + proc onSetClick( + ev: Event, n: VNode, + state: ResetPassword + ) = + state.loading = true + state.error = none[PostError]() + + let uri = makeUri("resetPassword", ("newPassword", $state.newPassword)) + ajaxPost(uri, @[], "", + (s: int, r: kstring) => onPost(s, r, state)) + + proc render*(state: ResetPassword): VNode = + if state.loading: + return buildHtml(tdiv(class="loading")) + + result = buildHtml(): + section(class="container grid-xl"): + tdiv(class="resetpassword"): + tdiv(class="title"): + p(): text "Reset Password" + tdiv(class="content"): + input(class="form-input", `type`="password", name="password", + placeholder="Type your new password here", + oninput=(e: Event, n: VNode) => onPassChange(e, n, state)) + if state.error.isSome(): + p(class="text-error"): + text state.error.get().message + tdiv(class="footer"): + button(class=class( + {"loading": state.loading}, + "btn btn-primary" + ), + onClick=(ev: Event, n: VNode) => + (onSetClick(ev, n, state))): + text "Set password" \ No newline at end of file diff --git a/src/utils.nim b/src/utils.nim index c4ecfec..069b503 100644 --- a/src/utils.nim +++ b/src/utils.nim @@ -1,5 +1,5 @@ import asyncdispatch, smtp, strutils, json, os, rst, rstgen, xmltree, strtabs, - htmlparser, streams, parseutils, options + htmlparser, streams, parseutils, options, logging from times import getTime, getGMTime, format # Used to be: @@ -178,4 +178,4 @@ proc rstToHtml*(content: string): string = result = "" add(result, node, indWidth=0, addNewLines=false) except: - echo("[WARNING] Could not parse rst html.") + warn("Could not parse rst html.")