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.0rc3] Permission issue, can start print from Cura without such permission #3398

Closed
foosel opened this issue Dec 18, 2019 · 9 comments
Closed
Labels
bug Issue describes a bug done Done but not yet released
Milestone

Comments

@foosel
Copy link
Member

foosel commented Dec 18, 2019

Problem

@foosel

Could you please check what appkeys exist in your system and for which users, and that the key in Cura is actually for the user you expect it to be? You can do so via Settings > Application Keys.

I just tested again, making sure to be logged in as the api_user to get the API key (that has > limited access) and although I can not move or preheat the printer within Cura, slicing and clicking "Print with OctoPrint" does in fact upload the file (allowed) and start the print (shouldn't be allowed).

Originally posted by @gcurtis79 in #3389 (comment)

Solution

Make sure PRINT permission gets checked on evaluation of select and print request parameters on upload API, probably not the case right now, causing this issue.

@foosel foosel added the triage This issue needs triage label Dec 18, 2019
@foosel foosel added this to the 1.4.0 milestone Dec 18, 2019
@fieldOfView
Copy link
Contributor

Is this something you plan to fix today (this year) still? If not this sounds like something I could make a PR for, for you to check out next year.

@foosel
Copy link
Member Author

foosel commented Dec 18, 2019

Not today, so sure, go ahead :)

@fieldOfView
Copy link
Contributor

It looks like the PRINT permission is properly checked on evaluation of select and print request parameters on upload API, however the PRINT permission seems broken.

All permissions which specify additional needs (FILES_DOWNLOAD, FILES_DELETE, FILES_SELECT, PRINT, GCODE_VIEWER and TIMELAPSE_ADMIN) seem to always be available. This can also be seen in the UI by revoking the File List permission; the file list should disappear but it doesn't. Same with removing the File Delete permission, which should remove the trash icon from the file list but doesn't.

Removing the FILE_LIST need from the definition of these permissions fixes this. Removing the FILE_SELECT from the PRINT permission fixes the issue reported by @gcurtis79.

I've been looking why multiple needs are not working and will continue to do so.

@fieldOfView
Copy link
Contributor

fieldOfView commented Jan 2, 2020

My previous assessment seems incorrect. The problem is not so much multiple needs, but the FILES_LIST permission seems suspect.

See below

@fieldOfView
Copy link
Contributor

My original assessment was correct. I understand the Needs and Permissions a bit better now.

When an OctoPrintPermission has multiple Needs, Permission.allows() checks if there's an intersection between the needs of the Permission and the provides of the Identity. Because the length of the intersection is not checked, this is true if one or more needs overlap. So because the PRINT permission also has the FILES_SELECT permission, Permission.PRINT.can() returns true if the user does not provide a print Needs but has a files_select Needs.

The quickest fix is to remove the multiple Needs in the OctoPrintPermissions definitions. A better fix is to check the length of the intersection between the Identity provides and Permission needs to be the same as the Permission needs in Permission.allows, but that's an upstream fix.

@fieldOfView
Copy link
Contributor

fieldOfView commented Jan 14, 2020

This is fixed by 3229e65, because that removes the multiple Needs.

By doing that, it also sort of removes the necessity of #3414, but that PR might still be useful as defensive programming in the case that multiple Needs ever make a return (the implementation of having multiple Needs on a Permission is still there).

@foosel
Copy link
Member Author

foosel commented Jan 14, 2020

Yeah, I'm actually just now looking through it and trying to figure out if it will have any problematic side effects. Leaning towards a merge though.

@foosel
Copy link
Member Author

foosel commented Jan 14, 2020

PR merged and permission modelling changed, so this should be solved and ready for 1.4.0rc4

@foosel foosel added done Done but not yet released bug Issue describes a bug and removed triage This issue needs triage labels Jan 14, 2020
@foosel
Copy link
Member Author

foosel commented Jan 28, 2020

1.4.0rc4 is out

@foosel foosel closed this as completed Jan 28, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue describes a bug done Done but not yet released
Projects
None yet
Development

No branches or pull requests

2 participants