wso2 / product-is

Welcome to the WSO2 Identity Server source code! For info on working with the WSO2 Identity Server repository and contributing code, click the link below.
http://wso2.github.io/
Apache License 2.0
746 stars 724 forks source link

Password grant does not trim the whitespaces in the username #10774

Open rksk opened 3 years ago

rksk commented 3 years ago

Describe the issue: If we invoke the password grant with whitespaces at the beginning or the end, the sub of the issued id_token contains the same username with whitespace while the real user on the userstore does not have whitespace in the username.

The same issue is existing in the basic authenticator and we have avoided it by trimming the username in the authentication endpoint.

How to reproduce:

Expected behavior: The sub should contain the correct username on the userstore

Environment information

mefarazath commented 3 years ago

@janakamarasena @senthalan Will we ever need to allow leading space or trailing in any request parameters in the /oauth2/token request?

I am thinking rather than handling this specifically for the password grant, to remove leading and trailing whitespaces on all grant type params. WDYT?

senthalan commented 3 years ago

@janakamarasena @senthalan Will we ever need to allow leading space or trailing in any request parameters in the /oauth2/token request?

I am thinking rather than handling this specifically for the password grant, to remove leading and trailing whitespaces on all grant type params. WDYT? +1

And I think it is better to handle trimming the space in the server-side as well. I hope this method will be used by all the authenticators which handler the username input.

https://github.com/wso2/carbon-identity-framework/blob/v5.18.218/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/util/FrameworkUtils.java#L2660

janakamarasena commented 3 years ago

+1 for the suggestions. We shouldn't need leading or trailing whitespace.

chamathns commented 3 years ago

Migara Pramod will work on this.