-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Closes #20834: Add support for enabling/disabling Tokens #20864
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closes #20834: Add support for enabling/disabling Tokens #20864
Conversation
|
I’ve opened this as a draft PR to get an initial round of feedback on the approach and implementation. Once the changes from #20823 are merged from In the meantime, any early feedback on the model changes, API surface, or tests is very welcome. Thanks in advance for taking a look! |
8640d0e to
d513cc8
Compare
|
I’ve just rebased this PR onto the newly merged branch and resolved the merge conflicts. The changes are now up to date and ready for review. Thank you very much for taking a look! |
jeremystretch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @pheus! Just had a few minor suggestions.
Introduce an `enabled` flag on the `Token` model to allow temporarily revoking API tokens without deleting them. Update forms, serializers, and views to expose the new field. Enforce the `enabled` flag in token authentication. Add model, API, and authentication tests for the new behavior. Fixes netbox-community#20834
d513cc8 to
ef89933
Compare
|
Thank you very much for the thoughtful review and suggestions! I really appreciate you taking the time to go through this. Your feedback is super helpful and will definitely help me improve! |
|
I’m really sorry about that. I ran the Thank you for pushing the fix and for your patience. I’ll make sure to run the full set of relevant tests next time. 🙏 |
|
No worries! |
Fixes: #20834
This PR introduces an
enabledfield on theTokenmodel to allow API tokens to be temporarily disabled without being deleted.Key changes:
enabledBoolean field toToken(defaulting toTruefor backwards compatibility)enabledflag in token authentication so disabled tokens are rejectedenabledvia the token API serializers (including provisioning), allowing tokens to be created and managed in a disabled stateExisting tokens remain enabled by default, so there is no change in behavior until a token is explicitly disabled.
Thanks in advance for reviewing!