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

fix ldap userId with case problem #2326

Merged
merged 5 commits into from
Jun 20, 2019
Merged

Conversation

idefav
Copy link
Contributor

@idefav idefav commented Jun 11, 2019

@idefav
Copy link
Contributor Author

idefav commented Jun 11, 2019

我采用ldap系统里面的用户ID代替用户登录ID,不然用户登录输入的用户ID可能大小写都不一样,这样收藏功能使用有问题
image

@idefav
Copy link
Contributor Author

idefav commented Jun 11, 2019

@nobodyiam 帮忙看看有没有问题

@idefav
Copy link
Contributor Author

idefav commented Jun 11, 2019

登录时候输入的用户ID
image
显示的ID
image

然后收藏功能基本上失效了

image

@codecov-io
Copy link

codecov-io commented Jun 11, 2019

Codecov Report

Merging #2326 into master will decrease coverage by 7.81%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2326      +/-   ##
============================================
- Coverage     57.69%   49.88%   -7.82%     
- Complexity     1130     1991     +861     
============================================
  Files           196      407     +211     
  Lines          5943    12457    +6514     
  Branches        636     1278     +642     
============================================
+ Hits           3429     6214    +2785     
- Misses         2254     5800    +3546     
- Partials        260      443     +183
Impacted Files Coverage Δ Complexity Δ
...portal/spi/configuration/LdapExtendProperties.java 0% <ø> (ø) 0 <0> (?)
...tal/spi/ldap/ApolloLdapAuthenticationProvider.java 0% <0%> (ø) 0 <0> (?)
.../portal/spi/configuration/LdapGroupProperties.java 0% <0%> (ø) 0 <0> (?)
...lo/portal/spi/configuration/AuthConfiguration.java 5.78% <0%> (ø) 1 <0> (?)
...ortal/spi/configuration/LdapMappingProperties.java 0% <0%> (ø) 0 <0> (?)
...work/apollo/biz/message/DatabaseMessageSender.java 56.25% <0%> (-10.42%) 6% <0%> (-2%)
...ework/apollo/portal/controller/ItemController.java 6.32% <0%> (ø) 1% <0%> (?)
...o/configservice/wrapper/DeferredResultWrapper.java 100% <0%> (ø) 12% <0%> (?)
...omponent/emailbuilder/GrayPublishEmailBuilder.java 11.53% <0%> (ø) 1% <0%> (?)
... and 208 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 3bfed3c...0787751. Read the comment docs.

@coveralls
Copy link

coveralls commented Jun 11, 2019

Coverage Status

Coverage decreased (-0.1%) to 53.492% when pulling c82cb3c on idefav:master into b84f5b3 on ctripcorp:master.

@nobodyiam
Copy link
Member

改动点有些大,如果只是解决收藏问题的话,是不是只要忽略前端传过来的userId,直接用当前用户的userId就可以了?

@idefav
Copy link
Contributor Author

idefav commented Jun 17, 2019

改动点有些大,如果只是解决收藏问题的话,是不是只要忽略前端传过来的userId,直接用当前用户的userId就可以了?

这样改是简单一些,但是userinfoHolder里面的userId还是用户输入的ID,这个地方是不是应该是ldap系统里面真实的userId
image

而且这个地方也需要修改
image

需要改几个地方

所以我这里通过重写LdapAuthenticationProvider里面的authenticate方法修改这个userId为Ldap系统里面的ID

@idefav
Copy link
Contributor Author

idefav commented Jun 18, 2019

已经修改了 @nobodyiam

Copy link
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

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

LGTM

@nobodyiam nobodyiam merged commit 1a5db6c into apolloconfig:master Jun 20, 2019
@nobodyiam nobodyiam added this to the 1.5.0 milestone Aug 10, 2019
CrackerCat pushed a commit to CrackerCat/apollo-1 that referenced this pull request Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants