Defect #34029
closed403 Forbidden error when non-member try to upload a file
100%
Description
Hello
Our users encountered an error in a specific case, when uploading files.
Here is a screenshot showing the 403-forbidden error after the upload:
Steps to reproduce¶
This error happens in a specific case when the user is not a member of the project.
Here are the steps to reproduce the issue:
- The current user is NOT member of any project
- The build-in role "non-member" has NO permission at all
- In the project's members tab, a role is set for the member "non-member" and this role has permission to create and update issues
Logs¶
On the server side, here are the logs:
Started POST "/uploads.js?attachment_id=1&filename=image-test.jpg&content_type=image%2Fjpeg" for 127.0.0.1 at 2020-09-24 10:30:51 +0200
Processing by AttachmentsController#upload as JS
Parameters: {"attachment_id"=>"1", "filename"=>"image-test.jpg", "content_type"=>"image/jpeg"}
Token Update All (12.1ms) UPDATE "tokens" SET "updated_on" = '2020-09-24 10:30:51.312455' WHERE "tokens"."user_id" = $1 AND "tokens"."value" = $2 AND "tokens"."action" = $3 [["user_id", 14], ["value", "7d688080432d1c8ceafbd03811ad81dbf8193f1f"], ["action", "session"]]
(0.6ms) SELECT MAX("settings"."updated_on") FROM "settings"
User Load (0.5ms) SELECT "users".* FROM "users" WHERE "users"."type" IN ('User', 'AnonymousUser') AND "users"."status" = $1 AND "users"."id" = $2 LIMIT $3 [["status", 1], ["id", 14], ["LIMIT", 1]]
Current user: visitor (id=14)
Role Load (1.0ms) SELECT DISTINCT "roles".* FROM "roles" INNER JOIN "member_roles" ON "member_roles"."role_id" = "roles"."id" INNER JOIN "members" ON "members"."id" = "member_roles"."member_id" INNER JOIN "projects" ON "projects"."id" = "members"."project_id" WHERE (projects.status <> 9) AND "members"."user_id" = 14
Role Load (0.2ms) SELECT "roles".* FROM "roles" WHERE "roles"."builtin" = $1 LIMIT $2 [["builtin", 1], ["LIMIT", 1]]
Filter chain halted as :authorize_global rendered or redirected
Completed 403 Forbidden in 20ms (ActiveRecord: 14.4ms)
Configuration¶
The bug has been confirmed on the latest Redmine version, with no plugin installed.
Environment: Redmine version 4.1.1.stable Ruby version 2.6.6-p146 (2020-03-31) [x86_64-darwin19] Rails version 5.2.4.2 Environment development Database adapter PostgreSQL Mailer queue ActiveJob::QueueAdapters::AsyncAdapter Mailer delivery smtp SCM: Subversion 1.13.0 Git 2.24.1 Filesystem Redmine plugins: no plugin installed
Files
Updated by Vincent Robert almost 4 years ago
- File patch.diff patch.diff added
- Target version set to 4.1.2
Please find attached a diff file which contains:
- a system test to reproduce the error
- a patch to fix it
Thank you for considering this patch.
def test_create_issue_with_attachment_when_user_is_not_a_member
set_tmp_attachments_directory
# Set no permission to non-member role
non_member_role = Role.where(:builtin => Role::BUILTIN_NON_MEMBER).first
non_member_role.permissions = []
non_member_role.save
# Set role "Reporter" to non-member users on project ecookbook
membership = Member.find_or_create_by(user_id: Group.non_member.id, project_id: 1)
membership.roles = [Role.find(3)] # Reporter
membership.save
log_user('someone', 'foo')
issue = new_record(Issue) do
visit '/projects/ecookbook/issues/new'
fill_in 'Subject', :with => 'Issue with attachment'
attach_file 'attachments[dummy][file]', Rails.root.join('test/fixtures/files/testfile.txt')
fill_in 'attachments[1][description]', :with => 'Some description'
click_on 'Create'
end
assert_equal 1, issue.attachments.count
assert_equal 'Some description', issue.attachments.first.description
end
# authorize if user has at least one role that has this permission
- roles = self.roles.to_a | [builtin_role]
+ roles = self.roles.to_a | [builtin_role] | Group.non_member.roles.to_a | Group.anonymous.roles.to_a
roles.any? {|role|
role.allowed_to?(action) && ...
Updated by Marius BĂLTEANU over 3 years ago
- Target version changed from 4.1.2 to 4.2.0
Moving this to a major version (4.2.0) because the proposed patch moves a method from one class to another.
Updated by Marius BĂLTEANU over 3 years ago
- Target version changed from 4.2.0 to 5.0.0
Need more time to review and fix.
Updated by Rob Logan almost 3 years ago
+1
Hand applied the patch to my 4.2.2 system ( app/models/user.rb reformatted def roles) and it appears to solve the problem.
Updated by Dariusz Makowski over 2 years ago
I have the same problem. I'm on :
Environment: Redmine version 4.2.3.stable Ruby version 2.6.9-p207 (2021-11-24) [x86_64-linux] Rails version 5.2.6 Environment production Database adapter Mysql2 Mailer queue ActiveJob::QueueAdapters::AsyncAdapter Mailer delivery smtp SCM: Subversion 1.14.1 Git 2.34.1 Filesystem Redmine plugins:
I tried to add the tweak to user.rb like Robert Hammer Logan did, but sadly it does not work for me :- (
It actually broke more and I have Internal Error instead now.
Any hints? I want to allow self-registered-authenticated users to attach attachments...
Updated by Marius BĂLTEANU over 2 years ago
Dariusz Makowski wrote:
I have the same problem. I'm on :
[...]
I tried to add the tweak to user.rb like Robert Hammer Logan did, but sadly it does not work for me :- (
It actually broke more and I have Internal Error instead now.Any hints? I want to allow self-registered-authenticated users to attach attachments...
Can you check in the logs what error do you receive?
Updated by Marius BĂLTEANU over 2 years ago
- File 0001-Include-GroupNonMember-and-GroupAnonymous-roles-3402.patch 0001-Include-GroupNonMember-and-GroupAnonymous-roles-3402.patch added
Vincent, what do you think about the following fix?
I tried to fix the method User#roles
to include the roles assigned to Group Non Member
or Group Anonymous Users
groups. For example, roles_for_project
already do this.
Also, only the roles assigned to public projects are included in the query.
Updated by Vincent Robert over 2 years ago
Hello Marius. Thank you for this update.
About the fix, I think we should cumulate these roles.
A member should never have fewer permissions than a non-member ; a non-member should never have fewer permissions than an anonymous.
As I see it, a member should have all the permissions granted to these roles : member.roles + non_member.roles + anonymous.roles
Updated by Marius BĂLTEANU over 2 years ago
roles_for_project
(source:/trunk/app/models/user.rb#L637) doesn't do this, it overrides the roles only if the user is not a member. At the same time, being a global context, I agree that it makes sense to cumulate them or do cumulate the roles from public projects where the user is not a member.
Being a quite important design choice, I would like more feedback.
What do you think if I commit the fix just for this issue for now to catch the next releases and discuss more in a different issue?
Updated by Go MAEDA over 2 years ago
Vincent Robert wrote:
About the fix, I think we should cumulate these roles.
A member should never have fewer permissions than a non-member ; a non-member should never have fewer permissions than an anonymous.
I am against the change. I think it is confusing if the set of permissions for a role is different from as configured. And the change may damage the flexibility of permissions and roles.
Updated by Vincent Robert over 2 years ago
It may actually be a different topic. I agree, we can first fix the bug with the 403 error, without modifying current roles' inheritance.
Updated by Marius BĂLTEANU over 2 years ago
- Status changed from New to Closed
- % Done changed from 0 to 100
Applied in changeset r21502.