Adding support for peer authentication

[giovannipizzi]

In Postgres it is possible to use peer authentication.

This is signaled by a hostname set to None by pgsu.
In this case, the part user:password@hostname:port of
the connection string of the SQLAlchemy engine should be left empty.

This fixes #4032

[giovannipizzi]

Question: how can we test it?

[ltalirz]

Question: how can we test it?

The proper way is to run the tests on an architecture that uses peer authentication for postgresql.
pgsu does this but
A) I don't think we want to run this type of test on every commit for aiida-core
B) while we could move the sqla connect string into pgsu, it doesn't really belong there

I originally thought that testing pgsu on all of these platforms would already be enough (and I think it has already reduced the number of issues) but it now seems that integration tests for verdi quicksetup might be useful to run on a number of platforms as well.

We could think about introducing a quicksetup workflow that runs e.g. weekly / ... on a wide range of platforms, building on the setup used in the pgsu tests

@csadorf What do you think?

[ltalirz]

@giovannipizzi For manual testing, one can simply follow the instructions from the documentation on Quantum Mobile (ubuntu has local peer authentication enabled by default).

I think (not 100% sure) these were originally contributed by @dev-zero , so I'm a little surprised that they should actually never have worked (?)...

[ltalirz]

After following the instructions, this is the config.json I currently get:

        "peer3": {
            "PROFILE_UUID": "e7126766fc0c4e659b70624bf817723f",
            "AIIDADB_ENGINE": "postgresql_psycopg2",
            "AIIDADB_BACKEND": "django",
            "AIIDADB_NAME": "aiidadb2",
            "AIIDADB_PORT": 5432,
            "AIIDADB_HOST": "",
            "AIIDADB_USER": "max",
            "AIIDADB_PASS": "\"\"",
            "AIIDADB_REPOSITORY_URI": "file:///home/max/.aiida/repository/peer3",
            "options": {},
            "default_user_email": "aiida@localhost"
        }

This is obviously not ideal, but it still seems to work (I can open the verdi shell, create new nodes, ...).
Printing the connect string shows:

/postgresql://max:""@:5432/aiidadb2

i.e. it would have worked even without the change proposed here.

It seems like the interpretation of the connect string is quite flexible and is able to deal with empty strings etc. (but chokes, when one explicitly provides a wrong hostname, such as "None")

[dev-zero]

@giovannipizzi For manual testing, one can simply follow the instructions from the documentation on Quantum Mobile (ubuntu has local peer authentication enabled by default).

I think (not 100% sure) these were originally contributed by @dev-zero , so I'm a little surprised that they should actually never have worked (?)...

For verdi setup: I always passed--db-host "" --db-port 5432 ---db-password "" instead of using the !. This worked for a long time already (I've been using it since I fixed it in AiiDA and recommended it to colleagues in my group).

@giovannipizzi to be precise: leaving the hostname empty in the underlying psycopyg (respectively libpq) means to use a socket connection where PostgreSQL is able to get the UID of the user connecting to the socket (which in turn allows for peer authentication). The default PostgreSQL configuration then is to try to match the system UID of the connecting user to a database user of the same name (as created by createuser).
When using a hostname peer authentication would still be possible, but requires to run an ident service to give the server the possibility to query the host for the user connecting from a specific port since an IP connection does not forward the UID of the connecting user.
So, peer authentication is always used (when enabled in the configuration - which is the default), but often fails because people are using a hostname (instead of a socket) or created a DB user different from their system user. Since having a stored password does not add to security I would always recommend using socket-based peer-authentication.
Btw, with the pg_ident.conf one could even manually map system users to database users in case they don't match.

As for the setup I just thought that we could simplify it even more: at the beginning of the setup try to connect to the socket (with the default postgres DB), if successful, fetch the list of databases where the current system/database user is listed as owner (and which is still empty), use this table (and the empty hostname) as proposal for the DB name.

[giovannipizzi]

@ltalirz yes, I agree that running every few days might be good
@dev-zero thanks for the clarifications!
Then, maybe this PR can be simplified even further, by just changing a single line and replacing it with

hostname=profile.database_hostname or "",

?

Also, @ltalirz why do you have your pwd set to '""'? Can't it just be the empty string ''?

Can @ltalirz and @chrisjsewell check if, with simple line change I'm mentioning above, this would work for them (I don't have a reproducible environment myself at the moment).

Thanks

[codecov]

Codecov Report

Merging #4255 into develop will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop    #4255      +/-   ##
===========================================
- Coverage    79.22%   79.21%   -0.00%     
===========================================
  Files          468      468              
  Lines        34559    34560       +1     
===========================================
- Hits         27375    27374       -1     
- Misses        7184     7186       +2     

Flag	Coverage Δ
#django	72.82% <100.00%> (+0.01%)	⬆️
#sqlalchemy	72.00% <100.00%> (-<0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
aiida/backends/utils.py	95.24% <100.00%> (+0.24%)	⬆️
aiida/engine/daemon/client.py	72.42% <0.00%> (-1.14%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 45d4cfa...cb53015. Read the comment docs.

[giovannipizzi]

I checked the simplified version and it works fine for me (I just tried a new installation from scratch using PSQL provided by HomeBrew). I fixed and rebased, it's ok to merge for me

[ltalirz]

thanks @giovannipizzi !

[sphuber]

Thanks @giovannipizzi . The build that @ltalirz approved failed because you used double quotes and the pre-commit changed them to single. I took the liberty to fix the problem, adjust the comment a bit. Hope that is ok