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

Implement password method #87

Closed
wants to merge 4 commits into from
Closed

Conversation

LucienHH
Copy link
Contributor

This PR clears up #9 and implements the password method directly within pauth with the added benefit of more descriptive errors i.e. if an account has 2FA enabled. With this change password auth can now do user, device and title authentication enabling developers to use password auth and join servers requiring title auth.

This is a breaking change due to live/sisu flow option needing to be set for password auth to work, it will not work with msal flow.

Copy link

Removed dependencies detected. Learn more about Socket for GitHub ↗︎

🚮 Removed packages: @xboxreplay/[email protected]

@extremeheat
Copy link
Member

I'm not convinced this is a good or reliable idea, this is some really hacky code that would need to remain maintained over long periods of time.

@LucienHH
Copy link
Contributor Author

LucienHH commented Dec 11, 2023

I'm not convinced this is a good or reliable idea, this is some really hacky code that would need to remain maintained over long periods of time.

Is the alternative to remove altogether or continue to rely on the dependency? I do agree it could be tricky to maintain but this solution has remained the same for many years and with it in the repo we can better support those who do use it with more informative errors/docs.

@extremeheat
Copy link
Member

Since this really is a hack, it's not something I think is appropriate to merge. If it's in some other package dependency we can kind of absolve responsibility for issues here, but putting it into an official PrismarineJS project creates responsibility on us for the creation of and maintenance of this code. If there is a documented and official way to do this, we could merge this, but at present I am not a fan of lowering the code standards to do hacky stuff like this (as in emulating login pages, scraping content, regex, etc). It's not illegal but still not good.

@LucienHH
Copy link
Contributor Author

Understood. Unfortunately ROPC isn't supported for non org accounts so I'll close this up.

@LucienHH LucienHH closed this Dec 14, 2023
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.

2 participants