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

[ISSUE #11957] Remove default password #11991

Merged
merged 4 commits into from
Apr 29, 2024

Conversation

godhth
Copy link
Contributor

@godhth godhth commented Apr 21, 2024

Please do not create a Pull Request without creating an issue first.

What is the purpose of the change

#11957

Brief changelog

XX

Verifying this change

XXXX

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
  • Run mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=true to make sure basic checks pass. Run mvn clean install -DskipITs to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.

@godhth godhth closed this Apr 21, 2024
@godhth godhth reopened this Apr 21, 2024
Copy link
Collaborator

@KomachiSion KomachiSion left a comment

Choose a reason for hiding this comment

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

  1. 好像测试有大规模的失败,建议排查一下原因
  2. 关于如果没有GLOBAL_ADMIN_ROLE用户的时候, 应该在更前面的地方判断,并且直接弹出,权限校验部分应该维持现状,就是没有user或没有权限的报错。

@KomachiSion
Copy link
Collaborator

测试用例已修复, 请同步develop分支后重新提交

@godhth
Copy link
Contributor Author

godhth commented Apr 23, 2024

done

Copy link
Collaborator

@KomachiSion KomachiSion left a comment

Choose a reason for hiding this comment

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

整体的方案我觉得可以重新考虑一下。

我觉得只需要判断一下是否数据库中有admin用户即可, 并且记录这个状态,然后暴露状态;

此时应该有以下几个行为:

  1. 控制台通过读取这个状态,当没有初始化admin用户时, 转跳到一个单独的页面,来创建admin用户(具体可以讨论,是用户密码都自定义,还是用户名是nacos,密码让用户自定义)。
  2. 提供一个新API,对于不方便使用控制台的用户,通过调用这个API来设置nacos用户的密码;该接口只能不带任何身份信息调用一次,如果已经存在admin级别的用户,这个接口应该拒绝访问。
  3. 其他的正常接口访问,不需要额外判断逻辑, 走正常鉴权的时候因为没有用户信息,正常报错即可。

@godhth godhth requested a review from KomachiSion April 26, 2024 01:28
@KomachiSion KomachiSion merged commit 70ad2eb into alibaba:develop Apr 29, 2024
7 checks passed
@KomachiSion KomachiSion added the kind/feature type/feature label Apr 29, 2024
@KomachiSion KomachiSion added this to the 2.4.0 milestone Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature type/feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants