add Oidc preferred username #3629

vdiskg · 2021-04-04T03:35:50Z

What's the purpose of this PR

add a human readable username for oidc user

Which issue(s) this PR fixes:

codecov-io · 2021-04-04T04:35:10Z

Codecov Report

Merging #3629 (7850814) into master (3de1d72) will decrease coverage by 0.50%.
The diff coverage is 4.50%.

@@             Coverage Diff              @@
##             master    #3629      +/-   ##
============================================
- Coverage     50.97%   50.47%   -0.51%     
- Complexity     2329     2330       +1     
============================================
  Files           449      451       +2     
  Lines         14026    14178     +152     
  Branches       1426     1448      +22     
============================================
+ Hits           7150     7156       +6     
- Misses         6389     6536     +147     
+ Partials        487      486       -1

Impacted Files	Coverage Δ	Complexity Δ
.../com/ctrip/framework/apollo/common/dto/AppDTO.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...com/ctrip/framework/apollo/common/dto/BaseDTO.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...ork/apollo/common/utils/PreferredUsernameUtil.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...work/apollo/portal/entity/bo/ReleaseHistoryBO.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...trip/framework/apollo/portal/entity/po/UserPO.java	`4.16% <0.00%> (-0.60%)`	`1.00 <0.00> (ø)`
...ctrip/framework/apollo/portal/spi/UserService.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...lo/portal/spi/configuration/AuthConfiguration.java	`5.55% <0.00%> (ø)`	`2.00 <0.00> (ø)`
...i/oidc/OidcAuthenticationSuccessEventListener.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...ollo/portal/spi/oidc/OidcLocalUserServiceImpl.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
.../spi/springsecurity/SpringSecurityUserService.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3de1d72...7850814. Read the comment docs.

nobodyiam · 2021-04-05T01:30:07Z

For oidc implementation, is it possible to follow the similar implementation of LdapUserService and CtripUserService to query the backend service when searching the users? So that no user info should be stored in local db and the user name could be retrieved on the fly.

https://github.com/ctripcorp/apollo/blob/943bbb4d56bd9d3bbec291ea17136099a3a60b7c/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/ldap/LdapUserService.java#L122-L129

https://github.com/ctripcorp/apollo/blob/943bbb4d56bd9d3bbec291ea17136099a3a60b7c/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/ctrip/CtripUserService.java#L100-L106

apollo-portal/src/main/resources/static/config.html

vdiskg · 2021-04-05T03:04:25Z

For oidc implementation, is it possible to follow the similar implementation of LdapUserService and CtripUserService to query the backend service when searching the users? So that no user info should be stored in local db and the user name could be retrieved on the fly.

https://github.com/ctripcorp/apollo/blob/943bbb4d56bd9d3bbec291ea17136099a3a60b7c/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/ldap/LdapUserService.java#L122-L129

https://github.com/ctripcorp/apollo/blob/943bbb4d56bd9d3bbec291ea17136099a3a60b7c/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/ctrip/CtripUserService.java#L100-L106

it seems impossible because there is no standard way to search users in the oidc scheme. some oidc provider such as keycloak, provided it's custom admin sdk for search users, while others like okta is not even provide a way to search users.

...lo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java

...t/java/com/ctrip/framework/apollo/configservice/integration/AbstractBaseIntegrationTest.java

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/CommitController.java

...al/src/main/java/com/ctrip/framework/apollo/portal/controller/NamespaceBranchController.java

...tal/src/main/java/com/ctrip/framework/apollo/portal/controller/ReleaseHistoryController.java

apollo-common/src/main/java/com/ctrip/framework/apollo/common/utils/PreferredUsernameUtil.java

apollo-portal/src/main/resources/static/scripts/directive/directive.js

apollo-portal/src/main/resources/static/views/component/namespace-panel-master-tab.html

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/NamespaceService.java

nobodyiam

Thanks for the great effort, it looks much better!
Please find the latest comments below.

...lo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ReleaseHistoryService.java

