Avoid toString in favor of getName in order to extract sid

[Atry]

There are some more sophisticated implementations of getName in AbstractAuthenticationToken, JwtAuthenticationToken and other Authentication classes.

[rwinch]

Thanks for the PR! Can you please provide additional details on why you feel like this change is needed?

[Atry]

When creating a resource server from spring-security-acl and spring-security-oauth2, you will have a JwtAuthenticationToken authentication, and by default store a sid of Jwt@12345678 in acl_sid table, which is a wrong behavior.

[Atry]

We have authentication.getPrincipal().toString(); for historical reason. The original code is written before the availability of getName, which is implemented since Spring Security 5.0.

[rwinch]

Authentication has extended Principal and AbstractAuthentication has implemented getName since the Acegi days (i.e. Spring Security 1.0.x), so I don't think this is why it is implemented this way.

I think it does make sense to update the code as you suggested. However, I think we want a few changes:

• Let's remove the UserDetails check since that is already done in AbstractAuthentication
• Please add the necessary tests to ensure it works properly

[rwinch]

I see you pushed some changes. However, I don't see any tests or removal of the UserDetails bits above. Did something go wrong?

[Atry]

I just rollbacked some changes Rob Winch <notifications@github.com> 于2019年1月11日周五 上午12:12写道：

…

***@***.**** requested changes on this pull request. I see you pushed some changes. However, I don't see any tests or removal of the UserDetails bits above. Did something go wrong? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6354 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAktuiVdnmUqalTAusarYyRswPdm5oT2ks5vB2ZggaJpZM4ZyxCd> .

-- 杨博 (Yang Bo)

[rwinch]

Can you please add the requested changes?

[eleftherias]

@Atry Can you please add the requested tests? They should check that the correct principal was set. They will be similar to the following tests.

spring-security/acl/src/test/java/org/springframework/security/acls/sid/SidTests.java

Line 208 in 971ae34

assertThat("johndoe".equals(principalSid.getPrincipal())).isTrue();

[eleftherias]

Merged via ea148d5.