Skip to content
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

feature: isCommonlyUsed password check not hardcoded #4018

Closed
DiegoKrupitza opened this issue Oct 7, 2021 · 11 comments · Fixed by #4207
Closed

feature: isCommonlyUsed password check not hardcoded #4018

DiegoKrupitza opened this issue Oct 7, 2021 · 11 comments · Fixed by #4207
Assignees
Labels
area/portal apollo-portal feature request Categorizes issue as related to a new feature.

Comments

@DiegoKrupitza
Copy link
Contributor

DiegoKrupitza commented Oct 7, 2021

Is your feature request related to a problem? Please describe.
Fragments of passwords that seem insecure may change from time to time and may be wanted to change. For example a company password policy might change and won't allow a certain pattern in their password (lets say the company name).

Describe the solution you'd like
I suggest that the LIST_OF_CODE_FRAGMENT List that is currently hardcoded in com.ctrip.framework.apollo.portal.util.checker.AuthUserPasswordChecker should be extracted into a file that can be change 24/7 (aka while running).
The administrator can define the location of the file inside a property file or smth similar like that.
If no location is defined there is a default file inside the project that could be used. The default file may contain the already existing hardcoded list.

Describe alternatives you've considered
Alternatively to storing the LIST_OF_CODE_FRAGMENTs inside a file, the list could be stored inside a database. This will make it easier to maintain inside the administration panel.

Additional context
I think this feature implemented in #4008 is really great but not hardcoding this fragment list may make it more future proof and better maintainable.

@nobodyiam
Copy link
Member

I think it makes sense to make LIST_OF_CODE_FRAGMENT configurable, @WillardHu what do you think?

@WillardHu
Copy link
Contributor

Okay, I'll fix it. Can I insert a row of data store configuration in the serverconfig table? @nobodyiam

@nobodyiam
Copy link
Member

@WillardHu I think it's ok to have the default configurations hardcoded in the program, what needs to be done is to add a logic to read from PortalConfig so that it could be customized by ServerConfig and spring configuration files like application.properties?

@WillardHu
Copy link
Contributor

@WillardHu I think it's ok to have the default configurations hardcoded in the program, what needs to be done is to add a logic to read from PortalConfig so that it could be customized by ServerConfig and spring configuration files like application.properties?

@nobodyiam Okey, I’m going to define the static constant as the default. And determine if ServerConfig has data to replace the default static constant. As for spring configuration files, I think it does not have the ability to dynamically modify at runtime.

@WillardHu
Copy link
Contributor

I can design the custom extension as an interface, first implementing the ServerConfig mode and reserving the Spring configuration file mode.

@nobodyiam
Copy link
Member

PortalConfig extends RefreshableConfig, which means it could be hot-reloaded. So I think it's the user to decide where to maintain the custom LIST_OF_CODE_FRAGMENT. If it is maintained in ServerConfig then it will be effective in 1 min. If it is maintained in application.properties, then it won't be effective until next reboot.

@DiegoKrupitza
Copy link
Contributor Author

PortalConfig extends RefreshableConfig, which means it could be hot-reloaded. So I think it's the user to decide where to maintain the custom LIST_OF_CODE_FRAGMENT. If it is maintained in ServerConfig then it will be effective in 1 min. If it is maintained in application.properties, then it won't be effective until next reboot.

I think it would be a really nice feature to make that configurable. Like the admin can decide if it should be hot-reloaded (since this takes more resources), in 1min interval or if it should be only reboot.

@WillardHu
Copy link
Contributor

PortalConfig extends RefreshableConfig, which means it could be hot-reloaded. So I think it's the user to decide where to maintain the custom LIST_OF_CODE_FRAGMENT. If it is maintained in ServerConfig then it will be effective in 1 min. If it is maintained in application.properties, then it won't be effective until next reboot.

Ok, I understand. Please assign this issue to me and I will completed it.

@DiegoKrupitza
Copy link
Contributor Author

@WillardHu I cant assign the issue to you. @nobodyiam has to do it

@DiegoKrupitza
Copy link
Contributor Author

@WillardHu are you still working on this?

@WillardHu
Copy link
Contributor

Sorry, I have been busy with my work recently and it will take some time to finish it

WillardHu added a commit to WillardHu/apollo that referenced this issue Jan 13, 2022
Signed-off-by: WillardHu <wei.hu@daocloud.io>
WillardHu added a commit to WillardHu/apollo that referenced this issue Jan 13, 2022
Signed-off-by: WillardHu <wei.hu@daocloud.io>
WillardHu added a commit to WillardHu/apollo that referenced this issue Jan 14, 2022
Signed-off-by: WillardHu <wei.hu@daocloud.io>
@Anilople Anilople added area/portal apollo-portal feature request Categorizes issue as related to a new feature. labels Jan 16, 2022
WillardHu added a commit to WillardHu/apollo that referenced this issue Jan 17, 2022
Signed-off-by: WillardHu <wei.hu@daocloud.io>
WillardHu added a commit to WillardHu/apollo that referenced this issue Jan 18, 2022
Signed-off-by: WillardHu <wei.hu@daocloud.io>
WillardHu added a commit to WillardHu/apollo that referenced this issue Jan 19, 2022
Signed-off-by: WillardHu <wei.hu@daocloud.io>
WillardHu added a commit to WillardHu/apollo that referenced this issue Jan 19, 2022
Signed-off-by: WillardHu <wei.hu@daocloud.io>
Anilople pushed a commit that referenced this issue Jan 19, 2022
Signed-off-by: WillardHu <wei.hu@daocloud.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/portal apollo-portal feature request Categorizes issue as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants