Skip to content

Commit

Permalink
Security-sensitive profile groups are handled uniformly
Browse files Browse the repository at this point in the history 
And note that developer settings are security-sensitive.
  • Loading branch information
@kohler
kohler committed Nov 25, 2024
1 parent abdf2ea commit aeaf266
Show file tree
Hide file tree
Showing 9 changed files with 141 additions and 95 deletions.
13 changes: 12 additions & 1 deletion etc/profilegroups.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@
{ "name": "__field/collaborators", "label": "Collaborators", "page": "main" },
{ "name": "__field/topics", "label": "Topics", "page": "main" },

{
"name": "__reauthenticate", "autosection": false,
"print_function": "Security_UserInfo::print_reauthenticate"
},

[ "main/main", 1000, "UserStatus::print_main" ],
[ "main/country", 3000, "UserStatus::print_country" ],
[ "main/roles", 5000, "UserStatus::print_roles" ],
Expand All @@ -39,11 +44,14 @@
{ "name": "security", "title": "Security", "order": 200,
"allow_if": "profile_security",
"print_function": "*Security_UserInfo::print",
"request_recent_authentication": true,
"request_function": "*Security_UserInfo::request",
"save_members": true,
"inputs": false },
[ "security/currentpassword", 10, "*Security_UserInfo::print_current_password" ],
{ "name": "security/reauthenticate",
"order": 1, "alias": "__reauthenticate" },
{ "name": "security/newpassword", "order": 100,
"title": "Change password",
"print_function": "*Security_UserInfo::print_new_password",
"request_function": "*Security_UserInfo::parse_qreq_new_password",
"save_function": "Security_UserInfo::save_new_password",
Expand All @@ -60,8 +68,11 @@
{ "name": "developer", "title": "Developer settings", "short_title": "Developer",
"order": 2000,
"allow_if": "auth_self",
"request_recent_authentication": true,
"request_function": "*Developer_UserInfo::request",
"save_members": true },
{ "name": "developer/reauthenticate",
"order": 1, "alias": "__reauthenticate" },
{ "name": "developer/tokens", "title": "API tokens", "order": 10,
"print_function": "*Developer_UserInfo::print_bearer_tokens",
"inputs": false },
Expand Down
12 changes: 8 additions & 4 deletions lib/login.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,13 +384,17 @@ static function login_error(Conf $conf, $email, $info, $ms) {
$conf->redirect($urlarg->value);
}

if ($ms) {
$ms->error_at(isset($info["email"]) ? "email" : "password", $msg);
if (!$ms) {
$conf->error_msg($msg);
} else if (isset($info["field"])) {
$ms->error_at($info["field"], $msg);
} else if (!isset($info["email"])) {
$ms->error_at("password", $msg);
} else {
$ms->error_at("email", $msg);
if (isset($info["password"])) {
$ms->error_at("password");
}
} else {
$conf->error_msg($msg);
}
}
}
7 changes: 0 additions & 7 deletions scripts/script.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13094,13 +13094,6 @@ handle_ui.on("js-profile-role", function () {
foldup.call(this, null, {n: 2, open: (pctype && pctype !== "none") || ass !== 0});
});

handle_ui.on("js-profile-current-password", function () {
if (this.value.trim() !== "") {
$(this.form).find(".need-profile-current-password").prop("disabled", false);
removeClass(this, "uii");
}
});

