Skip to content

Added support for for native OAuth login (#767) - #768

Draft
professorwaltwood wants to merge 24 commits into
ulyssa:mainfrom
professorwaltwood:main
Draft

professorwaltwood wants to merge 24 commits into
ulyssa:mainfrom
professorwaltwood:main

Conversation

@professorwaltwood

Copy link
Copy Markdown

PR for #767

First time contributing to iamb. Any issues or there's something I'm missing, please let me know

Comment thread src/worker.rs Outdated
Comment thread src/worker.rs Outdated
Comment thread src/worker.rs Outdated
Comment thread src/worker.rs Outdated
Comment thread src/worker.rs Outdated
ApplicationType::Native,
// We are going to use the Authorization Code flow.
vec![OAuthGrantType::AuthorizationCode {
redirect_uris: vec![ipv4_localhost_uri, ipv6_localhost_uri],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be the redirect_uri returned by LocalserverBuilder::build() to include the port?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

iirc, oauth clients aren't restricted based off ports just scheme, host and path. Otherwise, for native clients like this, the client would need to re-register every time it needs to re-auth. This would also only work for dynamic client registration (should probably include an option in this PR for pre-registered iamb client)

Comment thread src/worker.rs Outdated
Comment thread src/worker.rs Outdated
Comment thread src/worker.rs Outdated
Comment thread src/worker.rs Outdated
Comment thread src/worker.rs Outdated
@VAWVAW VAWVAW added the enhancement New feature or request label Oct 1, 2026
@professorwaltwood

professorwaltwood commented Oct 2, 2026 •

Copy link
Copy Markdown
Author

not sure if it's limited to the oauth login method but when logging out, the crypto store isn't removed so on re-login, the client throws an error like this:
Failed to get OAuth login: Matrix client error: failed to read or write to the crypto store the account in the store doesn't match the account in the constructor: expected @professor:waltwood.dev:deviceid_1, got @professor:waltwood.dev:deviceid_2

Comment thread src/worker.rs Outdated
Comment thread src/worker.rs Outdated
Comment thread src/worker.rs Outdated
@VAWVAW

VAWVAW commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

Another thing I noticed is that I can login with a different user id than I set in the config. We should set settings.profile.user_id = client.user_id() after session restore to make sure that we always have the right user. And we might make the user_id setting optional in a followup PR.

Comment thread src/worker.rs Outdated
Comment thread src/config/mod.rs Outdated
Comment thread src/worker.rs Outdated
Comment thread src/worker.rs Outdated
Comment thread src/worker.rs Outdated
@ulyssa ulyssa added this to the v0.0.13 milestone Oct 4, 2026

@VAWVAW VAWVAW left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there still something blocking this on your side? Otherwise I would like to merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants