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

Add basic support for login detection to the FFI. #774

Closed
wants to merge 2 commits into from

Conversation

pixlwave
Copy link
Member

@pixlwave pixlwave commented Jun 17, 2022

Draft for now as it will need the next version of Ruma with ruma/ruma#1193 included. The PR does the following:

  • Add a discover_client SDK function that creates a new client for a domain string.
  • Add a new_client SDK function that creates a new client for a homeserver URL (as a string through the FFI).
  • Add authentication_server() and supports_password_login() client methods to discover login options available to the client.

For now the authentication server method is using the identity server until Ruma is updated.

Comment on lines 3 to 6
Client new_client(string homeserver);

[Throws=ClientError]
Client discover_client(string domain);
Copy link
Member Author

@pixlwave pixlwave Jun 17, 2022

Choose a reason for hiding this comment

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

Ideally there would just be a single function here, that attempted to discover the server and if that failed, checked the domain to see if it is in fact the server. so the user could enter example.com / matrix.example.com / http://matrix.example.com:8008 and the SDK would do the right thing. (I've handled this in EXI for now though as I wasn't sure how to achieve that).

@@ -313,14 +314,20 @@ impl ClientBuilder {
err => ClientBuildError::Http(err),
})?;

if let Some(base_url) = well_known.identity_server.map(|server| server.base_url) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Using the identity_server for now as a placeholder for testing.

@codecov
Copy link

codecov bot commented Jun 17, 2022

Codecov Report

Merging #774 (9ce9a80) into main (091fab8) will decrease coverage by 0.01%.
The diff coverage is 57.14%.

@@            Coverage Diff             @@
##             main     #774      +/-   ##
==========================================
- Coverage   77.76%   77.75%   -0.02%     
==========================================
  Files          92       92              
  Lines       13675    13682       +7     
==========================================
+ Hits        10635    10639       +4     
- Misses       3040     3043       +3     
Impacted Files Coverage Δ
crates/matrix-sdk/src/client/mod.rs 84.51% <0.00%> (-0.19%) ⬇️
crates/matrix-sdk/src/client/builder.rs 71.57% <80.00%> (+0.46%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 091fab8...9ce9a80. Read the comment docs.

@pixlwave pixlwave force-pushed the doug/ffi_login_types branch from 785c24d to 9ce9a80 Compare June 28, 2022 13:24
@pixlwave
Copy link
Member Author

pixlwave commented Jul 6, 2022

Replaced by #820.

@pixlwave pixlwave closed this Jul 6, 2022
@pixlwave pixlwave deleted the doug/ffi_login_types branch July 8, 2022 11:15
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