1163399 - Autocompletion no longer works when impersonating a user

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Frédéric Buclin]

Autocompletion no longer works when impersonating a user. It was working fine with YUI. The error I get is: "HTTP/1.1 400 Bad Request". If I log in as this user directly, it's working fine. So I suspect this has something to do with a bad interaction between the user token and the sudoer login cookie, or something like that (I didn't look at the new code).

Comment 1

[David Lawrence [:dkl]]

Attachment: 1163399_1.patch

Comment 2

[Frédéric Buclin]

Comment on attachment 8604149 [details] [diff] [review]
1163399_1.patch

>diff --git a/template/en/default/admin/sudo.html.tmpl b/template/en/default/admin/sudo.html.tmpl

Err, this is not what I'm talking about. I'm talking about all Bugzilla pages where autocompletion is supposed to occur, such as in bug reports. We never used autocompletion for this sudo page.

Comment 3

[David Lawrence [:dkl]]

(In reply to Frédéric Buclin from comment #2)
> Comment on attachment 8604149 [details] [diff] [review]
> 1163399_1.patch
> 
> >diff --git a/template/en/default/admin/sudo.html.tmpl b/template/en/default/admin/sudo.html.tmpl
> 
> Err, this is not what I'm talking about. I'm talking about all Bugzilla
> pages where autocompletion is supposed to occur, such as in bug reports. We
> never used autocompletion for this sudo page.

Ah, misread the summary, etc. Will take a look at the proper issue.

Comment 4

[David Lawrence [:dkl]]

Attachment: 1163399_2.patch

- Fixes issue with autocomplete not working when sudo'ed as a different user
- Left in change to sudo.html.tmpl to allow lookup of a user to impersonate.

dkl

Comment 5

[Frédéric Buclin]

(In reply to David Lawrence [:dkl] from comment #4)
> - Left in change to sudo.html.tmpl to allow lookup of a user to impersonate.

You don't want that. You access the sudo page from editusers.cgi, where you have already selected a user and clicked "Impersonate this user". So the user field is already populated. Moreover, autocompletion would also list users who are protected by the bz_sudo_protect group.

Comment 6

[David Lawrence [:dkl]]

(In reply to Frédéric Buclin from comment #5)
> (In reply to David Lawrence [:dkl] from comment #4)
> > - Left in change to sudo.html.tmpl to allow lookup of a user to impersonate.
> 
> You don't want that. You access the sudo page from editusers.cgi, where you
> have already selected a user and clicked "Impersonate this user". So the
> user field is already populated.

Not if you come from https://bugzilla.mozilla.org/userprefs.cgi?tab=permissions :)

> Moreover, autocompletion would also list users who are protected by the bz_sudo_protect group.

But good point so I will attach a new patch with this removed.

dkl

Comment 7

[Frédéric Buclin]

Comment on attachment 8604165 [details] [diff] [review]
1163399_2.patch

per comment 6.

Comment 8

[David Lawrence [:dkl]]

Attachment: 1163399_3.patch

Comment 9

[Frédéric Buclin]

Comment on attachment 8604240 [details] [diff] [review]
1163399_3.patch

>     my $user = Bugzilla->user;
>+    # Allow certain UI components to work if impersonating another user.
>+    $user = Bugzilla->sudoer if Bugzilla->sudoer;

Merge both lines into: my $user = Bugzilla->sudoer || Bugzilla->user;

The comment should be improved to mention that REST will look at the sudoer login cookies (via Bugzilla::Auth::Login::Cookie::get_login_info()), and so the API token of the sudoer must be returned. It took me a long time to determine this. (This means that REST doesn't work without a valid login cookie. Is that correct? If I am wrong, then I don't understand why the API token of the sudoer is required, and my r+ would become a r-).


r=LpSolit with the changes above. But first please confirm that I understood the problem correctly.

Comment 10

[David Lawrence [:dkl]]

To ssh://[e-mail]/bugzilla/bugzilla.git
   2d96c58..d5c1c6f  master -> master

Comment 11

[David Lawrence [:dkl]]

(In reply to Frédéric Buclin from comment #9)
> Comment on attachment 8604240 [details] [diff] [review]
> 1163399_3.patch
> 
> >     my $user = Bugzilla->user;
> >+    # Allow certain UI components to work if impersonating another user.
> >+    $user = Bugzilla->sudoer if Bugzilla->sudoer;
> 
> Merge both lines into: my $user = Bugzilla->sudoer || Bugzilla->user;
> 
> The comment should be improved to mention that REST will look at the sudoer
> login cookies (via Bugzilla::Auth::Login::Cookie::get_login_info()), and so
> the API token of the sudoer must be returned. It took me a long time to
> determine this. (This means that REST doesn't work without a valid login
> cookie. Is that correct? If I am wrong, then I don't understand why the API
> token of the sudoer is required, and my r+ would become a r-).
> 
> 
> r=LpSolit with the changes above. But first please confirm that I understood
> the problem correctly.

According to Bugzilla->login, you need a valid 'sudo' token which is only stored as a cookie currently to be able to sudo to another user. With REST calls, we do not want to use cookies for anything if we can avoid it and the only reason it would work with the UI is it is same-domain. The better route for API calls would be to 1st sudo to another user, then go to their user preferences and obtain an API key. Then use the API key for REST calls. I don't feel that changing the current comment is needed as it already mentions that the change is for allowing UI components to work.

dkl