-
Notifications
You must be signed in to change notification settings - Fork 285
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
[Bug] Disabled users are able to login #264
Conversation
560646e
to
4429dec
Compare
@@ -32,8 +32,8 @@ public enum SeatunnelErrorEnum { | |||
|
|||
USERNAME_PASSWORD_NO_MATCHED( | |||
10007, | |||
"username and password no matched", | |||
"The user name and password do not match, please check your input"), | |||
"username and password no matched or user is disabled.", |
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.
please fix the typo,
"username and password no matched or user is disabled.", | |
"username and password not matched or user is disabled." |
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.
The existing message had the typo. I retained the same message initially, but it can be corrected. I have now corrected it.
@@ -94,7 +94,7 @@ | |||
select | |||
<include refid="Base_Column_List"/> | |||
from `user` | |||
where username = #{username,jdbcType=VARCHAR} and password = #{password,jdbcType=VARCHAR} | |||
where status = 0 and username = #{username,jdbcType=VARCHAR} and password = #{password,jdbcType=VARCHAR} |
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.
status=1 semantically makes sense for a user to be enabled, 0 should be considered disabled. Can we please check, what changes would be required for the same?
we can revisit UserServiceImpl#add
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.
This is how currently user status is stored in the database. In my opinion, reversing the status code does not constitute a significant code improvement. Therefore, I prefer to maintain the existing approach.
4429dec
to
ba841e2
Compare
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.
Thanks @arshadmohammad and @shashwatsai
Thanks @shashwatsai , @Hisoka-X for review. |
Purpose of this pull request
Check list
New License Guide