Fix up authentication issues with the HostingServiceAccount API.
Review Request #14128 — Created Aug. 29, 2024 and submitted
The
HostingServiceAccountResource
has always been primarily used in a
read capacity, and the implementation forcreate
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 newcredentials
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 thecredentials
dict. In the case of the reported bug, GitLab's
form uses aprivate_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.
Summary | ID |
---|---|
ab33f0a379dd5e5611dcbeb00f8a2c87973a0ab9 |
Description | From | Last Updated |
---|---|---|
line too long (99 > 79 characters) Column: 80 Error code: E501 |
reviewbot | |
whitespace after '{' Column: 22 Error code: E201 |
reviewbot | |
whitespace before '}' Column: 49 Error code: E202 |
reviewbot | |
Small nit, but let's alphabetize these keys. |
chipx86 | |
Feel free to discard: This could be simplified to a dictionary expression, and you'd get the typing for free. |
chipx86 |
- Commits:
-
Summary ID 128b2ec2fdb72b2cadfe49fbd2131181277647eb 8692f12259f59798ef9c44b13a83f9ce949c2077
Checks run (2 succeeded)
- Change Summary:
-
Mention that some services might need additional fields in the API POST docs.
- Commits:
-
Summary ID 8692f12259f59798ef9c44b13a83f9ce949c2077 38cc82f8df1c26579836904490a1eb3772c67fd5