Fix up authentication issues with the HostingServiceAccount API.

Review Request #14128 — Created Aug. 29, 2024 and submitted — Latest diff uploaded

Information

Review Board
release-7.x

Reviewers

The HostingServiceAccountResource has always been primarily used in a
read capacity, and the implementation for create wansn't able to
create hosting service accounts for any kind of authenticated services.
There are two big issues here:

  • HostingServiceAccountResource.create() was manually calling
    service.authorize, but with a legacy function signature that did not
    work for everything. We started the process of migrating
    HostingService subclasses to use a new credentials dict, but that
    still isn't implemented everywhere, and for services that do use it,
    the API endpoint was just completely broken.
  • Many HostingService instances have custom authentication forms which
    add or remove fields. These forms are what actually create the content
    of the credentials dict. In the case of the reported bug, GitLab's
    form uses a private_token field in credentials rather than
    password.

This change makes it so the API resource instantiates the hosting
service's auth form and then use that form to create the
HostingServiceAccount. This makes it so we're always calling authorize
with the expected kwargs for the given service, and any custom fields
which are relied on will be properly validated and applied.

This adds a few new unit tests that use the GitLab service, since that's
an example of something that uses a custom auth form and uses
non-standard keys in the credentials dict.

Ran unit tests.

Diff Revision 2

This is not the most recent revision of the diff. The latest diff is revision 4. See what's changed.

orig
1
2
3
4

Commits

First Last Summary ID Author
Fix up authentication issues with the HostingServiceAccount API.
The `HostingServiceAccountResource` has always been primarily used in a read capacity, and the implementation for `create` wansn't able to create hosting service accounts for any kind of authenticated services. There are two big issues here: - `HostingServiceAccountResource.create()` was manually calling `service.authorize`, but with a legacy function signature that did not work for everything. We started the process of migrating HostingService subclasses to use a new `credentials` dict, but that still isn't implemented everywhere, and for services that do use it, the API endpoint was just completely broken. - Many `HostingService` instances have custom authentication forms which add or remove fields. These forms are what actually create the content of the `credentials` dict. In the case of the reported bug, GitLab's form uses a `private_token` field in credentials rather than `password`. This change makes it so the API resource instantiates the hosting service's auth form and then use that form to create the HostingServiceAccount. This makes it so we're always calling `authorize` with the expected kwargs for the given service, and any custom fields which are relied on will be properly validated and applied. This adds a few new unit tests that use the GitLab service, since that's an example of something that uses a custom auth form and uses non-standard keys in the credentials dict. Testing Done: Ran unit tests.
8692f12259f59798ef9c44b13a83f9ce949c2077 David Trowbridge
reviewboard/webapi/resources/hosting_service_account.py
reviewboard/webapi/tests/test_hosting_service_account.py
Loading...