mORMot Open Source

DigDiver

Member

Registered: 2013-04-29

Posts: 137

MultiFieldValues, ExecuteList and Call.RestAccessRights

I think that we should add the check of Users rights to the method ExecuteORMGet.

When the client calls MultiFieldValues or ExecuteList, the User Rights are not checked in the method ExecuteORMGet.

procedure TSQLRestServerURIContext.ExecuteORMGet;
var SQLSelect, SQLWhere, SQLSort, SQLDir, SQL: RawUTF8;
    SQLStartIndex, SQLResults, SQLTotalRowsCount: integer;
    NonStandardSQLSelectParameter, NonStandardSQLWhereParameter: boolean;
    SQLisSelect: boolean;
    ResultList: TSQLTableJSON;
    P: PUTF8Char;
    i,j,L: integer;
    Blob: PPropInfo;
    tbIndex: integer;
begin
  case Method of
  mLOCK,mGET: begin
    if Table=nil then begin
      if (Method<>mLOCK) then begin
        if (Call.InBody='') and (Parameters<>nil) and
           (reUrlEncodedSQL in Call.RestAccessRights^.AllowRemoteExecute) then begin
          // GET with a SQL statement sent in URI, as sql=....
          while not UrlDecodeValue(Parameters,'SQL=',SQL,@Parameters) do
            if Parameters=nil then break;
        end else
          // GET with a SQL statement sent as UTF-8 body
          SQL := Call.InBody;
        SQLisSelect := isSelect(pointer(SQL));

//check User Access
        tbIndex :=  Server.Model.GetTableIndexFromSQLSelect(SQL, false); 
        if not (tbIndex in Call.RestAccessRights^.GET) then // check User Access
         begin
          Call.OutStatus := HTML_NOTALLOWED;
          exit;
         end;
// end ckeck User Access
...

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,661

Website

Re: MultiFieldValues, ExecuteList and Call.RestAccessRights

First point is to state that if security is a real concern, you should enable authentication and URI signature.
So that only trusted clients may access to the server.
This is the main security rule of the framework - per table access right is more a design rule than a strong security feature.

This was a known limitation.
There was this comment:

// no user check for SELECT: see TSQLAccessRights.GET comment

And in the TSQLAccessRights.GET comment:

/// GET method (retrieve record) table access bits
    // - note that a GET request with a SQL statement without a table (i.e.
    // on 'ModelRoot' URI with a SQL statement as SentData, as used in
    // TSQLRestClientURI.UpdateFromServer) is always valid, whatever the bits
    // here are: since TSQLRestClientURI.UpdateFromServer() is called only
    // for refreshing a direct statement, it will be OK; you can improve this
    // by overriding the TSQLRestServer.URI() method
    // - if the REST request is LOCK, the PUT access bits will be read instead
    // of the GET bits value
    GET: TSQLFieldTables;

But your patch is worth adding.
AFAIR there was no TSQLModel.GetTableIndexFromSQLSelect() method available when we wrote this part of the framework.
Now we can have a rough be at least existing check.

I've also added a new reSQLSelectWithoutTable flag for TSQLAccessRights.AllowRemoteExecute, which would handle the case of a SELECT without an identified table in its FROM clause.
Note that it may break existing code relying of execution of complex SQL

See http://synopse.info/fossil/info/16df9f62a0

Thanks a lot for the idea and patch!

DigDiver

Member

Registered: 2013-04-29

Posts: 137

Re: MultiFieldValues, ExecuteList and Call.RestAccessRights

Since User Access Right is not verified in the TSQLRestServer.EngineBatchSend function but the RecordCanBeUpdated event is called, is there a possibility to get the SessionID in the RecordCanBeUpdated event to get the user who is executing the current operation?

function TWPServer.OnUpdate(Sender: TSQLRestServer; Event: TSQLEvent;
  aTable: TSQLRecordClass; aID: integer): boolean;
var
 u : TAuthUser;
begin
 if (Event = seDelete) and (aTable = TEmails) then
  begin
    U := GroupServer.SessionGetUser(SessionID) <- How to get SessionID here?
  end 
end;

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,661

Website

Re: MultiFieldValues, ExecuteList and Call.RestAccessRights

There was no simple way to do it, and I found it not the best idea to add a parameter to the callback.

ServiceContext threadvar will now be set in all ORM and SOA process, to allow access to the execution context.
See http://synopse.info/fossil/info/c90684aab9