Skip to content

feature: enforce account initialization and disable default credentials #7261

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

Open
wants to merge 12 commits into
base: 2.x
Choose a base branch
from

Conversation

YongGoose
Copy link
Contributor

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

fix #6396

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@YongGoose
Copy link
Contributor Author

YongGoose commented Mar 31, 2025

@slievrly

Also, we need to consider how to handle this logic in the build workflow.
Currently, it expects username and password input, but input is not possible in a build workflow.

That's why the workflow below does not terminate.
https://github.com/apache/incubator-seata/actions/runs/14169655810/job/39690431239?pr=7261

@YongGoose
Copy link
Contributor Author

@slievrly

Also, we need to consider how to handle this logic in the build workflow. Currently, it expects username and password input, but input is not possible in a build workflow.

That's why the workflow below does not terminate. https://github.com/apache/incubator-seata/actions/runs/14169655810/job/39690431239?pr=7261

I resolved the problem in 78dd208 by adding environment variables during the CI stage.

Copy link

codecov bot commented Apr 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 55.76%. Comparing base (ac99e5b) to head (cfe6d24).

Additional details and impacted files
@@            Coverage Diff            @@
##                2.x    #7261   +/-   ##
=========================================
  Coverage     55.75%   55.76%           
  Complexity     7469     7469           
=========================================
  Files          1178     1178           
  Lines         41962    41962           
  Branches       4923     4923           
=========================================
+ Hits          23396    23399    +3     
+ Misses        16328    16323    -5     
- Partials       2238     2240    +2     

see 5 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@funky-eyes funky-eyes changed the title Feature: enforce account initialization and disable default credentials eature: enforce account initialization and disable default credentials Apr 29, 2025
@funky-eyes funky-eyes changed the title eature: enforce account initialization and disable default credentials feature: enforce account initialization and disable default credentials Apr 29, 2025
user.setUsername(username);
user.setPassword(new BCryptPasswordEncoder().encode(password));
public void init() throws IOException {
String envUsername = System.getenv("SEATA_CONSOLE_USERNAME");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requiring users to read credentials (username and password) directly from environment variables would impose significant changes to their workflow and create a poor user experience. Instead, we can default to a predefined username (e.g., seata) and remove any default password. If no password is configured, the system will automatically generate a random password, log it for user reference, and notify them:

"No password was configured. A random password has been generated for security purposes. You may either:

  1. Use the auto-generated password (see logs for details), or
  2. Set a custom password in the configuration."

This approach ensures backward compatibility while guiding users toward secure practices.

Copy link
Contributor Author

@YongGoose YongGoose Apr 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requiring users to read credentials (username and password) directly from environment variables would impose significant changes to their workflow and create a poor user experience. Instead, we can default to a predefined username (e.g., seata) and remove any default password. If no password is configured, the system will automatically generate a random password, log it for user reference, and notify them:

"No password was configured. A random password has been generated for security purposes. You may either:

  1. Use the auto-generated password (see logs for details), or
  2. Set a custom password in the configuration."

This approach ensures backward compatibility while guiding users toward secure practices.

Thank you for your comment!

As you pointed out, suddenly requiring users to provide credentials would lead to a poor user experience.
Following your suggestion, I will implement it so that if no password is set in the YAML configuration file, the system will generate a random password, log it, and provide guidance to the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in a02be95 !

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The modified result is now being logged as shown below!

22:56:58.549  INFO --- [                     main] [y.CustomUserDetailsServiceImpl] [                init]  [] : No password was configured. A random password has been generated for security purposes. You may either:
1. Use the auto-generated password: [424e287f]
2. Set a custom password in the configuration.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done

@funky-eyes funky-eyes added this to the 2.5.0 milestone Apr 29, 2025
@funky-eyes funky-eyes added module/console type: feature Category issues or prs related to feature request. labels Apr 29, 2025
@YongGoose YongGoose requested a review from funky-eyes April 29, 2025 13:56
@YongGoose
Copy link
Contributor Author

YongGoose commented Apr 29, 2025

The problem currently occurs only with JDK 17, and I'm in the process of analyzing it.

Caused by: java.lang.NoClassDefFoundError: Could not initialize class org.apache.catalina.startup.Tomcat
Caused by: java.lang.ExceptionInInitializerError

The most likely cause at this point appears to be a compatibility issue between JDK 17 and Tomcat version 9.0.99.
If this turns out to be the case, we plan to upgrade Tomcat to version 9.1.00 with separate PR.

@YongGoose
Copy link
Contributor Author

The problem currently occurs only with JDK 17, and I'm in the process of analyzing it.

Caused by: java.lang.NoClassDefFoundError: Could not initialize class org.apache.catalina.startup.Tomcat
Caused by: java.lang.ExceptionInInitializerError

The most likely cause at this point appears to be a compatibility issue between JDK 17 and Tomcat version 9.0.99. If this turns out to be the case, we plan to upgrade Tomcat to version 9.1.00 with separate PR.

See #7329

Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM @slievrly PTAL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/console type: feature Category issues or prs related to feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prohibiting Default Username and Password Logins.
2 participants