Feature: Add enforce HTTPS middleware #27

kokokuo · 2022-08-02T07:54:32Z

Description

Provide the enforce HTTPS middleware, the enforce https provide two different solution:

With Reverse Proxy

If you have a reverse proxy in the front of vulcan-sql server, then you could choose the solution to enforce request must be https.
In yaml, you could setup options in enforce-https ( Will move out to the options for following Unite extension loaders dicussion after first extension loader finished), like below:

enforce-https:
   options:
      type: X_FORWARDED_PROTO,
      redirectMethods:
         - GET

The above detail options could see in sslify. Also we change the resolver options to type and add proto for CUSTOM type.

Otherwise, the all type shown below to match different reverse proxies:

export enum ResolverType {
  /* use local server to run https server, suit for local usage. */
  LOCAL = 'LOCAL',
  /*
   * RFC standard header (RFC7239) to carry information in a organized way for reverse proxy used.
   *  However, currently only little reverse proxies support it. e.g: nginx supported.
   *  refer: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded
   *  refer: https://www.nginx.com/resources/wiki/start/topics/examples/forwarded/
   */
  FORWARDED = 'FORWARDED',
  /*
   * X-Forwarded-Proto header flag is one of the de-facto standard (But not RFC standard) to check and enforce https or not, almost reverse proxies supported.
   * e.g: Heroku, GKE ingress, AWS ELB, nginx.
   */
  X_FORWARDED_PROTO = 'X_FORWARDED_PROTO',
  /*
   * if use Azure Application Request Routing as reverse proxy, then it use X-ARR-SSL header flag to check and enforce https.
   * refer: https://abhimantiwari.github.io/blog/ARR/
   */
  AZURE_ARR = 'AZURE_ARR',
  /* customize the header flag to check and enforce https, when use the type, need to define an custom header flag for checking and enforcing https */
  CUSTOM = 'CUSTOM',
}

Without Reverse Proxy

If you only would like to run in the local, you could set type to LOCAL and provide ssl options in the yaml:

ssl: 
   keyFile: './server.key'
   certFile: './server.cert'

enforce-https:
   options:
      type: LOCAL,

Then the server will open HTTP and HTTP server to make you redirect when request with HTTP to the server, the HTTP default port is 3000, the HTTPS default port is 3001

How To Test / Expected Results

For the test result, please see the below test cases that passed the unit test:

Serve Package

Commit Message

b5c89ae - feat(serve): add enforcing https middleware.
- add EnforceHttpsMiddlewares to support enforcing https request.
- add enforcing HTTPS middleware test cases.
- add ssl-sslify package.
- provide runServer method for "VulcanApplication" to start server for local https according to config provide ssl file and set LOCAL mode or not.
9f17631 - fix(serve): make the local enforce https provide http and https server.
- move the run server method from app to server class.
- make the local enforce https provide http and https server.
- update app test cases.

kokokuo · 2022-08-17T07:58:50Z

Rebased done from #26 which based on #23

codecov-commenter · 2022-08-19T01:38:11Z

Codecov Report

Base: 91.78% // Head: 91.52% // Decreases project coverage by -0.25% ⚠️

Coverage data is based on head (c35b4b2) compared to base (6b094ef).
Patch coverage: 83.07% of modified lines in pull request are covered.

Additional details and impacted files

@@                    Coverage Diff                    @@
##           feature/authenticator      #27      +/-   ##
=========================================================
- Coverage                  91.78%   91.52%   -0.26%     
=========================================================
  Files                        192      193       +1     
  Lines                       2494     2548      +54     
  Branches                     293      304      +11     
=========================================================
+ Hits                        2289     2332      +43     
- Misses                       158      168      +10     
- Partials                      47       48       +1

