Openapi and api explorer to honor basePath

[dkrantsberg]

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

• npm test passes on your machine
• New tests added or existing tests modified to cover all changes
• Code conforms with the style guide
• API Documentation in code was updated
• Documentation in /docs/site was updated
• Affected artifact templates in packages/cli were updated
• Affected example projects in examples/* were updated

[bajtos]

Thank you for the pull request.

The proposed change has few subtle ramifications that need further discussion, see my comments below.

[bajtos]

I am also not sure we want these endpoints to honor basePath option.

@raymondfeng @strongloop/loopback-maintainers what's your opinion? When the application is configured with a custom basePath, e.g. /api, should we apply the base path to openapi endpoints too?

• Now we have /openapi.json and e.g. /api/products
• Proposed change: /api/openapi.json and e.g. /api/products

I am afraid the proposed implementation is a breaking change requiring a semver-major release. I think we should preserve backwards compatibility and introduce a new feature flag allowing users to decide whether to apply basePath configuration to openapi endpoints or not. By default, we should fall back to old behavior, but new projects scaffolded by lb4 cli should enable the new behavior.

[dkrantsberg]

@bajtos Having /explorer always running at the root creates problems when there's a reverse proxy running in front of the API. Here's our situation and I'm sure this is pretty common case for many production environments.

We have both UI and API applications running on the same host behind nginx.

UI runs at: http://www.mysite.com
Loopback API runs at: http://www.mysite.com/_api

Whenever a requests come for /_api/* nginx forwards it to Loopback API. All other requests to to the UI application.

As a result requests to http://www.mysite.com/explorer fail because they don't even reach the Loopback app. Hence my fix, which allows explorer to run at http://www.mysite.com/_api/explorer.

[raymondfeng]

I don't think basePath was introduced for this purpose. There are 3 paths that build a route to a controller method:

1. server-level basePath: it's used as _expressApp.use(basePath, <lb4-rest-router>);, such as /api
2. controller-class-level basePath (set by @api): it's prepended to the path from step 3, such as /products
3. controller-method-level path (set by @get, @post etc): local path of the operation, such as /{id}

Please note 1 is for express middleware mounting and it is transparent to our REST API routes. Express will strip /api from request.url property, which becomes /products/{id}.
2 + 3 is for internal routing by @loopback/rest.

Putting reverse proxy aside, the http url for a method is: /<serverBasePath>/<controllerBasePath>/<operationPath>, such as /api/products/{id}and api explorer is exposed at /explorer by default.

[dkrantsberg]

@raymondfeng Then my question is: is there a way to expose the API explorer at some different route (like /myprefix/explorer)?

I did create a workaround in my project but it's quite hacky and involves messing with the express route stack directly. I'd like to avoid doing it.

[bajtos]

@dkrantsberg I'd like to better understand your scenario.

We have both UI and API applications running on the same host behind nginx.

• UI runs at: http://www.mysite.com
• Loopback API runs at: http://www.mysite.com/_api

Do you have two servers, one for UI and another for LoopBack API? If that's the case, then I would expect you should be able to configure your nginx instance as follows:

• API requests (e.g. http://www.mysite.com/_api/products/{id}) are forwarded to LB instance and _api is stripped from the path (e.g. /products/{id}).
• As a result, explorer will become available at http://www.mysite.com/_api/explorer. Alternatively, I believe it should be possible to configure nginx and define a new virtual route (e.g. http://www.mysite.com/api-explorer) that will be forwarded to /explorer in LB.

I think the key is to keep empty basePath setting in LoopBack.

In case you are running both UI and LB API in the same server, how do you compose these two sets of routes?

[bajtos]

is there a way to expose the API explorer at some different route (like /myprefix/explorer)?

The self-hosted version of API Explorer that's provided by @loopback/rest-explorer is already honoring basePath setting and allows customization of the Explorer URL, see
https://github.com/strongloop/loopback-next/tree/master/packages/rest-explorer#basic-use

By default, API Explorer is mounted at /explorer. This path can be customized via RestExplorer configuration as follows:

this.bind(RestExplorerBindings.CONFIG).to({
  path: '/openapi/ui',
});

The redirection to externally-hosted API Explorer, which is built into @loopback/rest, does not support customization of the URL and it also ignores basePath setting. See here:
https://github.com/strongloop/loopback-next/blob/6e0eeb66b6a8c79d736fcc2dc7d9c76bfd57abd3/packages/rest/src/rest.server.ts#L288-L291

Please note that these redirects are deprecated, we want our users to use the self-hosted variant provided by @loopback/rest.

[bajtos]

IMO, we should preserve OPENAPI_SPEC_MAPPING as a constant config independent on basePath. The basePath setting should be applied when we are processing the mapping entries.

In your proposal, when app developers specify custom mapping, then their custom endpoints won't be prefixed with basePath. I would find that surprising.

In my proposal, basePath will be prepended to all mapping entries, regardless of whether they are provided by the user or come from the default mapping.

[dkrantsberg]

Leaving OPENAPI_SPEC_MAPPING where it is and applying basePath to it inside the RestServer constructor will mean recreating the entire OPENAPI_SPEC_MAPPING object with basePath added to the key names.
Is there a reason to do it considering that OPENAPI_SPEC_MAPPING constant isn't used anywhere else in the code other than RestServer constructor?

Yes, I propose that custom mapping should be truly custom and not be altered once it is set by a developer. IMO this will give developers more flexibility. But I'm fine either way as long as we have this PR moving forward.

My main blocker is inability to customize /explorer path.

[raymondfeng]

My main blocker is inability to customize /explorer path.

Yes, that's what I'm seeing now. Please note @loopback/rest-explorer mounts the UI as follows:

// explorePath is default to `/explorer`
application.static(explorerPath, swaggerUI.getAbsoluteFSPath()); 

But RestApplication.static() adds it to a sub router of the root router (/api). As a result, the UI is only available under /api/explorer while the spec is served at /openapi.json. Here is the express pipeline:

/ (RestServer._expressApp)
  /openapi.json
  /openapi.yaml
  /api
    /products
    /explorer (Explorer UI) 

If we want keep /openapi.json and /explorer regardless of basePath, we need to allow /explorer to be mounted at root RestServer._expressApp level.

[bajtos]

Leaving OPENAPI_SPEC_MAPPING where it is and applying basePath to it inside the RestServer constructor will mean recreating the entire OPENAPI_SPEC_MAPPING object with basePath added to the key names.

I am envisioning this differently. Instead of re-creating a new mapping object, we should apply basePath when creating Express routes here:

https://github.com/strongloop/loopback-next/blob/6e0eeb66b6a8c79d736fcc2dc7d9c76bfd57abd3/packages/rest/src/rest.server.ts#L281-L286

-    this._expressApp.get(p, (req, res) => 
+    this._expressApp.get(basePath + p, (req, res) => 

[raymondfeng]

To make it clear, I was able to reproduce the problem by adding basePath: '/api' to examples/todo/index.js. There are a few issues:

1. The url is printed as http://127.0.0.1:3000, which returns 404. I think the url should include the basePath, i.e., 'http://127.0.0.1:3000/api' (See fix(rest): make sure basePath is included in RestServer.url #2560)
2. The home page contains links to /openapi.json and /explorer. But /explorer is not working.
3. Go to http://127.0.0.1:3000/api/explorer manually and it fails again as the UI expects /api/openapi.json, which is not found.

[raymondfeng]

Now we have /openapi.json and e.g. /api/products
Proposed change: /api/openapi.json and e.g. /api/products

Can you do both as a short term solution?

[bajtos]

If we want keep /openapi.json and /explorer regardless of basePath, we need to allow /explorer to be mounted at root RestServer._expressApp level.

I was thinking about this a bit, also in the context of reworking /openapi.json into regular LB4 RouteEntry (instead of current Express-level route).

I am proposing to add a new flag to LB routes to allow them to ignore basePath setting. Then we can add a new option to @loopback/explorer to control whether the provided path (e.g. '/explorer') should be interpreted as rooted path (ignoring basePath) or a "regular" path (nested under basePath).

As for /openapi.json paths: in the current implementation, users can include basePath in the endpoint mapping config.

new RestApplication({
  rest: {
    basePath: '/api',
    openApiSpec.endpointMapping: {
      '/api/openapi.json': {version: '3.0.0', format: 'json'},
    },
  },
});

I acknowledge this is a not-very-pretty workaround, but it's something that works right now.

For longer term, I am proposing to introduce a feature flag, re-posting part of my older comment:

introduce a new feature flag allowing users to decide whether to apply basePath configuration to openapi endpoints or not. By default, we should fall back to old behavior, but new projects scaffolded by lb4 cli should enable the new behavior.

[ghost]

@bajtos @raymondfeng @dkrantsberg
sorry to interrupting, i don't think mount the openapi.json under base path /api/opeapi.json
as i know open api can honor multi server path as well lb4,
please read my comment in #2329
as short term workaround
this.basePath should be removed when the _redirectToSwaggerUI ask for the path .
https://github.com/strongloop/loopback-next/blob/b5f12be7c2f1f809360122ca72a3e55c5ef92bcb/packages/rest/src/rest.server.ts#L456

[raymondfeng]

@sanadHaj Thank you for chiming in. @dkrantsberg's use case is different. IIUC, his set up involves reverse proxies, which are not related to basePath settings.

[gordancso]

I would like to follow up with this issue.

If we want keep /openapi.json and /explorer regardless of basePath, we need to allow /explorer to be mounted at root RestServer._expressApp level.

I was thinking about this a bit, also in the context of reworking /openapi.json into regular LB4 RouteEntry (instead of current Express-level route).

I am proposing to add a new flag to LB routes to allow them to ignore basePath setting. Then we can add a new option to @loopback/explorer to control whether the provided path (e.g. '/explorer') should be interpreted as rooted path (ignoring basePath) or a "regular" path (nested under basePath).

As for /openapi.json paths: in the current implementation, users can include basePath in the endpoint mapping config.

new RestApplication({
  rest: {
    basePath: '/api',
    openApiSpec.endpointMapping: {
      '/api/openapi.json': {version: '3.0.0', format: 'json'},
    },
  },
});

I acknowledge this is a not-very-pretty workaround, but it's something that works right now.

For longer term, I am proposing to introduce a feature flag, re-posting part of my older comment:

introduce a new feature flag allowing users to decide whether to apply basePath configuration to openapi endpoints or not. By default, we should fall back to old behavior, but new projects scaffolded by lb4 cli should enable the new behavior.

@bajtos The workaround would not work as explorer will try to look for openapi.json file at a different location (with @loopback/rest 1.9.1). What returned in the explorer:

Errors

Fetch errorNot Found /api/api/openapi.json

/ (RestServer._expressApp)
  /openapi.json
  /openapi.yaml
  /api
    /products
    /explorer (Explorer UI) 

In the rest-explorer package, the below code is trying to locate the openapi.json under basePath (which is openApiSpecUrl in index()). For /api/explorer to load openapi.json successfully, a very quick fix is to remove this.request.baseUrl + from openApiSpecUrl, if it is by design the openapi.json is served at the same level as /api.
rest-explorer.controller

  index() {
        let openApiSpecUrl = this.openApiSpecUrl;
        if (this.request.baseUrl && this.request.baseUrl !== '/') {
            openApiSpecUrl = this.request.baseUrl + openApiSpecUrl;
        }
        const data = {
            openApiSpecUrl,
        };
        const homePage = templateFn(data);
        this.response
            .status(200)
            .contentType('text/html')
            .send(homePage);
    }

[bajtos]

@gordancso thanks for chiming in!

For /api/explorer to load openapi.json successfully, a very quick fix is to remove this.request.baseUrl + from openApiSpecUrl, if it is by design the openapi.json is served at the same level as /api.

This looks very reasonable to me. Could you please open a pull request to contribute this change? See Contributing code and Submitting a pull request to LoopBack 4 to get started.

[raymondfeng]

This looks very reasonable to me. Could you please open a pull request to contribute this change? See Contributing code and Submitting a pull request to LoopBack 4 to get started.

The baseUrl is used to support the case where LB4 app is mounted to an existing express app, such as:

expressApp.use('/lb4', lb4App);

[gordancso]

I found a little bit confusing here.

Inside @loopback/rest, in rest.server.ts, basePath actually return nothing as it is not inside the requestContext even when I configured basePath option inside application.ts.

  private async _serveOpenApiSpec(
    request: Request,
    response: Response,
    specForm?: OpenApiSpecForm,
  ) {
    const requestContext = new RequestContext(
      request,
      response,
      this,
      this.config,
    );

    specForm = specForm || {version: '3.0.0', format: 'json'};
    let specObj = this.getApiSpec();
    if (this.config.openApiSpec.setServersFromRequest) {
      specObj = Object.assign({}, specObj);
      specObj.servers = [{url: requestContext.requestedBaseUrl}];
    }

    const basePath = requestContext.basePath;
    if (specObj.servers && basePath) {
      for (const s of specObj.servers) {
        // Update the default server url to honor `basePath`
        if (s.url === '/') {
          s.url = basePath;
        }
      }
    }

    if (specForm.format === 'json') {
      const spec = JSON.stringify(specObj, null, 2);
      response.setHeader('content-type', 'application/json; charset=utf-8');
      response.end(spec, 'utf-8');
    } else {
      const yaml = safeDump(specObj, {});
      response.setHeader('content-type', 'text/yaml; charset=utf-8');
      response.end(yaml, 'utf-8');
    }
  }

Currently the issue I have is that, after build, the API explorer looking at /api/openapi.json but the openapi.json is served ignoring the basePath config, while all endpoints working normally as expected. @bajtos @raymondfeng may I know what the expected behaviour is of this basePath option is?

[raymondfeng]

even when I configured basePath option inside application.ts.

How did you do that?

[gordancso]

even when I configured basePath option inside application.ts.

How did you do that?

I tried to follow documentation Configure the Base Path for my LB4 application. There are actually two issues i found:

1. In home index.html, redirecting to /{basePath}/explorer will always return an error. It will only redirect to /explorer.
2. Failed to locate /{basePath}/openapi.json after setting basePath property in LB4 application.

This issue only affects how explorer display the openapi spec. Endpoint-wise there's not issue but I will have to stick with older rest-explorer and rest packages whenever I need to build my application image.

[bajtos]

Sorry for the long delay. I have a ton of other work on my plate and have difficulties finding enough time and energy to take a close look at this complex issue.

[bajtos]

@gordancso I think it would help a lot if you could:

• create a small application showing the configuration you are using
• describe exact steps we can run to reproduce the problems you are encountering
• explain what is the actual result vs. what did you expect to happen instead

Without that, we will have to create such app ourselves, and that's something we don't have bandwidth for :(

[gordancso]

@gordancso I think it would help a lot if you could:

• create a small application showing the configuration you are using
• describe exact steps we can run to reproduce the problems you are encountering
• explain what is the actual result vs. what did you expect to happen instead

Without that, we will have to create such app ourselves, and that's something we don't have bandwidth for :(

Current version of my dependencies:

"dependencies": {
    "@loopback/boot": "^1.2.2",
    "@loopback/context": "^1.12.0",
    "@loopback/core": "^1.5.0",
    "@loopback/openapi-v3": "^1.3.6",
    "@loopback/repository": "^1.5.0",
    "@loopback/rest": "^1.10.3",
    "@loopback/rest-explorer": "^1.1.17",
    "@loopback/service-proxy": "^1.1.5",
    "loopback-connector-mysql": "^5.3.1"
}

Steps to Reproduce:

Tried the below but no hope.

1. include this.basePath('api'); in application.ts;
2. include basePath: 'api' inside rest object of config;
3. include both this.basePath('api'); in application.ts and basePath: 'api' inside rest object of config.

In all of the above, Fetch error Not Found /api/openapi.json is returned, as openapi.json's location is one layer above basePath option.
Triggering an api GET request will return success (e.g. localhost:3000/api/user will return an array of users)

Expected Result

Expect to be able to use explorer with host:port/${basePath}/explorer.

[bajtos]

I managed to reproduce the problem and opened a new pull request to fix it - see #2854

[bajtos]

Closing as fixed via #2856