Skip to content

Bug 2073282 - Add deprecated api_key=<key> query-parameter fallback to native Mojo REST auth #2753

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Xzzz wants to merge 2 commits into
base: master
Choose a base branch
from
+20 −3
Open

Bug 2073282 - Add deprecated api_key=<key> query-parameter fallback to native Mojo REST auth #2753

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4e82224
Bug 2073282 - Add deprecated api_key=<key> query-parameter fallback t…
Xzzz Sep 18, 2026
ba30b0e
Bug 2073282 - Note the deprecated api_key query-parameter fallback in…
Xzzz Sep 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion Bugzilla/App/Plugin/Login.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,15 @@ sub register {

# For api requests, we check for the api key in the header
if ($usage_mode == USAGE_MODE_REST || $usage_mode == USAGE_MODE_MOJO_REST) {
if (my $api_key_text = $headers->header('x-bugzilla-api-key')) {

# Deprecated fallback for the legacy ?api_key=<key> query parameter,
# same as the legacy WebService dispatcher (see
# Bugzilla::WebService::Util::fix_credentials). This is a
# deprecation-pending stopgap, not a first-class supported method.
my $api_key_text
= $headers->header('x-bugzilla-api-key') || $c->param('api_key');

@dklawren dklawren Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two gaps versus the legacy dispatcher this is meant to match

  1. fix_credentials accepts both api_key and Bugzilla_api_key (Bugzilla/WebService/Util.pm:330 promotes the short form, and Bugzilla/Auth/Login/APIKey.pm:54 reads the long form directly). the docs warning this PR now points to also lists both. a k8s-style caller sending ?Bugzilla_api_key=<key> still gets a 401 on native resources, so the "undiscovered callers" case is only half covered

Looking through the logs, there are unfortunately some use of the Bugzilla_api_key query parameter in addition to api_key so we need to support both for now.

  1. $c->param reads urlencoded/multipart body params too, not just the query string. the code comment above says "query parameter", the docs change says "query parameter", and the test only covers the query string — so the deprecated surface being reopened is larger than what is described. either use $c->req->query_params->param('api_key') to keep the stopgap minimal, or say plainly that body params are accepted as well


if ($api_key_text) {
if (my $api_key = Bugzilla::User::APIKey->new({name => $api_key_text})) {
my $remote_ip = $c->tx->remote_address;
if (
Expand Down
6 changes: 4 additions & 2 deletions docs/en/rst/api/core/v1/general.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,10 @@ Send only one authentication method with each request. BMO does not combine
credentials or choose the strongest method when more than one is supplied.

Most resources have been migrated off the legacy authentication path onto BMO's
native REST framework, which accepts only a cookie, an ``X-Bugzilla-API-Key``
header, or an OAuth2 bearer token. Legacy ``Bugzilla_login`` and
native REST framework, which accepts a cookie, an ``X-Bugzilla-API-Key``

@dklawren dklawren Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"described in the warning below" overstates it — that warning covers Bugzilla_api_key or api_key, but only api_key works on native resources after this change. so we need to implement the long form too.

header, or an OAuth2 bearer token (plus the deprecated ``api_key`` query
parameter described in the warning below, kept only as a stopgap for
undiscovered callers). Legacy ``Bugzilla_login`` and
``Bugzilla_password`` credentials are **not** accepted on these resources, even
though the old WebService dispatcher underneath BMO still supports them for
resources not yet migrated (currently ``Bug``, ``Group``, ``Product``, and
Expand Down
7 changes: 7 additions & 0 deletions qa/t/rest_native_login.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ $t->get_ok($url . $endpoint)->status_is(401);
$t->get_ok($url . $endpoint => {'X-Bugzilla-API-Key' => $api_key})
->status_is(200)->json_has('/result');

#
# 2a. Deprecated fallback: an API key passed as the ?api_key= query parameter
# also works (bug 2073282), same as the legacy WebService dispatcher.
#
$t->get_ok($url . $endpoint . '&api_key=' . $api_key)

@dklawren dklawren Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

worth adding a case where both the header and ?api_key= are sent, asserting the header wins

that precedence is security relevant — fix_credentials documents it as stopping body-injected params from overriding gateway auth headers, and it already regressed once (t/webservice-fix-credentials.t:85, bug 2035598). the || here preserves it but nothing locks it in

->status_is(200)->json_has('/result');

#
# 3. Authentication via the login cookie + Bugzilla_api_token parameter works.
# This is the mechanism the web UI (Bugzilla.API) uses, and is the path that
Expand Down
Loading