Skip to content

Conversation

@veronoicc
Copy link
Contributor

Trying to make generic auth flows work, meaning that you can also use custom authentication servers.
Ik the code is horrible and missing documentation, feel free to help me out

@veronoicc
Copy link
Contributor Author

If i'm thinking correctly this would also fix #134

@veronoicc veronoicc marked this pull request as ready for review March 17, 2024 22:17
@veronoicc veronoicc marked this pull request as draft March 17, 2024 22:52
@veronoicc veronoicc marked this pull request as ready for review March 18, 2024 06:49
@mat-1
Copy link
Collaborator

mat-1 commented Sep 21, 2025

Closing since this PR is really old. Sorry that I never wrote a proper review.

I like the goal here; I've seen several people bring up this issue that creating Accounts from tokens or with custom auth servers is too annoying.

If you (or anyone else) wants to pick this up, the main things I'd change in this PR are:

  • Avoiding generic arguments (functions like add_account should take a boxed account instead so the user can mix accounts more easily).
  • Rename BoxedAccount to just Account and the account trait probably to AccountTrait, since the boxed account will be the one that the user will probably be interacting with the most. The boxed account should probably also get a Deref impl.
  • Preserving the existing Account::offline and Account::microsoft functions to avoid breaking changes and since imo these look nicer than OfflineAccount::new / MicrosoftAccount::new.
  • I think keeping the old fetch_certificates return type of Result<Certificates, FetchCertificatesError> and adding like an Unsupported variant to the error is more clear than returning Result<Option<Certificates>, FetchCertificatesError>.

@mat-1 mat-1 closed this Sep 21, 2025
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