Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.4.0rc2]global API KEY don't have all permission #3365

Closed
devildant opened this issue Dec 2, 2019 · 8 comments
Closed

[1.4.0rc2]global API KEY don't have all permission #3365

devildant opened this issue Dec 2, 2019 · 8 comments

Comments

@devildant
Copy link
Contributor

@devildant devildant commented Dec 2, 2019

What were you doing?

following the RC update, I see that it is no longer possible to call the API of plugins.
i take an error 403.

exemple with shutdown printer plugin :
url : /api/plugin/shutdownprinter
body : {"command":"enable", "eventView" : true}
API key in header : X-Api-Key=666666......

result :
http code: 403
body : Insufficient rights

inside method on_api_command (shutdown printer plugin) i put this control :

def on_api_command(self, command, data):
                if not user_permission.can():
                        return make_response("Insufficient rights", 403)

but in 1.3.12 no pb

I also try with user API it's work correctly, but with global api key no

What did you expect to happen?

What happened instead?

Did the same happen when running OctoPrint in safe mode?

Version of OctoPrint

Operating System running OctoPrint

Printer model & used firmware incl. version

Browser and version of browser, operating system running browser

Link to octoprint.log

Link to contents of terminal tab or serial.log

Link to contents of Javascript console in the browser

Screenshot(s)/video(s) showing the problem:

I have read the FAQ.

@GitIssueBot

This comment has been minimized.

Copy link
Collaborator

@GitIssueBot GitIssueBot commented Dec 2, 2019

Hi @devildant,

