From cdb6dd687afb25cccfff8f0d43dbff60cef5b7b3 Mon Sep 17 00:00:00 2001 From: Eddie Kohler Date: Wed, 4 Oct 2023 15:59:53 -0400 Subject: [PATCH] $Opt["disableNewUsers"] can be set to "other". Which prevents any real user from creating other accounts (because on the test site, account creation was abused). --- etc/distoptions.php | 1 + lib/mailer.php | 2 +- src/conference.php | 14 +++++++++--- src/contact.php | 49 ++++++++++++++++++++++++++--------------- src/logentry.php | 2 +- src/pages/p_profile.php | 4 ++-- src/reviewinfo.php | 2 +- src/userstatus.php | 28 ++++++++++++++--------- 8 files changed, 66 insertions(+), 36 deletions(-) diff --git a/etc/distoptions.php b/etc/distoptions.php index 164ba58a6..299bbcf86 100644 --- a/etc/distoptions.php +++ b/etc/distoptions.php @@ -101,6 +101,7 @@ // defaultEmailDomain Set to the default domain for account email addresses // when using httpAuthLogin. // disableNewUsers Don’t allow new users to register. +// disableNonPC Disable all accounts except PC and sysadmin accounts. // PASSWORD SECURITY diff --git a/lib/mailer.php b/lib/mailer.php index 4dee87aaa..9453374e6 100644 --- a/lib/mailer.php +++ b/lib/mailer.php @@ -527,7 +527,7 @@ function expand($text, $field = null) { } // lose newlines on header expansion - if ($this->context != self::CONTEXT_BODY) { + if ($this->context !== self::CONTEXT_BODY) { $text = rtrim(preg_replace('/[\r\n\f\x0B]+/', ' ', $text)); } diff --git a/src/conference.php b/src/conference.php index 569dcd8d3..539e153cc 100644 --- a/src/conference.php +++ b/src/conference.php @@ -1863,9 +1863,17 @@ function external_login() { /** @return bool */ function allow_user_self_register() { - return !$this->external_login() - && !$this->disable_non_pc - && !$this->opt("disableNewUsers"); + if ($this->external_login() || $this->disable_non_pc) { + return false; + } + $dnu = $this->opt("disableNewUsers"); + return !$dnu || $dnu === "other"; + } + + /** @return bool */ + function allow_user_activate_other() { + $dnu = $this->opt("disableNewUsers"); + return !$dnu || $dnu === true || $dnu === "self"; } diff --git a/src/contact.php b/src/contact.php index fec30d6c9..5ed101048 100644 --- a/src/contact.php +++ b/src/contact.php @@ -262,8 +262,8 @@ private function __construct($conf) { /** @return Contact */ static function make(Conf $conf) { $u = new Contact($conf); - $u->set_roles_properties(); $u->contactXid = self::$next_xid--; + $u->set_roles_properties(); return $u; } @@ -271,20 +271,30 @@ static function make(Conf $conf) { * @return Contact */ static function make_email(Conf $conf, $email) { $u = new Contact($conf); + $u->contactXid = self::$next_xid--; $u->email = $email ?? ""; $u->set_roles_properties(); + return $u; + } + + /** @param int $contactId + * @return Contact */ + static function make_placeholder(Conf $conf) { + $u = new Contact($conf); $u->contactXid = self::$next_xid--; + $u->disabled = self::DISABLEMENT_PLACEHOLDER; + $u->set_roles_properties(); return $u; } /** @param int $contactId * @return Contact */ - static function make_placeholder(Conf $conf, $contactId) { + static function make_deleted(Conf $conf, $contactId) { $u = new Contact($conf); $u->contactId = $contactId; $u->contactXid = $contactId > 0 ? $contactId : self::$next_xid--; - $u->email = "unknown"; - $u->disablement = self::DISABLEMENT_USER; + $u->email = ""; + $u->disablement = self::DISABLEMENT_DELETED; $u->set_roles_properties(); return $u; } @@ -293,10 +303,10 @@ static function make_placeholder(Conf $conf, $contactId) { * @return Contact */ static function make_cdb_email(Conf $conf, $email) { $u = new Contact($conf); + $u->contactXid = self::$next_xid--; $u->email = $email ?? ""; $u->cdb_confid = $conf->cdb_confid(); $u->set_roles_properties(); - $u->contactXid = self::$next_xid--; return $u; } @@ -307,6 +317,7 @@ static function make_keyed(Conf $conf, $args) { // the importable properties $u = new Contact($conf); $u->contactId = $args["contactId"] ?? 0; + $u->contactXid = $u->contactId > 0 ? $u->contactId : self::$next_xid--; $u->email = trim($args["email"] ?? ""); $u->firstName = $args["firstName"] ?? $args["first"] ?? ""; $u->lastName = $args["lastName"] ?? $args["last"] ?? ""; @@ -317,7 +328,6 @@ static function make_keyed(Conf $conf, $args) { $u->disablement = $args["disablement"] ?? 0; $u->disabled = $u->disablement & self::DISABLEMENT_DB; $u->set_roles_properties(); - $u->contactXid = $u->contactId ? : self::$next_xid--; return $u; } @@ -325,6 +335,7 @@ static function make_keyed(Conf $conf, $args) { * @return Contact */ static function make_site_contact(Conf $conf, $args) { $u = new Contact($conf); + $u->contactXid = self::$next_xid--; $u->email = $args["email"] ?? ""; $u->firstName = $args["firstName"] ?? ""; $u->lastName = $args["lastName"] ?? ""; @@ -332,7 +343,6 @@ static function make_site_contact(Conf $conf, $args) { $u->_root_user = true; $u->_dangerous_track_mask = 0; $u->set_roles_properties(); - $u->contactXid = self::$next_xid--; return $u; } @@ -1964,20 +1974,23 @@ function import_prop($src, $ifempty) { $this->set_prop($prop, $src->prop1($prop, $shape), $ifempty); } - // disablement requires special handling - if ($this->cdb_confid !== 0 - && $this->contactDbId === 0 - && $src->disablement !== 0) { - // creating a cdb user for a disabled local user: cdb is placeholder + // disablement import is special + // source is disabled local user, creating cdb user: cdb is placeholder + if ($src->disablement !== 0 + && $this->cdb_confid !== 0 + && $this->contactDbId === 0) { $this->set_prop("disabled", $this->disabled | self::DISABLEMENT_PLACEHOLDER); - } else if (($this->disabled & self::DISABLEMENT_PLACEHOLDER) !== 0 - && $src->disablement === 0) { - // saving a non-placeholder user: other user loses placeholder status + } + // source is non-disabled local user: this is not placeholder + if ($src->cdb_confid === 0 + && $src->disablement === 0 + && ($this->disabled & self::DISABLEMENT_PLACEHOLDER) !== 0) { $this->set_prop("disabled", $this->disabled & ~self::DISABLEMENT_PLACEHOLDER); } - if ($src->cdb_confid !== 0 - && ($src->disabled & self::DISABLEMENT_USER) !== 0) { - // globally disabled users are locally disabled too + // source is globally disabled: this local user is disabled + if (($src->disabled & self::DISABLEMENT_USER) !== 0 + && $src->cdb_confid !== 0 + && $this->cdb_confid === 0) { $this->set_prop("disabled", $this->disabled | self::DISABLEMENT_USER); } } diff --git a/src/logentry.php b/src/logentry.php index e7d3df890..62bd3b3b3 100644 --- a/src/logentry.php +++ b/src/logentry.php @@ -277,7 +277,7 @@ private function _make_users() { } if (!empty($this->need_users)) { foreach ($this->need_users as $cid => $x) { - $this->users[$cid] = Contact::make_keyed($this->conf, ["contactId" => $cid, "disablement" => Contact::DISABLEMENT_DELETED]); + $this->users[$cid] = Contact::make_deleted($this->conf, $cid); } $result = $this->conf->qe("select " . $this->conf->deleted_user_query_fields() . " from DeletedContactInfo where contactId?a", array_keys($this->need_users)); while (($user = Contact::fetch($result, $this->conf))) { diff --git a/src/pages/p_profile.php b/src/pages/p_profile.php index eb07cc5e6..a7f792445 100644 --- a/src/pages/p_profile.php +++ b/src/pages/p_profile.php @@ -115,7 +115,7 @@ private function find_user() { $user = $this->viewer; } else if ($this->viewer->privChair && ($u === "new" || $u === "bulk")) { - $user = Contact::make($this->conf); + $user = Contact::make_placeholder($this->conf); $this->page_type = $u === "new" ? 1 : 2; } else { $user = $this->handle_user_search($u); @@ -288,7 +288,7 @@ private function save_bulk($text, $filename) { $ustatus->add_csv_synonyms($csv); while (($line = $csv->next_row())) { - $ustatus->set_user(Contact::make($this->conf)); + $ustatus->set_user(Contact::make_placeholder($this->conf)); $ustatus->clear_messages(); $ustatus->jval = (object) ["id" => null]; $ustatus->csvreq = $line; diff --git a/src/reviewinfo.php b/src/reviewinfo.php index 738d97efe..34bc8b736 100644 --- a/src/reviewinfo.php +++ b/src/reviewinfo.php @@ -778,7 +778,7 @@ function reviewer() { if ($this->_reviewer === null) { $this->prow && $this->prow->ensure_reviewer_names(); $this->_reviewer = $this->conf->user_by_id($this->contactId, USER_SLICE) - ?? Contact::make_placeholder($this->conf, $this->contactId); + ?? Contact::make_deleted($this->conf, $this->contactId); } return $this->_reviewer; } diff --git a/src/userstatus.php b/src/userstatus.php index 2d41b5e81..a8e27b7a2 100644 --- a/src/userstatus.php +++ b/src/userstatus.php @@ -891,16 +891,15 @@ function save_user($cj, $old_user = null) { // ensure/create user $this->check_invariants($cj); $actor = $this->viewer->is_root_user() ? null : $this->viewer; - if ($old_user) { - $old_disablement = $old_user->disablement; - } else { - $user = Contact::make_keyed($this->conf, (array) $cj)->store(0, $actor); + if (!$old_user) { + $create_cj = array_merge((array) $cj, ["disablement" => Contact::DISABLEMENT_PLACEHOLDER]); + $user = Contact::make_keyed($this->conf, $create_cj)->store(0, $actor); $cj->email = $user->email; // adopt contactdb’s email capitalization - $old_disablement = Contact::DISABLEMENT_PLACEHOLDER; } if (!$user) { return null; } + $old_disablement = $user->disablement; // initialize assert(!isset($cj->email) || strcasecmp($cj->email, $user->email) === 0); @@ -1013,13 +1012,22 @@ static function save_main(UserStatus $us) { } // Disabled + $disablement = $user->disablement & Contact::DISABLEMENT_DB; if (isset($cj->disabled)) { - $user->set_prop("disabled", $cj->disabled ? Contact::DISABLEMENT_USER : 0); - if ($user->prop_changed("disabled")) { - $us->diffs[$cj->disabled ? "disabled" : "enabled"] = true; + if ($cj->disabled) { + $disablement |= Contact::DISABLEMENT_USER; + } else { + $disablement &= ~Contact::DISABLEMENT_USER; } - } else { // always revoke placeholder status - $user->activate_placeholder_prop(); + } + if ($disablement === Contact::DISABLEMENT_PLACEHOLDER + && ($us->viewer->is_root_user() + || $us->conf->allow_user_activate_other())) { + $disablement &= ~Contact::DISABLEMENT_PLACEHOLDER; + } + $user->set_prop("disabled", $disablement); + if ($user->prop_changed("disabled") && isset($cj->disabled)) { + $us->diffs[$cj->disabled ? "disabled" : "enabled"] = true; } // Follow