Add a management command for invalidating tokens

Review Request #12481 — Created July 21, 2022 and submitted

Information

Review Board
release-5.0.x

Reviewers

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.

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

Summary ID
Add a management command for invalidating tokens
ed9c78dbbe05445a93e064b345144889f200ffd0
Description From Last Updated

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

daviddavid

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

maubinmaubin

We should gettext_lazy() this.

chipx86chipx86

I wonder if instead we should go by usernames?

chipx86chipx86

We should _(...) all the help text.

chipx86chipx86

Needs a Raises for CommandError.

chipx86chipx86

We should end the strings with a period, make them proper sentences.

chipx86chipx86

No blank line here.

chipx86chipx86

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

daviddavid

I wonder if it wouldn't be better to have invalidate_tokens take a list of user PKs. That way we could …

daviddavid

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

chipx86chipx86

You can avoid the list comprehension. Given we're working with set, I'd also sort. Also, both variables are already a …

chipx86chipx86

The % should be on the next line.

chipx86chipx86
maubin
  1. 
      
  2. Show all issues

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

    1. Yep, that's the right place.

  3. 
      
david
  1. 
      
  2. Show all issues

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

  3. Show all issues

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

    1. Oops yep that's right.

  4. 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.

  5. 
      
chipx86
  1. 
      
  2. Show all issues

    We should gettext_lazy() this.

  3. Show all issues

    I wonder if instead we should go by usernames?

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

  4. Show all issues

    We should _(...) all the help text.

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

    Needs a Raises for CommandError.

  6. Show all issues

    We should end the strings with a period, make them proper sentences.

  7. Show all issues

    No blank line here.

  8. 
      
maubin
david
  1. Ship It!
  2. 
      
chipx86
  1. 
      
  2. 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().

  3. 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))
    
  4. Show all issues

    The % should be on the next line.

  5. 
      
maubin
chipx86
  1. Ship It!
  2. 
      
maubin
Review request changed
Status:
Completed
Change Summary:
Pushed to release-5.0.x (ff32f05)