Remove support for credentials #220
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
navytux
changed the title
|
See "d) removal of credentials support" in #195 (comment) . |
|
The comments on #53 clearly says that the support was experimental and it talks about future clients. That future never arrived. It makes sense to remove that experiment. |
dataflake
approved these changes
Jan 9, 2023
|
Jens, thanks for feedback. I will try to look into this once again properly, and especially with keeping new way of SSL authentication that ZEO5 started to use, after we hopefully settle with #217. |
1. Adjust text in changes. It is only support for `credentials` object that we drop here, not username nor password. For the reference from aa08949 (ClientStorage: Fix thinko in documentation for credentials/username/password/realm): d4805a0 (*: Documentation, Cosmetics) documented all credentials and username/password/realm as "credentials" and that it is ZEO4-only. It is not correct however: - username/password/realm are indeed related to basic authentication which, in ZEO5, has been _already_ superseded by SSL. Already because those parameters, besides the following assert assert not username or password or realm are otherwise ignored by ClientStorage constructor. - credentials however is ZEO5-only thing added in 2016 by Jim in commit dbb066d (Added the ability to pass credentials when creating client storages) with the following commit message: Added the ability to pass credentials when creating client storages. This is experimental in that passing credentials will cause connections to an ordinary ZEO server to fail, but it facilitates experimentation with custom ZEO servers. Doing this with custom ZEO clients would have been awkward due to the many levels of composition involved. In the future, we expect to support server security plugins that consume credentials for authentication (typically over SSL). Note that credentials are opaque to ZEO. They can be any object with a true value. The client mearly passes them to the server, which will someday pass them to a plugin. To my knowledge there is no use of such "credentials" feature, and regular ZEO server will just fail in register if any credentials object is passed. Still this feature is separate from ZEO4 basic authentication support. 2. Reenable test that verifies that `credentials` argument is ignored.
Noticed while rereading those.
navytux
changed the title
d-maurer
approved these changes
Jan 24, 2023
Starting from 2016 ZEO5 server never returns AuthError because support for user/password/realm authentication was dropped in favour of SSL. So nowdays this exception class appears to be unused: if client will use wrong SSL certificates, connecting to server will be rejected by SSL layer. Suggested by @d-maurer at #220 (review) See also c7f2138 for details.
|
Thanks, Dieter. I did so in 12af50c. |
|
All good with tests -> merging. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
kirr:
Extracted from #195