From 3f7769d2ba4047e8766e511b7a42c7aa4721f6f8 Mon Sep 17 00:00:00 2001 From: Yorhel Date: Thu, 3 Oct 2019 17:33:33 +0200 Subject: Require email confirmation when changing email address This ensures that the email address linked to a user is always valid and actually belong(s|ed) to that user. --- elm/Lib/Api.elm | 1 + elm/User/Edit.elm | 36 +++++++++++++++----------- lib/VNWeb/Auth.pm | 18 +++++++------ lib/VNWeb/Elm.pm | 1 + lib/VNWeb/User/Edit.pm | 53 ++++++++++++++++++++++++++++++++++++--- util/sql/func.sql | 30 +++++++++++++++++----- util/sql/schema.sql | 3 ++- util/updates/update_20191003b.sql | 4 ++- 8 files changed, 113 insertions(+), 33 deletions(-) diff --git a/elm/Lib/Api.elm b/elm/Lib/Api.elm index 4df99fde..283cbe3c 100644 --- a/elm/Lib/Api.elm +++ b/elm/Lib/Api.elm @@ -41,6 +41,7 @@ showResponse res = DoubleEmail -> "Email address already used for another account." DoubleIP -> "You can only register one account from the same IP within 24 hours." BadCurPass -> "Current password is invalid." + MailChange -> unexp expectResponse : (Response -> msg) -> Http.Expect msg diff --git a/elm/User/Edit.elm b/elm/User/Edit.elm index 4bdffad9..544b2fe1 100644 --- a/elm/User/Edit.elm +++ b/elm/User/Edit.elm @@ -23,25 +23,27 @@ main = Browser.element type alias Model = - { state : Api.State - , data : GUE.Send - , cpass : Bool - , pass1 : String - , pass2 : String - , opass : String - , passNeq : Bool + { state : Api.State + , data : GUE.Send + , cpass : Bool + , pass1 : String + , pass2 : String + , opass : String + , passNeq : Bool + , mailConfirm : Bool } init : GUE.Send -> Model init d = - { state = Api.Normal - , data = d - , cpass = False - , pass1 = "" - , pass2 = "" - , opass = "" - , passNeq = False + { state = Api.Normal + , data = d + , cpass = False + , pass1 = "" + , pass2 = "" + , opass = "" + , passNeq = False + , mailConfirm = False } @@ -115,7 +117,8 @@ update msg model = else ({ model | state = Api.Loading }, Api.post "/u/edit" (GUE.encode model.data) Submitted) -- TODO: This reload is only necessary for the skin and customcss options to apply, but it's nicer to do that directly from JS. - Submitted GApi.Success -> (model, load <| "/u" ++ String.fromInt model.data.id ++ "/edit") + Submitted GApi.Success -> (model, load <| "/u" ++ String.fromInt model.data.id ++ "/edit") + Submitted GApi.MailChange -> ({ model | mailConfirm = True, state = Api.Normal }, Cmd.none) Submitted r -> ({ model | state = Api.Error r }, Cmd.none) @@ -188,5 +191,8 @@ view model = ] , div [ class "mainbox" ] [ fieldset [ class "submit" ] [ submitButton "Submit" model.state (not model.passNeq) False ] + , if not model.mailConfirm then text "" else + div [ class "notice" ] + [ text "A confirmation email has been sent to your new address. Your address will be updated after following the instructions in that mail." ] ] ] diff --git a/lib/VNWeb/Auth.pm b/lib/VNWeb/Auth.pm index 0b43074a..7143f203 100644 --- a/lib/VNWeb/Auth.pm +++ b/lib/VNWeb/Auth.pm @@ -238,13 +238,17 @@ sub setpass { } -# Change a users' password, requires that the current logged in user is an admin. -sub admin_setpass { - my($self, $uid, $pass) = @_; - my $encpass = $self->_preparepass($pass); - tuwf->dbVali(select => - sql_func user_admin_setpass => \$uid, \$self->{uid}, sql_fromhex($self->{token}), sql_fromhex($encpass) - ) +sub setmail_token { + my($self, $mail) = @_; + my $token = unpack 'H*', urandom(20); + tuwf->dbExeci(select => sql_func user_setmail_token => \$self->uid, sql_fromhex($self->token), sql_fromhex(sha1_hex lc $token), \$mail); + $token; +} + + +sub setmail_confirm { + my(undef, $uid, $token) = @_; + tuwf->dbVali(select => sql_func user_setmail_confirm => \$uid, sql_fromhex sha1_hex lc $token); } diff --git a/lib/VNWeb/Elm.pm b/lib/VNWeb/Elm.pm index 78fad8c8..e83faf25 100644 --- a/lib/VNWeb/Elm.pm +++ b/lib/VNWeb/Elm.pm @@ -45,6 +45,7 @@ my %apis = ( DoubleEmail => [], # Account with same email already exists DoubleIP => [], # Account with same IP already exists BadCurPass => [], # Current password is incorrect when changing password + MailChange => [], # A confirmation mail has been sent to change a user's email address ); diff --git a/lib/VNWeb/User/Edit.pm b/lib/VNWeb/User/Edit.pm index 8b1f1ea2..4e67bf4e 100644 --- a/lib/VNWeb/User/Edit.pm +++ b/lib/VNWeb/User/Edit.pm @@ -34,6 +34,11 @@ my $FORM = form_compile in => { elm_form UserEdit => undef, $FORM; +sub _getmail { + my $uid = shift; + tuwf->dbVali(select => sql_func user_getmail => \$uid, \auth->uid, sql_fromhex auth->token); +} + TUWF::get qr{/$RE{uid}/edit}, sub { my $u = tuwf->dbRowi(q{ SELECT id, username, perm, ign_votes, hide_list, show_nsfw, traits_sexual, @@ -43,7 +48,7 @@ TUWF::get qr{/$RE{uid}/edit}, sub { return tuwf->resNotFound if !can_edit u => $u; - $u->{email} = tuwf->dbVali(select => sql_func user_getmail => \$u->{id}, \auth->uid, sql_fromhex auth->token); + $u->{email} = _getmail $u->{id}; $u->{authmod} = auth->permUsermod; $u->{password} = undef; $u->{skin} ||= config->{skin_default}; @@ -65,6 +70,8 @@ TUWF::get qr{/$RE{uid}/edit}, sub { json_api qr{/u/edit}, $FORM, sub { my $data = shift; + my $username = tuwf->dbVali('SELECT username FROM users WHERE id =', \$data->{id}); + return tuwf->resNotFound if !$username; return elm_Unauth if !can_edit u => $data; if(auth->permUsermod) { @@ -88,12 +95,52 @@ json_api qr{/u/edit}, $FORM, sub { } } - tuwf->dbExeci(select => sql_func user_setmail => \$data->{id}, \auth->uid, sql_fromhex(auth->token), \$data->{email}); + my $ret = \&elm_Success; + + my $oldmail = _getmail $data->{id}; + if($data->{email} ne $oldmail) { + if(auth->permUsermod) { + tuwf->dbExeci(select => sql_func user_admin_setmail => \$data->{id}, \auth->uid, sql_fromhex(auth->token), \$data->{email}); + } else { + my $token = auth->setmail_token($data->{email}); + my $body = sprintf + "Hello %s," + ."\n\n" + ."To confirm that you want to change the email address associated with your VNDB.org account from %s to %s, click the link below:" + ."\n\n" + ."%s" + ."\n\n" + ."vndb.org", + $username, $oldmail, $data->{email}, tuwf->reqBaseURI()."/u$data->{id}/setmail/$token"; + + tuwf->mail($body, + To => $data->{email}, + From => 'VNDB ', + Subject => "Confirm e-mail change for $username", + ); + $ret = \&elm_MailChange; + } + } $data->{skin} = '' if $data->{skin} eq config->{skin_default}; auth->prefSet($_, $data->{$_}, $data->{id}) for qw/hide_list show_nsfw traits_sexual tags_all tags_cont tags_ero tags_tech spoilers skin customcss/; - elm_Success + $ret->(); +}; + + +TUWF::get qr{/$RE{uid}/setmail/(?[a-f0-9]{40})}, sub { + my $success = auth->setmail_confirm(tuwf->capture('id'), tuwf->capture('token')); + my $title = $success ? 'E-mail confirmed' : 'Error confirming email'; + framework_ title => $title, index => 0, sub { + div_ class => 'mainbox', sub { + h1_ $title; + div_ class => $success ? 'notice' : 'warning', sub { + p_ "Your e-mail address has been updated!" if $success; + p_ "Invalid or expired confirmation link." if !$success; + }; + }; + }; }; 1; diff --git a/util/sql/func.sql b/util/sql/func.sql index 51dc2aac..0ffe902d 100644 --- a/util/sql/func.sql +++ b/util/sql/func.sql @@ -781,10 +781,10 @@ $$ LANGUAGE SQL SECURITY DEFINER; -- calling this function doesn't even get the token, and thus can't get access -- to someone's account. But alas, that'd require a separate process. CREATE OR REPLACE FUNCTION user_resetpass(text, bytea) RETURNS integer AS $$ - WITH uid(uid) AS ( - SELECT id FROM users WHERE lower(mail) = lower($1) AND length($2) = 20 AND perm & 128 = 0 - ) - INSERT INTO sessions (uid, token, expires, type) SELECT uid, $2, NOW()+'1 week', 'pass' FROM uid RETURNING uid + INSERT INTO sessions (uid, token, expires, type) + SELECT id, $2, NOW()+'1 week', 'pass' FROM users + WHERE lower(mail) = lower($1) AND length($2) = 20 AND perm & 128 = 0 + RETURNING uid $$ LANGUAGE SQL SECURITY DEFINER; @@ -822,8 +822,21 @@ CREATE OR REPLACE FUNCTION user_getmail(integer, integer, bytea) RETURNS text AS $$ LANGUAGE SQL SECURITY DEFINER; -CREATE OR REPLACE FUNCTION user_setmail(integer, integer, bytea, text) RETURNS void AS $$ - UPDATE users SET mail = $4 WHERE id = $1 AND user_isauth($1, $2, $3) +-- Set a token to change a user's email address. +-- Args: uid, web-token, new-email-token, email +CREATE OR REPLACE FUNCTION user_setmail_token(integer, bytea, bytea, text) RETURNS void AS $$ + INSERT INTO sessions (uid, token, expires, type, mail) + SELECT id, $3, NOW()+'1 week', 'mail', $4 FROM users + WHERE id = $1 AND user_isauth($1, $1, $2) AND length($3) = 20 +$$ LANGUAGE SQL SECURITY DEFINER; + + +-- Actually change a user's email address, given a valid token. +CREATE OR REPLACE FUNCTION user_setmail_confirm(integer, bytea) RETURNS boolean AS $$ + WITH u(mail) AS ( + DELETE FROM sessions WHERE uid = $1 AND token = $2 AND type = 'mail' AND expires > NOW() RETURNING mail + ) + UPDATE users SET mail = (SELECT mail FROM u) WHERE id = $1 AND EXISTS(SELECT 1 FROM u) RETURNING true; $$ LANGUAGE SQL SECURITY DEFINER; @@ -838,3 +851,8 @@ CREATE OR REPLACE FUNCTION user_admin_setpass(integer, integer, bytea, bytea) RE ) DELETE FROM sessions WHERE uid IN(SELECT id FROM upd) $$ LANGUAGE SQL SECURITY DEFINER; + + +CREATE OR REPLACE FUNCTION user_admin_setmail(integer, integer, bytea, text) RETURNS void AS $$ + UPDATE users SET mail = $4 WHERE id = $1 AND user_isauth(-1, $2, $3) +$$ LANGUAGE SQL SECURITY DEFINER; diff --git a/util/sql/schema.sql b/util/sql/schema.sql index 57136b0f..46bf3d27 100644 --- a/util/sql/schema.sql +++ b/util/sql/schema.sql @@ -65,7 +65,7 @@ CREATE TYPE release_type AS ENUM ('complete', 'partial', 'trial'); CREATE TYPE tag_category AS ENUM('cont', 'ero', 'tech'); CREATE TYPE vn_relation AS ENUM ('seq', 'preq', 'set', 'alt', 'char', 'side', 'par', 'ser', 'fan', 'orig'); CREATE TYPE resolution AS ENUM ('unknown', 'nonstandard', '640x480', '800x600', '1024x768', '1280x960', '1600x1200', '640x400', '960x600', '960x640', '1024x576', '1024x600', '1024x640', '1280x720', '1280x800', '1366x768', '1600x900', '1920x1080'); -CREATE TYPE session_type AS ENUM ('web', 'pass'); +CREATE TYPE session_type AS ENUM ('web', 'pass', 'mail'); -- Sequences used for ID generation of items not in the DB CREATE SEQUENCE covers_seq; @@ -456,6 +456,7 @@ CREATE TABLE sessions ( added timestamptz NOT NULL DEFAULT NOW(), expires timestamptz NOT NULL, type session_type NOT NULL, + mail text, PRIMARY KEY (uid, token) ); diff --git a/util/updates/update_20191003b.sql b/util/updates/update_20191003b.sql index 11b1a5d9..a7342463 100644 --- a/util/updates/update_20191003b.sql +++ b/util/updates/update_20191003b.sql @@ -4,13 +4,15 @@ UPDATE sessions SET expires = expires + '1 month'::interval; ALTER TABLE sessions ALTER COLUMN expires DROP DEFAULT; -- Support different session types -CREATE TYPE session_type AS ENUM ('web', 'pass'); +CREATE TYPE session_type AS ENUM ('web', 'pass', 'mail'); ALTER TABLE sessions ADD COLUMN type session_type NOT NULL DEFAULT 'web'; ALTER TABLE sessions ALTER COLUMN type DROP DEFAULT; +ALTER TABLE sessions ADD COLUMN mail text; DROP FUNCTION user_isloggedin(integer, bytea); DROP FUNCTION user_update_lastused(integer, bytea); DROP FUNCTION user_isvalidtoken(integer, bytea); +DROP FUNCTION user_setmail(integer, integer, bytea, text); -- Convert old password reset tokens to the new session format INSERT INTO sessions (uid, token, expires, type) -- cgit v1.2.3