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

Auth Customization Options #159

Merged
merged 18 commits into from
Aug 11, 2024
Merged

Auth Customization Options #159

merged 18 commits into from
Aug 11, 2024

Conversation

AS1100K
Copy link
Contributor

@AS1100K AS1100K commented Jun 19, 2024

What this PR adds?

This PR addresses and closes the issue #130. It introduces two new functions in azalea_client::Account:

  • microsoft_with_custom_client_id_and_scope
  • with_microsoft_access_token_and_custom_client_id_and_scope.

This PR will allow the developer, to customize the client_id and scope, or use the nintendo switch one. Also, some other functions in auth.rs like refresh_ms_auth_token, get_ms_link_code, interactive_get_ms_auth_token, etc. are modified and takes some extra parameters now. See the changes in auth.rs for more information.

Backwards Compatibility?

The existing functions microsoft and with_microsoft_access_token will maintain their current functionality. However, their implementations have been refactored to depend on the new functions microsoft_with_custom_client_id_and_scope and with_microsoft_access_token_and_custom_client_id_and_scope to avoid code duplication and facilitate easier maintenance.

@1zun4
Copy link
Contributor

1zun4 commented Jun 27, 2024

This is great, but you are missing two important options:

If you are using your own ClientID, these changes are also mandatory. Otherwise this option is useless. I saw that you added a custom scope, but then removed it as it was apparently not needed? What is the reason for this? - d2bb6f4

@AS1100K
Copy link
Contributor Author

AS1100K commented Jun 27, 2024

I saw that you added a custom scope, but then removed it as it was apparently not needed? What is the reason for this? - d2bb6f4

I removed the custom scope because that time i thought, that modifying permissions might not be required but rethinking it now, i realize that how can this be useful. Like you liquidboune client_id only works with other scope i.e. not the default one. Also, may be you can take extra permissions and integrate your bot with that.

I will add custom scope and RpsTicket format and commit the changes

@veronoicc
Copy link
Contributor

veronoicc commented Jul 7, 2024

I just looked trough it an saw a lot of parameters with &'static str, and to my knowledge getting somethink as an &'static is not really possible (unless with a hardcoded str in this case) without leaking memory. Maybe using a non 'static str here is better?

@veronoicc
Copy link
Contributor

Meant fields in the AuthOps struct mb

Copy link
Collaborator

@mat-1 mat-1 left a comment

Choose a reason for hiding this comment

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

Thank you for making this, I'm sorry for taking so long to get to it. This looks good, the main thing I think should be changed is using Option<&str> instead of &str for all the public functions that take a client_id and scope. I can resolve everything here tomorrow (I'm going to sleep after writing this) unless you want to.

By the way, I recommend you also run cargo fmt to standardize the formatting on some of the code.

azalea-client/src/account.rs Outdated Show resolved Hide resolved
azalea-auth/src/auth.rs Outdated Show resolved Hide resolved
azalea-auth/src/auth.rs Outdated Show resolved Hide resolved
azalea-auth/src/auth.rs Outdated Show resolved Hide resolved
azalea-auth/src/auth.rs Outdated Show resolved Hide resolved
azalea-auth/src/auth.rs Outdated Show resolved Hide resolved
azalea-auth/src/auth.rs Outdated Show resolved Hide resolved
azalea-client/src/account.rs Outdated Show resolved Hide resolved
@AS1100K AS1100K requested a review from mat-1 August 11, 2024 12:10
@mat-1 mat-1 merged commit 13afc1d into azalea-rs:main Aug 11, 2024
1 check passed
@AS1100K AS1100K deleted the feature/custom-auth branch August 12, 2024 05:45
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.

4 participants