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/async trait auth #37

Merged
merged 4 commits into from
Sep 15, 2023
Merged

Feat/async trait auth #37

merged 4 commits into from
Sep 15, 2023

Conversation

dizda
Copy link
Owner

@dizda dizda commented Jun 23, 2023

Based on the work of #36 (thank you)

More flexibility has been added, such as

  • async_trait is being used, which makes it easier to use async fn for authentication
  • allow_no_auth flag, for scenario where we could auth the client via other parameters such as IP address, etc. so the client doesn't need to send credentials, but will be checked by the Authentication trait anyway.
  • user's account can be returned once the socket has been successfully upgraded to socks5 (hence successfully authenticated), so no need to use black magic anymore to do so

nemosupremo and others added 3 commits May 8, 2023 22:31
…a other parameters such as headers

feat: credentials can be fetch once the socket has been successfully upgraded to socks5 (hence successfully authenticated)
@dizda dizda self-assigned this Jun 23, 2023
@dizda
Copy link
Owner Author

dizda commented Jun 23, 2023

Feel free to review @nemosupremo

@nemosupremo
Copy link
Contributor

Ah looks good, I was a bit confused on how you planned to return the users credentials and spent some time on that, but the get_credentials makes more sense.

Only thing I would add is maybe SimpleUserPassword should return an AuthenticationMethod::Password like struct so that users who might depend on AuthenticationMethod::Password with the username and password have a clear upgrade path.

@dizda
Copy link
Owner Author

dizda commented Jun 26, 2023

Only thing I would add is maybe SimpleUserPassword should return an AuthenticationMethod::Password like struct so that users who might depend on AuthenticationMethod::Password with the username and password have a clear upgrade path.

Thanks for the feedback, I've updated the server example accordingly.

@dizda dizda merged commit 81f8eb3 into master Sep 15, 2023
@dizda
Copy link
Owner Author

dizda commented Sep 15, 2023

Released on 0.9.0

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.

None yet

2 participants