Summary:

Update WebAPITokenManager to use token generators

Review Request #12419 — Created June 28, 2022 and submitted Aug. 3, 2022, 10:19 a.m.

Information
Owner:	maubin
Repository:	Djblets
Branch:	release-3.x
Bugs:
Depends On:	~~12341~~, ~~12415~~
Reviewers
Groups:	djblets
People:

Description

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 in WebAPITokenManager.
To do this we add two new parameters to WebAPITokenManager,
token_generator_id to decide what token generator to use and token_info to
pass along information needed for token generation to the token generator.

Testing Done


Updated WebAPITokenManager's unit tests to account for the use

  of token generators.
Ran all unit tests in djblets.webapi.tests with success.

Commits

Summary
Update WebAPITokenManager to use Token Generators

Screenshots

Issues

Description	From	Last Updated
SyntaxError: invalid syntax Column: 1 Error code: E999	reviewbot	July 15, 2022, 1:56 p.m.
Switch these two lines	david	July 20, 2022, 4:26 p.m.
Instead of instantiating the registry as an attribute on this manager class, we shoud have a global within djblets. That …	david	July 20, 2022, 4:26 p.m.
We should probably verify that this returns a value, and if not, raise WebAPITokenGenerationError.	david	July 26, 2022, 11:58 a.m.
Can we swap these two args? That way all the query args are together.	david	July 26, 2022, 12:43 p.m.
While you're here, can you move this import up into the 3rd party modules group?	david	July 20, 2022, 4:28 p.m.
Let's put a Version Added in each of these new ones. Helps to have a high-level in the function's description, …	chipx86	July 26, 2022, 11:34 a.m.
No need for the parens here, or the trailing comma.	chipx86	July 26, 2022, 11:34 a.m.
No need for the parens here, or the trailing comma.	chipx86	July 26, 2022, 11:35 a.m.
No blank line here.	chipx86	July 26, 2022, 11:36 a.m.
Let's do import kgb and use attribute accesses from there. That's the pattern we've been establishing elsewhere.	chipx86	July 26, 2022, 11:39 a.m.
Can we also check the warning text, to ensure we're getting what we think we're getting?	chipx86	July 26, 2022, 11:41 a.m.
Let's move this after auto_generated. Since Python by default allows positional arguments to be specified for keyword arguments (unless using …	chipx86	July 28, 2022, 10:04 a.m.

flake8 passed.

JSHint passed.

```
Ship It!
```

Change Summary:


Added deprecation warnings for when token_generator_id or token_info aren't set.

Commits:

	Summary
-	Update WebAPITokenManager to use Token Generators
+	Update WebAPITokenManager to use Token Generators

Diff:

Revision 2 (+244 -26)

Show changes

	djblets/webapi/managers.py
	djblets/webapi/tests/test_api_token.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

djblets/webapi/tests/test_api_token.py (Diff revision 2)
Show all issues
```
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:

	Summary
-	Update WebAPITokenManager to use Token Generators
+	Update WebAPITokenManager to use Token Generators

Diff:

Revision 3 (+236 -26)

Show changes

	djblets/webapi/managers.py
	djblets/webapi/tests/test_api_token.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

djblets/webapi/managers.py (Diff revision 3)
Show all issues
```
Switch these two lines
```

djblets/webapi/managers.py (Diff revision 3)

Show all issues

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 use djblets.registries.importer.lazy_import_registry.

djblets/webapi/tests/test_api_token.py (Diff revision 3)

Show all issues

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:

	Summary
-	Update WebAPITokenManager to use Token Generators
+	Update WebAPITokenManager to use Token Generators

Diff:

Revision 4 (+228 -24)

Show changes

	djblets/webapi/managers.py
	djblets/webapi/tests/test_api_token.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Added methods for invalidating a token and a set of tokens to the manager.

Commits:

	Summary
-	Update WebAPITokenManager to use Token Generators
+	Update WebAPITokenManager to use Token Generators

Diff:

Revision 5 (+536 -24)

Show changes

	djblets/webapi/managers.py
	djblets/webapi/tests/test_api_token.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

djblets/webapi/managers.py (Diff revisions 3 - 5)

Show all issues

We should probably verify that this returns a value, and if not, raise WebAPITokenGenerationError.

djblets/webapi/managers.py (Diff revisions 3 - 5)

Show all issues

Can we swap these two args? That way all the query args are together.

djblets/webapi/managers.py (Diff revision 5)

Show all issues

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/managers.py (Diff revision 5)

Show all issues

No need for the parens here, or the trailing comma.

djblets/webapi/managers.py (Diff revision 5)

Show all issues

No need for the parens here, or the trailing comma.

djblets/webapi/managers.py (Diff revision 5)
Show all issues
```
No blank line here.
```

djblets/webapi/tests/test_api_token.py (Diff revision 5)

Show all issues

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)

Show all issues

Can we also check the warning text, to ensure we're getting what we think we're getting?

Commits:

	Summary
-	Update WebAPITokenManager to use Token Generators
+	Update WebAPITokenManager to use Token Generators

Diff:

Revision 6 (+626 -28)

Show changes

	djblets/webapi/managers.py
	djblets/webapi/tests/test_api_token.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

djblets/webapi/managers.py (Diff revision 6)

Show all issues

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:

	Summary
-	Update WebAPITokenManager to use Token Generators
+	Update WebAPITokenManager to use Token Generators

Diff:

Revision 7 (+632 -28)

Show changes

	djblets/webapi/managers.py
	djblets/webapi/tests/test_api_token.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-3.x (b7f84ba)