-
Notifications
You must be signed in to change notification settings - Fork 89
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: argon2id configuration override #96
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master goravel/framework#96 +/- ##
==========================================
+ Coverage 51.52% 51.55% +0.02%
==========================================
Files 103 103
Lines 7028 7032 +4
==========================================
+ Hits 3621 3625 +4
Misses 3126 3126
Partials 281 281
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
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.
Great! By the way, could we add a unit test to cover the issue?
Add it. |
@@ -59,6 +59,12 @@ func (s *ApplicationTestSuite) TestCheckHash() { | |||
} | |||
} | |||
|
|||
func (s *ApplicationTestSuite) TestConfigurationOverride() { | |||
value := "$argon2id$v=19$m=65536,t=8,p=1$NlVjQm5PQUdWTHVTM1RBUg$Q5T7WfeCI7ucIdk6Na6AdQ" |
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.
我们可以根据你自定义配置的发生问题的过程生成这个加密串么?而不是固定写死,这样测试是不是不太能覆盖到问题?
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.
我们可以根据你自定义配置的发生问题的过程生成这个加密串么?而不是固定写死,这样测试是不是不太能覆盖到问题?
问题的哈希是从数据库取出来的,因为是其他程序生成的,所以参数不一样。
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.
LGTM
Resolve goravel/goravel#158