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

Inconsistent Guest Permissions in printerstate.js #4992

Open
1 of 4 tasks
codeceptsDE opened this issue Apr 16, 2024 · 1 comment
Open
1 of 4 tasks

Inconsistent Guest Permissions in printerstate.js #4992

codeceptsDE opened this issue Apr 16, 2024 · 1 comment
Labels
bug Issue describes a bug done Done but not yet released
Milestone

Comments

@codeceptsDE
Copy link

codeceptsDE commented Apr 16, 2024

The problem

When accessing OctoPrint as guest, you can (if set up correctly) start a print, pause a print, but never cancel a print.
If you can start a print, and are also allowed to pause a print (potentially ruining the part, too), you should be allowed to cancel a print. This behavior is inconsistent otherwise.
Any logged in user, even with the most restrictive permissions, would be able to cancel a print.

This issue is due to the way permissions are checked in printerstate.js:

self.enablePrint = ko.pureComputed(function () {
return (
self.isOperational() &&
(self.isReady() || self.isPaused()) &&
!self.isPrinting() &&
!self.isCancelling() &&
!self.isPausing() &&
self.loginState.hasPermission(self.access.permissions.PRINT) &&
self.filename()
);
});
self.enablePause = ko.pureComputed(function () {
return (
self.isOperational() &&
(self.isPrinting() || self.isPaused()) &&
!self.isCancelling() &&
!self.isPausing() &&
self.loginState.hasPermission(self.access.permissions.PRINT)
);
});
self.enableCancel = ko.pureComputed(function () {
return (
self.isOperational() &&
(self.isPrinting() || self.isPaused()) &&
!self.isCancelling() &&
!self.isPausing() &&
self.loginState.loggedIn()
);
});

The print and pause button check self.loginState.hasPermission(self.access.permissions.PRINT), while cancel requires self.loginState.loggedIn(). It should be the former as well, or maybe self.loginState.loggedIn() || self.loginState.hasPermission(self.access.permissions.PRINT) to prevent guests in an unsecured environment from messing with prints.

I am not sure if there are changes required in the backend, too.
As far as I am aware, the variant with the OR above has no adverse security implications, as by default, guests do not have print permission.

I use OctoPrint in an isolated LAN environment, and I want all users to be able to use the basic functionalities of the printer without requiring a login. This currently works, besides this limitation that guests with print permission cannot cancel a print.

Did the issue persist even in safe mode?

Yes, it did persist

If you could not test in safe mode, please state why ("currently printing" is NOT an excuse!)

No response

Version of OctoPrint

1.9.3

Operating system running OctoPrint

OctoPi

Printer model & used firmware incl. version

No response

Browser and version of browser, operating system running browser

No response

Checklist of files to include below

  • Systeminfo Bundle (always include!)
  • Contents of the JavaScript browser console (always include in cases of issues with the user interface)
  • Screenshots and/or videos showing the problem (always include in case of issues with the user interface)
  • GCODE file with which to reproduce (always include in case of issues with GCODE analysis or printing behaviour)

Additional information & file uploads

No response
octoprint-systeminfo-20240416093746.zip

@github-actions github-actions bot added the triage This issue needs triage label Apr 16, 2024
@foosel foosel added this to the 1.10.x milestone May 21, 2024
@foosel foosel added bug Issue describes a bug and removed triage This issue needs triage labels May 21, 2024
foosel added a commit that referenced this issue May 21, 2024
The cancel button was deactivated if the user was logged out,
even if they have the PRINT permission. That was probably
a left-over from the time before the granular permission system
existed.

Closes #4992
@foosel
Copy link
Member

foosel commented May 21, 2024

That was indeed only the permission check in the frontend that was wonky, and purely visual too. The backend already was adjusted to only require PRINT, and the API client didn't do any further checks browser-side. This was probably a leftover from the time prior to having the granular permission system and simply slipped through so far. Thanks for spotting this! It will be fixed in 1.10.2.

@foosel foosel added the done Done but not yet released label May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue describes a bug done Done but not yet released
Projects
Status: Done
Development

No branches or pull requests

2 participants