Impacted Files	Coverage Δ
packages/serve/src/lib/server.ts	`75.00% <61.90%> (-11.96%)`	⬇️
...serve/src/lib/middleware/enforceHttpsMiddleware.ts	`92.50% <92.50%> (ø)`
packages/serve/src/lib/app.ts	`83.33% <100.00%> (+2.68%)`	⬆️
packages/serve/src/lib/middleware/index.ts	`100.00% <100.00%> (ø)`
...e/src/lib/middleware/response-format/middleware.ts	`76.19% <0.00%> (-23.81%)`	⬇️
...ges/serve/src/models/extensions/routeMiddleware.ts	`91.66% <0.00%> (+16.66%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

oscar60310

LGTM

oscar60310 · 2022-08-30T02:51:19Z

packages/serve/src/lib/server.ts

+   */
+  private runServer(app: VulcanApplication, port: number, httpsPort: number) {
+    const options = getEnforceHttpsOptions(this.config['enforce-https']);
+    if (options && this.config.ssl) {


Should we create https server without enforce-https middleware enabled?

Thanks @oscar60310 for reviewing and suggestion, but I could not understand why we need also to create https server when enforcec-https options are disabled ? ( I need you tell more reason ~ )

In general, we usually open http server in the local machine, if user would like to create https server, it could use the reverse proxy ?

If you would like to create local https server, it should set the LOCAL undertans the enforce-https options.

Hi @oscar60310 thanks for discussing with me, after discussion, I have made the HTTPS server open when enforce-https options use defualt value ( default is LOCAL type with enabled` ).

Besides user setup the disable for enforce-https options, or the http server and https server will run both.

oscar60310 · 2022-08-30T02:59:01Z

packages/serve/src/lib/server.ts

+   * @param port the http port for server start, default is 3000
+   * @param httpsPort the https port for https server start when you set "type" = LOCAL in "enforce-https" middleware and provide ssl file, default port is 3001
+   */
+  public async start(port = 3000, httpsPort = 3001) {


NIT: We'll need to set https port here and the option of koa sslify, maybe we can find a way the share it. e.g. store them in config file ...

NIT: Would you update CLI too?

Thanks @oscar60310 for reviewing and suggestion, I have moved the httpsPort setting into config under enforce-https options, because the https server is only created when enforce-https set LOCAL type.

Btw, after I moved to the config, seems CLI no need to update too

Hi @oscar60310, thanks for discussing with me, I have also move the http port into config for reading!

oscar60310 · 2022-08-30T03:08:25Z

packages/serve/src/lib/server.ts

+      const options = {
+        key: fs.readFileSync(this.config.ssl.keyFile),
+        cert: fs.readFileSync(this.config.ssl.certFile),
+      };


We should provide "ca" property for CA bundle/chain.
Ref
https://medium.com/@superseb/get-your-certificate-chain-right-4b117a9c0fce
https://www.namecheap.com/support/knowledgebase/article.aspx/9705/33/installing-an-ssl-certificate-on-nodejs/

Thanks @oscar60310 for reviewing and suggestion, also thanks for sharing the document to let me know we should also provide CA bundle chain!

- add "EnforceHttpsMiddleware" to support enforcing https request. - add enforcing https middleware test cases. - add "ssl-sslify" package. - provide "runServer" method for "VulcanApplication" to start server for local https according config provide ssl file and set LOCAL mode or not.

- move the run server method from app to server class. - make the local enforce https provide http and https server. - update app test cases.

- setup the https port by config when enable enforce-https with LOCAL type. - make the https server open when enforce-https options is enabled and type is LOCAL - add ca bundle file for creating https server.

kokokuo · 2022-08-31T04:07:13Z

Hi @oscar60310 thanks for reviewing, suggesting and discussed with me, the PR has been fixed, please check it :)

kokokuo changed the base branch from develop to feature/authenticator August 2, 2022 07:56

kokokuo changed the title ~~Feature: Provide enforce HTTPS middleware~~ [WIP] Feature: Provide enforce HTTPS middleware Aug 2, 2022

kokokuo force-pushed the feature/enforce-https branch 2 times, most recently from 5ed6c17 to 6738279 Compare August 2, 2022 09:39

kokokuo marked this pull request as ready for review August 2, 2022 09:41

kokokuo changed the title ~~[WIP] Feature: Provide enforce HTTPS middleware~~ Feature: Provide enforce HTTPS middleware Aug 2, 2022

