continuwuation/continuwuity
9
17
Fork 10
Code Issues 78 Pull requests 9 Projects Releases 16 Packages 1 Activity Actions

WIP: act as a Matrix OIDC auth provider #810

Draft
lafleur wants to merge 20 commits from lafleur/continuwuity:as-oidc-provider into main
pull from: lafleur/continuwuity:as-oidc-provider
merge into: continuwuation:main
Conversation 20 Commits 20 Files changed 31 +1573 -257
lafleur commented 2025-04-28 23:28:12 +00:00
First-time contributor
Copy link

This MR proposes to implement acting as an OIDC authentication provider following the expectations of

  • MSC2965 advertise the auth issuer
  • MSC2964 serve the OIDC token flow
  • MSC2966 dynamic clients registration

The OIDC token flow implies an OAuth transaction implemented with the oxide-auth crate. An enhancement could be to add html templates support for the login and consentment pages (and the future account registration/management pages).

The token flow was tested against areweoidcyet.com, and passes all the steps. Unfortunately, I was unable to use that flow with any app supporting new-style authentication.

MSC4254 (token revocation) should be straightforward to implement, I'll do it soon.

This MR proposes to implement acting as an OIDC authentication provider following the expectations of - [MSC2965](https://github.com/sandhose/matrix-spec-proposals/blob/msc/sandhose/oidc-discovery/proposals/2965-auth-metadata.md) advertise the auth issuer - [MSC2964](https://github.com/sandhose/matrix-spec-proposals/blob/msc/sandhose/oauth2-profile/proposals/2964-oauth2-profile.md) serve the OIDC token flow - [MSC2966](https://github.com/sandhose/matrix-spec-proposals/blob/msc/sandhose/oauth2-dynamic-registration/proposals/2966-oauth2-dynamic-registration.md) dynamic clients registration The OIDC token flow implies an OAuth transaction implemented with the [`oxide-auth`](https://docs.rs/oxide-auth/latest/oxide_auth/) crate. An enhancement could be to add html templates support for the login and consentment pages (and the future account registration/management pages). The token flow was tested against [areweoidcyet.com](https://areweoidcyet.com/client-implementation-guide/), and passes all the steps. Unfortunately, I was unable to use that flow with any app supporting new-style authentication. [MSC4254](https://github.com/matrix-org/matrix-spec-proposals/blob/quenting/oauth2-revocation/proposals/4254-oauth2-revocation.md) (token revocation) should be straightforward to implement, I'll do it soon.
❤️ 4
lafleur commented 2025-04-29 00:05:00 +00:00
Author
First-time contributor
Copy link

These MSCs form MSC3861, that recently landed in the spec - see their blog.

These MSCs form [MSC3861](https://github.com/matrix-org/matrix-spec-proposals/blob/hughns/delegated-oidc-architecture/proposals/3861-next-generation-auth.md), that recently landed in the spec - see [their blog](https://matrix.org/blog/2025/04/25/this-week-in-matrix-2025-04-25/).
Jade commented 2025-04-30 16:23:42 +00:00
Owner
Copy link

Thank you for the PR! I'd like to get #801 merged, then we can work on a basic HTML login page, then we can get login working with Element Web. & Element X as our test targets. We'll also need to add the database tables for dynamic client registration - as it's entirely new tables, that shouldn't cause any database compatibility issues with other conduits, but we'll need to pay attention if any modifications are required elsewhere to the database in the process.
I also want to see if we can move the routing stuff to ruwuma, although that's a lower-priority change.

All that said, this is an excellent start, thank you again! I'm quite happy with using oxide-auth here to avoid maintaining that code ourselves, and it looks well-maintained from a quick glance.

Thank you for the PR! I'd like to get https://forgejo.ellis.link/continuwuation/continuwuity/pulls/801 merged, then we can work on a basic HTML login page, then we can get login working with Element Web. & Element X as our test targets. We'll also need to add the database tables for dynamic client registration - as it's entirely new tables, that shouldn't cause any database compatibility issues with other conduits, but we'll need to pay attention if any modifications are required elsewhere to the database in the process. I also want to see if we can move the routing stuff to ruwuma, although that's a lower-priority change. All that said, this is an excellent start, thank you again! I'm quite happy with using `oxide-auth` here to avoid maintaining that code ourselves, and it looks well-maintained from a quick glance.
nex added this to the 0.5.0 milestone 2025-04-30 16:28:09 +00:00
Jade added the
Database
Enhancement
Matrix/MSC
Matrix/Auth
 labels 2025-04-30 16:34:03 +00:00
lafleur commented 2025-04-30 17:20:46 +00:00
Author
First-time contributor
Copy link

Great ! I noticed you added the askama crate in #801, I should be able to update this MR once you merge it to use its templating system in the current login page - and perhaps the css you added, if it's ment to be site-wide.

Currently I implemented the clients db as a volatile struct using oxide-auth's tooling. This means it's reset at server shutdown. I thought it was fine because I thought clients had to register on each connection anyway, but now I understand that they expect their registration to stay valid across server restart. So yes, I guess we have to add new tables.

BTW, I believe the implementation of msc2965 implies that user registration takes place at a new web endpoint, /_matrix/static/client/register (which is a bit confusing for an user registration endpoint if you ask me). So I'd like to implement that endpoint too, even if I couldn't find the relevant msc.

Great ! I noticed you added the askama crate in #801, I should be able to update this MR once you merge it to use its templating system in the current login page - and perhaps the css you added, if it's ment to be site-wide. Currently I implemented the clients db as a volatile struct using oxide-auth's tooling. This means it's reset at server shutdown. I thought it was fine because I thought clients had to register on each connection anyway, but now I understand that they expect their registration to stay valid across server restart. So yes, I guess we have to add new tables. BTW, I believe the implementation of msc2965 implies that user registration takes place at a new web endpoint, `/_matrix/static/client/register` (which is a bit confusing for an user registration endpoint if you ask me). So I'd like to implement that endpoint too, even if I couldn't find the relevant msc.
Jade commented 2025-04-30 18:06:26 +00:00
Owner
Copy link

Great ! I noticed you added the askama crate in #801, I should be able to update this MR once you merge it to use its templating system in the current login page - and perhaps the css you added, if it's ment to be site-wide.

That all sounds good to me! The CSS in there at the moment is a pretty rough draft, but ideally, we'd have one piece of base CSS (although perhaps split across multiple actual CSS files) and then per-page tweaks for things that can't be reused.

BTW, I believe the implementation of msc2965 implies that user registration takes place at a new web endpoint, /_matrix/static/client/register (which is a bit confusing for an user registration endpoint if you ask me). So I'd like to implement that endpoint too, even if I couldn't find the relevant msc.

From what I've found, this isn't in the spec and is a synapse-ism, and is what elements will use if a path isn't specified. We can use whatever endoint we want.


/**
 * Path to use when the client does not supported any or all registration flows.
 * Not documented.
 */
internal const val REGISTER_FALLBACK_PATH = "/_matrix/static/client/register/"
> Great ! I noticed you added the askama crate in #801, I should be able to update this MR once you merge it to use its templating system in the current login page - and perhaps the css you added, if it's ment to be site-wide. That all sounds good to me! The CSS in there at the moment is a pretty rough draft, but ideally, we'd have one piece of base CSS (although perhaps split across multiple actual CSS files) and then per-page tweaks for things that can't be reused. > BTW, I believe the implementation of msc2965 implies that user registration takes place at a new web endpoint, /_matrix/static/client/register (which is a bit confusing for an user registration endpoint if you ask me). So I'd like to implement that endpoint too, even if I couldn't find the relevant msc. From what I've found, this isn't in the spec and is a synapse-ism, and is what elements will use if a path isn't specified. We can use whatever endoint we want. ```kt /** * Path to use when the client does not supported any or all registration flows. * Not documented. */ internal const val REGISTER_FALLBACK_PATH = "/_matrix/static/client/register/" ```
👍 1
lafleur commented 2025-05-06 14:30:00 +00:00
Author
First-time contributor
Copy link

I noticed #801 was merged, so I rebased on it and am trying to use askama templates for login and consent webpages. There's a serious issue here : oxide-auth-axum is implemented as a synchronous backend, and porting it to async is non-trivial (see an old MR at their site - that's 1500 lines of code to port oxide-auth-actix to async). But askama is async, and the consent action needs its output in a sync function.

So I'm stuck here right now. I could try and implement that async port in the future, but that will probably take quite some time - and I'm going to have less free time in the weeks to come.

Besides, my main motivation to do this is to support external OIDC providers in the end. But I started thinking that such a feature would have no code in common with this MR.

So the alternative is either more work for the "act as OIDC provider" feature, or start another MR to implement OIDC delegation straight away.

Any opinions on that ?

I noticed #801 was merged, so I rebased on it and am trying to use askama templates for login and consent webpages. There's a serious issue here : oxide-auth-axum is implemented as a synchronous backend, and porting it to async is non-trivial (see [an old MR at their site](https://github.com/HeroicKatora/oxide-auth/pull/165) - that's 1500 lines of code to port oxide-auth-actix to async). But askama _is_ async, and the consent action needs its output in a sync function. So I'm stuck here right now. I could try and implement that async port in the future, but that will probably take quite some time - and I'm going to have less free time in the weeks to come. Besides, my main motivation to do this is to support external OIDC providers in the end. But I started thinking that such a feature would have no code in common with this MR. So the alternative is either more work for the "act as OIDC provider" feature, or start another MR to implement OIDC delegation straight away. Any opinions on that ?
lafleur commented 2025-05-06 14:46:07 +00:00
Author
First-time contributor
Copy link

For the record, someone already implemented async in oxide-auth with axum : https://github.com/mtelahun/axum-oauth

For the record, someone already implemented async in oxide-auth with axum : https://github.com/mtelahun/axum-oauth
Jade commented 2025-05-06 15:10:50 +00:00
Owner
Copy link

From what I understand, and what I can see in that PR, there is nothing in oxide-auth itself that needs to be changed to be asynchronous as there is no network or disk interaction within the library. That PR adds an example of how you can use asynchronous frameworks with the library.

askama, in contrast, is completely synchronous within its templates. To use asynchronous code you have to run it before rendering the templates and pass it into the template.

Regarding OIDC, this PR from what I understand is a necessary step, and third party OIDC will build upon it. It would be an addition of a new authentication method to the HTML login page. Although this has nothing in common with legacy OIDC (implemented as a different matrix-native authentication flow), that is depreciated and will stop working in some clients soon.

Don't worry about timelines, you're under absolutely no pressure here. I personally am pretty busy with exams right now!

From what I understand, and what I can see in that PR, there is nothing in oxide-auth itself that needs to be changed to be asynchronous as there is no network or disk interaction within the library. That PR adds an example of how you can use asynchronous frameworks with the library. askama, in contrast, is completely synchronous within its templates. To use asynchronous code you have to run it before rendering the templates and pass it into the template. Regarding OIDC, this PR from what I understand is a necessary step, and third party OIDC will build upon it. It would be an addition of a new authentication method to the HTML login page. Although this has nothing in common with legacy OIDC (implemented as a different matrix-native authentication flow), that is depreciated and will stop working in some clients soon. Don't worry about timelines, you're under absolutely no pressure here. I personally am pretty busy with exams right now!
lafleur commented 2025-05-07 12:00:59 +00:00
Author
First-time contributor
Copy link

Aha, I thought askama was asynchronous because I had introduced an async relay function, so I freaked out. Indeed, with your advice I succeeded in returning templates in oxide-auth's authentication flow.

The problem that I have now is that the returned login form is not clickable because of CSP policies. And I couldn't find a way to add headers to OAuthResponses. ATM I'm looking into implementing an OwnerSolicitor that would accept custom headers. It's a bit of a rabbit hole, but hopefully I'll manage something out of it.

Aha, I thought askama was asynchronous because I had introduced an async relay function, so I freaked out. Indeed, with your advice I succeeded in returning templates in oxide-auth's authentication flow. The problem that I have now is that the returned login form is not clickable because of CSP policies. And I couldn't find a way to add headers to `OAuthResponse`s. ATM I'm looking into implementing an `OwnerSolicitor` that would accept custom headers. It's a bit of a rabbit hole, but hopefully I'll manage something out of it.
❤️ 1
lafleur force-pushed as-oidc-provider from 7f531034bf to 9a52b6cf9e 2025-05-07 12:02:17 +00:00 Compare
Jade commented 2025-05-07 13:30:50 +00:00
Owner
Copy link

Good work! I'm excited to see where this goes!

I assume you're running into issues with the default CSP. You'll need to set a CSP for the page using something like https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/src/web/mod.rs#L32 rather than just directly returning the string.

Form actions don't support nonces (https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Security-Policy/form-action) so you'll probably want form-action 'self' In addition to the policy on the index page.

Good work! I'm excited to see where this goes! I assume you're running into issues with the default CSP. You'll need to set a CSP for the page using something like https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/src/web/mod.rs#L32 rather than just directly returning the string. Form actions don't support nonces (https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Security-Policy/form-action) so you'll probably want `form-action 'self'` In addition to the policy on the index page.
lafleur force-pushed as-oidc-provider from 9a52b6cf9e to a78887d979 2025-05-09 10:13:12 +00:00 Compare
lafleur commented 2025-05-09 10:24:45 +00:00
Author
First-time contributor
Copy link

I had to implement a custom OidcResponse that has CSP header capabilities. In the process I ditched the dependency on oxide-auth-axum. The MR now ships with askama templates support !

The result builds and passes areweoidcyet.com's implementation guide. Docstrings also have been rewritten with a newcomer's standpoint in mind.

I had to implement a custom OidcResponse that has CSP header capabilities. In the process I ditched the dependency on oxide-auth-axum. The MR now ships with askama templates support ! The result builds and passes areweoidcyet.com's [implementation guide](https://areweoidcyet.com/client-implementation-guide/). Docstrings also have been rewritten with a newcomer's standpoint in mind.
❤️ 1
lafleur reviewed 2025-05-09 10:36:07 +00:00
lafleur force-pushed as-oidc-provider from a78887d979 to eef6e60826 2025-05-09 10:43:07 +00:00 Compare
lafleur reviewed 2025-05-09 10:44:59 +00:00
src/web/oidc/response.rs Outdated
@ -0,0 +13,4 @@
/// A Web response that can be processed by the OIDC authentication flow before
/// being sent over.
#[derive(Default, Clone, Debug)]
pub struct OidcResponse {
lafleur commented 2025-05-09 10:44:59 +00:00
Author
First-time contributor
Copy link

I'm not positive that OidcResponse and OidcRequest should live in src/web - I first thought they belonged in src/api/client/oidc but that results in a cyclic dependency because that API depends on src/web/oidc itself.

Maybe could they be buried deeper into the structure ?

I'm not positive that `OidcResponse` and `OidcRequest` should live in `src/web` - I first thought they belonged in `src/api/client/oidc` but that results in a cyclic dependency because that API depends on `src/web/oidc` itself. Maybe could they be buried deeper into the structure ?
Jade commented 2025-05-10 13:54:09 +00:00
Owner
Copy link

I think you're right that they belong in API, or at least with the OIDC code. There shouldn't be a cyclic dependency as web doesn't depend on API or router.
If you carry on struggling, you could move the OIDC stuff into its own crate.

I think you're right that they belong in API, or at least with the OIDC code. There shouldn't be a cyclic dependency as web doesn't depend on API or router. If you carry on struggling, you could move the OIDC stuff into its own crate.
Jade force-pushed as-oidc-provider from eef6e60826 to 0c4917874e 2025-05-10 11:59:40 +00:00 Compare
Jade added 2 commits 2025-05-10 12:30:38 +00:00
chore(typos): Ignore certificate files
Some checks failed
Release Docker Image / define-variables (pull_request) Successful in 1s
Details
Documentation / Build and Deploy Documentation (pull_request) Failing after 22s
Details
Release Docker Image / build-image (linux/amd64, linux-amd64) (pull_request) Failing after 42s
Details
Release Docker Image / build-image (linux/arm64, linux-arm64) (pull_request) Failing after 53s
Details
Release Docker Image / merge (pull_request) Has been skipped
Details
a025c08fed
Jade added 3 commits 2025-05-10 13:24:37 +00:00
fix: Don't crash when the client URL doesn't have a domain
Some checks failed
Release Docker Image / define-variables (pull_request) Successful in -2s
Details
Documentation / Build and Deploy Documentation (pull_request) Failing after 23s
Details
Release Docker Image / build-image (linux/arm64, linux-arm64) (pull_request) Failing after 45s
Details
Release Docker Image / build-image (linux/amd64, linux-amd64) (pull_request) Failing after 1m9s
Details
Release Docker Image / merge (pull_request) Has been skipped
Details
7ff9d770e3 
Having a URL with an IP literal, for example, is allowed
Jade approved these changes 2025-05-10 13:58:30 +00:00
Jade left a comment
Owner
Copy link

I haven't reviewed this for spec yet, just code quality. Ran into a bug I couldn't easily fix before I finished going through 'are we oidc yet'.

Ran clippy, cargo +nightly fmt and typos over it, which created the big diff.

OidcRequest/Response should be located in the same crate as use of oxide-auth. It seems like dependencies have been added in many places to support code being spread out - should be avoided if possible.

Ignoring the templates themselves as I'm assuming they're placeholder for now.

I haven't reviewed this for spec yet, just code quality. Ran into a bug I couldn't easily fix before I finished going through 'are we oidc yet'. Ran clippy, `cargo +nightly fmt` and `typos` over it, which created the big diff. OidcRequest/Response should be located in the same crate as use of oxide-auth. It seems like dependencies have been added in many places to support code being spread out - should be avoided if possible. Ignoring the templates themselves as I'm assuming they're placeholder for now.
conduwuit-example.toml Outdated
@ -58,6 +58,10 @@
#
#port = 8008
# This item is undocumented. Please contribute documentation for it.
Jade commented 2025-05-10 13:36:12 +00:00
Owner
Copy link

Shouldn't be here. Check to see what well_known does

Shouldn't be here. Check to see what `well_known` does
lafleur commented 2025-05-10 17:20:46 +00:00
Author
First-time contributor
Copy link

fixed in 97b66d2f3e

fixed in 97b66d2f3e
lafleur marked this conversation as resolved
src/api/client/oidc/authorize.rs Outdated
@ -0,0 +77,4 @@
/// Authorize the device based on the user's consent. If the user allows
/// it to access their data, the client may request a token at the
/// [super::token::token] endpoint.
pub(crate) async fn authorize_consent(
Jade commented 2025-05-10 13:26:53 +00:00
Owner
Copy link

Testing going through https://areweoidcyet.com/ with the server running with the following config:

[global]
server_name = "test.local"
database_path = "./target/database"
auth = { enable_oidc_login = true, enable_oidc_account_management = true }
well_known = { client = "http://127.0.0.1:8008" }

When we get to this point, we get a crash as this seems to return an empty body

Testing going through https://areweoidcyet.com/ with the server running with the following config: ```toml [global] server_name = "test.local" database_path = "./target/database" auth = { enable_oidc_login = true, enable_oidc_account_management = true } well_known = { client = "http://127.0.0.1:8008" } ``` When we get to this point, we get a crash as this seems to return an empty body
lafleur commented 2025-05-10 19:34:25 +00:00
Author
First-time contributor
Copy link

Ahh I get it - if you set

well_known = { client = "http://localhost:8008" }

it succeeds, right ? Then it's solved in 2baeba41

Ahh I get it - if you set ``` toml well_known = { client = "http://localhost:8008" } ``` it succeeds, right ? Then it's solved in 2baeba41
Jade commented 2025-05-10 19:57:19 +00:00
Owner
Copy link

Nope, not solved. I fixed the issue with IP literals in particular in 7ff9d770e3, this is a different issue

[global]
server_name = "test.local"
database_path = "./target/database"
auth = { enable_oidc_login = true, enable_oidc_account_management = true }
well_known = { client = "http://127.0.0.1:8008" }

http://127.0.0.1:8008/_matrix/client/unstable/org.matrix.msc2964/authorize
?response_type = code
&response_mode = fragment
&client_id = tQwCbMNIZU
&redirect_uri = https%3A%2F%2Fareweoidcyet.com%2Fclient-implementation-guide%2Fcallback
&scope = urn%3Amatrix%3Aorg.matrix.msc2967.client%3Aapi%3A*%20urn%3Amatrix%3Aorg.matrix.msc2967.client%3Adevice%3AEIv9AHM9TmWkhAXS
&state = rdgVyeK58yYEGpFt
&code_challenge_method = S256
&code_challenge = DV2ZxyJZ0MWLPBUMmKjsN35LdjFoVmEe8YSbsRKTLx0

2025-05-10T19:53:08.906177Z DEBUG router{method=GET path=/_matrix/client/unstable/org.matrix.msc2964/authorize}: tower_http::trace::on_response: finished processing request latency=0 ms status=200
2025-05-10T19:54:14.518548Z  INFO router{method=POST path=/_matrix/client/unstable/org.matrix.msc2964/login}:request:handle{active=0 handled=8}: conduwuit_api::client::oidc::login: logging in: "@jade:test.local"
2025-05-10T19:54:14.518974Z DEBUG router{method=POST path=/_matrix/client/unstable/org.matrix.msc2964/login}: tower_http::trace::on_response: finished processing request latency=37 ms status=200
2025-05-10T19:54:16.098437Z DEBUG router{method=POST path=/_matrix/client/unstable/org.matrix.msc2964/authorize}:request:handle{active=0 handled=9}: conduwuit_api::client::oidc::authorize: processing user's consent: true - OidcRequest { auth: None, query: Some(NormalizedParameter { inner: {"response_type": Some("code"), "state": Some("rdgVyeK58yYEGpFt"), "response_mode": Some("fragment"), "code_challenge_method": Some("S256"), "allow": Some("true"), "client_id": Some("tQwCbMNIZU"), "code_challenge": Some("DV2ZxyJZ0MWLPBUMmKjsN35LdjFoVmEe8YSbsRKTLx0"), "scope": Some("urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:EIv9AHM9TmWkhAXS"), "redirect_uri": Some("https://areweoidcyet.com/client-implementation-guide/callback")} }), body: Some(NormalizedParameter { inner: {} }) }

thread 'conduwuit:worker' panicked at src/web/oidc/response.rs:37:30:
body
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
  2025-05-10T19:54:16.099814Z ERROR conduwuit_router::layers: body
    at src/router/layers.rs:190 on conduwuit:worker ThreadId(7)
    in conduwuit_router::layers::panic
    in conduwuit_router::request::handle with active=0 handled=9
    in conduwuit_router::request::request
    in conduwuit_router::layers::router with method=POST path=/_matrix/client/unstable/org.matrix.msc2964/authorize

  2025-05-10T19:54:16.100235Z ERROR conduwuit_router::request: 500 Internal Server Error, method: POST, uri: /_matrix/client/unstable/org.matrix.msc2964/authorize?client_id=tQwCbMNIZU&redirect_uri=https%3A%2F%2Fareweoidcyet%2Ecom%2Fclient%2Dimplementation%2Dguide%2Fcallback&scope=urn%3Amatrix%3Aorg%2Ematrix%2Emsc2967%2Eclient%3Aapi%3A%2A%20urn%3Amatrix%3Aorg%2Ematrix%2Emsc2967%2Eclient%3Adevice%3AEIv9AHM9TmWkhAXS&state=rdgVyeK58yYEGpFt&code_challenge=DV2ZxyJZ0MWLPBUMmKjsN35LdjFoVmEe8YSbsRKTLx0&code_challenge_method=S256&response_type=code&response_mode=fragment&allow=true
    at src/router/request.rs:105 on conduwuit:worker ThreadId(7)
    in conduwuit_router::request::request
    in conduwuit_router::layers::router with method=POST path=/_matrix/client/unstable/org.matrix.msc2964/authorize

2025-05-10T19:54:16.100306Z DEBUG router{method=POST path=/_matrix/client/unstable/org.matrix.msc2964/authorize}: tower_http::trace::on_response: finished processing request latency=2 ms status=500
  2025-05-10T19:54:16.100339Z ERROR tower_http::trace::on_failure: response failed, classification: Status code: 500 Internal Server Error, latency: 2 ms
    at /Users/jade/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tower-http-0.6.2/src/trace/on_failure.rs:93 on conduwuit:worker ThreadId(7)
    in conduwuit_router::layers::router with method=POST path=/_matrix/client/unstable/org.matrix.msc2964/authorize

2025-05-10T19:54:16.137886Z ERROR router{method=GET path=/favicon.ico}:request: conduwuit_router::request: 404 Not Found method=GET uri=/favicon.ico
2025-05-10T19:54:16.137974Z DEBUG router{method=GET path=/favicon.ico}: tower_http::trace::on_response: finished processing request latency=0 ms status=404
Nope, not solved. I fixed the issue with IP literals in particular in https://forgejo.ellis.link/continuwuation/continuwuity/commit/7ff9d770e318fdae2ae673a57daf551d005be6e0, this is a different issue ```toml [global] server_name = "test.local" database_path = "./target/database" auth = { enable_oidc_login = true, enable_oidc_account_management = true } well_known = { client = "http://127.0.0.1:8008" } ``` ``` http://127.0.0.1:8008/_matrix/client/unstable/org.matrix.msc2964/authorize ?response_type = code &response_mode = fragment &client_id = tQwCbMNIZU &redirect_uri = https%3A%2F%2Fareweoidcyet.com%2Fclient-implementation-guide%2Fcallback &scope = urn%3Amatrix%3Aorg.matrix.msc2967.client%3Aapi%3A*%20urn%3Amatrix%3Aorg.matrix.msc2967.client%3Adevice%3AEIv9AHM9TmWkhAXS &state = rdgVyeK58yYEGpFt &code_challenge_method = S256 &code_challenge = DV2ZxyJZ0MWLPBUMmKjsN35LdjFoVmEe8YSbsRKTLx0 ``` ``` 2025-05-10T19:53:08.906177Z DEBUG router{method=GET path=/_matrix/client/unstable/org.matrix.msc2964/authorize}: tower_http::trace::on_response: finished processing request latency=0 ms status=200 2025-05-10T19:54:14.518548Z INFO router{method=POST path=/_matrix/client/unstable/org.matrix.msc2964/login}:request:handle{active=0 handled=8}: conduwuit_api::client::oidc::login: logging in: "@jade:test.local" 2025-05-10T19:54:14.518974Z DEBUG router{method=POST path=/_matrix/client/unstable/org.matrix.msc2964/login}: tower_http::trace::on_response: finished processing request latency=37 ms status=200 2025-05-10T19:54:16.098437Z DEBUG router{method=POST path=/_matrix/client/unstable/org.matrix.msc2964/authorize}:request:handle{active=0 handled=9}: conduwuit_api::client::oidc::authorize: processing user's consent: true - OidcRequest { auth: None, query: Some(NormalizedParameter { inner: {"response_type": Some("code"), "state": Some("rdgVyeK58yYEGpFt"), "response_mode": Some("fragment"), "code_challenge_method": Some("S256"), "allow": Some("true"), "client_id": Some("tQwCbMNIZU"), "code_challenge": Some("DV2ZxyJZ0MWLPBUMmKjsN35LdjFoVmEe8YSbsRKTLx0"), "scope": Some("urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:EIv9AHM9TmWkhAXS"), "redirect_uri": Some("https://areweoidcyet.com/client-implementation-guide/callback")} }), body: Some(NormalizedParameter { inner: {} }) } thread 'conduwuit:worker' panicked at src/web/oidc/response.rs:37:30: body note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace 2025-05-10T19:54:16.099814Z ERROR conduwuit_router::layers: body at src/router/layers.rs:190 on conduwuit:worker ThreadId(7) in conduwuit_router::layers::panic in conduwuit_router::request::handle with active=0 handled=9 in conduwuit_router::request::request in conduwuit_router::layers::router with method=POST path=/_matrix/client/unstable/org.matrix.msc2964/authorize 2025-05-10T19:54:16.100235Z ERROR conduwuit_router::request: 500 Internal Server Error, method: POST, uri: /_matrix/client/unstable/org.matrix.msc2964/authorize?client_id=tQwCbMNIZU&redirect_uri=https%3A%2F%2Fareweoidcyet%2Ecom%2Fclient%2Dimplementation%2Dguide%2Fcallback&scope=urn%3Amatrix%3Aorg%2Ematrix%2Emsc2967%2Eclient%3Aapi%3A%2A%20urn%3Amatrix%3Aorg%2Ematrix%2Emsc2967%2Eclient%3Adevice%3AEIv9AHM9TmWkhAXS&state=rdgVyeK58yYEGpFt&code_challenge=DV2ZxyJZ0MWLPBUMmKjsN35LdjFoVmEe8YSbsRKTLx0&code_challenge_method=S256&response_type=code&response_mode=fragment&allow=true at src/router/request.rs:105 on conduwuit:worker ThreadId(7) in conduwuit_router::request::request in conduwuit_router::layers::router with method=POST path=/_matrix/client/unstable/org.matrix.msc2964/authorize 2025-05-10T19:54:16.100306Z DEBUG router{method=POST path=/_matrix/client/unstable/org.matrix.msc2964/authorize}: tower_http::trace::on_response: finished processing request latency=2 ms status=500 2025-05-10T19:54:16.100339Z ERROR tower_http::trace::on_failure: response failed, classification: Status code: 500 Internal Server Error, latency: 2 ms at /Users/jade/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tower-http-0.6.2/src/trace/on_failure.rs:93 on conduwuit:worker ThreadId(7) in conduwuit_router::layers::router with method=POST path=/_matrix/client/unstable/org.matrix.msc2964/authorize 2025-05-10T19:54:16.137886Z ERROR router{method=GET path=/favicon.ico}:request: conduwuit_router::request: 404 Not Found method=GET uri=/favicon.ico 2025-05-10T19:54:16.137974Z DEBUG router{method=GET path=/favicon.ico}: tower_http::trace::on_response: finished processing request latency=0 ms status=404 ```
lafleur commented 2025-05-11 00:24:59 +00:00
Author
First-time contributor
Copy link

OK, obviously I was too quick using OidcResponse in api/client/oidc/discovery.rs. This specific issue should be fixed with
-e09d60a6df- - discarded by 357c2335 .

I'm a bit jealous of your runtime logs, I only have access to a release binary, because I don't have enough disk space to compile debug builds. If I had access to that when debugging oxide-auth at the beggining, I'd have saved quite some hours !

OK, obviously I was too quick using `OidcResponse` in `api/client/oidc/discovery.rs`. This specific issue should be fixed with -e09d60a6df- - discarded by 357c2335 . I'm a bit jealous of your runtime logs, I only have access to a release binary, because I don't have enough disk space to compile debug builds. If I had access to that when debugging oxide-auth at the beggining, I'd have saved quite some hours !
lafleur commented 2025-05-11 10:59:30 +00:00
Author
First-time contributor
Copy link

Actually, OidcResponse needs a deeper rewrite. I thought I had copied over all functionalities from oxide-auth-axum's OAuthResponse, but it appears I copied the wrong implementation. I'm on it now. BTW my machine built a debug build this night, thanks to nix --store ... and the workaround I published in #822

Actually, `OidcResponse` needs a deeper rewrite. I thought I had copied over all functionalities from oxide-auth-axum's `OAuthResponse`, but it appears I copied the wrong implementation. I'm on it now. BTW my machine built a debug build this night, thanks to `nix --store ...` and the workaround I published in #822
lafleur commented 2025-05-11 13:21:12 +00:00
Author
First-time contributor
Copy link

Done in 357c2335 - rewrite of OidcResponse's IntoResponse implementation. Should fix the issue at hand (the location header was not set), and a bunch of others.

Done in 357c2335 - rewrite of `OidcResponse`'s `IntoResponse` implementation. Should fix the issue at hand (the location header was not set), and a bunch of others.
lafleur commented 2025-05-12 11:45:59 +00:00
Author
First-time contributor
Copy link

Can you confirm this build passes areweoidcyet.com's test page ?

Can you confirm this build passes areweoidcyet.com's test page ?
src/api/client/oidc/discovery.rs Outdated
@ -0,0 +34,4 @@
http::StatusCode::NOT_FOUND,
ClientErrorBody::Standard {
kind: ClientErrorKind::Unrecognized,
message: "This homeserver doesn't do OIDC authentication.".to_owned(),
Jade commented 2025-05-10 13:38:54 +00:00
Owner
Copy link

'This homeserver has disabled OIDC authentication'

'This homeserver has disabled OIDC authentication'
lafleur commented 2025-05-10 17:22:08 +00:00
Author
First-time contributor
Copy link

fixed in f4e0ff9e13

fixed in f4e0ff9e13
lafleur marked this conversation as resolved
src/api/client/oidc/discovery.rs Outdated
@ -0,0 +71,4 @@
.unwrap(),
),
revocation_endpoint: issuer
.join("_matrix/client/unstable/org.matrix.msc2964/revoke")
Jade commented 2025-05-10 13:39:30 +00:00
Owner
Copy link

Missing starting slash here?

Missing starting slash here?
lafleur commented 2025-05-10 17:23:11 +00:00
Author
First-time contributor
Copy link

Nice catch ! fixed in 0781453572

Nice catch ! fixed in 0781453572
lafleur marked this conversation as resolved
src/api/router.rs Outdated
@ -117,6 +117,21 @@ pub fn build(router: Router<State>, server: &Server) -> Router<State> {
.ruma_route(&client::get_protocols_route)
.route("/_matrix/client/unstable/thirdparty/protocols",
get(client::get_protocols_route_unstable))
// MSC2965 route.
Jade commented 2025-05-10 13:29:20 +00:00
Owner
Copy link

Nitpicking but include the MSC titles please

Nitpicking but include the MSC titles please
lafleur commented 2025-05-10 17:23:53 +00:00
Author
First-time contributor
Copy link

Right ! fixed in f4e0ff9e13

Right ! fixed in f4e0ff9e13
lafleur marked this conversation as resolved
src/core/config/check.rs Outdated
@ -274,0 +275,4 @@
if auth.enable_oidc_login && config.well_known.client.is_none() {
return Err!(Config(
"auth.enable_oidc_login",
"Oidc authentication is enabled but the well-known client is not set."
Jade commented 2025-05-10 13:28:45 +00:00
Owner
Copy link

OIDC / HTML URL shouldn't be tied to the client-server API URL. Can use it as a default, but should be a separate config option

OIDC / HTML URL shouldn't be tied to the client-server API URL. Can use it as a default, but should be a separate config option
lafleur commented 2025-05-10 16:53:44 +00:00
Author
First-time contributor
Copy link

I believed that until we implement delegation, the OIDC issuer should run at the same URL as the server. Should that be config.well_known.server ?

And, if we make this a configurable default, then if the admin sets it to some other server, that server will expect all sorts of things like client provisioning. I naïvely thought we could avoid that, and implement delegation over standard OIDC servers in a later MR.

Maybe it's silly - or too ambitious. I don't get why Matrix did that overlay over OIDC. I hope Conduwuit can support standard OIDC issuers like Kanidm. Does that sound realistic ?

I believed that until we implement delegation, the OIDC issuer should run at the same URL as the server. Should that be `config.well_known.server` ? And, if we make this a configurable default, then if the admin sets it to some other server, that server will expect all sorts of things like client provisioning. I naïvely thought we could avoid that, and implement delegation over standard OIDC servers in a later MR. Maybe it's silly - or too ambitious. I don't get why Matrix did that overlay over OIDC. I hope Conduwuit can support standard OIDC issuers like Kanidm. Does that sound realistic ?
Jade commented 2025-05-10 17:01:52 +00:00
Owner
Copy link

So this is not requesting delegation, but for the case where, for example, continuwuity is running on matrix.somedomain.com, but the user wants login pages, etc to appear on somedomain.com (and they've set their reverse proxy up correctly for that). For that to work, they'd need to be able to set the URL to a different value to the C/S URL

So this is not requesting delegation, but for the case where, for example, continuwuity is running on matrix.somedomain.com, but the user wants login pages, etc to appear on somedomain.com (and they've set their reverse proxy up correctly for that). For that to work, they'd need to be able to set the URL to a different value to the C/S URL
Jade commented 2025-05-10 17:05:19 +00:00
Owner
Copy link

In fact, this but might not even be necessary, looks like it defaults to "Continuwuity" in the only place it's used
image

In fact, this but might not even be necessary, looks like it defaults to "Continuwuity" in the only place it's used ![image](/attachments/97d30e0d-40bf-49d5-9ed1-75b724d76961)
image.png
50 KiB
lafleur commented 2025-05-10 17:41:23 +00:00
Author
First-time contributor
Copy link

The well-known client is used in src/api/client/oidc/discovery.rs to advertise the OIDC issuer. That's why I check on it at startup - if we are the OIDC issuer, we need an URL to advertise.

That should be the well-known server instead, right ?

On the other hand, I wanted the login and consent page to display the current server name as a title. That's what the hostname variable is for (that's in src/api/client/oidc/login.rs). The well-known client was a hasty fill for the job. Is there a config variable that would be more adapted ? Or should we add eg an advertised_servername in [global.auth] ?

The well-known client is used in `src/api/client/oidc/discovery.rs` to advertise the OIDC issuer. That's why I check on it at startup - if we are the OIDC issuer, we need an URL to advertise. That should be the well-known server instead, right ? On the other hand, I wanted the login and consent page to display the current server name as a title. That's what the `hostname` variable is for (that's in `src/api/client/oidc/login.rs`). The well-known client was a hasty fill for the job. Is there a config variable that would be more adapted ? Or should we add eg an `advertised_servername` in `[global.auth]` ?
Jade commented 2025-05-10 17:55:08 +00:00
Owner
Copy link

For displaying the server name, you want config.server_name, which is what ends up being a part of the username.

For the OIDC Discovery document, client is OK as a default, given that it's usually clients accessing those endpoints. I'll have a more in-depth look at that when I review for spec/functionality.

For displaying the server name, you want `config.server_name`, which is what ends up being a part of the username. For the OIDC Discovery document, client is OK as a default, given that it's usually clients accessing those endpoints. I'll have a more in-depth look at that when I review for spec/functionality.
lafleur commented 2025-05-10 19:13:42 +00:00
Author
First-time contributor
Copy link

OK ! Added in 2baeba41 - proxy users would set that variable anyway, right ? So this settles that, doesn't it ?

OK ! Added in 2baeba41 - proxy users would set that variable anyway, right ? So this settles that, doesn't it ?
Jade commented 2025-05-10 19:40:25 +00:00
Owner
Copy link

Yep, you have to set server_name to start up the server

Yep, you *have* to set `server_name` to start up the server
Jade marked this conversation as resolved
src/core/config/mod.rs Outdated
@ -1882,0 +1892,4 @@
/// The URL where the user is able to access the account management
/// capabilities of the homeserver. Only used if `enable_oidc_login` is set.
/// Unset by default.
pub enable_oidc_account_management: bool,
Jade commented 2025-05-10 13:27:46 +00:00
Owner
Copy link

Doc comment doesn't match here. Should have a default set.

Doc comment doesn't match here. Should have a default set.
lafleur commented 2025-05-10 17:28:13 +00:00
Author
First-time contributor
Copy link

I updated it to match the implementation in 97b66d2f3e - I guess the question to use a set or just a boolean is dependent on our other conversation on src/core/config/check.rs.

I updated it to match the implementation in 97b66d2f3e - I guess the question to use a set or just a boolean is dependent on our other conversation on `src/core/config/check.rs`.
Jade marked this conversation as resolved
src/service/Cargo.toml Outdated
@ -111,6 +111,7 @@ webpage.workspace = true
webpage.optional = true
blurhash.workspace = true
blurhash.optional = true
oxide-auth.workspace = true
Jade commented 2025-05-10 13:43:44 +00:00
Owner
Copy link

Is there a way we can avoid adding the oxide-auth dependency to so many crates? Reexporting necessary types, maybe

Is there a way we can avoid adding the oxide-auth dependency to so many crates? Reexporting necessary types, maybe
lafleur commented 2025-05-10 19:38:51 +00:00
Author
First-time contributor
Copy link

That would need a separate crate as you proposed.

That would need a separate crate as you proposed.
lafleur added 3 commits 2025-05-10 17:18:31 +00:00
fix oidc_provider discovery message and docstrings
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been cancelled
Details
Release Docker Image / define-variables (pull_request) Has been cancelled
Details
Release Docker Image / build-image (linux/amd64, linux-amd64) (pull_request) Has been cancelled
Details
Release Docker Image / build-image (linux/arm64, linux-arm64) (pull_request) Has been cancelled
Details
Release Docker Image / merge (pull_request) Has been cancelled
Details
f4e0ff9e13
lafleur added 3 commits 2025-05-10 19:12:45 +00:00 
I don't have the hd space to do debug builds, so I use tracing::info to debug
on release builds. Silly, right ?
oidc: small cosmetics + typos
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been cancelled
Details
Release Docker Image / define-variables (pull_request) Has been cancelled
Details
Release Docker Image / build-image (linux/amd64, linux-amd64) (pull_request) Has been cancelled
Details
Release Docker Image / build-image (linux/arm64, linux-arm64) (pull_request) Has been cancelled
Details
Release Docker Image / merge (pull_request) Has been cancelled
Details
23b581dd98
lafleur commented 2025-05-10 19:36:59 +00:00
Author
First-time contributor
Copy link

BTW I'm sorry, I can't run cargo +nightly fmt on my machine right now, lack of resources here. I'll try to set it up.

BTW I'm sorry, I can't run `cargo +nightly fmt` on my machine right now, lack of resources here. I'll try to set it up.
Jade commented 2025-05-10 19:47:36 +00:00
Owner
Copy link

DW, I'm about to do that + fix some compile errors

DW, I'm about to do that + fix some compile errors
Jade added 1 commit 2025-05-10 19:51:08 +00:00
chore: fix up
Some checks failed
Release Docker Image / define-variables (pull_request) Successful in 1s
Details
Documentation / Build and Deploy Documentation (pull_request) Failing after 24s
Details
Release Docker Image / build-image (linux/amd64, linux-amd64) (pull_request) Failing after 1m9s
Details
Release Docker Image / build-image (linux/arm64, linux-arm64) (pull_request) Failing after 51s
Details
Release Docker Image / merge (pull_request) Has been skipped
Details
9f0fce843c
lafleur added 1 commit 2025-05-11 00:17:52 +00:00
fix OidcResponse: tolerate empty body
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been cancelled
Details
Release Docker Image / define-variables (pull_request) Has been cancelled
Details
Release Docker Image / build-image (linux/amd64, linux-amd64) (pull_request) Has been cancelled
Details
Release Docker Image / build-image (linux/arm64, linux-arm64) (pull_request) Has been cancelled
Details
Release Docker Image / merge (pull_request) Has been cancelled
Details
e09d60a6df
lafleur reviewed 2025-05-11 00:29:15 +00:00
src/service/oidc/mod.rs Outdated
@ -0,0 +48,4 @@
pub fn preconfigured() -> Self {
Self {
registrar: Mutex::new(
vec![Client::public(
lafleur commented 2025-05-11 00:29:15 +00:00
Author
First-time contributor
Copy link

Before I forget about this old commit: this is purely illustrative, stating how to populate the initial client registry with an app. We should remove that sample client before long.

Before I forget about this old commit: this is purely illustrative, stating how to populate the initial client registry with an app. We should remove that sample client before long.
👍 1
lafleur force-pushed as-oidc-provider from e09d60a6df to 357c233576 2025-05-11 13:18:11 +00:00 Compare
Jade added 1 commit 2025-05-11 19:59:10 +00:00
fixup! fix OidcResponse: reimplement IntoResponse
Some checks failed
Release Docker Image / define-variables (pull_request) Failing after 0s
Details
Release Docker Image / build-image (linux/amd64, linux-amd64) (pull_request) Has been skipped
Details
Release Docker Image / build-image (linux/arm64, linux-arm64) (pull_request) Has been skipped
Details
Release Docker Image / merge (pull_request) Has been skipped
Details
Documentation / Build and Deploy Documentation (pull_request) Failing after 41s
Details
9c7c7e7798
lafleur reviewed 2025-05-12 11:50:28 +00:00
src/api/router.rs Outdated
@ -118,2 +118,4 @@
.route("/_matrix/client/unstable/thirdparty/protocols",
get(client::get_protocols_route_unstable))
// MSC2965: OAuth 2.0 Authorization Server Metadata discovery.
.route("/_matrix/client/unstable/org.matrix.msc2965/auth_metadata",
lafleur commented 2025-05-12 11:50:28 +00:00
Author
First-time contributor
Copy link

Any idea why Element X doesn't use the OIDC login endpoint ? I thought that maybe they don't use the unstable endpoints URLs anymore - the related MSCs were merged recently. Should I try to remove the unstable prefixes from the routes ?

Any idea why Element X doesn't use the OIDC login endpoint ? I thought that maybe they don't use the unstable endpoints URLs anymore - the related MSCs were merged recently. Should I try to remove the unstable prefixes from the routes ?
Jade commented 2025-05-12 16:27:04 +00:00
Owner
Copy link

You can try that, don't forget you can register the same handler on multiple routes.

You can try that, don't forget you can register the same handler on multiple routes.
lafleur commented 2025-05-20 10:08:12 +00:00
Author
First-time contributor
Copy link

I did try that (remove the unstable prefix from the OIDC discovery endpoint), the issue is still the same.

I must say I don't know what to do else. I did have some oauth knowledge before starting this MR because I already had implemented a client in react, but I'm definitely no expert in Matrix clients - I just used to run a conduwuit server for my family. This MR already contains all I can contribute at the moment, I'm afraid. Besides, I have plenty of paid work in another field coming up for the next two months, so I can't invest much time in debugging this issue.

If someone comes up with something else to try, I might find a bit of time for that though.

I did try that (remove the unstable prefix from the OIDC discovery endpoint), the issue is still the same. I must say I don't know what to do else. I did have some oauth knowledge before starting this MR because I already had implemented a client in react, but I'm definitely no expert in Matrix clients - I just used to run a conduwuit server for my family. This MR already contains all I can contribute at the moment, I'm afraid. Besides, I have plenty of paid work in another field coming up for the next two months, so I can't invest much time in debugging this issue. If someone comes up with something else to try, I might find a bit of time for that though.
Jade commented 2025-05-21 11:50:45 +00:00
Owner
Copy link

I'll have a look and see what I can figure out, but if I can't get it to work, I guess we can continue without that for now.

I'll have a look and see what I can figure out, but if I can't get it to work, I guess we can continue without that for now.
nex commented 2025-05-31 18:49:59 +00:00
Owner
Copy link

Taking a look at my Element X IOS Nightly logs, I see this:

2025-05-31T18:44:13.477754Z DEBUG matrix_sdk::http_client::native: Sending request num_attempt=1 | crates/matrix-sdk/src/http_client/native.rs:78 | spans: send{request_id="REQ-3" method=GET uri="https://matrix-client.matrix.org/_matrix/client/unstable/org.matrix.msc2965/auth_metadata"}

I'll see if I can figure out why it's not making the request here

Taking a look at my Element X IOS Nightly logs, I see this: `2025-05-31T18:44:13.477754Z DEBUG matrix_sdk::http_client::native: Sending request num_attempt=1 | crates/matrix-sdk/src/http_client/native.rs:78 | spans: send{request_id="REQ-3" method=GET uri="https://matrix-client.matrix.org/_matrix/client/unstable/org.matrix.msc2965/auth_metadata"}` I'll see if I can figure out why it's not making the request here
Jade force-pushed as-oidc-provider from 9c7c7e7798 to bb9e8af4e0 2025-05-21 11:48:14 +00:00 Compare
nex added 1 commit 2025-05-31 20:23:47 +00:00
fix build errors
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Failing after 34s
Details
9dbd0e654c
nex requested changes 2025-05-31 23:02:09 +00:00
src/web/oidc/authorize.rs
@ -0,0 +9,4 @@
pub redirect_uri: Url,
pub scope: String,
pub state: String,
pub code_challenge: String,
nex commented 2025-05-31 23:02:02 +00:00
Owner
Copy link

Now that clients are confirmed to be connecting correctly (confirmed in #development:continuwuity.org, I got mxtoken and Element X iOS to correctly detect and start using OIDC), there's an issue with the initial redirect to /authorize:

Failed to deserialize query string: missing field code_challenge

In the initial request, this, and other fields are not provided, but required by this struct (and similar ones like LoginQuery).

The only five query params provided at the start of the flow are client_id, response_type, redirect_uri, state, and scope. The rest are seemingly used later on?

Now that clients are confirmed to be connecting correctly (confirmed in `#development:continuwuity.org`, I got [mxtoken](https://timedout.uk/mxtoken.html) and Element X iOS to correctly detect and start using OIDC), there's an issue with the initial redirect to `/authorize`: `Failed to deserialize query string: missing field code_challenge` In the initial request, this, and other fields are not provided, but required by this struct (and similar ones like LoginQuery). The only five query params provided at the start of the flow are `client_id`, `response_type`, `redirect_uri`, `state`, and `scope`. The rest are seemingly used later on?
ty commented 2025-06-01 15:03:11 +00:00
First-time contributor
Copy link

Both code_challenge and code_challenge_method are necessary for PKCE, which is required as per the MSC on OIDC login. If a client is truly MSC2964 compatible, those parameters must be sent.

The same MSC also appears to suggest the response_mode parameter is required (it is explicitly mentioned multiple times without a default, and has the client check the server for compatibility with different modes), so it should also be sent every time. Which client wasn't sending these?

Both `code_challenge` and `code_challenge_method` are necessary for PKCE, which is required as per the MSC on OIDC login. If a client is truly MSC2964 compatible, those parameters must be sent. The same MSC also appears to suggest the `response_mode` parameter is required (it is explicitly mentioned multiple times without a default, and has the client check the server for compatibility with different modes), so it should also be sent every time. Which client wasn't sending these?
nex commented 2025-05-31 23:04:32 +00:00
Owner
Copy link

I'm going to spend some time on this over the next week to see if we can get it working. It doesn't seem far off being finished.

I'm going to spend some time on this over the next week to see if we can get it working. It doesn't seem far off being finished.
lafleur commented 2025-05-31 23:20:11 +00:00
Author
First-time contributor
Copy link

Great ! I'm totally busy until the 8th of june, so I won't be able to help this week, but I'll try to follow your advances remotely - indeed, I think the MR is pretty close to the needed functionality, and I'm confident you'll find out how to make it work.

I guess the code will need some cleanup once it works, I think I'll focus on that when I'm back.

Thanks @nex

Great ! I'm totally busy until the 8th of june, so I won't be able to help this week, but I'll try to follow your advances remotely - indeed, I think the MR is pretty close to the needed functionality, and I'm confident you'll find out how to make it work. I guess the code will need some cleanup once it works, I think I'll focus on that when I'm back. Thanks @nex
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Failing after 34s
Details
This pull request has changes conflicting with the target branch.
  • Cargo.lock
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u as-oidc-provider:lafleur-as-oidc-provider
git checkout lafleur-as-oidc-provider

Merge

Merge the changes and update on Forgejo.
git checkout main
git merge --no-ff lafleur-as-oidc-provider
git checkout lafleur-as-oidc-provider
git rebase main
git checkout main
git merge --ff-only lafleur-as-oidc-provider
git checkout lafleur-as-oidc-provider
git rebase main
git checkout main
git merge --no-ff lafleur-as-oidc-provider
git checkout main
git merge --squash lafleur-as-oidc-provider
git checkout main
git merge --ff-only lafleur-as-oidc-provider
git checkout main
git merge lafleur-as-oidc-provider
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
Jade
nex
Labels
Clear labels   
Bug

Something isn't working as intended

  
Cherry-picking

Commits picked from other conduit projects

  
Database

This requires or includes changes to the database

  
Dependencies

Something dependency related

  
Documentation

Improvements or additions to documentation

  
Enhancement

New feature or request

  
Good first issue

Good for newcomers

  
Help wanted

Additional eyes and keyboards are required for this one

  
Inherited

Issues that have been inhereted from the project pre-fork

  
Matrix/Administration

Features pertaining to homeserver administration

  
Matrix/Appservices

Features pertaining to the appservice API

  
Matrix/Auth

Features pertaining to authentication

  
Matrix/Client

Features pertaining to client-to-server interactions

  
Matrix/Core

   
Matrix/Federation

Features pertaining to server-to-server interactions

  
Matrix/MSC

Features pertaining to unstable matrix features

  
Matrix/Media

Features pertaining to media interactions

  
Meta

Related to housekeeping, maintenance, or other repo-meta.

  
Meta/Packaging

Packaging

  
Priority
Blocking
This issue is blocking the next release

  
Priority
High
This issue is very important

  
Priority
Low
This issue is of a rather low priority

  
Security

This item is related to general security

  
Status
Confirmed

  
Status
Duplicate
This issue or pull request already exists

  
Status
Invalid
This issue doesn't seem right

  
Status
Needs Investigation
This issue needs further investigation

  
Wont fix

This will not be worked on

  
old/ci/cd

Ci/CD

Archived

  
old/rust

Pull requests that update Rust code

Archived

No labels
Bug
Cherry-picking
Database
Dependencies
Documentation
Enhancement
Good first issue
Help wanted
Inherited
Matrix/Administration
Matrix/Appservices
Matrix/Auth
Matrix/Client
Matrix/Core
Matrix/Federation
Matrix/MSC
Matrix/Media
Meta
Meta/Packaging
Priority
Blocking
Priority
High
Priority
Low
Security
Status
Confirmed
Status
Duplicate
Status
Invalid
Status
Needs Investigation
Wont fix
old/ci/cd
old/rust
Milestone
Clear milestone
No items
No milestone
0.5.0
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: continuwuation/continuwuity#810
No description provided.
Delete branch "lafleur/continuwuity:as-oidc-provider"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?