Skip to content

Commit

Permalink
🥅 Make access control mandatory
Browse files Browse the repository at this point in the history
Apparently no matter how often you tell people to not put their stuff
unsecured on the public internet, they still will do that happily and
then I get the resulting fallout from the security community.

So you now will HAVE to enable access control. No more disabled ACL.
Apparently you can't trust people with this kind of responsibility.

If you want a bit more convenience, look into the trusted network
settings in config.yaml. And get a password manager. And stop port
forwarding and exposing your OctoPrint on the public internet please -
it can be found, you are not invisible, and you are putting yourself at
risk. Learn what a VPN is and to set one up on your router, that is a
valuable skill in any case.
  • Loading branch information
foosel committed Oct 21, 2020
1 parent fd319fc commit c0c6dd7
Show file tree
Hide file tree
Showing 19 changed files with 96 additions and 230 deletions.
30 changes: 3 additions & 27 deletions src/octoprint/access/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ def __init__(self, group_manager, settings=None):
self._logger = logging.getLogger(__name__)
self._session_users_by_session = {}
self._sessionids_by_userid = {}
self._enabled = True

if settings is None:
settings = s()
Expand All @@ -50,29 +49,14 @@ def unregister_login_status_listener(self, listener):
self._login_status_listeners.remove(listener)

def anonymous_user_factory(self):
if self.enabled:
return AnonymousUser([self._group_manager.guest_group])
else:
return AdminUser(
[self._group_manager.admin_group, self._group_manager.user_group]
)
return AnonymousUser([self._group_manager.guest_group])

def api_user_factory(self):
return ApiUser([self._group_manager.admin_group, self._group_manager.user_group])

@property
def enabled(self):
return self._enabled

@enabled.setter
def enabled(self, value):
self._enabled = value

def enable(self):
self._enabled = True

def disable(self):
self._enabled = False
return True

def login_user(self, user):
self._cleanup_sessions()
Expand Down Expand Up @@ -112,7 +96,7 @@ def login_user(self, user):
return user

def logout_user(self, user, stale=False):
if user is None or user.is_anonymous or isinstance(user, AdminUser):
if user is None or user.is_anonymous:
return

if isinstance(user, LocalProxy):
Expand Down Expand Up @@ -1444,11 +1428,3 @@ def __repr__(self):
class ApiUser(User):
def __init__(self, groups):
User.__init__(self, "_api", "", True, [], groups)


##~~ User object to use when access control is disabled


class AdminUser(User):
def __init__(self, groups):
User.__init__(self, "_admin", "", True, [], groups)
2 changes: 0 additions & 2 deletions src/octoprint/cli/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@ def user(ctx):
err=True,
)
user_manager = FilebasedUserManager(group_manager, settings=settings)
finally:
user_manager.enabled = settings.getBoolean(["accessControl", "enabled"])

ctx.obj.user_manager = user_manager

Expand Down
6 changes: 1 addition & 5 deletions src/octoprint/plugins/appkeys/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,11 +419,7 @@ def _user_for_api_key(self, api_key):
with self._keys_lock:
for user_id, data in self._keys.items():
if any(filter(lambda x: x.api_key == api_key, data)):
if self._user_manager.enabled:
return self._user_manager.find_user(userid=user_id)
elif user_id == "_admin" or user_id == "dummy":
# dummy = backwards compatible
return self._user_manager.anonymous_user_factory()
return self._user_manager.find_user(userid=user_id)
return None

