refactor(oidc): Move qrcode module inside oidc
#4687
Conversation
Signed-off-by: Kévin Commaille <[email protected]>
Will allow to share code when the backend is switched to the oauth2 crate too. It will also allow to expose the device authorization grant directly in Oidc, if necessary. Signed-off-by: Kévin Commaille <[email protected]>
Signed-off-by: Kévin Commaille <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4687 +/- ##
==========================================
- Coverage 85.89% 85.87% -0.02%
==========================================
Files 292 291 -1
Lines 33912 33930 +18
==========================================
+ Hits 29129 29139 +10
- Misses 4783 4791 +8 ☔ View full report in Codecov by Sentry. |
| #[cfg(all(feature = "e2e-encryption", not(target_arch = "wasm32")))] | ||
| async fn request_device_authorization( | ||
| &self, | ||
| _device_authorization_endpoint: Url, | ||
| _client_id: oauth2::ClientId, | ||
| _scopes: Vec<oauth2::Scope>, | ||
| ) -> Result< | ||
| oauth2::StandardDeviceAuthorizationResponse, | ||
| oauth2::basic::BasicRequestTokenError<oauth2::HttpClientError<reqwest::Error>>, | ||
| > { | ||
| unimplemented!() | ||
| } | ||
|
|
||
| #[cfg(all(feature = "e2e-encryption", not(target_arch = "wasm32")))] | ||
| async fn exchange_device_code( | ||
| &self, | ||
| _token_endpoint: Url, | ||
| _client_id: oauth2::ClientId, | ||
| _device_authorization_response: &oauth2::StandardDeviceAuthorizationResponse, | ||
| ) -> Result< | ||
| OidcSessionTokens, | ||
| oauth2::RequestTokenError< | ||
| oauth2::HttpClientError<reqwest::Error>, | ||
| oauth2::DeviceCodeErrorResponse, | ||
| >, | ||
| > { | ||
| unimplemented!() | ||
| } |
There was a problem hiding this comment.
To be honest, I don't really like this backend trait and that we're using it to test the OIDC submodule.
I wish we properly mocked things instead and added a oidc() subtype to the MatrixMockServer.
I know it's annoying to mock the OIDC stuff, but if the server changes under our feet we don't have any proof that it did so. Furthermore we don't have any integration tests that use MAS either.
I guess it's still a good step to centralize the logic instead of having this additional OIDC client thing, and then we can mock things using wiremock and MatrixMockServer and eventually get rid of this mock backend and trait.
There was a problem hiding this comment.
I had planned to slightly refactor the tests before changing to the oauth2 crate, to reduce duplication and simplify the migration. Maybe I can do this instead then.
Because it uses the OIDC API and it is accessed via
Oidc::login_with_qr_code().The second commit makes
LoginWithQrCodeuseOidcBackendinstead of its ownOauthClient. It will allow to share code when the server backend is switched to the oauth2 crate too. It will also allow to expose the device authorization grant directly inOidc, if necessary.