1477163 - REST API authentication fails with X-Bugzilla-Token header

Status: RESOLVED INVALID

(bugzilla.mozilla.org :: API, defect)

Description

[Kohei Yoshino [:kohei]]

REST API with `X-Bugzilla-API-Key` HTTP header: OK
REST API with `X-Bugzilla-Token` HTTP header: *NG*

REST API with `Bugzilla_api_key` param: OK
REST API with `Bugzilla_api_token` param: OK

JSON-RPC API with `X-Bugzilla-API-Key` HTTP header: OK
JSON-RPC API with `X-Bugzilla-Token` HTTP header: OK

JSON-RPC API with `Bugzilla_api_key` param: OK
JSON-RPC API with `Bugzilla_api_token` param: OK

Comment 1

[Dylan Hardison [:dylan] (he/him)]

On most accounts, API token will only work if cookies are also sent.

Comment 2

[Kohei Yoshino [:kohei]]

I think I’ve found the culprit: https://github.com/mozilla-bteam/bmo/blob/master/Bugzilla/Auth/Login/Cookie.pm#L57-L77

Comment 3

[Dylan Hardison [:dylan] (he/him)]

Which fixes a security bug.

Here's what we should actually do (in about that place)

trust cookies for API calls, if a custom header is set and the Origin header is valid.

Comment 4

[Kohei Yoshino [:kohei]]

Maybe I shouldn’t touch this stuff 😉

Basically removing the `elsif` clause solves the issue: trust cookies for REST API calls just like JSON-RPC.

There are too many authentication methods... Should we remove the token support in favour of cookies?

Comment 5

[Kohei Yoshino [:kohei]]

Some context in Bug 1139257.

If we fix the issue straightforwardly, these 2 lines have to be altered to check the header as well:

```Perl
if (exists Bugzilla->input_params->{Bugzilla_api_token}) {
  my $api_token = Bugzilla->input_params->{Bugzilla_api_token};
```

Comment 6

[Dylan Hardison [:dylan] (he/him)]

(In reply to Kohei Yoshino [:kohei] from comment #4)
> Maybe I shouldn’t touch this stuff 😉
> 
> Basically removing the `elsif` clause solves the issue: trust cookies for
> REST API calls just like JSON-RPC.
> 
> There are too many authentication methods... Should we remove the token
> support in favour of cookies?

The landscape changed a bit with SameSite cookies too.. I'll tell you what, I'll fix this so that the REST api just uses cookies + the current recommendations for preventing request forgery.

Comment 7

[Kohei Yoshino [:kohei]]

Bug 1380437 should be solved first so the `Bugzilla_api_token` param can be removed easily from JavaScript at one place.

Comment 8

[Kohei Yoshino [:kohei]]

I thought tokens had been used only on this site, but just realized it was available via the API [1]. In that case, I assume we can’t remove the support now. Deprecating it is fine though. Maybe we should fix the original issue I reported so the X-Bugzilla-Token HTTP request header works as expected, as said in Comment 5.

[1] https://bmo.readthedocs.io/en/latest/api/core/v1/user.html

Comment 9

[Kohei Yoshino [:kohei]]

Also, an interesting fact is, according to the document, the official query param name is not `Bugzilla_api_token` used on Bugzilla but `Bugzilla_token` or `token`. Both don’t work.

https://bmo.readthedocs.io/en/latest/api/core/v1/general.html#authentication

Comment 10

[Kohei Yoshino [:kohei]]

Reverting the summary and adding the facts mentioned in my Comment 9.

Comment 11

[Kohei Yoshino [:kohei]]

Attachment: pull request

Sent a PR to solve the original issue.

Comment 12

[Kohei Yoshino [:kohei]]

Hmm, `token` and `Bugzilla_token` query params are somewhat different, so the tests are failing. I’m just fixing the header issue then.

Comment 13

[Kohei Yoshino [:kohei]]

Then the `X-Bugzilla-Token` header is also different.

The token returned by `/rest/login` is Bugzilla_login + Bugzilla_logincookie:

https://github.com/mozilla-bteam/bmo/blob/master/Bugzilla/WebService/User.pm#L472

It's called _login_ token, not an API token :(

https://github.com/mozilla-bteam/bmo/blob/master/Bugzilla/Auth/Login/Cookie.pm#L146

In that case, this bug is INVALID. Or we could support a different header, like `X-Bugzilla-API-Token`.

Comment 14

[Kohei Yoshino [:kohei]]

Let’s revisit the auth methods while designing REST API v2.