REST error on GET... Logged-out users cannot use the "ids" argument to this function to access any user information."

VERIFIED FIXED

Status

VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: njn, Assigned: gps)

Tracking

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
+++ This bug was initially created as a clone of Bug #1121328 +++

I'm still getting the same problem that I originally reported in bug 1121328 -- '"Logged-out users cannot use the "ids" argument to this function to access any user information."' -- even after updating my version-control-tools repo.
(Assignee)

Comment 1

4 years ago
Created attachment 8549286 [details] [diff] [review]
bzexport: tests for invalid cookie

We had zero test coverage for cookie auth in bzexport. We suspect that
issues with cookie validity are causing bugs. This patch starts the
process of better handling and testing cookie auth.
Attachment #8549286 - Flags: review?(sphink)
(Assignee)

Updated

4 years ago
Assignee: nobody → gps
Status: NEW → ASSIGNED
(Assignee)

Comment 2

4 years ago
I've created a test that reliably reproduces this failure. Working on a fix now.
(Assignee)

Comment 3

4 years ago
Created attachment 8549309 [details] [diff] [review]
bzexport: revert back to bzAPI for resolving the user name

bzexport always needs to know the Bugzilla username associated with a
request. When we use cookies extracted from a Firefox profile, the
username is not known and must be resolved by querying Bugzilla.

Unfortunately, it appears the REST API as deployed on BMO does not
currently allow us to use cookies to do what we want. If we attempt a
GET /rest/user/%s, Bugzilla fails with "Logged-out users cannot use the
"ids" argument to this function to access any user information."
Bugzilla thinks we are logged out because cookie auth is only allowed on
POST and PUT HTTP methods. When specified on GET, etc requests, cookies
are silently discarded (for the record, we currently send cookies on GET
requests).

Ideally, we'd exchange cookies for a token to use with the REST API.
However, the REST API's login endpoint is only accessible via GET. And,
when you specify cookies via GET, they are ignored. So, there appears to
be no way to obtain a REST token nor perform authenticated GET requests
when all we have are cookies.

bzAPI doesn't have the restriction that cookies can't be used with GET
requests. Instead, you toss some cookie values in the query string and
it happily uses them for authentication.

In this patch, we revert user lookups to go through bzAPI.

The aforementioned "Logged-out users cannot use..." error message was
observed as part of developing this patch and the switch to bzAPI was
confirmed to make it go away.

A bug should be filed against Bugzilla's REST API to allow clients
starting with cookies only to be able to use all REST APIs.
Attachment #8549309 - Flags: review?(sphink)
(Assignee)

Comment 4

4 years ago
dkl, mcote: could you please read the commit message on the patch I just uploaded and let me know if my assessment is accurate and Bugzilla's REST API needs to be improved.
Flags: needinfo?(mcote)
Flags: needinfo?(dkl)
(Assignee)

Comment 5

4 years ago
Until the aforementioned patches are applied, cookie auth for `hg bzexport` is essentially busted. The workaround is to use password auth.

Given the impact and annoyance of this regression, if anyone wants to jump in and take the review and possibly land things if r+ is granted, go for it.
Attachment #8549286 - Flags: review?(sphink) → review+
Attachment #8549309 - Flags: review?(sphink) → review+
(Assignee)

Comment 7

4 years ago
Reopen if you see the "Logged out users cannot use the 'ids' argument..." error message after this change.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Reporter)

Comment 8

4 years ago
I updated. Now I get this error:

> abort: REST error on GET to https://bugzilla.mozilla.org/rest/user: Logged-out users cannot use the "match" argument to this function to access any user information.

(That's it; no stack trace.)

And it doesn't upload the patch. So for me things are worse than they were before the change.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 9

4 years ago
And I get the same failure if I try to use the username/password technique from https://bitbucket.org/sfink/bzexport/.
(Assignee)

Comment 10

4 years ago
Sounds like the same problem: GET requests using cookies aren't authenticated. Probably the same workaround to fix it.

Regressions were expected. If you want stability back, update version-control-tools to a changeset before all the changes, like 7a0f787a44a9. We prefix commit messages in version-control-tools with the subcomponent that changed. So do `hg log -k bzexport` to roughly find all the commits involving bzexport.

I'm not going to let these regressions slip through the cracks. Keep filing bugs and we'll fix them. And they shouldn't regress now that we have test coverage, which we didn't have Monday. We just need to find all the magic permutations of tests, that's all.
(Assignee)

Comment 11

4 years ago
Created attachment 8549397 [details] [diff] [review]
bzexport: revert back to bzAPI for searching for users

More fallout from REST not being able to authenticate cookies on GET
requests.

Converting an existing test to use cookies was enough to reproduce the
issue. Moving back to bzAPI made the bug go away.
Attachment #8549397 - Flags: review?(sphink)
(Assignee)

Comment 12

4 years ago
We still have one use of GET on REST: fetching attachments. This is likely only an issue when interacting with private bugs.

I'm inclined to wait for a needinfo response from a BMO expert before I patch around that too.
(Assignee)

Updated

4 years ago
Status: REOPENED → ASSIGNED
(In reply to Gregory Szorc [:gps] from comment #4)
> dkl, mcote: could you please read the commit message on the patch I just
> uploaded and let me know if my assessment is accurate and Bugzilla's REST
> API needs to be improved.

It is correct. Normal REST requests using Native and BzAPI compat endpoint only allow cookie auth with doing POST/PUT and not GET. Although we are supposed to allow GET /rest/login to bypass the check so that cookie auth works. There is a bug I just found that, when you do pass cookies to /rest/login, but no login/password values, it will ignore the cookies and error that a login/password pair was provided. 

I will fix that issue in the next update. The second issue after that is that if you are successfully authenticated to /rest/login with cookies only, it only returns the user's id and not a token value since they did not login fresh with a login/password. Getting the token value (same as the Bugzilla_logincookie) is not easy to reproduce.

A workaround for you would be to create your token manually using the Firefox profiles Bugzilla_logincookie value and the user id value from Bugzilla_login. Then you can just pass that to any calls you make and it will work just the same (as long as their cookie is not tied to an IP).

So for example:

Bugzilla_login: 5898
Bugzilla_logincookie: 2jkadsf783

the token you pass would be:

/rest/bug/35?token=5898-2jkadsf783

You should be able to do that now without change on BMO since you have the cookie values from the Firefox Profile the client is using.

dkl
Flags: needinfo?(dkl)
(Assignee)

Comment 14

4 years ago
Created attachment 8549845 [details] [diff] [review]
bzexport: use API token from cookie values and use REST API again

It turns out that the REST API token is a composition of cookie values!
So, we revert our bzAPI reverts and teach REST requests how to use
tokens from cookie values. The REST API again works with GET requests
when we only have cookie authentication!
Attachment #8549845 - Flags: review?(sphink)
(Assignee)

Updated

4 years ago
Attachment #8549397 - Attachment is obsolete: true
Attachment #8549397 - Flags: review?(sphink)
(Assignee)

Comment 15

4 years ago
dkl: thanks for the info!

I confirmed that composing tokens from cookies works for our needs.

One thing I did notice as part of testing is that the error message for invalid token is slightly less actionable than invalid cookie.

With invalid cookies as part of a POST, I get an error like: The cookies or token provide were not valid or have expired. You may login again to get new cookies or a new token

But if I remove cookies from the request and only send a token, I get an error like: You must log in before using this part of Bugzilla.

This is on the POST rest/bug/%s/attachment handler in case it makes a difference.
Flags: needinfo?(mcote)
Comment on attachment 8549845 [details] [diff] [review]
bzexport: use API token from cookie values and use REST API again

Review of attachment 8549845 [details] [diff] [review]:
-----------------------------------------------------------------

Hah! This is an amusing resolution to the problem, but it WFM.
Attachment #8549845 - Flags: review?(sphink) → review+
(Assignee)

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
(Reporter)

Comment 18

4 years ago
It works! Thank you.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.