[auth_dbname] introduce auto-registration option in case auto-database is configured

[goksgie]

When the database section has an auto-database, '*', the expected behavior for login experience to have client's database registered when not found. However, this experience would be blocked if we configure auth_dbname to fall to auto-database. Hence, the main goal of this PR is to ensure that auth_dbname can still function on cases where it is not configured, but auto-database is provided.

To achieve that, this PR makes already existing function, find_or_register_database, public and uses it from various places to simplify code.

Edit:

Quoting the targeted use case of this change from @JelteF 's comment, which makes the change more presentable:

[databases]
* = host=localhost

[pgbouncer]
auth_dbname=postgres

Where, we will use auth_dbname=postgres with auto-registered database's connection string.

[eulerto]

We discussed this topic when the auth_dbname feature was reviewed. This idea was rejected and removed from the patch.

I cannot imagine strong arguments to allow such option. You generally create some objects (table, view, function) to centralized the authentication into one specific database. This proposal is against this idea; indeed, it is similar to the pre-auth_dbname (create objects in multiple databases). What happen if you create a new database and forget to create the authentication objects?

I'm also concerned about security. If you have a credentials table in database A and in database B but you shouldn't use that table from database B. Depending on your setup, you might allow access to it.

[goksgie]

I cannot imagine strong arguments to allow such option. You generally create some objects (table, view, function) to centralized the authentication into one specific database. This proposal is against this idea; indeed, it is similar to the pre-auth_dbname (create objects in multiple databases). What happen if you create a new database and forget to create the authentication objects?

I'm also concerned about security. If you have a credentials table in database A and in database B but you shouldn't use that table from database B. Depending on your setup, you might allow access to it.

Users are still required to configure their authentication databases manually, whether it is through setting "global" or per database. This change lifts the requirement where auth_dbname needs to be an entry under [databases] section.

In terms of security, I do not think this PR poses any other risk than what already exists. Because:

1-> We enforce you to configure auth_dbname, whether globally or per database. This can only be performed from admin console, or through configuration file. If attacker has access to any of them, they can do anything with the old implementation as well.
2-> If the authentication database is the same as the client's target database, and it is not an entry under [databases], then we auto_register before calling the prepare_auth_database as per client routine. Hence, the use case is already partially supported.

Please let me know if you can think of other potentially issues, would be happy to learn.

Edit: I've updated the title as this might be misleading to other discussion where we discussed fallback option to client's database when auth_database is disabled.

[JelteF]

We discussed this topic when the auth_dbname feature was reviewed. This idea was rejected and removed from the patch.

@eulerto So to be clear, this idea in this PR is quite different from the one discussed in that comment (afaict from my memory and re-reading the comment). The type of config that we decided to disallow there was:

[databases]
mydb = dbname=mydb host=localhost

[pgbouncer]
auth_dbname=postgres

The initial version of #764 would then fall back to using mydb as the auth_db. I still think this should fail hard. Because falling back to using a different database indeed seems like a security issue

The type of config that this PR intends to support instead is:

[databases]
* = host=localhost

[pgbouncer]
auth_dbname=postgres

And I think it makes sense to allow this type of config. We'd still connect to the postgres database. But instead of requiring a dedicated connection string to connect to it, we can generate the connection string using the wildcard connection string.

[eulerto]

And I think it makes sense to allow this type of config. We'd still connect to the postgres database. But instead of requiring a dedicated connection string to connect to it, we can generate the connection string using the wildcard connection string.

I misunderstood the proposal because the description wasn't clear enough. I'm fine with this proposal.