Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CORS whitelist #679

Closed
fastbike opened this issue Aug 13, 2023 · 10 comments
Closed

CORS whitelist #679

fastbike opened this issue Aug 13, 2023 · 10 comments
Assignees
Labels
accepted Issue has been accepted and inserted in a future milestone enhancement
Milestone

Comments

@fastbike
Copy link
Contributor

We have a requirement where we want to have a whitelist of origins that the CORS headers can be accepted.

For example we have three external application providers we work with that have web based applications. These will be something like:

How do we add the CORS middlware to the server with a whitelist of allowed domains, when we have to specify the allowed host in the constructor ?
At the point this code is run (TWebMod.WebModuleCreate) we do not have access to the WebContext to check if the request is coming from one of the allowed domains.

I was thinking we could subclass the CORS middleware and override the OnBeforeRouting method, except it is not declared as virtual.

@fastbike
Copy link
Contributor Author

fastbike commented Aug 14, 2023

The MDN documentation states

Limiting the possible Access-Control-Allow-Origin values to a set of allowed origins requires code on the server side to check the value of the Origin request header, compare that to a list of allowed origins, and then if the Origin value is in the list, set the Access-Control-Allow-Origin value to the same value as the Origin value.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin

This would imply that we should have some way of iterating through that list. I could imagine it being provided in the constructor as a comma separated list, or providing an anonymous method that allows the list to be interrogated when the OnBeforeRouting is being called so the Origin header can be checked and the correct response header added.
e.g.
TMVCCORSCheckOriginProc = reference to procedure(AContext: TWebContext; ResponseHeaders: TStrings);

@wuhao13
Copy link

wuhao13 commented Aug 14, 2023

Does overriding OnBeforeAction in the Controller to handle the request meet your requirements?

@fastbike
Copy link
Contributor Author

Does overriding OnBeforeAction in the Controller to handle the request meet your requirements?

I'd need to add it to all of the controllers, so I think a change to the middleware would be a better solution.

@fastbike
Copy link
Contributor Author

I'd imagine using it like this in the web module

  FMVC.AddMiddleware(TMVCCORSMiddleware.Create('', TMVCCORSDefaults.ALLOWS_CREDENTIALS, 
    TMVCCORSDefaults.EXPOSE_HEADERS, TMVCCORSDefaults.ALLOWS_HEADERS,  TMVCCORSDefaults.ALLOWS_METHODS,
    procedure(AContext: TWebContext; ResponseHeaders: TStrings)
    var
      RequestOrigin: string;
      AllowedOrigins: TArray<string>;
    begin
      RequestOrigin := AContext.Request.Headers['Origin'];
      if RequestOrigin <> '' then
      begin
        AllowedOrigins := Settings.ReadString('CORS', 'Access_Control_Allow_Origin', '').Split([',']);
        for var I := Low(AllowedOrigins) to High(AllowedOrigins) do
          if SameText(RequestOrigin, AllowedOrigins[I]) then
          begin
            ResponseHeaders.Values['Access-Control-Allow-Origin'] := AllowedOrigins[I];
            Break;
          end;
      end;
    end));

@fastbike
Copy link
Contributor Author

Also just reading through the MDN docs, there is a recommendation to omit the Access-Control-Allow-Credentials header if it is not true

So either Access-Control-Allow-Credentials: true
or nothing
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Credentials

@wuhao13
Copy link

wuhao13 commented Aug 14, 2023

Yes, you are right.

@danieleteti danieleteti self-assigned this Aug 15, 2023
@danieleteti danieleteti added this to the 3.4.0-neon milestone Aug 15, 2023
@danieleteti danieleteti added enhancement accepted Issue has been accepted and inserted in a future milestone labels Aug 15, 2023
@danieleteti
Copy link
Owner

dfe3943 should implement all the necessary to solve the problem. Let me know. There is also a new example in the folder 'samples\middleware_cors' (launch the project middleware_cors then launch simplewebserver and click the button in the resultant webbrowser).

@fastbike
Copy link
Contributor Author

Almost, The constructor is missing the assignment of the (split) AllowedOriginalURLs to the private field.

// ***************************************************************************
//
// Delphi MVC Framework
//
// Copyright (c) 2010-2023 Daniele Teti and the DMVCFramework Team
//
// https://github.com/danieleteti/delphimvcframework
//
// ***************************************************************************
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
// *************************************************************************** }

unit MVCFramework.Middleware.CORS;

{$I dmvcframework.inc}

interface

uses
  System.StrUtils,
  MVCFramework,
  MVCFramework.Commons;

type
  TMVCCORSDefaults = class sealed
  public
    const
    ALLOWS_ORIGIN_URL = '*';
    ALLOWS_CREDENTIALS = True;
    /// <summary>
    /// The Access-Control-Expose-Headers response header indicates which headers can be exposed as part of the response by listing their names.
    /// By default, only the 6 simple response headers are exposed: Cache-Control, Content-Language, Content-Type, Expires, Last-Modified, Pragma
    /// If you want clients to be able to access other headers, you have to list them using the Access-Control-Expose-Headers header.
    /// Source: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Expose-Headers
    /// </summary>
    EXPOSE_HEADERS = '';
    /// <summary>
    /// The Access-Control-Allow-Headers response header is used in response to a preflight request which includes the Access-Control-Request-Headers to indicate which HTTP headers can be used during the actual request.
    /// The simple headers, Accept, Accept-Language, Content-Language, Content-Type (but only with a MIME type of its parsed value (ignoring parameters) of either application/x-www-form-urlencoded, multipart/form-data, or text/plain), are always available and don't need to be listed by this header.
    /// This header is required if the request has an Access-Control-Request-Headers header.
    /// Source: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Headers
    /// </summary>
    ALLOWS_HEADERS = 'Content-Type, Accept, jwtusername, jwtpassword, authentication, authorization';
    /// <summary>
    /// The Access-Control-Allow-Methods response header specifies the method or methods allowed when accessing the resource in response to a preflight request.
    /// Source: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Methods
    /// </summary>
    ALLOWS_METHODS = 'POST,GET,OPTIONS,PUT,DELETE';
  end;

  TMVCCORSMiddleware = class(TInterfacedObject, IMVCMiddleware)
  strict protected
    FAllowedOriginURLs: TArray<String>;
    FAllowsCredentials: Boolean;
    FAllowsMethods: string;
    FExposeHeaders: string;
    FAllowsHeaders: string;
  protected
    function GetAllowedOriginURL(AContext: TWebContext): String; virtual;

    procedure OnBeforeRouting(
      AContext: TWebContext;
      var AHandled: Boolean
      ); virtual;

    procedure OnBeforeControllerAction(
      AContext: TWebContext;
      const AControllerQualifiedClassName: string;
      const AActionName: string;
      var AHandled: Boolean
      ); virtual;

    procedure OnAfterControllerAction(
      AContext: TWebContext;
      const AControllerQualifiedClassName: string; const AActionName: string;
      const AHandled: Boolean); virtual;

    procedure OnAfterRouting(
      AContext: TWebContext;
      const AHandled: Boolean
      ); virtual;

  public
    constructor Create(
      const AAllowedOriginURLs: string = TMVCCORSDefaults.ALLOWS_ORIGIN_URL;
      const AAllowsCredentials: Boolean = TMVCCORSDefaults.ALLOWS_CREDENTIALS;
      const AExposeHeaders: String = TMVCCORSDefaults.EXPOSE_HEADERS;
      const AAllowsHeaders: String = TMVCCORSDefaults.ALLOWS_HEADERS;
      const AAllowsMethods: string = TMVCCORSDefaults.ALLOWS_METHODS
      ); virtual;
  end;

  TCORSMiddleware = TMVCCORSMiddleware;

implementation

uses
  System.SysUtils;

{ TMVCCORSMiddleware }

constructor TMVCCORSMiddleware.Create(
  const AAllowedOriginURLs: string;
  const AAllowsCredentials: Boolean;
  const AExposeHeaders: String;
  const AAllowsHeaders: String;
  const AAllowsMethods: string
  );
begin
  inherited Create;
  FAllowedOriginURLs := AAllowedOriginURLs.Split([',']);
  FAllowsCredentials := AAllowsCredentials;
  FExposeHeaders := AExposeHeaders;
  FAllowsHeaders := AAllowsHeaders;
  FAllowsMethods := AAllowsMethods;
end;

function TMVCCORSMiddleware.GetAllowedOriginURL(AContext: TWebContext): String;
var
  lRequestOrigin: string;
  lAllowed: String;
begin
  Result := '';
  lRequestOrigin := AContext.Request.Headers['Origin'];
  if lRequestOrigin <> '' then
  begin
    for var I := Low(FAllowedOriginURLs) to High(FAllowedOriginURLs) do
    begin
      lAllowed := FAllowedOriginURLs[I].Trim;
      if SameText(lRequestOrigin, lAllowed) or (lAllowed = '*') then
      begin
        Exit(lAllowed);
      end;
    end;
  end;
end;

procedure TMVCCORSMiddleware.OnAfterControllerAction(
      AContext: TWebContext;
      const AControllerQualifiedClassName: string; const AActionName: string;
      const AHandled: Boolean);
begin
  // Implement as needed
end;

procedure TMVCCORSMiddleware.OnAfterRouting(AContext: TWebContext; const AHandled: Boolean);
begin
  // Implement as needed
end;

procedure TMVCCORSMiddleware.OnBeforeControllerAction(
  AContext: TWebContext; const AControllerQualifiedClassName,
  AActionName: string; var AHandled: Boolean);
begin
  // Implement as needed
end;

procedure TMVCCORSMiddleware.OnBeforeRouting(AContext: TWebContext; var AHandled: Boolean);
begin
  AContext.Response.RawWebResponse.CustomHeaders.Values['Access-Control-Allow-Origin'] := GetAllowedOriginURL(AContext);
  AContext.Response.RawWebResponse.CustomHeaders.Values['Access-Control-Allow-Methods'] := FAllowsMethods;
  AContext.Response.RawWebResponse.CustomHeaders.Values['Access-Control-Allow-Headers'] := FAllowsHeaders;

  if FAllowsCredentials then
  begin
    // Omit Access-Control-Allow-Credentials if <> true
    // https://github.com/danieleteti/delphimvcframework/issues/679#issuecomment-1676535853
    AContext.Response.RawWebResponse.CustomHeaders.Values['Access-Control-Allow-Credentials'] := 'true';
  end;
  AContext.Response.RawWebResponse.CustomHeaders.Values['Access-Control-Expose-Headers'] := FExposeHeaders;

  // allows preflight requests
  if (AContext.Request.HTTPMethod = httpOPTIONS) then
  begin
    AContext.Response.StatusCode := HTTP_STATUS.OK;
    AHandled := True;
  end;
end;

end.

@danieleteti
Copy link
Owner

In the meantime I did some refactoring and some additions. Can you check if it is OK from your side?

@danieleteti danieleteti reopened this Aug 16, 2023
@fastbike
Copy link
Contributor Author

All good, although remove unused private field on line 70. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Issue has been accepted and inserted in a future milestone enhancement
Projects
None yet
Development

No branches or pull requests

3 participants