fix: use double encoded resource ids to support special characters in…

… email address as user login (#939, #940) BREAKING CHANGES: Introduced double encoding of resource ids in REST API calls for user login (see [Migrations / 2.4 to 3.0](https://github.com/intershop/intershop-pwa/blob/develop/docs/guides/migrations.md#2.4-to-30) for more details).
intershop · Jul 19, 2022 · 2e8f7e4 · 2e8f7e4
1 parent 7c6a226
commit 2e8f7e4
Show file tree

Hide file tree

Showing 15 changed files with 75 additions and 42 deletions.
diff --git a/docs/guides/migrations.md b/docs/guides/migrations.md
@@ -42,6 +42,11 @@ We no longer use the verbose way of injecting the `PLATFORM_ID` and check it wit
 This way still works but it is discouraged by a new ESLint rule that suggests using the new `SSR` variable instead.
 So running `npm run lint` will help with finding custom code that still relies on the platform checks.
 
+To support e.g. special characters in email addresses with newer versions of ICM (7.10.38.x), like `+`, double encoding of resource ids in the REST API calls is necessary.
+With the method `encodeResourceID` we provide a central place that implements the fitting resource encoding.
+In the PWA this was applied to all user logins in REST API calls.
+For project customizations the usage of the native `encodeURIComponent` functionality should be replaced with `encodeResourceID` for user logins in REST calls as well.
+
 ## 2.3 to 2.4
 
 The PWA 2.4 contains an Angular update to version 13.3.10 and many other dependencies updates.

diff --git a/projects/organization-management/src/app/services/cost-centers/cost-centers.service.spec.ts b/projects/organization-management/src/app/services/cost-centers/cost-centers.service.spec.ts
@@ -111,7 +111,7 @@ describe('Cost Centers Service', () => {
     costCentersService.updateCostCenterBuyer('123', costCenterBuyer).subscribe(() => {
       verify(apiService.patch(anything(), anything())).once();
       expect(capture(apiService.patch).last()[0]).toMatchInlineSnapshot(
-        `"customers/4711/costcenters/123/buyers/pmiller@test.intershop.de"`
+        `"customers/4711/costcenters/123/buyers/pmiller%2540test.intershop.de"`
       );
       done();
     });
@@ -121,7 +121,7 @@ describe('Cost Centers Service', () => {
     costCentersService.deleteCostCenterBuyer('123', 'pmiller@test.intershop.de').subscribe(() => {
       verify(apiService.delete(anything())).once();
       expect(capture(apiService.delete).last()[0]).toMatchInlineSnapshot(
-        `"customers/4711/costcenters/123/buyers/pmiller@test.intershop.de"`
+        `"customers/4711/costcenters/123/buyers/pmiller%2540test.intershop.de"`
       );
       done();
     });

diff --git a/projects/organization-management/src/app/services/cost-centers/cost-centers.service.ts b/projects/organization-management/src/app/services/cost-centers/cost-centers.service.ts
@@ -10,6 +10,7 @@ import { Link } from 'ish-core/models/link/link.model';
 import { ApiService, AvailableOptions } from 'ish-core/services/api/api.service';
 import { getLoggedInCustomer } from 'ish-core/store/customer/user';
 import { whenTruthy } from 'ish-core/utils/operators';
+import { encodeResourceID } from 'ish-core/utils/url-resource-ids';
 
 @Injectable({ providedIn: 'root' })
 export class CostCentersService {
@@ -152,7 +153,10 @@ export class CostCentersService {
     return this.currentCustomer$.pipe(
       switchMap(customer =>
         this.apiService
-          .patch(`customers/${customer.customerNo}/costcenters/${costCenterId}/buyers/${buyer.login}`, buyer)
+          .patch(
+            `customers/${customer.customerNo}/costcenters/${costCenterId}/buyers/${encodeResourceID(buyer.login)}`,
+            buyer
+          )
           .pipe(concatMap(() => this.getCostCenter(costCenterId)))
       )
     );
@@ -176,7 +180,7 @@ export class CostCentersService {
     return this.currentCustomer$.pipe(
       switchMap(customer =>
         this.apiService
-          .delete(`customers/${customer.customerNo}/costcenters/${costCenterId}/buyers/${login}`)
+          .delete(`customers/${customer.customerNo}/costcenters/${costCenterId}/buyers/${encodeResourceID(login)}`)
           .pipe(concatMap(() => this.getCostCenter(costCenterId)))
       )
     );

diff --git a/...services/organization-management-breadcrumb/organization-management-breadcrumb.service.ts b/...services/organization-management-breadcrumb/organization-management-breadcrumb.service.ts
@@ -6,6 +6,7 @@ import { map, switchMap, withLatestFrom } from 'rxjs/operators';
 import { AppFacade } from 'ish-core/facades/app.facade';
 import { BreadcrumbItem } from 'ish-core/models/breadcrumb-item/breadcrumb-item.interface';
 import { whenFalsy, whenTruthy } from 'ish-core/utils/operators';
+import { encodeResourceID } from 'ish-core/utils/url-resource-ids';
 
 import { OrganizationManagementFacade } from '../../facades/organization-management.facade';
 
@@ -42,7 +43,7 @@ export class OrganizationManagementBreadcrumbService {
                     { key: 'account.organization.user_management', link: `${prefix}/users` },
                     {
                       text: `${translation} - ${user.firstName} ${user.lastName}`,
-                      link: `${prefix}/users/${user.login}`,
+                      link: `${prefix}/users/${encodeResourceID(user.login)}`,
                     },
                     {
                       key: `account.user.update_${path.substring(path.lastIndexOf('/') + 1)}.heading`,

diff --git a/projects/organization-management/src/app/services/users/users.service.spec.ts b/projects/organization-management/src/app/services/users/users.service.spec.ts
@@ -63,7 +63,7 @@ describe('Users Service', () => {
       verify(apiService.get(anything())).once();
       expect(capture(apiService.get).last()).toMatchInlineSnapshot(`
         Array [
-          "customers/4711/users/pmiller%40test.intershop.de",
+          "customers/4711/users/pmiller%2540test.intershop.de",
         ]
       `);
       done();
@@ -75,7 +75,7 @@ describe('Users Service', () => {
       verify(apiService.delete(anything())).once();
       expect(capture(apiService.delete).last()).toMatchInlineSnapshot(`
         Array [
-          "customers/4711/users/pmiller%40test.intershop.de",
+          "customers/4711/users/pmiller%2540test.intershop.de",
         ]
       `);
       done();
@@ -98,7 +98,7 @@ describe('Users Service', () => {
     usersService.updateUser(user).subscribe(() => {
       verify(apiService.put(anything(), anything())).once();
       expect(capture(apiService.put).last()[0]).toMatchInlineSnapshot(
-        `"customers/4711/users/pmiller%40test.intershop.de"`
+        `"customers/4711/users/pmiller%2540test.intershop.de"`
       );
       done();
     });
@@ -116,7 +116,7 @@ describe('Users Service', () => {
       verify(apiService.put(anything(), anything())).once();
       expect(capture(apiService.put).last()).toMatchInlineSnapshot(`
         Array [
-          "customers/4711/users/pmiller%40test.intershop.de/roles",
+          "customers/4711/users/pmiller%2540test.intershop.de/roles",
           Object {
             "userRoles": Array [],
           },
@@ -140,7 +140,7 @@ describe('Users Service', () => {
         verify(apiService.put(anything(), anything())).once();
         expect(capture(apiService.put).last()).toMatchInlineSnapshot(`
           Array [
-            "customers/4711/users/pmiller%40test.intershop.de/budgets",
+            "customers/4711/users/pmiller%2540test.intershop.de/budgets",
             Object {
               "budget": Object {
                 "currency": "USD",

diff --git a/projects/organization-management/src/app/services/users/users.service.ts b/projects/organization-management/src/app/services/users/users.service.ts
@@ -8,6 +8,7 @@ import { PriceHelper } from 'ish-core/models/price/price.helper';
 import { ApiService, unpackEnvelope } from 'ish-core/services/api/api.service';
 import { getLoggedInCustomer } from 'ish-core/store/customer/user';
 import { whenTruthy } from 'ish-core/utils/operators';
+import { encodeResourceID } from 'ish-core/utils/url-resource-ids';
 
 import { B2bRoleData } from '../../models/b2b-role/b2b-role.interface';
 import { B2bRoleMapper } from '../../models/b2b-role/b2b-role.mapper';
@@ -52,7 +53,7 @@ export class UsersService {
     return this.currentCustomer$.pipe(
       switchMap(customer =>
         this.apiService
-          .get(`customers/${customer.customerNo}/users/${encodeURIComponent(login)}`)
+          .get(`customers/${customer.customerNo}/users/${encodeResourceID(login)}`)
           .pipe(map(B2bUserMapper.fromData))
       )
     );
@@ -119,7 +120,7 @@ export class UsersService {
     return this.currentCustomer$.pipe(
       switchMap(customer =>
         this.apiService
-          .put(`customers/${customer.customerNo}/users/${encodeURIComponent(user.login)}`, {
+          .put(`customers/${customer.customerNo}/users/${encodeResourceID(user.login)}`, {
             ...customer,
             ...user,
             preferredInvoiceToAddress: { urn: user.preferredInvoiceToAddressUrn },
@@ -146,9 +147,7 @@ export class UsersService {
     }
 
     return this.currentCustomer$.pipe(
-      switchMap(customer =>
-        this.apiService.delete(`customers/${customer.customerNo}/users/${encodeURIComponent(login)}`)
-      )
+      switchMap(customer => this.apiService.delete(`customers/${customer.customerNo}/users/${encodeResourceID(login)}`))
     );
   }
 
@@ -172,7 +171,7 @@ export class UsersService {
     return this.currentCustomer$.pipe(
       switchMap(customer =>
         this.apiService
-          .put(`customers/${customer.customerNo}/users/${encodeURIComponent(login)}/roles`, { userRoles })
+          .put(`customers/${customer.customerNo}/users/${encodeResourceID(login)}/roles`, { userRoles })
           .pipe(
             unpackEnvelope<B2bRoleData>('userRoles'),
             map(data => data.map(r => r.roleID))
@@ -196,7 +195,7 @@ export class UsersService {
     return this.currentCustomer$.pipe(
       switchMap(customer =>
         this.apiService.put<UserBudget>(
-          `customers/${customer.customerNo}/users/${encodeURIComponent(login)}/budgets`,
+          `customers/${customer.customerNo}/users/${encodeResourceID(login)}/budgets`,
           budget
         )
       )

diff --git a/src/app/core/services/api/api.service.ts b/src/app/core/services/api/api.service.ts
@@ -27,6 +27,7 @@ import {
 import { communicationTimeoutError, serverError } from 'ish-core/store/core/error';
 import { getLoggedInCustomer, getLoggedInUser, getPGID } from 'ish-core/store/customer/user';
 import { whenTruthy } from 'ish-core/utils/operators';
+import { encodeResourceID } from 'ish-core/utils/url-resource-ids';
 
 /**
  * Pipeable operator for elements translation (removing the envelope).
@@ -308,30 +309,26 @@ export class ApiService {
       get: <T>(path: string, options?: AvailableOptions) =>
         ids$.pipe(
           concatMap(([user, customer]) =>
-            this.get<T>(`customers/${customer.customerNo}/users/${encodeURIComponent(user.login)}/${path}`, options)
+            this.get<T>(`customers/${customer.customerNo}/users/${encodeResourceID(user.login)}/${path}`, options)
           )
         ),
       delete: <T>(path: string, options?: AvailableOptions) =>
         ids$.pipe(
           concatMap(([user, customer]) =>
-            this.delete<T>(`customers/${customer.customerNo}/users/${encodeURIComponent(user.login)}/${path}`, options)
+            this.delete<T>(`customers/${customer.customerNo}/users/${encodeResourceID(user.login)}/${path}`, options)
           )
         ),
       put: <T>(path: string, body = {}, options?: AvailableOptions) =>
         ids$.pipe(
           concatMap(([user, customer]) =>
-            this.put<T>(
-              `customers/${customer.customerNo}/users/${encodeURIComponent(user.login)}/${path}`,
-              body,
-              options
-            )
+            this.put<T>(`customers/${customer.customerNo}/users/${encodeResourceID(user.login)}/${path}`, body, options)
           )
         ),
       patch: <T>(path: string, body = {}, options?: AvailableOptions) =>
         ids$.pipe(
           concatMap(([user, customer]) =>
             this.patch<T>(
-              `customers/${customer.customerNo}/users/${encodeURIComponent(user.login)}/${path}`,
+              `customers/${customer.customerNo}/users/${encodeResourceID(user.login)}/${path}`,
               body,
               options
             )
@@ -341,7 +338,7 @@ export class ApiService {
         ids$.pipe(
           concatMap(([user, customer]) =>
             this.post<T>(
-              `customers/${customer.customerNo}/users/${encodeURIComponent(user.login)}/${path}`,
+              `customers/${customer.customerNo}/users/${encodeResourceID(user.login)}/${path}`,
               body,
               options
             )

diff --git a/src/app/core/services/authorization/authorization.service.ts b/src/app/core/services/authorization/authorization.service.ts
@@ -7,6 +7,7 @@ import { AuthorizationMapper } from 'ish-core/models/authorization/authorization
 import { Customer } from 'ish-core/models/customer/customer.model';
 import { User } from 'ish-core/models/user/user.model';
 import { ApiService } from 'ish-core/services/api/api.service';
+import { encodeResourceID } from 'ish-core/utils/url-resource-ids';
 
 @Injectable({ providedIn: 'root' })
 export class AuthorizationService {
@@ -21,7 +22,7 @@ export class AuthorizationService {
     }
 
     return this.apiService
-      .get<AuthorizationData>(`customers/${customer.customerNo}/users/${encodeURIComponent(user.login)}/roles`)
+      .get<AuthorizationData>(`customers/${customer.customerNo}/users/${encodeResourceID(user.login)}/roles`)
       .pipe(map(data => this.authorizationMapper.fromData(data)));
   }
 }
diff --git a/src/app/core/services/user/user.service.spec.ts b/src/app/core/services/user/user.service.spec.ts
@@ -13,6 +13,7 @@ import { User } from 'ish-core/models/user/user.model';
 import { ApiService, AvailableOptions } from 'ish-core/services/api/api.service';
 import { getUserPermissions } from 'ish-core/store/customer/authorization';
 import { getLoggedInCustomer, getLoggedInUser } from 'ish-core/store/customer/user';
+import { encodeResourceID } from 'ish-core/utils/url-resource-ids';
 
 import { UserService } from './user.service';
 
@@ -352,7 +353,9 @@ describe('User Service', () => {
 
     it("should get eligible cost centers for business user when 'getEligibleCostCenters' is called", done => {
       userService.getEligibleCostCenters().subscribe(() => {
-        verify(apiServiceMock.get(`customers/${customer.customerNo}/users/${user.login}/costcenters`)).once();
+        verify(
+          apiServiceMock.get(`customers/${customer.customerNo}/users/${encodeResourceID(user.login)}/costcenters`)
+        ).once();
         done();
       });
     });

diff --git a/src/app/core/services/user/user.service.ts b/src/app/core/services/user/user.service.ts
@@ -26,6 +26,7 @@ import { ApiService, AvailableOptions, unpackEnvelope } from 'ish-core/services/
 import { getUserPermissions } from 'ish-core/store/customer/authorization';
 import { getLoggedInCustomer, getLoggedInUser } from 'ish-core/store/customer/user';
 import { whenTruthy } from 'ish-core/utils/operators';
+import { encodeResourceID } from 'ish-core/utils/url-resource-ids';
 
 /**
  * The User Service handles the registration related interaction with the 'customers' REST API.
@@ -312,12 +313,10 @@ export class UserService {
     ]).pipe(
       take(1),
       switchMap(([customer, user]) =>
-        this.apiService
-          .get(`customers/${customer.customerNo}/users/${encodeURIComponent(user.login)}/costcenters`)
-          .pipe(
-            unpackEnvelope(),
-            map((costCenters: UserCostCenter[]) => costCenters)
-          )
+        this.apiService.get(`customers/${customer.customerNo}/users/${encodeResourceID(user.login)}/costcenters`).pipe(
+          unpackEnvelope(),
+          map((costCenters: UserCostCenter[]) => costCenters)
+        )
       )
     );
   }

diff --git a/src/app/core/utils/url-resource-ids.spec.ts b/src/app/core/utils/url-resource-ids.spec.ts
@@ -0,0 +1,11 @@
+import { encodeResourceID } from './url-resource-ids';
+
+describe('Url Resource Ids', () => {
+  describe('encode ids twice', () => {
+    it('should always return a double encoded string', () => {
+      expect(encodeResourceID('123456abc')).toEqual(`123456abc`);
+      expect(encodeResourceID('d.ori+6@test.intershop.de')).toEqual(`d.ori%252B6%2540test.intershop.de`);
+      expect(encodeResourceID('pmiller@test.intershop.de')).toEqual(`pmiller%2540test.intershop.de`);
+    });
+  });
+});
diff --git a/src/app/core/utils/url-resource-ids.ts b/src/app/core/utils/url-resource-ids.ts
@@ -0,0 +1,10 @@
+/**
+ * Encodes a given resource ID in a way that can be processed by ICM.
+ * To support e.g. special characters in email addresses, like '+', double encoding is necessary.
+ *
+ * @param resourceID    The resource ID to be encoded.
+ * @returns             The encoded resource ID.
+ */
+export function encodeResourceID(resourceID: string): string {
+  return encodeURIComponent(encodeURIComponent(resourceID));
+}
diff --git a/src/app/extensions/punchout/services/punchout/punchout.service.ts b/src/app/extensions/punchout/services/punchout/punchout.service.ts
@@ -12,6 +12,7 @@ import { getCurrentBasketId } from 'ish-core/store/customer/basket';
 import { getLoggedInCustomer } from 'ish-core/store/customer/user';
 import { CookiesService } from 'ish-core/utils/cookies/cookies.service';
 import { whenTruthy } from 'ish-core/utils/operators';
+import { encodeResourceID } from 'ish-core/utils/url-resource-ids';
 
 import { PunchoutSession } from '../../models/punchout-session/punchout-session.model';
 import { PunchoutType, PunchoutUser } from '../../models/punchout-user/punchout-user.model';
@@ -122,7 +123,7 @@ export class PunchoutService {
           .put<PunchoutUser>(
             `customers/${customer.customerNo}/punchouts/${this.getResourceType(
               user.punchoutType
-            )}/users/${encodeURIComponent(user.login)}`,
+            )}/users/${encodeResourceID(user.login)}`,
             user,
             {
               headers: this.punchoutHeaders,
@@ -146,7 +147,9 @@ export class PunchoutService {
     return this.currentCustomer$.pipe(
       switchMap(customer =>
         this.apiService.delete<void>(
-          `customers/${customer.customerNo}/punchouts/${this.getResourceType(user.punchoutType)}/users/${user.login}`,
+          `customers/${customer.customerNo}/punchouts/${this.getResourceType(
+            user.punchoutType
+          )}/users/${encodeResourceID(user.login)}`,
           {
             headers: this.punchoutHeaders,
           }

diff --git a/src/app/extensions/quoting/models/quoting/quoting.mapper.spec.ts b/src/app/extensions/quoting/models/quoting/quoting.mapper.spec.ts
@@ -21,7 +21,7 @@ describe('Quoting Mapper', () => {
     it('should map incoming quote request link to model data', () => {
       const data: QuoteData = {
         type: 'Link',
-        uri: 'inSPIRED-inTRONICS_Business-Site/-;loc=en_US;cur=USD/customers/OilCorp/users/jlink@test.intershop.de/quoterequests/ioMKCgoEcC4AAAF0BEAGFSQ5',
+        uri: 'inSPIRED-inTRONICS_Business-Site/-;loc=en_US;cur=USD/customers/OilCorp/users/jlink%2540test.intershop.de/quoterequests/ioMKCgoEcC4AAAF0BEAGFSQ5',
         title: 'ioMKCgoEcC4AAAF0BEAGFSQ5',
       };
       const mapped = quoteMapper.fromData(data, 'QuoteRequest');
@@ -37,7 +37,7 @@ describe('Quoting Mapper', () => {
     it('should map incoming quote link to model data', () => {
       const data: QuoteData = {
         type: 'Link',
-        uri: 'inSPIRED-inTRONICS_Business-Site/-;loc=en_US;cur=USD/customers/OilCorp/users/jlink@test.intershop.de/quotes/IpMKCgoEYBcAAAF0TgwGFSQd',
+        uri: 'inSPIRED-inTRONICS_Business-Site/-;loc=en_US;cur=USD/customers/OilCorp/users/jlink%2540test.intershop.de/quotes/IpMKCgoEYBcAAAF0TgwGFSQd',
         title: 'IpMKCgoEYBcAAAF0TgwGFSQd',
       };
       const mapped = quoteMapper.fromData(data, 'Quote');
@@ -53,7 +53,7 @@ describe('Quoting Mapper', () => {
     it('should map incoming quote request link with attributes to model data', () => {
       const data: QuoteData = {
         type: 'Link',
-        uri: 'inSPIRED-inTRONICS_Business-Site/-;loc=en_US;cur=USD/customers/OilCorp/users/jlink@test.intershop.de/quoterequests/ioMKCgoEcC4AAAF0BEAGFSQ5',
+        uri: 'inSPIRED-inTRONICS_Business-Site/-;loc=en_US;cur=USD/customers/OilCorp/users/jlink%2540test.intershop.de/quoterequests/ioMKCgoEcC4AAAF0BEAGFSQ5',
         title: 'ioMKCgoEcC4AAAF0BEAGFSQ5',
         attributes: [
           { name: 'number', type: 'String', value: '0000003' },
@@ -84,7 +84,7 @@ describe('Quoting Mapper', () => {
     it('should map incoming quote link with attributes to model data', () => {
       const data: QuoteData = {
         type: 'Link',
-        uri: 'inSPIRED-inTRONICS_Business-Site/-;loc=en_US;cur=USD/customers/OilCorp/users/jlink@test.intershop.de/quotes/IpMKCgoEYBcAAAF0TgwGFSQd',
+        uri: 'inSPIRED-inTRONICS_Business-Site/-;loc=en_US;cur=USD/customers/OilCorp/users/jlink%2540test.intershop.de/quotes/IpMKCgoEYBcAAAF0TgwGFSQd',
         title: 'IpMKCgoEYBcAAAF0TgwGFSQd',
         attributes: [
           { name: 'number', type: 'String', value: '0000003' },
@@ -268,7 +268,7 @@ describe('Quoting Mapper', () => {
         items: [
           {
             type: 'Link',
-            uri: 'inSPIRED-inTRONICS_Business-Site/-;loc=en_US;cur=USD/customers/OilCorp/users/jlink@test.intershop.de/quoterequests/Of4KCgoEGrcAAAF0kM8GFSQc/items/yMUKCgoEgGkAAAF0AdEGFSQc',
+            uri: 'inSPIRED-inTRONICS_Business-Site/-;loc=en_US;cur=USD/customers/OilCorp/users/jlink%2540test.intershop.de/quoterequests/Of4KCgoEGrcAAAF0kM8GFSQc/items/yMUKCgoEgGkAAAF0AdEGFSQc',
             title: 'yMUKCgoEgGkAAAF0AdEGFSQc',
           },
         ],

diff --git a/src/app/shared/forms/validators/special-validators.ts b/src/app/shared/forms/validators/special-validators.ts
@@ -59,7 +59,7 @@ export class SpecialValidators {
      * - no IPs allowed for login emails
      * - only some special characters allowed
      */
-    return /^([\w\-\~]+\.)*[\w\-\~]+@(([\w][\w\-]*)?[\w]\.)+[a-zA-Z]{2,}$/.test(control.value)
+    return /^([\w\-\~\+]+\.)*[\w\-\~\+]+@(([\w][\w\-]*)?[\w]\.)+[a-zA-Z]{2,}$/.test(control.value)
       ? undefined
       : { email: { valid: false } };
   }