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

nacos认证时loadUserByUsername()方法存在缺陷 #9318

Closed
David-wu91 opened this issue Oct 13, 2022 · 12 comments
Closed

nacos认证时loadUserByUsername()方法存在缺陷 #9318

David-wu91 opened this issue Oct 13, 2022 · 12 comments
Labels
kind/discussion Category issues related to discussion plugin

Comments

@David-wu91
Copy link
Contributor

Describe the bug
A clear and concise description of what the bug is.
在进行用户认证时nacos配置文件中nacos.core.auth.caching.enabled=true是默认开启的,目前使用ldap认证方式时,用户首次认证成功以后会将用户账号和密码写入users表中,而ldap协议的服务是不区分用户名大小写的,因此不论用户名大写(TESTUSER001)、小写(testuser001)或者大小写混写(testUSER001)均能通过认证,然后数据认证通过以后写入数据库,同时将数据加入缓存中,但实际情况会出现下述错误。

Expected behavior
TESTUSER001、testuser001、testUSER001三种方式均能在ldap服务认证通过,因此在console页面登录时也应该能正常登录才对。

Actually behavior
当用户第一次登录时输入TESTUSER001并登录成功,那么用户以后每一次登录都必须如此填写才能登录成功,如果输入testuser001或testUSER001进行登录时,后台会报插入users表主键冲突,同时页面会提示登录错误。
问题出在com.alibaba.nacos.plugin.auth.impl.users.NacosUserDetailsServiceImpl实现类的reload()和loadUserByUsername(),username在数据库中一般是大小写不敏感的,但是将username通过ConcurrentHashMap进行缓存时则是大小写敏感的,因此在首次登录写入数据时是TESTUSER001,然后在后续登录时输入testuser001,无法在缓存中查到,程序会认为这是一个新用户就写入users表,而username在数据库中已经存在,所以会报Duplicate entry 'LDAP_TESTUSER001' for key 'PRIMARY'。

How to Reproduce
建议可以采用两种方式解决该问题:
方案一:在console登录页面用户登录时,无论输入什么,提交的时候页面统一转为小写(或大写)传入后端;
方案二:在NacosUserDetailsServiceImpl实现类的reload()方法第62行map.put()将用户名统一转小写(或大写)并写入缓存,然后loadUserByUsername()第73行userMap.get(username)时统一将username和reload()方法中的转换逻辑一致

目前我们通过关闭缓存nacos.core.auth.caching.enabled=false解决了这个问题

Desktop (please complete the following information):

  • OS: [e.g. Centos]
  • Version [e.g. nacos-server 1.3.1, nacos-client 1.3.1]
  • Module [e.g. naming/config]
  • SDK [e.g. original, spring-cloud-alibaba-nacos, dubbo]

Additional context
Add any other context about the problem here.

@KomachiSion
Copy link
Collaborator

本来ladp和nacos的鉴权实现就是两个插件实现,方案一和二都不对,默认插件实现就是大小写敏感的,如果要解决这个问题,就应该缓存保留大小写敏感,然后在ladp插件里面去转大写或小写。

@KomachiSion KomachiSion added plugin kind/discussion Category issues related to discussion labels Oct 14, 2022
@David-wu91
Copy link
Contributor Author

本来ladp和nacos的鉴权实现就是两个插件实现,方案一和二都不对,默认插件实现就是大小写敏感的,如果要解决这个问题,就应该缓存保留大小写敏感,然后在ladp插件里面去转大写或小写。

