Insecure oAuth Client Configuration

meeque commented 5 years ago

The Installing SAP Commerce Cloud for use with Spartacus guide contains a section on Configuring OCC Credentials. That section describes how to setup oAuth clients in SAP Commerce Cloud for use with Spartacus.

Risk

The current recommendation for oAuth clients seems bloated, inaccurate, and confusing. Therefore the recommended configuration exposes too much attack surface. Some of the exposed oAuth features are not required for Spartacus.

Points of Criticism

Currently the guide recommends oAuth client configuration based on the following ImpEx data:

INSERT_UPDATE OAuthClientDetails;clientId[unique=true]    ;resourceIds       ;scope        ;authorizedGrantTypes                                            ;authorities             ;clientSecret    ;registeredRedirectUri
                                ;client-side              ;hybris            ;basic        ;implicit,client_credentials                                     ;ROLE_CLIENT             ;secret          ;http://localhost:9001/authorizationserver/oauth2_implicit_callback;
                                ;mobile_android           ;hybris            ;basic        ;authorization_code,refresh_token,password,client_credentials    ;ROLE_CLIENT             ;secret          ;http://localhost:9001/authorizationserver/oauth2_callback;

This raises the following problems:

Excessive number of oAuth clients

The guide recommends 2 distinct oAuth clients (client-side and mobile_android) but one of them may be sufficient. It seems that only the mobile_android client is ever referenced in other Spartacus configuration documentation. Thus it appears to be the only one actually used by Spartacus.

If that is the case, the client-side client should be removed from documentation.

Confusing oAuth client ids

The recommended clientIds for oAuth clients (e.g. mobile_android) do not express the relationship of between the clients and Spartacus. Thus customers may be lead to reuse these clients for unrelated purposes, which could weaken their security configuration. Or, customers might accidentally delete the clients, which would lead to unavailability of the Spartacus storefront.

Consider renaming the client to something more descriptive, like spartacus or spartacus-storefront.

Unnecessary Grant Types

The recommended authorizedGrantTypes contain entries that are not required for Spartacus. Currently, Spartacus only makes use of the following grant types:

client_credentials
password

The recommended grant types should be restricted accordingly.

Broken Redirect URLs

None of the above grant types make use of redirect URLs, but the guide recommends to fill the registeredRedirectUri attribute nevertheless. Moreover, the recommended values for redirect URLs do not make any sense.

Consider dropping the redirect URL configuration altogether.

If Spartacus should ever require the use of redirect URLs, more documentation will be needed. The redirect URLs should point to a Spartacus storefront itself, not to SAP Commerce. Such redirect URLs will vary according to the customers' domain configuration.

Misleading Client Secret

In the Spartacus scenario, the clientSecret is not kept secret, but is published in non-confidential Spartacus configuration files. This is acceptable for a non-confidential oAuth client, but it should be documented.

Possibly the value should be adjusted accordingly, e.g. from secret to not-a-secret.

Suggested Mitigation:

Verify whether the above points can be applied to the oAuth client configuration without degrading Spartacus functionality.
Adjust oAuth client documentation accordingly.
Adjust related Spartacus documentation as needed. (E.g. adjust app.module.ts documentation, to match new client id and client secret.)

meeque commented 5 years ago

I've done some preliminary testing with the following client configuration ImpEx:

INSERT_UPDATE OAuthClientDetails;clientId[unique=true]   ;resourceIds   ;scope   ;authorizedGrantTypes          ;authorities   ;clientSecret
                                ;spartacus               ;hybris        ;basic   ;password,client_credentials   ;ROLE_CLIENT   ;

And in my Spartacus app.module.ts:

authentication: {
  client_id: 'spartacus',
  client_secret: ''
},

So far, everything works like a charm. Have not tested full checkout yet.

danilchican commented 4 years ago

@giancorderoortiz , maybe it make sense to prioritize this task?

SAP / spartacus