Skip to content

Add support for SCRAM-SHA-256 authentication.#437

Merged
elprans merged 1 commit into
MagicStack:masterfrom
jkatz:scram
May 8, 2019
Merged

Add support for SCRAM-SHA-256 authentication.#437
elprans merged 1 commit into
MagicStack:masterfrom
jkatz:scram

Conversation

@jkatz

@jkatz jkatz commented Apr 30, 2019

Copy link
Copy Markdown
Contributor

SCRAM-SHA-256 authentication was introduced in PostgreSQL 10 as a better way
for handling password based authentication.

This implementation follows the guidance provided in the documentation, i.e.

https://www.postgresql.org/docs/current/sasl-authentication.html#SASL-SCRAM-SHA-256

A detailed, high-level explanation for how it works is provided in the definition for the SCRAMAuthentication class.

This implementation does not support channel binding (added in PostgreSQL 11) as there is still some ongoing discussion in the community for how it should be handled.

This should satisfy the requirements for #314

@jkatz

jkatz commented Apr 30, 2019

Copy link
Copy Markdown
Contributor Author

OK, after a couple of tries through the full test suite, all the tests are now passing (most of the adjustments were around making the code Python 3.5 compatible).

Open to feedback. Thanks!

@elprans elprans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work! Please address the review comments. Thanks!

Comment thread asyncpg/protocol/coreproto.pyx Outdated
Comment thread asyncpg/protocol/coreproto.pyx Outdated
self._push_result()

elif self.auth_msg is not None:
# First, need to determine if this is

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Comment thread asyncpg/protocol/coreproto.pyx Outdated
Comment thread tests/test_connect.py
@jkatz

jkatz commented May 1, 2019

Copy link
Copy Markdown
Contributor Author

@elprans Thanks for the review! I've made all the requested changes.

For the implementation of SASLPrep I followed along the C implementation as while the docs are great, I figured this is the authority.

It also looks like one of the tests failed due to a momentary DNS issue with Github: https://ci.appveyor.com/project/MagicStack/asyncpg/builds/24236176/job/i0xdab9iw4knis1x

Comment thread asyncpg/protocol/coreproto.pyx Outdated
@elprans

elprans commented May 6, 2019

Copy link
Copy Markdown
Member

One last thing: please move class SCRAMAuthentication into a separate pyx file and include it in coreproto.pyx. The latter is big as it is. Other than that, looks good! Thanks!

SCRAM-SHA-256 authentication was introduced in PostgreSQL 10 as a better way
for handling password based authentication.

This implementation follows the guidance provided in the documentation, i.e.

https://www.postgresql.org/docs/current/sasl-authentication.html#SASL-SCRAM-SHA-256
@jkatz

jkatz commented May 7, 2019

Copy link
Copy Markdown
Contributor Author

@elprans Agreed; I have broken it out into a separate file. Thanks!

@elprans elprans merged commit 2d76f50 into MagicStack:master May 8, 2019
@elprans

elprans commented May 8, 2019

Copy link
Copy Markdown
Member

Merged. Thanks!

@jkatz

jkatz commented May 8, 2019

Copy link
Copy Markdown
Contributor Author

Awesome, thanks for working on this with me @elprans!

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