[Plus symbol issue] - UserAuthenticationTokenService sends requests with 'Content-Type' - 'application/x-www-form-urlencoded'

[DENCREAT]

If a user login or password contains a symbol + it would be replaced with whitespaces 'cause Content-Type is equal to application/x-www-form-urlencoded. As a result, we have problems with users authorization.

Environment Details

• Spartacus: 1.0.4

Steps to Reproduce

• Go to the login page
• Type a login
• Type a password which contains a plus or pluses (e.g. 12345+qq)

Observed Results

All pluses would be replaced with whitespaces when we send form data.

Expected Results

Login and password are sent without changes.

Additional Information

As I can see UserAuthenticationTokenService sends requests with 'Content-Type' - 'application/x-www-form-urlencoded', Probably a problem is here.

https://spartacus-storefront.slack.com/archives/CD16V16FR/p1566915251244300

[giancorderoortiz]

See also: https://github.com/SAP/cloud-commerce-spartacus-storefront/issues/4038

[meeque]

Well, ultimately the code that builds the above request should perform appropriate URL encoding for all data that it submits to the oAuth server.

I'm a little surprised that the http.post method of the HttpParams class does not take care of this? Not sure who else would be responsible for URL encoding?

Alternatively, the problem could be on the other side: maybe the URL decoding logic in the SAP Commerce oAuth extension is buggy?

Could someone post a dump of a HTTP request that triggers this bug?

[meeque]

Hm, I have now dumped the HTTP body that is sent when I enter !@#$%^&*+= in the login field:

 client_id=spartacus&client_secret=&grant_type=password&username=test@example.net&password=!@%23$%25%5E%26*+=

So it seams that most characters that require URL encoding get encoded properly. However the + character does not get encoded, which causes the confusion. I also wonder why the @ character is not encoded.

Could this be a bug in the underlying rest framework?

[meeque]

Fyi, here's some sources of how URL encoding should be done properly:

https://developer.mozilla.org/en-US/docs/Glossary/percent-encoding https://tools.ietf.org/html/rfc3986#section-2.1

[meeque]

And here is what the HttpParams class does by default:

https://github.com/angular/angular/blob/8.2.4/packages/common/http/src/params.ts#L81

Frankly this looks rather dodgy to me. They first use the browser's encodeURIComponent() method, but then they revert the encoding of several characters, including the + sign. I wonder what's the purpose of that? But it seems just plain wrong.

However, one could pass in a custom implementation of a HttpParameterCodec that would perform sane URL encoding. I wonder if that could fix the problem?

[KateChuen]

basically we cannot have + signs in email addresses. But if we have + signs in passwords, and it becomes a whitespace, then it might be a problem.

1) Please validate that we only accept characters that the specs(RFC5322) mention. 2) Verify that passwords indeed do accept all characters (according to backend definition) and they are not escaped. Break it into 2 tickets if necessary.

[meeque]

Frankly, I think that the question of having + signs in email addresses is off point here. That is ultimately a decision that the server-side has to make. The problem is that our frontend code does not transmit + signs to the server-side API correctly. This seems to be a general problem when working with Angular's rest client. Imho we need a general solution for that!

That said, I've read parts of RFC5322, and there is no reason to believe that + signs are forbidden in the local-part of an address. See addr-spec and dot-atom...

[DENCREAT]

Hi everyone! What news?)

[tobi-or-not-tobi]

Thanks @meeque for chiming in on this topic. Angular indeed doesn't support clean encoding/decoding, which is why we do not get proper encoding for various characters (@:$,;+=?/). I've gone through various resources related to this topic, and I'm not sure why these characters are skipped.

We should allow for proper encoding, but not without an option to customise this. My recommendation is to:

• implement a HttpParameterCodec that solely uses the encodeURIComponent and decodeURIComponent from the browser
• use the encoder wherever we use HttpParameters, but at least in the area of creating users and passwords
• allow for overridden this behaviour, either by DI or by configuring a class in the config module

[artem-zur]

@tobi-or-not-tobi,

Hi! Do you have any updates about this issue ? :)

[giancorderoortiz]

@Xymmer, @hackergil , @tobi-or-not-tobi are there any updates on this issue?

[Xymmer]

let's aim to do this one for 2.1 or during summer

[halilugur]

Hi everyone Unfortunately, we had the same problem. As a workaround, we encoded the password and sent it like that. For this, we had to override both the frontend and the backend. Frontend

const pass = encodeURIComponent(password.value);
this.auth.authorize(userId.value.toLowerCase(), pass);

Backtend This had to be override CoreAuthenticationProvider.

if (credential instanceof String) {
    // if credential is a string use URLDecode class. 
    credential = URLDecoder.decode(String.valueOf(credential), StandardCharsets.UTF_8);
    if (!user.checkPassword((String) credential)) {
        throw new BadCredentialsException(this.messages.getMessage("CoreAuthenticationProvider.badCredentials", "Bad credentials"));
    }
} else {
    if (!(credential instanceof LoginToken)) {
        throw new BadCredentialsException(this.messages.getMessage("CoreAuthenticationProvider.badCredentials", "Bad credentials"));
    }
   if (!user.checkPassword((LoginToken) credential)) {
       throw new BadCredentialsException(this.messages.getMessage("CoreAuthenticationProvider.badCredentials", "Bad credentials"));
     }
}

I hope it helps,

[meeque]

Hm, it seems weird that users like @halilugur still need to implement custom work-arounds for this issue. It's even worse that this requires additional changes on the server-side. Imho, the server-side is not at fault here.

Has there ever been an attempt to solve this for good, as @tobi-or-not-tobi suggested over a year ago?

[DENCREAT]

are there any news?

[MangoMan996]

For me the fix was using the below encoder with HttpPrams instead of the default encoder (spartacus 4.3.6, angular 12); Note the below code is a part of HTTP interceptor custom class that I implemented, it basically intercept the http call and clone the request with the new encoder with the original request params.

import { HttpParamsURIEncoder } from '@spartacus/core';

  copy(source: HttpParams): HttpParams {
    let target = new HttpParams({ encoder: new **HttpParamsURIEncoder**() });

    source.keys().forEach((key) => {
      source.getAll(key)?.forEach((val) => {
        target = target.append(key, val);
      });
    });

    return target;
  }