Update WebAPITokenManager to use token generators
Review Request #12419 — Created June 28, 2022 and submitted
Currently, token generation is baked into
djblets.webapi.managers.WebAPITokenManager
. With our move to API Tokens v2,
we don't want token generation to be tightly coupled with Web API tokens. We
want tokens to be usable outside of just the API.This change moves token generation out of
WebAPITokenManager
by making use of
token generators instead of directly generating tokens inWebAPITokenManager
.
To do this we add two new parameters toWebAPITokenManager
,
token_generator_id
to decide what token generator to use andtoken_info
to
pass along information needed for token generation to the token generator.
- Updated
WebAPITokenManager
's unit tests to account for the use
of token generators. - Ran all unit tests in
djblets.webapi.tests
with success.
Summary | ID |
---|---|
d39b84516abefb900321e80dd1d18f4524b0eb5d |
Description | From | Last Updated |
---|---|---|
SyntaxError: invalid syntax Column: 1 Error code: E999 |
![]() |
|
Switch these two lines |
|
|
Instead of instantiating the registry as an attribute on this manager class, we shoud have a global within djblets. That … |
|
|
We should probably verify that this returns a value, and if not, raise WebAPITokenGenerationError. |
|
|
Can we swap these two args? That way all the query args are together. |
|
|
While you're here, can you move this import up into the 3rd party modules group? |
|
|
Let's put a Version Added in each of these new ones. Helps to have a high-level in the function's description, … |
|
|
No need for the parens here, or the trailing comma. |
|
|
No need for the parens here, or the trailing comma. |
|
|
No blank line here. |
|
|
Let's do import kgb and use attribute accesses from there. That's the pattern we've been establishing elsewhere. |
|
|
Can we also check the warning text, to ensure we're getting what we think we're getting? |
|
|
Let's move this after auto_generated. Since Python by default allows positional arguments to be specified for keyword arguments (unless using … |
|

Change Summary:
- Added deprecation warnings for when
token_generator_id
ortoken_info
aren't set.
Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+244 -26) |
Checks run (1 failed, 1 succeeded)
flake8
-
djblets/webapi/tests/test_api_token.py (Diff revision 2) SyntaxError: invalid syntax Column: 1 Error code: E999

Change Summary:
Took out some stuff from patching in other changes that was left in by accident
Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+236 -26) |
Checks run (2 succeeded)
-
-
-
djblets/webapi/managers.py (Diff revision 3) Instead of instantiating the registry as an attribute on this manager class, we shoud have a global within djblets. That way extensions or other code can register additional generators easily.
This should live in
djblets/secrets/token_generators/__init__.py
and usedjblets.registries.importer.lazy_import_registry
. -
djblets/webapi/tests/test_api_token.py (Diff revision 3) While you're here, can you move this import up into the 3rd party modules group?

Change Summary:
Uses the global token generator registry instead of making it an attribute of the manager class.
Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 4 (+228 -24) |
Checks run (2 succeeded)

Change Summary:
Added methods for invalidating a token and a set of tokens to the manager.
Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 5 (+536 -24) |
Checks run (2 succeeded)
-
-
djblets/webapi/managers.py (Diff revisions 3 - 5) We should probably verify that this returns a value, and if not, raise
WebAPITokenGenerationError
. -
djblets/webapi/managers.py (Diff revisions 3 - 5) Can we swap these two args? That way all the query args are together.
-
-
djblets/webapi/managers.py (Diff revision 5) Let's put a
Version Added
in each of these new ones.Helps to have a high-level in the function's description, but also to see this on each parameter.
-
-
-
-
djblets/webapi/tests/test_api_token.py (Diff revision 5) Let's do
import kgb
and use attribute accesses from there. That's the pattern we've been establishing elsewhere. -
djblets/webapi/tests/test_api_token.py (Diff revision 5) Can we also check the warning text, to ensure we're getting what we think we're getting?

Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 6 (+626 -28) |
Checks run (2 succeeded)
-
-
djblets/webapi/managers.py (Diff revision 6) Let's move this after
auto_generated
. Since Python by default allows positional arguments to be specified for keyword arguments (unless using keyword-only arguments in Python 3), introducing a new argument in the middle is technically an API-breaking change.This is why I want to eventually move more things to keyword-only arguments (but that has its own deprecation challenges).

Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 7 (+632 -28) |