你说得对,因为你们在初始化表结构的脚本中使用的是utf8_bin字符集,确实是大小写敏感的,但是我们公司统一使用的是utf8_general_ci,是大小写不敏感的,所以我已此为基础提供了上面的两个建议方案。
确实最好的办法是LdapAuthenticationProvider实现类中最大小转统一,这样能不影响nacos鉴权的大小敏感设计(严格根据创nacos-mysql.sql脚本进行初始化能保证);但直接修改LdapAuthenticationProvider实现类进行统一大小写会影响默认ldap鉴权方式大小写敏的设计,它能解决我们公司目前使用微软AD域控(大小写不敏感)作为ldap服务的问题,但可能对某些大小写敏感的LDAP服务产生影响;
因此综合考虑,是否可以将默认的LDAP鉴权方式的大小写敏感作为一个配置项,让各公司自己选择;LDAP鉴权是否支持大小写敏感更为合适呢?

@KomachiSion
Copy link
Collaborator

那最好的办法是开发自己公司的LDAP鉴权插件, 而不是作为一个特殊需求提交到社区。

@David-wu91
Copy link
Contributor Author

目前console模块的鉴权实现LdapAuthenticationProvider支持自定义吗?我看到代码里面是写死的,SPI的鉴权自定义好像改不了这部分的实现呢。

NacosAuthConfig实现类:
@OverRide
protected void configure(AuthenticationManagerBuilder auth) throws Exception {
if (AuthSystemTypes.NACOS.name().equalsIgnoreCase(authConfigs.getNacosAuthSystemType())) {
auth.userDetailsService(userDetailsService).passwordEncoder(passwordEncoder());
} else if (AuthSystemTypes.LDAP.name().equalsIgnoreCase(authConfigs.getNacosAuthSystemType())) {
auth.authenticationProvider(ldapAuthenticationProvider);
}
}

@KomachiSion
Copy link
Collaborator

都用SPI写自己的插件了, 这个默认的LDAP完全都不走。 和这个代码没什么关系。

@KomachiSion
Copy link
Collaborator

可以征集一下社区意见,如果这个大小写敏感有很多人也遇到了,可以作为一个插件的配置项添加。 不过之前一直都没有人提交,我估计这个需求可能比较少。

@David-wu91
Copy link
Contributor Author

可以征集一下社区意见,如果这个大小写敏感有很多人也遇到了,可以作为一个插件的配置项添加。 不过之前一直都没有人提交,我估计这个需求可能比较少。

确实在市场上使用ldap协议的服务都使用得比较少,使用微软AD域控的公司可能就更少了,因此遇到这个问题的公司不会太多。但是我想LDAP协议认证作为社区官方提供的标准实现之一,从这个插件的健全性来看添加一个配置项确实更完整一些。

@KomachiSion
Copy link
Collaborator

那你提一个PR修改一下吧, 默认值保持现状即可。

@David-wu91
Copy link
Contributor Author

那你提一个PR修改一下吧, 默认值保持现状即可。

好的,我本地已经修改好验证过了,之前我提过另关于LDAP鉴权插件的问题(#9175)你们已经改好了,这次改动需要在那个代码的基础上再进行提交吗?

@David-wu91
Copy link
Contributor Author

那你提一个PR修改一下吧, 默认值保持现状即可。

代码提交时报错了,请问是不是需要社区授权以后才能push代码呢?

Push failed:
Remote: Not Found repository 'https://[email protected]/David-wu91/alibaba/nacos.git/' not found

@KomachiSion
Copy link
Collaborator

David-wu91 pushed a commit to David-wu91/nacos that referenced this issue Nov 1, 2022
@David-wu91
Copy link
Contributor Author

David-wu91 added a commit to David-wu91/nacos that referenced this issue Nov 1, 2022
David-wu91 added a commit to David-wu91/nacos that referenced this issue Nov 3, 2022
David-wu91 added a commit to David-wu91/nacos that referenced this issue Nov 3, 2022
KomachiSion pushed a commit that referenced this issue Nov 7, 2022
* resolve issue #9318 add ldap filter config:caseSensitive

* resolve issue #9318 change code style

* resolve issue #9318 add unit test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/discussion Category issues related to discussion plugin
Projects
None yet
Development

No branches or pull requests

2 participants