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. 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. Can we add module docstrings to the new empty __init__.py files?

  3. 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. 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. We should gettext_lazy() this.

  3. 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. We should _(...) all the help text.

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

    Needs a Raises for CommandError.

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

  7. No blank line here.

  8. 
      
maubin
david
  1. Ship It!
  2. 
      
chipx86
  1. 
      
  2. 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. 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. The % should be on the next line.

  5. 
      
maubin
chipx86
  1. Ship It!
  2. 
      
maubin
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (ff32f05)
Loading...