silverstripe / silverstripe-mfa

MultiFactor Authentication for Silverstripe CMS
BSD 3-Clause "New" or "Revised" License
11 stars 24 forks source link

JS `api()` issue with trailing slashes #475

Open OldStarchy opened 1 year ago

OldStarchy commented 1 year ago

A trailing slash in a URL can break the endpoint hit by the JavaScript api wrapper https://github.com/silverstripe/silverstripe-mfa/blob/8e6a6c83a52d8cdebe8c8586c905a0ac81ba2fcd/client/src/lib/api.js?rgh-link-date=2022-12-07T10%3A35%3A21Z#L5-L8

Please see more information in the original issue here https://github.com/xddesigners/otp-authenticator/issues/1#issuecomment-1340484353

When accessing the API from javascript with api('mfa/otp/registerto') the URL that gets hit depends on the current URL and the existence of a tag.

The existence of a trailing slash causes the URL to change.

/Security/login/default/mfa => /Security/login/default/mfa/otp/registerto /Security/login/default/mfa/ => /Security/login/default/mfa/mfa/otp/registerto

We use axllent/silverstripe-trailing-slash and would have been getting this error however another developer had implemented a workaround.

The fix I implemented manually works out the full URL

fetch(
  location.origin + location.pathname.replace(/(^|\/)[^\/]+\/?$/, '$1') + endpoint,

This regex removes the last segment in the URL regardless of a trailing slash. Though I believe a better fix can be implemented by passing a base API endpoint from the PHP somehow, though I'm not familiar enough with the inner workings to know how to implement that.

michalkleiner commented 1 year ago

Yeah, the path should get joined correctly regardless of the trailing slash.

emteknetnz commented 1 year ago

I did a quick little investigation into this - endpoints come from here on the PHP side:

I tried installing https://github.com/axllent/silverstripe-trailing-slash as well - though looking at the XHR response from /Security/login/default/mfa/schema (or /Security/login/default/mfa/schema/ when the trailing slash module was installed) when I visited /Security/login/default/mfa - the json response was the same

{
        ...
    "endpoints": {
        "register": "/Security/login/default/mfa/register/{urlSegment}",
        "verify": "/Security/login/default/mfa/verify/{urlSegment}",
        "complete": "/Security/login/default/mfa/complete",
        "skip": "/Security/login/default/mfa/skip"
    }
}

The {urlSegment} is string replace client side in a few places e.g. https://github.com/silverstripe/silverstripe-mfa/blob/4.7/client/src/components/Register.js#L87

I won't investigate any further at this point - though I can't help thinking this may be something more to with otp-authenticator, or just the trailing slash module (both not officially supported module) rather than the mfa module itself?

e.g. It seems like the otp-authenticator module doesn't use the same 'register' functionality that e.g. it uses this endpoint for registration instead of the 'register' endpoint in the json sample above