Summary

Add a management command for invalidating tokens

Review Request #12481 — Created July 21, 2022 and submitted Aug. 3, 2022, 10:22 a.m.

Information

Owner

maubin

Repository

Review Board

Branch

release-5.0.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

This change adds an rb-site manage /path/to/site invalidate-api-tokens command
that allows administrators to invalidate the API tokens for a set of users. Now
tokens can be invalidated individually via the API, or in batch with this
command.

Testing Done

Manually tested the command and checked that the right tokens were
being invalidated.

Commits

Summary	ID
Add a management command for invalidating tokens	ed9c78dbbe05445a93e064b345144889f200ffd0

Issues

Description	From	Last Updated
Can we add module docstrings to the new empty __init__.py files?	david	July 27, 2022, 9:58 a.m.
Should I write documentation for this here: https://www.reviewboard.org/docs/manual/4.0/admin/sites/management-commands/?	maubin	July 27, 2022, 12:02 p.m.
We should gettext_lazy() this.	chipx86	July 27, 2022, 10:27 a.m.
I wonder if instead we should go by usernames?	chipx86	July 27, 2022, 10:42 a.m.
We should _(...) all the help text.	chipx86	July 27, 2022, 10:42 a.m.
Needs a Raises for CommandError.	chipx86	July 27, 2022, 11:08 a.m.
We should end the strings with a period, make them proper sentences.	chipx86	July 27, 2022, 10:45 a.m.
No blank line here.	chipx86	July 27, 2022, 10:09 a.m.
Doesn't this already basically exist if you call invalidate_tokens with the default args (i.e. users=None)?	david	July 27, 2022, 10:08 a.m.
I wonder if it wouldn't be better to have invalidate_tokens take a list of user PKs. That way we could …	david	July 27, 2022, 10:26 a.m.
I think there's one-too-many ( and ). I don't think you'll need the list() though. The * should still work …	chipx86	July 28, 2022, 1 p.m.
You can avoid the list comprehension. Given we're working with set, I'd also sort. Also, both variables are already a …	chipx86	July 28, 2022, 1:05 p.m.
The % should be on the next line.	chipx86	July 28, 2022, 1:07 p.m.

flake8 passed.

JSHint passed.

The issue has been resolved. Show all issues

Should I write documentation for this here: https://www.reviewboard.org/docs/manual/4.0/admin/sites/management-commands/?

david July 21, 2022, 4:41 p.m.
```
Yep, that's the right place.
```

The issue has been resolved. Show all issues

Can we add module docstrings to the new empty __init__.py files?

reviewboard/webapi/management/commands/invalidate-api-tokens.py (Diff revision 1)

The issue has been resolved. Show all issues

Doesn't this already basically exist if you call invalidate_tokens with the default args (i.e. users=None)?

maubin July 22, 2022, 1:32 p.m.
```
Oops yep that's right.
```

reviewboard/webapi/management/commands/invalidate-api-tokens.py (Diff revision 1)

The issue has been resolved. Show all issues

I wonder if it wouldn't be better to have invalidate_tokens take a list of user PKs. That way we could avoid the entire Users query.

maubin July 22, 2022, 1:32 p.m.
```
Agreed.
```

reviewboard/webapi/management/commands/invalidate-api-tokens.py (Diff revision 1)
The issue has been resolved. Show all issues
```
We should gettext_lazy() this.
```

reviewboard/webapi/management/commands/invalidate-api-tokens.py (Diff revision 1)

The issue has been resolved. Show all issues

I wonder if instead we should go by usernames?

maubin July 26, 2022, 1:45 p.m.

Yeah, it's probably easier for admins to input usernames here instead of finding people's IDs and using those.

reviewboard/webapi/management/commands/invalidate-api-tokens.py (Diff revision 1)
The issue has been resolved. Show all issues
```
We should _(...) all the help text.
```

reviewboard/webapi/management/commands/invalidate-api-tokens.py (Diff revision 1)

The issue has been resolved. Show all issues

Needs a Raises for CommandError.

reviewboard/webapi/management/commands/invalidate-api-tokens.py (Diff revision 1)
The issue has been resolved. Show all issues
```
We should end the strings with a period, make them proper sentences.
```

reviewboard/webapi/management/commands/invalidate-api-tokens.py (Diff revision 1)

The issue has been resolved. Show all issues

No blank line here.

Commits:

	Summary	ID
	Add a management command for invalidating tokens	8a10c5e52a53407b4f49d4a72ec62285c9b93f18
	Add a management command for invalidating tokens	abff830ead0168987e64523665dc7947666e022e

Diff:

Revision 2 (+274)

Show changes

	docs/manual/admin/sites/management-commands.rst
	reviewboard/webapi/management/__init__.py
	reviewboard/webapi/management/commands/__init__.py
	reviewboard/webapi/management/commands/invalidate-api-tokens.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

reviewboard/webapi/management/commands/invalidate-api-tokens.py (Diff revision 2)

The issue has been resolved. Show all issues

I think there's one-too-many ( and ).
I don't think you'll need the list() though. The * should still work with the return type for values_list().

reviewboard/webapi/management/commands/invalidate-api-tokens.py (Diff revision 2)

The issue has been resolved. Show all issues

You can avoid the list comprehension.

Given we're working with set, I'd also sort.

Also, both variables are already a set.

So:

', '.join(sorted(usernames - found_usernames))

reviewboard/webapi/management/commands/invalidate-api-tokens.py (Diff revision 2)
The issue has been resolved. Show all issues
```
The % should be on the next line.
```

Commits:

	Summary	ID
	Add a management command for invalidating tokens	abff830ead0168987e64523665dc7947666e022e
	Add a management command for invalidating tokens	ed9c78dbbe05445a93e064b345144889f200ffd0

Diff:

Revision 3 (+290)

Show changes

	docs/manual/admin/sites/management-commands.rst
	reviewboard/webapi/management/__init__.py
	reviewboard/webapi/management/commands/__init__.py
	reviewboard/webapi/management/commands/invalidate-api-tokens.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-5.0.x (ff32f05)