Skip to content

Conversation

@jkatz
Copy link
Contributor

@jkatz jkatz commented Apr 30, 2019

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
Copy link
Contributor Author

jkatz commented Apr 30, 2019

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!

Copy link
Member

@elprans elprans left a comment

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!

self._push_result()

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

Choose a reason for hiding this comment

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

?

@jkatz
Copy link
Contributor Author

jkatz commented May 1, 2019

@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

@elprans
Copy link
Member

elprans commented May 6, 2019

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
Copy link
Contributor Author

jkatz commented May 7, 2019

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

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

elprans commented May 8, 2019

Merged. Thanks!

@jkatz
Copy link
Contributor Author

jkatz commented May 8, 2019

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