handle_ui.on("js-profile-token-add", function () {
this.disabled = true;
var nbt = document.getElementById("new-api-token").closest(".form-section");
Expand Down
21 changes: 15 additions & 6 deletions src/pages/p_profile.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ function __construct(Contact $viewer, Qrequest $qreq) {
$this->user = $viewer;
$this->ustatus = new UserStatus($viewer);
$this->ustatus->set_user($viewer);
$this->ustatus->qreq = $qreq;
$this->ustatus->set_qreq($qreq);
}


Expand Down Expand Up @@ -508,7 +508,8 @@ private function handle_delete() {

function handle_request() {
$this->find_user();
if ($this->qreq->cancel) {
if ($this->qreq->cancel
|| ($this->qreq->reauth && $this->qreq->valid_post())) {
$this->conf->redirect_self($this->qreq);
} else if ($this->qreq->savebulk
&& $this->page_type !== 0
Expand Down Expand Up @@ -739,10 +740,7 @@ function print() {
if ($this->page_type === 2) {
$this->ustatus->print_members("__bulk");
} else {
$this->ustatus->cs()->print_body_members($this->topic);
if ($this->ustatus->inputs_printed()) {
$this->ustatus->print_actions();
}
$this->print_topic();
echo "</div>"; // foldaccount
}

Expand All @@ -754,6 +752,17 @@ function print() {
$this->qreq->print_footer();
}

private function print_topic() {
$this->ustatus->cs()->print_body_members($this->topic);
if (!$this->ustatus->inputs_printed()
|| (!$this->ustatus->has_recent_authentication()
&& ($gj = $this->ustatus->cs()->get($this->topic))
&& ($gj->request_recent_authentication ?? false))) {
return;
}
$this->ustatus->print_actions();
}


static function go(Contact $user, Qrequest $qreq) {
if ($qreq->changeemail
Expand Down
53 changes: 28 additions & 25 deletions src/userinfo/u_developer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ static function unparse_last_used($time) {
}

function print_new_bearer_token(UserStatus $us) {
if (!$us->is_auth_self()) {
if (!$us->is_auth_self()
|| !$us->has_recent_authentication()) {
return;
} else if ($us->user->security_locked()) {
$us->conf->warning_msg("<0>This account’s security settings are locked, so its API tokens cannot be changed.");
Expand Down Expand Up @@ -219,14 +220,15 @@ function request_new_bearer_token(UserStatus $us) {
}

function save_new_bearer_token(UserStatus $us) {
if ($this->_new_token !== null) {
$this->_new_token->set_token_pattern("hct_[30]")->insert();
if ($this->_new_token->stored()) {
$us->diffs["API tokens"] = true;
} else {
$us->error_at(null, "<0>Error while creating new API token");
$this->_new_token = null;
}
if ($this->_new_token === null) {
return;
}
$this->_new_token->set_token_pattern("hct_[30]")->insert();
if ($this->_new_token->stored()) {
$us->diffs["API tokens"] = true;
} else {
$us->error_at(null, "<0>Error while creating new API token");
$this->_new_token = null;
}
}

Expand All @@ -244,25 +246,26 @@ function request_delete_bearer_tokens(UserStatus $us) {
}

function save_delete_bearer_tokens(UserStatus $us) {
if ($this->_delete_tokens !== null) {
$toks = self::all_active_bearer_tokens($us->user);
$deleteables = [];
foreach ($toks as $tok) {
foreach ($this->_delete_tokens as $dt) {
if ($tok->timeCreated === $dt[0]
&& $tok->is_cdb === $dt[1]
&& str_starts_with($tok->salt, $dt[2])) {
$deleteables[] = $tok;
}
if ($this->_delete_tokens === null) {
return;
}
$toks = self::all_active_bearer_tokens($us->user);
$deleteables = [];
foreach ($toks as $tok) {
foreach ($this->_delete_tokens as $dt) {
if ($tok->timeCreated === $dt[0]
&& $tok->is_cdb === $dt[1]
&& str_starts_with($tok->salt, $dt[2])) {
$deleteables[] = $tok;
}
}
if (!empty($deleteables)
&& count($deleteables) <= count($this->_delete_tokens)) {
foreach ($deleteables as $tok) {
$tok->delete();
}
$us->diffs["API tokens"] = true;
}
if (!empty($deleteables)
&& count($deleteables) <= count($this->_delete_tokens)) {
foreach ($deleteables as $tok) {
$tok->delete();
}
$us->diffs["API tokens"] = true;
}
}
}
60 changes: 18 additions & 42 deletions src/userinfo/u_security.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,48 +6,23 @@ class Security_UserInfo {
/** @var UserStatus
* @readonly */
public $us;
/** @var bool */
private $_approved = false;
/** @var bool */
private $_complained = false;
/** @var ?list<MessageItem> */
private $_approval_errors;
/** @var ?array{string,string} */
private $_req_passwords;

function __construct(UserStatus $us) {
$this->us = $us;
$this->_approved = $us->allow_some_security()
&& UpdateSession::usec_query($us->qreq, $us->viewer->email, 0, 1, Conf::$now - 300);
}

function request(UserStatus $us) {
if (!$us->allow_some_security()) {
return;
}
$pw = trim($us->qreq->oldpassword ?? "");
if ($pw === "") {
if (!$this->_approved) {
$this->_approval_errors[] = MessageItem::error_at("oldpassword", "<0>Current password required");
}
} else {
$info = $us->viewer->check_password_info($pw);
UpdateSession::usec_add_list($us->qreq, $us->viewer->email, $info["usec"] ?? [], 1);
$this->_approved = $info["ok"];
if (!$this->_approved) {
$this->_approval_errors[] = MessageItem::error_at("oldpassword", "<0>Incorrect current password");
}
}
$us->request_group("security");
}

/** @return bool */
function allow_security_changes() {
if (!$this->_approved && !$this->_complained) {
$this->_approval_errors[] = new MessageItem("oldpassword", "<0>Changes to security settings were ignored.", MessageSet::INFORM);
$this->us->append_list($this->_approval_errors);
}
return $this->_approved;
return $this->us->has_recent_authentication();
}

function print(UserStatus $us) {
Expand All @@ -69,7 +44,7 @@ function parse_qreq_new_password(UserStatus $us) {
$pw2 = trim($us->qreq->upassword2 ?? "");
$this->_req_passwords = [$us->qreq->upassword ?? "", $us->qreq->upassword2 ?? ""];
if (($pw === "" && $pw2 === "")
|| !$this->allow_security_changes()
|| !$us->has_recent_authentication()
|| !$us->viewer->can_edit_password($us->user)) {
return;
}
Expand All @@ -85,29 +60,30 @@ function parse_qreq_new_password(UserStatus $us) {
}
}

function print_current_password(UserStatus $us) {
if ($this->_approved) {
static function print_reauthenticate(UserStatus $us) {
if ($us->has_recent_authentication()) {
return;
}
$us->cs()->add_section_class("form-outline-section tag-yellow w-text")
->print_start_section("Confirm account", "reauth");
echo '<p>You must confirm your signin credentials before changing sensitive security settings.</p>';
$original_ignore_msgs = $us->swap_ignore_messages(false);
if ($us->is_auth_self()) {
$us->msg_at("oldpassword", "<0>Enter your current password to make changes to security settings", MessageSet::WARNING_NOTE);
} else {
$us->msg_at("oldpassword", "<0>Enter your current password to make changes to other users’ security settings", MessageSet::WARNING_NOTE);
}
$us->swap_ignore_messages($original_ignore_msgs);
echo '<div class="', $us->control_class("oldpassword", "f-i w-text"), '">',
'<label for="oldpassword">',
echo '<div class="', $us->control_class("reauth:password", "f-i w-text"), '">',
'<label for="reauth:password">',
$us->is_auth_self() ? "Current password" : "Current password for " . htmlspecialchars($us->viewer->email),
'</label>',
$us->feedback_html_at("oldpassword"),
Ht::entry("viewer_email", $us->viewer->email, ["autocomplete" => "username", "class" => "hidden ignore-diff", "readonly" => true]),
Ht::password("oldpassword", "", ["size" => 52, "autocomplete" => "current-password", "class" => "ignore-diff uii js-profile-current-password", "id" => "oldpassword", "autofocus" => true]),
Ht::password("reauth:password", "", ["size" => 52, "autocomplete" => "current-password", "class" => "ignore-diff", "id" => "reauth:password", "autofocus" => true]),
'</div>',
'<div class="aab aabig mb-0">',
Ht::submit("reauth", "Confirm account", ["class" => "btn-success mt-0"]),
'</div>';
}

function print_new_password(UserStatus $us) {
if (!$us->viewer->can_edit_password($us->user)) {
if (!$us->viewer->can_edit_password($us->user)
|| !$us->has_recent_authentication()) {
return;
}
$us->print_start_section("Change password");
Expand All @@ -117,19 +93,19 @@ function print_new_password(UserStatus $us) {
echo '<div class="has-fold foldc ui-fold js-fold-focus">';
if (!$open) {
echo '<div class="fn">',
Ht::button("Change password", ["class" => "ui js-foldup need-profile-current-password", "disabled" => !$this->_approved]),
Ht::button("Change password", ["class" => "ui js-foldup"]),
'</div><div class="fx">';
}
echo '<div class="', $us->control_class("password", "f-i w-text"), '">',
'<label for="upassword">New password</label>',
$us->feedback_html_at("password"),
$us->feedback_html_at("upassword"),
Ht::password("upassword", $pws[0], ["size" => 52, "autocomplete" => $us->autocomplete("new-password"), "disabled" => !$this->_approved, "class" => "need-profile-current-password want-focus"]),
Ht::password("upassword", $pws[0], ["size" => 52, "autocomplete" => $us->autocomplete("new-password"), "class" => "want-focus"]),
'</div>',
'<div class="', $us->control_class("upassword2", "f-i w-text"), '">',
'<label for="upassword2">Repeat new password</label>',
$us->feedback_html_at("upassword2"),
Ht::password("upassword2", $pws[1], ["size" => 52, "autocomplete" => $us->autocomplete("new-password"), "disabled" => !$this->_approved, "class" => "need-profile-current-password"]),
Ht::password("upassword2", $pws[1], ["size" => 52, "autocomplete" => $us->autocomplete("new-password")]),
'</div>', $open ? '' : '</div>', '</div>';
$us->mark_inputs_printed();
}
Expand Down
Loading

0 comments on commit aeaf266

Please sign in to comment.