CartoDB / cartodb

Location Intelligence & Data Visualization tool
http://carto.com
BSD 3-Clause "New" or "Revised" License
2.76k stars 650 forks source link

Auth API: OGR CARTO Driver and System tables #13952

Open dgaubert opened 6 years ago

dgaubert commented 6 years ago

While using regular api-keys the OGR CARTO driver fails because it assumes access to the system tables in order to figure out column names and types of tables. Also, in order to handle SRS, it’ll hit spatial_ref_sys to dereference SRID numbers.

Regular API key should always have access to spatial_ref_sys and also have access to geometry_columns.

cc/ @rafatower & @pramsey

rafatower commented 6 years ago

I just want to call attention on the fact that this is not exclusive of GDAL OGR CARTO driver but we shall see in the light of other clients/apps using Engine.

rafatower commented 6 years ago

@CartoDB/backend I'd like to bring this to the table. To me this is kind of an important usability and security concern to be addressed.

rafatower commented 6 years ago

From a short chat with @amartinj : the role for an auth token is created around these lines:

https://github.com/CartoDB/cartodb/blob/13925cf3ca8c5cb96a79be8a7062c622a43690af/app/models/carto/api_key.rb?utf8=%E2%9C%93#L285

https://github.com/CartoDB/cartodb/blob/13925cf3ca8c5cb96a79be8a7062c622a43690af/app/models/carto/api_key.rb?utf8=%E2%9C%93#L211

Most likely a clean solution is to grant perms on those tables to either the new role or to user.service.database_public_username role.

javitonino commented 6 years ago

We are doing this more or less on purpose, we decided to restrict regular API Keys as much as possible. We granted the database public role in the past, but that has a lot of consequences, granting access to a lot of things we might not want (like the analysis catalog, see https://github.com/CartoDB/cartodb/issues/13528). So the decision was to be as restrictive as possible, while supporting the desired flows. Of course, if we feel we need more permissions, we can do it, but keeping in mind what's discussed in that issue, consistency between org and non-org users (

I think this is very related to the Import API permission on API keys (which will also need this privileges and maybe some more), so maybe we want to frame the discussion there.

pramsey commented 6 years ago

My hope is that we can address much of this by altering the way OGR works to try and avoid system table queries. Things like schema info, etc, can be gathered from the table directly. It might need some SQL API changes to be more specific about data types in the JSON return, I'm not sure yet.

rafatower commented 6 years ago

You're right, but at the same time a feature that used to work does not anymore, and it slipped fairly unnoticed (and that makes me feel personally uncomfortable).

You know, the server(s) can be changed rather quickly on our side, but the patches for GDAL can take longer to actually reach users (because of packaging, dependencies, availability in different OS's, etc).

There is something to learn from this, but I haven't fully figured it out yet. Does nobody use GDAL CARTO driver? or don't people reach out when they try and it fails?

pramsey commented 6 years ago

Well, anyone using the GDAL/OGR driver is probably using it with the master key, and that still works, it's just not something we want people to do anymore. So our desires (use restricted keys, don't use master) are in conflict with our actions (using restricted keys actually doesn't work for some clients).