kokokuo requested a review from oscar60310 August 2, 2022 09:41

kokokuo changed the title ~~Feature: Provide enforce HTTPS middleware~~ Feature: Add enforce HTTPS middleware Aug 2, 2022

kokokuo force-pushed the feature/authenticator branch 2 times, most recently from a22b01b to 02a19de Compare August 17, 2022 06:44

kokokuo force-pushed the feature/enforce-https branch from 6738279 to 9f17631 Compare August 17, 2022 07:57

kokokuo force-pushed the feature/authenticator branch 3 times, most recently from 05e1464 to 6b094ef Compare August 19, 2022 01:25

kokokuo force-pushed the feature/enforce-https branch from 9f17631 to c35b4b2 Compare August 19, 2022 01:33

kokokuo force-pushed the feature/authenticator branch 5 times, most recently from 9f31a85 to c95709b Compare August 25, 2022 12:01

kokokuo force-pushed the feature/enforce-https branch from c35b4b2 to 8f5e50d Compare August 26, 2022 01:57

oscar60310 approved these changes Aug 30, 2022

View reviewed changes

Base automatically changed from feature/authenticator to develop August 30, 2022 05:55

kokokuo added 2 commits August 30, 2022 14:21

fix(serve): make the local enforce https provide http and https server.

1a62af2

- move the run server method from app to server class. - make the local enforce https provide http and https server. - update app test cases.

kokokuo force-pushed the feature/enforce-https branch from 8f5e50d to 1a62af2 Compare August 30, 2022 06:23

fix(serve): setup the https port by config, add ca bundle file

1437c32

- setup the https port by config when enable enforce-https with LOCAL type. - make the https server open when enforce-https options is enabled and type is LOCAL - add ca bundle file for creating https server.

kokokuo force-pushed the feature/enforce-https branch from 0351e1f to 1437c32 Compare August 31, 2022 04:02

oscar60310 merged commit a608f57 into develop Aug 31, 2022

oscar60310 deleted the feature/enforce-https branch August 31, 2022 04:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Add enforce HTTPS middleware #27

Feature: Add enforce HTTPS middleware #27

kokokuo commented Aug 2, 2022 •

edited

Loading

kokokuo commented Aug 17, 2022

codecov-commenter commented Aug 19, 2022

oscar60310 left a comment

oscar60310 Aug 30, 2022

kokokuo Aug 31, 2022 •

edited

Loading

kokokuo Aug 31, 2022

oscar60310 Aug 30, 2022 •

edited

Loading

kokokuo Aug 31, 2022

kokokuo Aug 31, 2022

oscar60310 Aug 30, 2022

kokokuo Aug 31, 2022

kokokuo commented Aug 31, 2022

Feature: Add enforce HTTPS middleware #27

Feature: Add enforce HTTPS middleware #27

Conversation

kokokuo commented Aug 2, 2022 • edited Loading

Description

With Reverse Proxy

Without Reverse Proxy

How To Test / Expected Results

Serve Package

Commit Message

kokokuo commented Aug 17, 2022

codecov-commenter commented Aug 19, 2022

Codecov Report

oscar60310 left a comment

Choose a reason for hiding this comment

oscar60310 Aug 30, 2022

Choose a reason for hiding this comment

kokokuo Aug 31, 2022 • edited Loading

Choose a reason for hiding this comment

kokokuo Aug 31, 2022

Choose a reason for hiding this comment

oscar60310 Aug 30, 2022 • edited Loading

Choose a reason for hiding this comment

kokokuo Aug 31, 2022

Choose a reason for hiding this comment

kokokuo Aug 31, 2022

Choose a reason for hiding this comment

oscar60310 Aug 30, 2022

Choose a reason for hiding this comment

kokokuo Aug 31, 2022

Choose a reason for hiding this comment

kokokuo commented Aug 31, 2022

kokokuo commented Aug 2, 2022 •

edited

Loading

kokokuo Aug 31, 2022 •

edited

Loading

oscar60310 Aug 30, 2022 •

edited

Loading