Inherited bless permissions without inherited membership do not work

RESOLVED FIXED in Bugzilla 2.20

Status

()

Bugzilla
Administration
--
major
RESOLVED FIXED
14 years ago
13 years ago

People

(Reporter: Wurblzap, Assigned: Wurblzap)

Tracking

({regression})

2.19.2
Bugzilla 2.20
regression
Bug Flags:
approval +
approval2.20 +
blocking2.20 +

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

14 years ago
(In reply to bug 119485, comment #36)

This addresses the following:

> >+    $vars->{'restrictablegroups'} = groupsUserMayBless($user, 'id', 'name');
> 
>   That duplicates Bugzilla->user->bless_groups, I'm pretty sure, since I
> checked that code in.
[...]
> >+# Give a list of IDs of groups the user may bless.
> >+sub groupsUserMayBless {
> 
>   As I mentioned above, you can do most of this with
> Bugzilla->user->bless_groups. You could have also done it with
> UserCanBlessGroups() or whatever it was called before in globals.pl.
(Assignee)

Updated

13 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 2.20
(Assignee)

Comment 1

13 years ago
It turns out groupsUserMayBless is doing its indirect bless permission check
incorrectly, which makes a move to bless_groups even more interesting.
Requesting blocking2.20.
Flags: blocking2.20?

Comment 2

13 years ago
Maybe 2.20.1 -- I *think* the only thing this will cause will be cosmetic errors
on the editusers.cgi page when you are looking at what groups a user inherits
for blessing, in a few cases.

However, if we can get the patch r+'ed before we release 2.20, I'd be all for
getting it in on the 2.20 branch. :-)
Flags: blocking2.20? → blocking2.20-
(Assignee)

Comment 3

13 years ago
Created attachment 190224 [details] [diff] [review]
Patch

(In reply to comment #2)
> Maybe 2.20.1 -- I *think* the only thing this will cause will be cosmetic

Unfortunately not...

o Let user UA be a member of group GA
o Let group GA inherit bless membership for group GB
o User UA should be allowed to bless GB, but isn't :(
Attachment #190224 - Flags: review?
(Assignee)

Updated

13 years ago
Flags: blocking2.20- → blocking2.20?

Comment 4

13 years ago
Yeah, in light of the fact that this actually breaks bless inheritance, it
should block. :-(
Severity: normal → major
Flags: blocking2.20? → blocking2.20+

Updated

13 years ago
Keywords: regression
Summary: Make editusers.cgi use Bugzilla::User::bless_groups instead of local groupsUserMayBless → Inherited bless permissions without inherited membership do not work
(Assignee)

Comment 5

13 years ago
When reviewing the patch, please note especially that groupsUserMayBless
contained a derive_groups call while bless_groups does not. This works for me.
(Assignee)

Updated

13 years ago
Attachment #190224 - Flags: review?
(Assignee)

Comment 6

13 years ago
Created attachment 190387 [details] [diff] [review]
Patch

Unrotted after checkin of bug 284264.
Attachment #190224 - Attachment is obsolete: true
Attachment #190387 - Flags: review?
(Assignee)

Updated

13 years ago
Whiteboard: [Patch awaiting review]

Comment 7

13 years ago
Comment on attachment 190387 [details] [diff] [review]
Patch

code reads OK.	I just need to test it.  Has anyone else already tested it?
(Assignee)

Comment 8

13 years ago
No, probably just I.

Comment 9

13 years ago
Comment on attachment 190387 [details] [diff] [review]
Patch

r=joel
I'd like either a 2xr or someone to do some more testing before it lands.
Attachment #190387 - Flags: review+
(Assignee)

Updated

13 years ago
Whiteboard: [Patch awaiting review] → [patch awaiting second-review]

Comment 10

13 years ago
Comment on attachment 190387 [details] [diff] [review]
Patch

works well. r=LpSolit
Attachment #190387 - Flags: review? → review+

Updated

13 years ago
Flags: approval?
Flags: approval2.20?
Whiteboard: [patch awaiting second-review]

Comment 11

13 years ago
oops. The patch doesn't apply cleanly on 2.20.
Flags: approval?
Flags: approval2.20?
Flags: approval2.20+
Flags: approval+
(Assignee)

Comment 12

13 years ago
Created attachment 192064 [details] [diff] [review]
2.20 backport

(In reply to comment #11)
> oops. The patch doesn't apply cleanly on 2.20.

Yes, that's because bug 284264 didn't go onto the branch. The backport
therefore contains parts of it.
Attachment #192064 - Flags: review?(LpSolit)
(Assignee)

Updated

13 years ago
Whiteboard: [ready for trunk][2.20 patch awaiting review]

Comment 13

13 years ago
This should get approval re-requested when the branch patch is ready.
Flags: approval2.20+
Flags: approval+

Comment 14

13 years ago
Comment on attachment 192064 [details] [diff] [review]
2.20 backport

tested, works correctly.
Attachment #192064 - Flags: review?(LpSolit) → review+

Comment 15

13 years ago
requesting approval again
Flags: approval?
Flags: approval2.20?
Whiteboard: [ready for trunk][2.20 patch awaiting review]
Flags: approval?
Flags: approval2.20?
Flags: approval2.20+
Flags: approval+

Comment 16

13 years ago
tip:

Checking in editusers.cgi;
/cvsroot/mozilla/webtools/bugzilla/editusers.cgi,v  <--  editusers.cgi
new revision: 1.96; previous revision: 1.95
done
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v  <--  User.pm
new revision: 1.73; previous revision: 1.72
done


2.20rc2:

Checking in editusers.cgi;
/cvsroot/mozilla/webtools/bugzilla/editusers.cgi,v  <--  editusers.cgi
new revision: 1.90.2.3; previous revision: 1.90.2.2
done
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v  <--  User.pm
new revision: 1.61.2.5; previous revision: 1.61.2.4
done
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.