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

Issue #456: Use of Django connection name rather than alias as lookup #458

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sdether
Copy link

@sdether sdether commented Aug 31, 2021

This commit changes connection behavior from storing the database
connection name to using the database alias mapped by SQL Explorer
instead.

The reason for this change is two-fold:

  1. Views take the connection name as input, allowing anyone who knows
    Django connection names to query those databases, even if SQL
    does not expose the connection directly.
  2. Query stores the connection name, which means that if the
    Django connection name changes or a different connection should
    be used (for example, one with reduced permissions) the stored
    Query will either stop working or at least continue using the old
    connection

This change modifies ExplorerConnections from being a dictionary
that proxies the Django connection dictionary to a dictionary-like
object that uses EXPLORER_CONNECTIONS to lookup and validate
the requested connection alias.

In addition all code that used to the EXPLORER_CONNECTIONS value
now uses the key instead.

For backwards compatibility, a migration will back-populate the alias
into Query instances (and fail if the mapping no longer exists),
EXPLORER_DEFAULT_CONNECTION is re-written on start-up to use the
alias in case it still uses the Django Connection name and
ExplorerConnections will still accept a Django Connection name as
long as that name is exposed by some alias in EXPLORER_CONNECTIONS.

… as lookup

This commit changes connection behavior from storing the database
connection name to using the database alias mapped by SQL Explorer
instead.

The reason for this change is two-fold:
1) Views take the connection name as input, allowing anyone who knows
   Django connection names to query those databases, even if SQL
   does not expose the connection directly.
2) `Query` stores the connection name, which means that if the
   Django connection name changes or a different connection should
   be used (for example, one with reduced permissions) the stored
   Query will either stop working or at least continue using the old
   connection

This change modifies `ExplorerConnections` from being a dictionary
that proxies the Django connection dictionary to a dictionary-like
object that uses `EXPLORER_CONNECTIONS` to lookup and validate
the requested connection alias.

In addition all code that used to the `EXPLORER_CONNECTIONS` value
now uses the key instead.

For backwards compatibility, a migration will back-populate the alias
into `Query` instances (and fail if the mapping no longer exists),
`EXPLORER_DEFAULT_CONNECTION` is re-written on start-up to use the
alias in case it still uses the Django Connection name and
`ExplorerConnections` will still accept a Django Connection name as
long as that name is exposed by some alias in `EXPLORER_CONNECTIONS`.
@marksweb marksweb self-requested a review August 31, 2021 06:59
marksweb
marksweb previously approved these changes Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants