- Notifications
You must be signed in to change notification settings - Fork928
fix: allow regular users to push files#4500
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
- As part of merging support for Template RBAC and user groups a permission check on reading files was relaxed. With the addition of admin roles on individual templates, regular users are now able to push template versions if they have inherited the 'admin' role for a template. In order to do so they need to be able to create and read their own files. Since collisions on hash in the past were ignored, this means that a regular user who pushes a template version with a file hash that collides with an existing hash will not be able to read the file (since it belongs to another user). This commit fixes the underlying problem which was that the files table had a primary key on the 'hash' column. This was not a problem at the time because only template admins and other users with similar elevated roles were able to read all files regardless of ownership. To fix this a new column and primary key 'id' has been introduced to the files table. The unique constraint has been updated to be hash+created_by. Tables (provisioner_jobs) that referenced files.hash have been updated to reference files.id. Relevant API endpoints have also been updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LG. PR comment is great.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Should we mark this a breaking change or add backwards compat? It looks like it prevents newer clients from uploading to previous versions of the server (the error is not very user friendly):
|
Uh oh!
There was an error while loading.Please reload this page.
As part of merging support for Template RBAC
and user groups a permission check on reading files
was relaxed.
With the addition of admin roles on individual templates, regular
users are now able to push template versions if they have
inherited the 'admin' role for a template. In order to do so
they need to be able to create and read their own files. Since
collisions on hash in the past were ignored, this means that a regular user
who pushes a template version with a file hash that collides with
an existing hash will not be able to read the file (since it belongs to
another user).
This commit fixes the underlying problem which was that
the files table had a primary key on the 'hash' column.
This was not a problem at the time because only template
admins and other users with similar elevated roles were
able to read all files regardless of ownership. To fix this
a new column and primary key 'id' has been introduced to the files
table. The unique constraint has been updated to be hash+created_by.
Tables (provisioner_jobs) that referenced files.hash have been updated
to reference files.id. Relevant API endpoints have also been updated.
fixes#4415