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	ID
Update WebAPITokenManager to use Token Generators	d39b84516abefb900321e80dd1d18f4524b0eb5d

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	ID
	Update WebAPITokenManager to use Token Generators	1bc481db2f099cc4127f48a2c3e6961a42bae5e0
	Update WebAPITokenManager to use Token Generators	6fe60bdab5c0235031d62e7a701f672cf524a7d9

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	ID
	Update WebAPITokenManager to use Token Generators	6fe60bdab5c0235031d62e7a701f672cf524a7d9
	Update WebAPITokenManager to use Token Generators	de8050d5aefafacc856f48f45846d45666f2f59b

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	ID
	Update WebAPITokenManager to use Token Generators	de8050d5aefafacc856f48f45846d45666f2f59b
	Update WebAPITokenManager to use Token Generators	0f547dcb63b355db12cd6257b7a0b97d6e71f469

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	ID
	Update WebAPITokenManager to use Token Generators	0f547dcb63b355db12cd6257b7a0b97d6e71f469
	Update WebAPITokenManager to use Token Generators	9c8ee3d75c9ef6c9517c82c1b87f48bd57054487

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	ID
	Update WebAPITokenManager to use Token Generators	9c8ee3d75c9ef6c9517c82c1b87f48bd57054487
	Update WebAPITokenManager to use Token Generators	d7a8c2e918c57aadbacbaa0024943d937ee2ee73

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	ID
	Update WebAPITokenManager to use Token Generators	d7a8c2e918c57aadbacbaa0024943d937ee2ee73
	Update WebAPITokenManager to use Token Generators	d39b84516abefb900321e80dd1d18f4524b0eb5d

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)