sqlpad icon indicating copy to clipboard operation
sqlpad copied to clipboard

Connection access sharing for longer that 1 day

Open benjamin2605 opened this issue 5 years ago • 6 comments

Hi, I just started setting up sqlpad in a more productive setting and using it with other users. I would like to permanently share a connection with a weak db user with everybody, while keeping other connections private.

However, I ran into an issue there: when I leave the duration field blank, or setting it so something longer than a day I get an "Internal Server error" in the UI. The logs pointed me to server/sequelize-db/connection-accesses, where I quickly found the validation that duration must be non-null and smaller than 86400.

As a quick fix, I set the the validation max from 86400s to 100 years and everything works as expected. What is the reasoning behind limiting this to a day?

Also, on a related note: is there any way to share a specific connection with Everybody during deployment? I would really like to be able to seed connection accesses.

benjamin2605 avatar Apr 29 '20 06:04 benjamin2605

Thanks for the feedback @benjamin2605 -- is this running from latest/master branch by chance? This could have been accidental breakage with the recent migration work.

Duration of 0 should set connection access indefinitely.

As for max allowed, I'm open to removing that restriction. It isn't something I had given much thought to. @koszti any thought on extending that to any amount of time?

Regarding connection access seed... not currently any way to do that but I can add it!

rickbergfalk avatar Apr 29 '20 23:04 rickbergfalk

Thanks for the quick response! Yes, I am running sqlpad in a freshly built docker (master from yesterday).

And yes, a duration of 0 also works for indefinite connection access, so it is only a bug the leaving it empty does not work. And I would vote for removing/strongly expanding the max allowed.

For the connection access seed I was surprised to be able to control access to queries in such a fine grained way, but the connection access was not controllable at all. We would appreciate if after a new deployment, all users can already use sqlpad without needing an admin to manually share the default connections each time. (We use the Auth Proxy, that has been working great, but that is why the connection access is the only feature that is preventing users from being able to use sqlpad again right after a deployment).

So if you could add such a functionality, that would be great. I myself have not a lot of experience with nodejs unfortunately. Should I add a new issue as feature request and close this one or is it ok that I hijack this bug thread for the feature?

benjamin2605 avatar Apr 30 '20 07:04 benjamin2605

Great to hear auth proxy is working out for you! That is relatively new and I'm glad it is getting used so quickly :)

We can keep this issue open for both purposes. Fixing the default and expanding the default duration shouldn't be an issue and can be done pretty easily.

It shouldn't be too difficult to add seed support for connection access as well.

Are the default connections you're looking to seed something shared with everyone or specific individuals?

rickbergfalk avatar Apr 30 '20 14:04 rickbergfalk

@benjamin2605 latest docker image / running from master should now be fixed to allow duration to be optional (sets to 0 by default like it used to). I've also removed the max restriction on duration.

Need to think a bit on how to represent the connection access seed data.

Is there any preference on whether these are standalone files vs something in the connection file itself?

For example, the connection JSON could be made out like the query JSON where you would have a field to specify user ids or email addresses. Probably not durations since these are loaded ahead of time and you probably won't be managing these via seed data? Things could get tricky if you set them short term, then restart SQLPad at a later date and they are restored with new times.

For example:

{
  "id": "seed-connection-1",
  "name": "postgres seed connection",
  "driver": "postgres",
  "host": "pghost",
  "acl": [
    { "userId" : "users sqlpad userid if you know it" }, 
    { "userEmail" : "users email you probably know" }
  ]
}

BTW forgot to mention about query access -- the back end supports more then the UI does today. Today the UI assumes the user can save the query if they can see it. The acl is honored server side however. If a user is not given write access, the UI will look like they can save, but they will get an error.

Plan is to take this further on the UI at some point.

rickbergfalk avatar Apr 30 '20 22:04 rickbergfalk

@rickbergfalk : Oh wow, thanks for the quick fix.

Yes, how you showed the example above is exactly what I would expect for connection sharing seeding. Keeping it as close to the query access sharing seeding as possible makes sense imo. (Meaning in the same json and with the same syntax). Being able to specify access by userId rather than email is also nice and fits well together with the auth proxy usage. (Email is more likely to change and more difficult to get).

For the duration feature: We would not limit the duration of the access so we don't need it, but one suggestion to get around this duration problem during restarting would be to set an end date rather than a duration when seeding the access.

The query access feature disparity is good to know, thank you, I think I have already experienced this problem when playing around with these features.

benjamin2605 avatar May 04 '20 06:05 benjamin2605

Hello, I'm having a usecase that I want to have a stateless container and so would be nice to be able to share a connection from the config.

luchees avatar Jun 01 '21 16:06 luchees