def _api_keys_for_user(self, user_id):
Expand Down
15 changes: 7 additions & 8 deletions src/octoprint/plugins/backup/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1101,14 +1101,13 @@ def _restore_backup(
with io.open(configfile, "rt", encoding="utf-8") as f:
configdata = yaml.safe_load(f)

if configdata.get("accessControl", {}).get("enabled", True):
userfile = os.path.join(temp, "basedir", "users.yaml")
if not os.path.exists(userfile):
if callable(on_invalid_backup):
on_invalid_backup("Backup lacks users.yaml")
if callable(on_restore_failed):
on_restore_failed(path)
return False
userfile = os.path.join(temp, "basedir", "users.yaml")
if not os.path.exists(userfile):
if callable(on_invalid_backup):
on_invalid_backup("Backup lacks users.yaml")
if callable(on_restore_failed):
on_restore_failed(path)
return False

if callable(on_log_progress):
on_log_progress("Unpacked")
Expand Down
2 changes: 1 addition & 1 deletion src/octoprint/plugins/corewizard/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def add_result(key, method):
return result

def get_wizard_version(self):
return 3
return 4

# ~~ helpers

Expand Down
77 changes: 32 additions & 45 deletions src/octoprint/plugins/corewizard/static/js/corewizard.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,19 @@ $(function () {
self.confirmedPassword = ko.observable(undefined);

self.setup = ko.observable(false);
self.decision = ko.observable();

self.required = false;

self.passwordMismatch = ko.pureComputed(function () {
return self.password() != self.confirmedPassword();
return self.password() !== self.confirmedPassword();
});

self.validUsername = ko.pureComputed(function () {
return self.username() && self.username().trim() != "";
return self.username() && self.username().trim() !== "";
});

self.validPassword = ko.pureComputed(function () {
return self.password() && self.password().trim() != "";
return self.password() && self.password().trim() !== "";
});

self.validData = ko.pureComputed(function () {
Expand All @@ -31,47 +30,38 @@ $(function () {
);
});

self.keepAccessControl = function () {
self.createAccount = function () {
if (!self.validData()) return;

var data = {
ac: true,
user: self.username(),
pass1: self.password(),
pass2: self.confirmedPassword()
};
self._sendData(data);
};

self.disableAccessControl = function () {
var message = gettext(
"If you disable Access Control <strong>and</strong> your OctoPrint installation is accessible from the internet, your printer <strong>will be accessible by everyone - that also includes the bad guys!</strong>"
);
showConfirmationDialog({
message: message,
onproceed: function (e) {
var data = {
ac: false
};
self._sendData(data);
}
});
};

self._sendData = function (data, callback) {
OctoPrint.postJson("plugin/corewizard/acl", data).done(function () {
self.setup(true);
self.decision(data.ac);
if (data.ac) {
// we now log the user in
var user = data.user;
var pass = data.pass1;
self.loginStateViewModel.login(user, pass, true).done(function () {
if (callback) callback();
});
} else {

// we now log the user in
var user = data.user;
var pass = data.pass1;
self.loginStateViewModel.login(user, pass, true).done(function () {
if (callback) callback();
}
});
});
};

self._showDecisionNeededDialog = function () {
showMessageDialog({
title: gettext("Please set up Access Control"),
message: gettext(
"You haven't yet set up access control. You need to setup a " +
'username and password and click "Create Account" before ' +
"continuing."
)
});
};

Expand All @@ -85,12 +75,17 @@ $(function () {
) {
return true;
}
showMessageDialog({
title: gettext("Please set up Access Control"),
message: gettext(
'You haven\'t yet set up access control. You need to either setup a username and password and click "Keep Access Control Enabled" or click "Disable Access Control" before continuing'
)
});

self._showDecisionNeededDialog();
return false;
};

self.onBeforeWizardFinish = function () {
if (!self.required) return true;

if (self.setup()) return true;

self._showDecisionNeededDialog();
return false;
};

Expand All @@ -102,14 +97,6 @@ $(function () {
response.corewizard.details.acl &&
response.corewizard.details.acl.required;
};

self.onWizardFinish = function () {
if (!self.required) return;

if (!self.decision()) {
return "reload";
}
};
}

function CoreWizardWebcamViewModel(parameters) {
Expand Down
19 changes: 5 additions & 14 deletions src/octoprint/plugins/corewizard/subwizards.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ def _get_webcam_wizard_name(self):
# noinspection PyUnresolvedReferences,PyMethodMayBeStatic
class AclSubwizard(object):
def _is_acl_wizard_firstrunonly(self):
return True
return False

def _is_acl_wizard_required(self):
return self._user_manager.enabled and not self._user_manager.has_been_customized()
return not self._user_manager.has_been_customized()

def _get_acl_wizard_details(self):
return {"required": self._is_acl_wizard_required()}
Expand All @@ -81,11 +81,11 @@ def _get_acl_additional_wizard_template_data(self):
def acl_wizard_api(self):
from flask import abort, request

from octoprint.server.api import NO_CONTENT, valid_boolean_trues
from octoprint.server.api import NO_CONTENT

if (
not self._settings.global_get(["server", "firstRun"])
or self._user_manager.has_been_customized()
and self._user_manager.has_been_customized()
):
abort(404)

Expand All @@ -94,17 +94,12 @@ def acl_wizard_api(self):
data = request.values

if (
"ac" in data
and data["ac"] in valid_boolean_trues
and "user" in data
"user" in data
and "pass1" in data
and "pass2" in data
and data["pass1"] == data["pass2"]
):
# configure access control
self._settings.global_set_boolean(["accessControl", "enabled"], True)
self._user_manager.enable()

self._user_manager.add_user(
data["user"],
data["pass1"],
Expand All @@ -113,10 +108,6 @@ def acl_wizard_api(self):
[USER_GROUP, ADMIN_GROUP],
overwrite=True,
)
elif "ac" in data and data["ac"] not in valid_boolean_trues:
# disable access control
self._settings.global_set_boolean(["accessControl", "enabled"], False)
self._user_manager.disable()
self._settings.save()
return NO_CONTENT

Expand Down
Original file line number Diff line number Diff line change
@@ -1,26 +1,10 @@
<h3>{{ _('Access Control') }}</h3>

{% trans safe_remote_url='https://octoprint.org/blog/2018/09/03/safe-remote-access/' %}<p>
<strong>Please read the following, it is very important for your printer's health!</strong>
</p>
<p>
OctoPrint by default ships with Access Control enabled, meaning you won't be able to do anything with the
printer unless you login first as a configured user. This is to <strong>prevent strangers - possibly with
malicious intent - to gain access to your printer</strong> via an untrustworthy network
and using it in such a way that it is damaged or worse (i.e. causes a fire).
</p>
<p>
If you plan on accessing OctoPrint remotely over the internet, <strong>you should not only rely on the built-in
Access Control mechanisms</strong> however -
<a href="{{ safe_remote_url }}" target="_blank" rel="noopener noreferrer">take additional precautions</a>.
Your OctoPrint enabled printer is a critical home appliance you really should not give anyone who happens to be
connected to the internet access to - even read-only! <strong>An instance available publicly on the internet
will be found, and people will try to break it open.</strong>
</p>
<p>
It looks like you haven't configured access control yet. Please <strong>set up a username and password</strong> for the
initial administrator account who will have full access to both the printer and OctoPrint's settings, then click
on "Keep Access Control Enabled":
{% trans %}<p>
It looks like you haven't configured access control yet, which is now mandatory.
Please <strong>set up a username and password</strong> for the
initial administrator account who will have full access to both the printer and
OctoPrint's settings:
</p>{% endtrans %}
<form class="form-horizontal" onsubmit="return false;">
<div class="control-group" data-bind="css: {success: validUsername()}">
Expand All @@ -42,26 +26,7 @@
<span class="help-inline" data-bind="visible: passwordMismatch()">{{ _('Passwords do not match') }}</span>
</div>
</div>
<div class="controls">
<a href="#" class="btn btn-primary" data-bind="click: function() { if(!setup()){createAccount()}}, enable: !setup() && validData(), css: {disabled: !validData() || setup()}">{{ _('Create Account') }}</a>
</div>
</form>
{% trans %}<p>
<strong>Note:</strong> In case that your OctoPrint installation is only accessible from within a trustworthy local network and you don't
need Access Control for other reasons, you may alternatively disable Access Control. You should only
do this if you are absolutely certain that only people you know and trust will be able to connect to it.
</p>
<p>
<strong>Do NOT underestimate the risk of access from the internet to your printer and OctoPrint instance!</strong>
</p>{% endtrans %}

<div class="row-fluid">
<a href="#" class="btn btn-danger span6" data-bind="click: function() { if(!setup()){disableAccessControl()}}, enable: !setup(), css: {disabled: setup()}">{{ _('Disable Access Control') }}</a>
<a href="#" class="btn btn-primary span6" data-bind="click: function() { if(!setup()){keepAccessControl()}}, enable: !setup() && validData(), css: {disabled: !validData() || setup()}">{{ _('Keep Access Control Enabled') }}</a>
</div>

<div class="acl_decision" style="display: none" data-bind="visible: setup()">
<div class="text-center" style="display: none" data-bind="visible: decision()">{% trans %}
Access control is <strong class="text-success">enabled</strong>.
{% endtrans %}</div>
<div class="text-center" style="display: none" data-bind="visible: !decision()">{% trans %}
Access control is <strong class="text-danger">disabled</strong>.
{% endtrans %}</div>
</div>
8 changes: 2 additions & 6 deletions src/octoprint/server/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ def on_user_logged_out(sender, user=None):


def load_user(id):
if id is None or not userManager.enabled:
if id is None:
return None

if id == "_api":
Expand Down Expand Up @@ -553,10 +553,6 @@ def on_settings_update(*args, **kwargs):
"falling back to FilebasedUserManager!".format(user_manager_name)
)
userManager = octoprint.access.users.FilebasedUserManager(groupManager)
finally:
userManager.enabled = self._settings.getBoolean(
["accessControl", "enabled"]
)
components.update({"user_manager": userManager})

# create printer instance
Expand Down Expand Up @@ -1349,7 +1345,7 @@ def _get_locale(self):
if "X-Locale" in request.headers:
return Locale.negotiate([request.headers["X-Locale"]], LANGUAGES)

if hasattr(g, "identity") and g.identity and userManager.enabled:
if hasattr(g, "identity") and g.identity:
userid = g.identity.id
try:
user_language = userManager.get_user_setting(
Expand Down
Loading

0 comments on commit c0c6dd7

Please sign in to comment.