caicloud / nirvana

Golang Restful API Framework for Productivity
https://caicloud.github.io/nirvana/
Apache License 2.0
520 stars 105 forks source link

feat(cors): Add a plugin to handle to handle CORS related requests #454

Closed Tomilla closed 3 years ago

Tomilla commented 3 years ago

What this PR does / why we need it:

Add a plugin to handle to handle CORS(Cross-Origin Resource Sharing) related requests(via simple encapsulation of this go package https://github.com/rs/cors). All its Supported Options as follow:

  1. set AllowedOrigins: a list of origins a cross-domain request can be executed from(for example the domain which hosts front-end pages is different from the back-end domain)
  2. set AllowedMethods: A list of methods the client is allowed to use with cross-domain requests(for example: GET, POST, PATCH, DELETE)
  3. set AllowOriginFunc func (origin string) bool: A custom function to validate the origin
  4. set AllowOriginRequestFunc func (r *http.Request origin string) bool: A custom function to validate the origin
  5. set AllowedHeaders []string: A list of non simple headers the client is allowed to use with cross-domain requests.
  6. set ExposedHeaders []string: Indicates which headers are safe to expose to the API of a CORS API specification
  7. set AllowCredentials bool: Indicates whether the request can include user credentials like cookies, HTTP authentication or client side SSL certificates.
  8. MaxAge int: Indicates how long (in seconds) the results of a preflight request can be cached.
  9. OptionsPassthrough bool: Instructs preflight to let other potential next handlers to process the OPTIONS method. Turn this on if your application handles OPTIONS.

Which issue(s) this PR is related to (optional, link to 3rd issue(s)):

Fixes #

Reference to #

Special notes for your reviewer:

/cc @iawia002

Since the cors exposes some function fields to public, However due to we cannot add function as command flag, So we need to skip those function fields when nirvana is calling the register fields methods registerFields.

related code https://github.com/caicloud/nirvana/blob/98146ec4aa3cdbada6878d1f560494d15b4c8bf6/config/config.go#L333-L492

Code freeze questions

  1. What causes this PR to not be merged before code freeze?
  2. Why this PR is absolutely necessary for this version? Paste a screenshot of smoke testing docs if you could.

    Error: Access to fetch at 'http://localhost:8080/apis/v1/messages?count=10' from origin 'http://localhost:8081' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. If an opaque response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled.

image-20210122162116623

  1. What's the effects after merging it?

image

  1. Is there anyway we can skip this to not affect the overall process? Without this new field(kind), Is there a way to detect a pointer is function or not during registerFields?

Release note:

  1. Introduce a new dependency https://github.com/rs/cors) into go.mod
  2. Implement implementing [http://www.w3.org/TR/cors/](Cross Origin Resource Sharing W3 specification) for Nirvana.
NONE
caicloud-bot commented 3 years ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: To fully approve this pull request, please assign additional approvers. We suggest the following additional approver: supereagle

Assign the PR to them by writing /assign @supereagle in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/caicloud/nirvana/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
iawia002 commented 3 years ago

I think it would be more appropriate for us to make this a middleware, it would be simpler and easier to use (e.g. we can configure different rules on different API groups).

caicloud-bot commented 3 years ago

@Tomilla: PR needs rebase.

Instructions for interacting with me using PR comments are available [here](https://github.com/caicloud/engineering/blob/master/docs/caicloud_bot.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
Tomilla commented 3 years ago

I think it would be more appropriate for us to make this a middleware, it would be simpler and easier to use (e.g. we can configure different rules on different API groups).

Okay. This plugin was defined to apply CORS policy to the root path. I respect your approach very much. During the transition period, I temporarily use this PR in my projects.