It looks like there is some information missing from your bug report that will be needed in order to solve the problem. Read the Contribution Guidelines which will provide you with a template to fill out here so that your bug report is ready to be investigated (I promise I'll go away then too!).

If you did not intend to report a bug but wanted to request a feature or brain storm about some kind of development, please take special note of the title format to use as described in the Contribution Guidelines.

Please do not abuse the bug tracker as a support forum - that can be found at discourse.octoprint.org. Go there for any kind of issues with network connectivity, webcam functionality, printer detection or any other kind of such support requests or general questions.

Also make sure you are at the right place - this is the bug tracker of the official version of OctoPrint, not the Raspberry Pi image OctoPi nor any unbundled third party OctoPrint plugins or unofficial versions. Make sure too that you have read through the Frequently Asked Questions and searched the existing tickets for your problem - try multiple search terms please.

I'm marking this one now as needing some more information. Please understand that if you do not provide that information within the next two weeks (until 2019-12-16 22:10 UTC) I'll close this ticket so it doesn't clutter the bug tracker. This is nothing personal, so please just be considerate and help the maintainers solve this problem quickly by following the guidelines linked above. Remember, the less time the devs have to spend running after information on tickets, the more time they have to actually solve problems and add awesome new features. Thank you!

Best regards,
~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being, so don't expect any replies from me :) Your ticket is read by humans too, I'm just not one of them.

@devildant

This comment has been minimized.

Copy link
Contributor Author

@devildant devildant commented Dec 2, 2019

I try to replace the permission control and it seems to work

if not user_permission.can():
                        return make_response("Insufficient rights", 403)

become

 user_id = current_user.get_name()
                if not user_id or not admin_permission.can():
                        return make_response("Insufficient rights", 403)

but inside the octoprint code i see that :

# we set admin_permission to a GroupPermission with the default admin group
admin_permission = octoprint.util.variable_deprecated("admin_permission has been deprecated, "
                                                      "please use individual Permissions instead",
                                                      since="1.4.0")(groups.GroupPermission(groups.ADMIN_GROUP))

# we set user_permission to a GroupPermission with the default user group
user_permission = octoprint.util.variable_deprecated("user_permission has been deprecated, "
                                                     "please use individual Permissions instead",
                                                     since="1.4.0")(groups.GroupPermission(groups.USER_GROUP))

what is the right solution for controlling permissions on plug-in APIs?

@foosel foosel changed the title [RC 1.4.0rc2]global API KEY don't have all permission [1.4.0rc2]global API KEY don't have all permission Dec 3, 2019
@foosel

This comment has been minimized.

Copy link
Owner

@foosel foosel commented Dec 3, 2019

what is the right solution for controlling permissions on plug-in APIs?

Declaring them for your plugins and using them instead of the generic and deprecated admin/user permissions. There's a new hook for that called octoprint.access.permissions (which I just realized I forgot to document, so thanks for that reminder!) and you can see how that is used in the various bundled plugins, e.g. the appkeys plugin.

Still, the global API key should indeed always work, so I'll look into that as well here.

foosel added a commit that referenced this issue Dec 3, 2019
Closes #3365
@foosel foosel added this to the 1.4.0 milestone Dec 3, 2019
foosel added a commit that referenced this issue Dec 3, 2019
@foosel

This comment has been minimized.

Copy link
Owner

@foosel foosel commented Dec 3, 2019

Should be fixed and I also added the missing docs.

@devildant

This comment has been minimized.

Copy link
Contributor Author

@devildant devildant commented Dec 3, 2019

thx @foosel ,
I just have a comment, it missing in the documentation the import necessary for use permission hook, and there will be problems of backward compatibility with version 1.3.12 (for those who will not update octoprint)
I fix this in the example below:

# coding=utf-8
from __future__ import absolute_import
from flask import make_response


#for retrocompatibility
try:
	from octoprint.access.permissions import Permissions, ADMIN_GROUP, USER_GROUP
except (ImportError, RuntimeError):
	from octoprint.server import current_user, admin_permission, user_permission

from flask_babel import gettext

import octoprint.plugin

class TestPlugin(octoprint.plugin.StartupPlugin,
                       octoprint.plugin.TemplatePlugin,
                       octoprint.plugin.SettingsPlugin,
					   octoprint.plugin.SimpleApiPlugin,
                       octoprint.plugin.AssetPlugin):
	def on_after_startup(self):
		self._logger.info("test init")

	def get_api_commands(self):
		return dict()
	
	def get_additional_permissions(self):
		return [
				dict(key="ADMIN",
					name="test plugin",
					description=gettext("Allows to access to api of plugin test."),
					default_groups=[ADMIN_GROUP],
					roles=["admin"],
					dangerous=True)
				]	
				
	def on_api_command(self, command, data):
		#for retrocompatibility
		try:
			plugin_permission = Permissions.PLUGIN_TEST_ADMIN.can()
		except:
			plugin_permission = user_permission.can()
		if not plugin_permission:
			return make_response("Insufficient rights", 403)
			
	def get_settings_defaults(self):
		return dict()

	def get_template_configs(self):
		return [
			dict(type="settings", custom_bindings=False)
		]

	def get_assets(self):
		return dict(
			js=["js/test.js"],
			css=["css/test.css"]
		)

__plugin_name__ = "Test"

def __plugin_load__():
	global __plugin_implementation__
	__plugin_implementation__ = TestPlugin()

	global __plugin_hooks__
	__plugin_hooks__ = {
		"octoprint.access.permissions": __plugin_implementation__.get_additional_permissions
	}

import necessary :
from octoprint.access.permissions import Permissions, ADMIN_GROUP, USER_GROUP
from flask_babel import gettext

@devildant

This comment has been minimized.

Copy link
Contributor Author

@devildant devildant commented Dec 6, 2019

it's just a small recommendation for the doc :)

@foosel

This comment has been minimized.

Copy link
Owner

@foosel foosel commented Dec 9, 2019

I know. I'd rather have plugin authors encourage people to update to newer OctoPrint versions though ;) I'll think about it, currently still hunting down a few reported bugs, that takes priority.

@foosel

This comment has been minimized.

Copy link
Owner

@foosel foosel commented Dec 12, 2019

1.4.0rc3 has just been released

@foosel foosel closed this Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.