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
Implement GSSAPI authentication #1122
Conversation
f7bd646
to
8c3e144
Compare
@elprans appreciate if you could take a look! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, thanks for contributing! One thing I'm wondering about is if there's a straightforward way to test this, e.g a Kerberos-enabled Postgres image that we can use in a test workflow?
if 'krbsrvname' in query: | ||
val = query.pop('krbsrvname') | ||
if krbsrvname is None: | ||
krbsrvname = val |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add handling of the PGKRBSRVNAME
environment variable as well to stay compatible with libpq.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -2235,6 +2236,10 @@ async def connect(dsn=None, *, | |||
or the value of the ``PGTARGETSESSIONATTRS`` environment variable, | |||
or ``"any"`` if neither is specified. | |||
|
|||
:param str krbsrvname: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mention the new parameter in a .. versionadded::
block below please.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
8c3e144
to
073ff14
Compare
@elprans I was wondering about testing as well, but did not look very carefully. First, I don't know if the version of postgres used in CI (https://github.com/MagicStack/asyncpg/blob/master/.github/workflows/install-postgres.sh) is even compiled with gssapi support. If not, this will be the first thing to fix. Once this is solved, we can use something like k5test (https://github.com/pythongssapi/k5test) to set up a kerberos environment, start a server and test that auth works. |
Most commonly used with Kerberos. Closes: MagicStack#769
073ff14
to
c19cd56
Compare
@elprans I fixed the test. |
c19cd56
to
0b29765
Compare
@elprans I added unit tests using k5test. From my testing so far it appears that Ubuntu builds of postgres have kerberos support of out the box, so this is fairly straightforward. There is a good chance this will not work in CI on the first try, so I would appreciate if you could help. |
469aab2
to
cfd647a
Compare
I think this is good to go. Thanks @eltoder! |
@elprans Any chance of a patch release (0.29.1) in the near future? |
Most commonly used with Kerberos.
Closes: #769