-
Notifications
You must be signed in to change notification settings - Fork 5k
feat: support create/drop totp_secret statement #34162
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
Conversation
Summary of ChangesHello @localvar, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances user security by introducing dedicated SQL statements for managing Time-based One-Time Password (TOTP) secrets, moving away from a deprecated user option. It also adds a new granular control for user resource allocation related to vnodes. These changes provide more robust authentication mechanisms and finer-grained resource management within the system. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces support for CREATE TOTP_SECRET and DROP TOTP_SECRET statements, which is a great enhancement for user security. The implementation spans across documentation, client, server, and parser components.
My review has identified a few issues, including two critical ones:
- A security vulnerability in
taosSafeRandByteswhere it falls back to an insecure random number generator. - A memory corruption bug in the client-side message handler
buildCreateTotpSecretBlock.
I've also found some minor issues like a typo in the English documentation and some inconsistencies. Please see the detailed comments.
A couple of other points:
- The PR description has a typo: "dorp" should be "drop".
- The changes related to
VNODE_PER_CALLseem unrelated to this feature. It might be better to move them to a separate pull request to keep this one focused.
Overall, this is a good feature addition, but the critical issues must be addressed before merging.
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.
Pull request overview
This PR adds support for CREATE/DROP TOTP_SECRET statements to replace the deprecated TOTPSEED approach in TDengine. The changes introduce a new SQL syntax for managing TOTP secrets that provides better security and usability.
Key changes:
- Introduces
CREATE TOTP_SECRET FOR USER <user_name>andDROP TOTP_SECRET FROM USER <user_name>statements - Removes deprecated
TOTPSEEDuser option andgenerate_totp_secret()function from autocomplete - Implements secure random byte generation using system cryptographic functions
- Adds base32 encoding for TOTP secret display
- Updates documentation in both English and Chinese
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/shell/src/shellEngine.c | Added display width handling for TOTP secret output |
| tools/shell/src/shellAuto.c | Removed deprecated TOTPSEED and generate_totp_secret from autocomplete, added new TOTP_SECRET statements |
| tools/shell/inc/shellInt.h | Defined SHELL_SHOW_TOTP_SECRET_DISPLAY_WIDTH constant |
| source/libs/parser/src/parTokenizer.c | Added TK_TOTP_SECRET token |
| source/libs/parser/src/parAstCreater.c | Implemented AST creation functions for CREATE/DROP TOTP_SECRET statements |
| source/libs/parser/inc/sql.y | Added grammar rules for CREATE/DROP TOTP_SECRET statements, moved ALTER DNODES RELOAD line |
| source/libs/parser/inc/parAst.h | Added function declarations for TOTP secret statement creators |
| source/libs/parser/src/parTranslater.c | Implemented translation functions and result schema extraction for TOTP secret operations |
| source/libs/nodes/src/nodesUtilFuncs.c | Added node type handling for CREATE/DROP_TOTP_SECRET_STMT |
| source/dnode/mnode/impl/src/mndUser.c | Implemented core TOTP secret management logic with base32 encoding |
| source/dnode/mnode/impl/src/mndTrans.c | Added transaction response handling for CREATE TOTP_SECRET |
| source/dnode/mnode/impl/src/mndPrivilege.c | Added stub privilege check function for community edition |
| source/dnode/mnode/impl/inc/mndUser.h | Added mndBuildSMCreateTotpSecretResp declaration |
| source/dnode/mnode/impl/inc/mndPrivilege.h | Added mndCheckTotpSecretPrivilege declaration |
| source/dnode/mgmt/mgmt_mnode/src/mmHandle.c | Registered message handlers for CREATE/DROP TOTP_SECRET |
| source/common/src/msg/tmsg.c | Implemented serialization/deserialization functions for TOTP secret messages |
| source/libs/scheduler/src/schRemote.c | Added response handling for CREATE TOTP_SECRET message type |
| source/client/src/clientMsgHandler.c | Implemented client-side response handling and result building for TOTP secret operations |
| source/client/test/clientTests.cpp | Added base32 decode function and updated TOTP test to use new CREATE TOTP_SECRET statement |
| source/os/src/osRand.c | Implemented taosSafeRandBytes for secure random byte generation |
| include/os/osRand.h | Added taosSafeRandBytes function declaration |
| include/libs/nodes/cmdnodes.h | Defined result constants and statement structures for TOTP secret operations |
| include/common/tmsgdef.h | Added TDMT_MND_CREATE_TOTP_SECRET and TDMT_MND_DROP_TOTP_SECRET message types |
| include/common/tmsg.h | Added TOTP secret request/response structures and node types |
| test/cases/81-Tools/02-Taos/test_tool_taos_cli.py | Updated test to use new CREATE TOTP_SECRET statement |
| docs/zh/14-reference/03-taos-sql/60-user.md | Added Chinese documentation for TOTP secret management and VNODE_PER_CALL parameter |
| docs/en/14-reference/03-taos-sql/60-user.md | Added English documentation for TOTP secret management and VNODE_PER_CALL parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
support create/dorp totp_secret statement.
Issue(s)
Checklist
Please check the items in the checklist if applicable.