...main/java/com/ctrip/framework/apollo/portal/service/AdditionalUserInfoEnrichServiceImpl.java

nobodyiam · 2021-05-01T02:31:45Z

scripts/apollo-on-kubernetes/db/portal-db/apolloportaldb.sql

@@ -284,6 +284,7 @@ CREATE TABLE `Users` (
  `Id` int(10) unsigned NOT NULL AUTO_INCREMENT COMMENT '自增Id',
  `Username` varchar(64) NOT NULL DEFAULT 'default' COMMENT '用户名',
  `Password` varchar(64) NOT NULL DEFAULT 'default' COMMENT '密码',
+  `PreferredUsername` varchar(512) NOT NULL DEFAULT 'default' COMMENT '用户名称',


I'm wondering is it better to call it UserDisplayName or simply DisplayName so that the intention is only for display purpose?
Currently we already have this concept in CtripUserService and LdapUserService:

https://github.com/ctripcorp/apollo/blob/de05540e6c9a18cdf370ae95ca5e544d7b8d04bf/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/ctrip/CtripUserService.java#L100-L106

https://github.com/ctripcorp/apollo/blob/de05540e6c9a18cdf370ae95ca5e544d7b8d04bf/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/ldap/LdapUserService.java#L122-L129

If we agree on this, then we may also update the variable names in the programs. I understand it will cause a big refactoring but I think it's worth to clarify the concept.

…name # Conflicts: # apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java

github-actions · 2021-05-15T12:58:40Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

vdiskg · 2021-05-15T13:20:52Z

I have read the CLA Document and I hereby sign the CLA

vdiskg · 2021-05-15T13:21:10Z

recheck

nobodyiam · 2021-05-16T02:26:02Z

...tal/src/main/java/com/ctrip/framework/apollo/portal/enricher/AdditionalUserInfoEnricher.java

@@ -0,0 +1,22 @@
+package com.ctrip.framework.apollo.portal.enricher;
+
+import com.ctrip.framework.apollo.common.dto.BaseDTO;


please remove the unused imports

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/NamespaceService.java

nobodyiam · 2021-05-16T02:30:59Z

...rc/main/java/com/ctrip/framework/apollo/portal/enricher/adapter/UserInfoEnrichedAdapter.java

+/**
+ * @author vdisk <vdisk@foxmail.com>
+ */
+public interface UserInfoEnrichedAdapter {


This abstraction looks good to me.

nobodyiam · 2021-05-20T13:12:41Z

@vdisk-group please check your email account (***@foxmail.com), there should be an invitation mail.

…service/NamespaceService.java Co-authored-by: Jason Song <nobodyiam@gmail.com>

...tal/src/main/java/com/ctrip/framework/apollo/portal/enricher/AdditionalUserInfoEnricher.java

…name

nobodyiam

LGTM

vdiskg added 6 commits April 3, 2021 21:38

table Users add column PreferredUsername

55e471e

Users add PreferredUsername sql

e97c949

Users add PreferredUsername page

48ef240

Users add PreferredUsername app page

a9d4510

update user's info after login

1561c5e

sql

6487608

vdiskg mentioned this pull request Apr 4, 2021

Username is not corrected with ocdi user's subject #3625

Closed

fix unit test

0f7f5e9

nobodyiam reviewed Apr 5, 2021

View reviewed changes

apollo-portal/src/main/resources/static/config.html Show resolved Hide resolved

vdiskg added 6 commits April 5, 2021 11:26

change the page

20f8f39

fix the unit test timeout

723e068

commit page add preferred username

db6afc9

namespace page add preferred username

1d4f575

release history page add preferred username

7850814

preferred username sql

4f477e7

nobodyiam reviewed Apr 10, 2021

View reviewed changes

vdiskg and others added 4 commits April 10, 2021 18:01

move logic to service

106d1f8

fix unit test time out

abe329b

namespaceService unit test

4d074e8

Merge branch 'master' into oidc-preferred-username

21d928a

nobodyiam reviewed Apr 11, 2021

View reviewed changes

vdiskg added 6 commits April 17, 2021 10:55

enricher

5fd63e2

name space enrich change

148289e

CommitService enrich change

a61e796

revert NamespaceBOUtils

78da8f1

namespace branch page

5358d34

enricher adjust

e4de699

nobodyiam reviewed May 1, 2021

View reviewed changes

vdiskg and others added 7 commits May 8, 2021 18:39

temp

1e63a3b

preferred username enrich adjust

21580b3

UserDisplayName

f7ae63a

fix unit test

8e7b163

Merge branch 'fix-unit-test' into oidc-pre-username-temp

030ac20

Merge remote-tracking branch 'apollo/master' into oidc-preferred-user…

1b39773

…name # Conflicts: # apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java

Merge branch 'master' into oidc-preferred-username

9e67af2

nobodyiam reviewed May 16, 2021

View reviewed changes

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/NamespaceService.java Outdated Show resolved Hide resolved

nobodyiam reviewed May 16, 2021

View reviewed changes

vdiskg and others added 3 commits May 22, 2021 10:07

Update apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/…

223ef17

…service/NamespaceService.java Co-authored-by: Jason Song <nobodyiam@gmail.com>

cleanup code

87b0833

Merge branch 'master' into oidc-preferred-username

44b6a75

nobodyiam reviewed May 22, 2021

View reviewed changes

...tal/src/main/java/com/ctrip/framework/apollo/portal/enricher/AdditionalUserInfoEnricher.java Show resolved Hide resolved

License

e04ee8e

nobodyiam added this to the 1.9.0 milestone May 23, 2021

vdiskg added 5 commits May 25, 2021 09:51

add License

cf72a14

License

2c58545

Merge branch 'oidc-pre-temp-2' into oidc-preferred-username

021da89

Merge remote-tracking branch 'apollo/master' into oidc-preferred-user…

31c4f79

…name

add License

191cdd4

nobodyiam approved these changes May 25, 2021

View reviewed changes

nobodyiam merged commit 150955f into apolloconfig:master May 25, 2021

github-actions bot locked and limited conversation to collaborators May 25, 2021

vdiskg deleted the oidc-preferred-username branch June 27, 2021 08:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add Oidc preferred username #3629

add Oidc preferred username #3629

vdiskg commented Apr 4, 2021

codecov-io commented Apr 4, 2021 •

edited

Loading

nobodyiam commented Apr 5, 2021

vdiskg commented Apr 5, 2021

nobodyiam left a comment

nobodyiam May 1, 2021

nobodyiam May 1, 2021

github-actions bot commented May 15, 2021 •

edited

Loading

vdiskg commented May 15, 2021

vdiskg commented May 15, 2021

nobodyiam May 16, 2021

nobodyiam May 16, 2021

nobodyiam commented May 20, 2021

nobodyiam left a comment

		@@ -0,0 +1,22 @@
		package com.ctrip.framework.apollo.portal.enricher;

		import com.ctrip.framework.apollo.common.dto.BaseDTO;

add Oidc preferred username #3629

add Oidc preferred username #3629

Conversation

vdiskg commented Apr 4, 2021

What's the purpose of this PR

Which issue(s) this PR fixes:

codecov-io commented Apr 4, 2021 • edited Loading

Codecov Report

nobodyiam commented Apr 5, 2021

vdiskg commented Apr 5, 2021

nobodyiam left a comment

Choose a reason for hiding this comment

nobodyiam May 1, 2021

Choose a reason for hiding this comment

nobodyiam May 1, 2021

Choose a reason for hiding this comment

github-actions bot commented May 15, 2021 • edited Loading

vdiskg commented May 15, 2021

vdiskg commented May 15, 2021

nobodyiam May 16, 2021

Choose a reason for hiding this comment

nobodyiam May 16, 2021

Choose a reason for hiding this comment

nobodyiam commented May 20, 2021

nobodyiam left a comment

Choose a reason for hiding this comment

codecov-io commented Apr 4, 2021 •

edited

Loading

github-actions bot commented May 15, 2021 •

edited

Loading