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

Allow a password to be callable (e.g. a secretbox abstraction) #135

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sdvillal
Copy link
Contributor

@sdvillal sdvillal commented May 2, 2019

Could there be interest on a small new feature allowing passwords to be callables instead of the password itself? If so I can polish this PR.

@miketeo
Copy link
Owner

miketeo commented May 2, 2019

I have taken a quick look at your changes. It looks like it's doable with minimal impact on the API. Will you be able to checkout and submit PRs against the dev-1.1.x branch? The master branch is currently tied to the dev-2.0.x branch where the development is stuck at the SMB2.1/SMB3 protocol implementation.

@sdvillal
Copy link
Contributor Author

sdvillal commented May 2, 2019

It is indeed a minimal change.

I have open a PR for the 1.1.x branch #136.

I have added a short note to the docs.

Regarding tests, I feel it would be enough copy any connection test in in test_auth and, instead of passing the password, pass a function returning the password. I have pushed such test in the two PRs. Since I have not setup the infrastructure to run directly the tests, feel free to tell me or edit directly to my branch if they do not pass.

@sdvillal sdvillal changed the title (WIP) Allow a password to be callable (e.g. a secretbox abstraction) Allow a password to be callable (e.g. a secretbox abstraction) May 2, 2019
@sdvillal
Copy link
Contributor Author

@miketeo Should we merge or close this too?

@miketeo miketeo added the todo label May 11, 2019
@miketeo
Copy link
Owner

miketeo commented May 11, 2019

If it's ok, I want to delay this merge as it is against the master branch. I have marked this PR as todo. When the changes are ready on dev-2.0, I will merge the branch into master and then merge in your PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants