Console icon indicating copy to clipboard operation
Console copied to clipboard

Calls to the "restfulv2" service do not check supplied password

Open jomiham opened this issue 6 years ago • 3 comments

Expected Behavior

After enabling the restfulv2 service, requests to /-/script/v2/master/<scriptname>?user=remoteuser&password=invalidpassword should result in a permission denied response.

This would be in line with the documentation and tests:

Actual Behavior

As long as the user is valid and has access to the restfulv2 service, the script is run regardless of the password that is sent.

Steps to Reproduce the Problem

Enable the restfulv2 service, e.g. with:

<configuration xmlns:patch="http://www.sitecore.net/xmlconfig/">
  <sitecore>
    <powershell>
      <services>
        <restfulv2 patch:instead="restfulv2" enabled="true" requireSecureConnection="false">
          <authorization>
            <add Permission="Allow" IdentityType="Role" Identity="sitecore\PowerShell Extensions Remoting"/>
          </authorization>
        </restfulv2>
      </services>
    </powershell>
  </sitecore>
</configuration>

Create a user "remoteuser" with password "secret" and add to the "PowerShell Extensions Remoting" role.

Create a PowerShell module with a "Web api" endpoint and add a simple "ping" script:

echo "pong!"

Call the script via the rest api (from outside Sitecore):

Invoke-RestMethod -Method Post -Uri "<sitecoreuri>/-/script/v2/master/ping?user=sitecore\remoteuser&password=invalidpw"

As far as I can tell, this seems to be deliberate in RemoteScriptCall.ashx.cs:

        private static bool CheckServiceAuthentication(HttpContext context, string apiVersion, string serviceName, bool isAuthenticated)
        {
            var skipAuthentication = false;

            switch (apiVersion)
            {
                case "1":
                case "2":
                    skipAuthentication = true;
                    break;
                default:
                    if (!isAuthenticated)
                    {
                        const string disabledMessage =
                            "The request could not be completed because the service requires authentication.";

                        context.Response.StatusCode = 401;
                        context.Response.StatusDescription = disabledMessage;
                        context.Response.SuppressFormsAuthenticationRedirect = true;
                        PowerShellLog.Error($"Attempt to call the {serviceName} service failed as - user not logged in, authentication failed, or no credentials provided.");
                    }

                    break;
            }

            return skipAuthentication || isAuthenticated;
        }

Please include the version number of SPE and Sitecore.

  • Sitecore v8.2-Update 5

  • SPE v4.7 and v5.0

  • [ ] Tested issue with clean install of Sitecore and the latest available version of SPE.

  • [x] Asked questions on the Sitecore Slack Chat channel.

  • [x] Reviewed questions and answers on the Sitecore Stack Exchange.

jomiham avatar Mar 04 '19 14:03 jomiham

would you expect the authentication to happen only for master? What would you expect for web?

michaellwest avatar Mar 04 '19 14:03 michaellwest

My assumption would be that if you either allow anonymous access, or require some credentials. Requiring an account name, but no password/token/whatever seems very strange to me. That said, there may very well be some use case where this makes sense...

I guess that calls to "web" would be things like content apis and would be directed to the CD servers? If you have the case where unpublished "master" content should be protected, but published "web" content should be public, maybe just rely on config roles and let the CDs declare their own <authorization> config as required?

jomiham avatar Mar 04 '19 15:03 jomiham

From what I can remember, this was intended for internal access to web (CM) when wanting to use the Web API to run scripts, such as an HTML report for your pointy-haired boss. If the script was missing then 404, otherwise get an output from the report.

I would think this should be updated to authenticate when master is the database. Then add a way to force authentication always to enable backwards compatibility.

michaellwest avatar Mar 04 '19 17:03 michaellwest