Skip to content
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

[feat/basic-auth] password encryption #533

Merged
merged 3 commits into from
Sep 21, 2015
Merged

Conversation

thibaultcha
Copy link
Member

On the contrary of #527, this only allows for sha1 encryption. The
reason is that due to the current architecture, we cannot support two
types at the same time (supporting plain is a bad practice anyways).
Because a basicauth_credential has no relation to a plugin entity (they
are not semantically related anyways), we cannot now how the
password is stored/encrypted.

I also took the opportunity of 0.5.0 and the migration script to make
that decision. The migration script will be updated to also migrate the
current passwords.

This does a bit more than #527:

  • encryption at the model layer
  • unit tests
  • support encryption in unit test mode (with a mock using a vendor sha1
    library)
  • comparison of the hash at the proxy level (for actual authentication)

Resolves #33

On the contrary of #527, this only allows for sha1 encryption. The
reason is that due to the current architecture, we cannot support two
types at the same time (supporting plain is a bad practice anyways).
Because a basicauth_credential has no relation to a plugin entity (they
are **not** semantically related anyways), we cannot now how the
password is stored/encrypted.

I also took the opportunity of 0.5.0 and the migration script to make
that decision. The migration script will be updated to also migrate the
current passwords.

This does a bit more than #527:

- unit tests
- support encryption in unit test mode (with a mock using a vendor sha1
  library)
- comparison of the hash at the proxy level (for actual authentication)

Resolves #33
@thibaultcha thibaultcha added pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release) and removed NEEDS REVIEW labels Sep 21, 2015
thibaultcha added a commit that referenced this pull request Sep 21, 2015
[feat/basic-auth] password encryption
@thibaultcha thibaultcha merged commit 33c9608 into master Sep 21, 2015
@thibaultcha thibaultcha deleted the feature/basic-auth-sha1 branch September 21, 2015 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant