-
Notifications
You must be signed in to change notification settings - Fork 5k
fix: fix three bugs #34314
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
fix: fix three bugs #34314
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 addresses two critical bugs: one preventing the correct enforcement of password reuse policies and another causing the 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 addresses two bugs: one related to reusable passwords and another concerning the allow_datetime feature. The changes in mndUser.c correctly adjust the logic for dropping old passwords from history, ensuring the password reuse policy is effective. The fix in tmsg.c aligns the allow_datetime check with the server's local time, as documented. Additionally, the TOTP login tests have been significantly improved for better coverage and robustness. The changes are well-implemented, but I have a couple of minor suggestions to improve code quality and documentation.
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 fixes two bugs related to password reuse and datetime whitelist functionality. The first bug prevented reusable passwords from being reused correctly due to incorrect expire time calculation. The second bug caused allow_datetime to not work as expected because it wasn't respecting the server's local timezone.
Changes:
- Fixed password reuse logic by correcting expire time calculation in
dropOldPasswordsfunction - Fixed datetime whitelist validation by converting UTC timestamps to local time before calculating week seconds
- Updated documentation to clarify that datetime restrictions are based on server's local timezone
- Enhanced test coverage for TOTP login functionality with comprehensive interval testing
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| source/dnode/mnode/impl/src/mndUser.c | Fixed password reuse bug by using the correct password's setTime as expire time and added dropOldPasswords call in mndUserDupObj |
| source/common/src/msg/tmsg.c | Fixed allow_datetime bug by converting timestamps to local time before calculating week seconds |
| docs/zh/14-reference/03-taos-sql/60-user.md | Updated Chinese documentation to clarify that datetime restrictions use server's local timezone |
| docs/en/14-reference/03-taos-sql/60-user.md | Updated English documentation to clarify that datetime restrictions use server's local timezone |
| test/cases/24-Users/test_user_totp.py | Enhanced test coverage with comprehensive TOTP interval testing and removed obsolete test setup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
fix bugs:
Issue(s)
Close/close/Fix/fix/Resolve/resolve:
Checklist
Please check the items in the checklist if applicable.