oauth2-types for clients

[matrixbot]

This issue was originally created by @zecakeh at matrix-org/matrix-authentication-service#313.

I'm working on adding support for OIDC in the matrix-rust-sdk, at least as a PoC for now (so clients like ElementX could test it).

I've tested locally based on two crates:

• openidconnect: it seems to work fine apart from a little incompatibility where it expects the OP to return the client metadata when dynamic registration is successful. It's a SHOULD in the OpenID Connect Dynamic Client Registration spec, so the crate should probably not require it.

• this project's oauth2-types: I had to make a few modifications to be able to build some types that had private fields, but other than that it's pretty usable.

I preferred to work with oauth2-types even if everything needs to be done manually.

Is it planned for oauth2-types to have first class client support? I already see in the docs that it's mentioned it might be worth publishing this crate.

By first-class I mean integrating with http to build requests or extract responses for example. There could also be client vs server features, to improve compile time by only compiling what is necessary (e.g. for a client, a request doesn't need to be deserialized, and a response doesn't need to be serialized).

[matrixbot]

This comment was originally posted by @sandhose at matrix-org/matrix-authentication-service#313 (comment).

I'm working on matrix-org/matrix-rust-sdk#859, at least as a PoC for now (so clients like ElementX could test it).

That is really good to hear! I planned to do that anyway once I'm back (currently out of office until the 1st of august), so I would be glad to help you on that

Is it planned for oauth2-types to have first class client support? I already see in the docs that it's mentioned it might be worth publishing this crate.

We need OIDC 'client' features in MAS anyway at some point, because we will need to support upstream OPs (see #19).

By first-class I mean integrating with http to build requests or extract responses for example.

For doing HTTP request, I would like to leverage tower. You would provide a Service<http::Request, Response = http::Response>, with whatever layer you want (see for example what I'm doing here for the HTTP client I'm using for requesting clients JWKS).

---

Have you also looked at what's in the JOSE crate? It handles JWT-related stuff, including extracting and validating claims from JWTs.

Another useful thing you might be interested in, is the certification suite from the OpenID foundation, which I found very useful for validating the 'server' (OP) implementation, and it also has a bunch of 'client' (RP) test suites.

[matrixbot]

This comment was originally posted by @zecakeh at matrix-org/matrix-authentication-service#313 (comment).

That is really good to hear! I planned to do that anyway once I'm back (currently out of office until the 1st of august), so I would be glad to help you on that

Thanks! I won't hesitate to come to you if I have questions. Do you have a Matrix room where I could ask questions in a less formal way? My Matrix ID is @zecakeh:tedomum.net if you want to invite me (when you're back of course).

For doing HTTP request, I would like to leverage tower. You would provide a Service<http::Request, Response = http::Response>, with whatever layer you want (see for example what I'm doing here for the HTTP client I'm using for requesting clients JWKS).

I'm not sure how that would look like. Do we create endpoints that implement a Layer that populates the request, and defines the response type and error type?

Have you also looked at what's in the JOSE crate? It handles JWT-related stuff, including extracting and validating claims from JWTs.

Yes, it's very useful. I'm already using it to validate the claims of the received ID token.

Another useful thing you might be interested in, is the certification suite from the OpenID foundation, which I found very useful for validating the 'server' (OP) implementation, and it also has a bunch of 'client' (RP) test suites.

Nice, when I'm confident enough in my implementation I'll definitely use that to validate it.

---

Right now I've managed to proceed to the client registration and "login" and get the access token from MAS but my code is not really organized.

As a first step, I'd like to improve types:

• Make sure they are easy to construct, i.e. making sure fields are public and eventually adding constructors.
• Improve correctness regarding the spec: make required fields non optional. Should we respect the OAuth2.0 spec or the OpenID Connect 1.0 spec, since the latter makes some optional fields from the former required?
• Make stronger typed errors, because I noticed a few of them return anyhow::Error, which is fine for a binary but less for a library.
• Move some types or create "redundant" types in oauth2-types, e.g. something like mas_axum_utils::client_authorization::Credentials would be useful to make an auth request, although we don't need the methods.

[matrixbot]

This comment was originally posted by @sandhose at matrix-org/matrix-authentication-service#313 (comment).

Thanks! I won't hesitate to come to you if I have questions. Do you have a Matrix room where I could ask questions in a less formal way? My Matrix ID is @zecakeh:tedomum.net if you want to invite me (when you're back of course).

There is #matrix-auth:matrix.org for Matrix+OIDC related discussions

I'm not sure how that would look like. Do we create endpoints that implement a Layer that populates the request, and defines the response type and error type?

Something like this

async fn token_exchange<S, ResBody>(client: &mut S) -> Result<(), Error>
where
    S: tower::Service<http::Request<String>, Response = http::Response<ResBody>>,
    ResBody: http_body::Body,
    Error: From<ResBody::Error> + From<S::Error>,
{
    // Build the request
    let request = http::Request::builder()
        .uri("https://example.com")
        .body("{}".into())?;
    // Send it
    let response = client.call(request).await?;

    // Do something with the response
    let (parts, body) = response.into_parts();
    let body = hyper::body::to_bytes(body).await?;

    Ok(())
}

and then use it like that

let http = hyper::client::HttpConnector::new();
let https = hyper_rustls::HttpsConnectorBuilder::new()
    .with_native_roots()
    .https_or_http()
    .enable_http1()
    .enable_http2()
    .wrap_connector(http);

let client = hyper::Client::builder().build(https);

let mut client = ServiceBuilder::new()
    .layer(DecompressionLayer::new())
    .layer(SetRequestHeaderLayer::overriding(
        USER_AGENT,
        "whatever/1.0",
    ))
    .layer(ConcurrencyLimitLayer::new(10))
    .layer(FollowRedirectLayer::new())
    .layer(TimeoutLayer::new(Duration::from_secs(10)))
    .service(client);

token_exchange(&mut client).await?;

This allows the user to provide any kind of HTTP client, with any stack of tower layers, like to decompress, set the user agent, add a global timeout, inject/extract tracing informations, etc.)

Of course, we would probably provide a default HTTP client with sane default to avoid some of this boilerplate for simpler use cases, but I like the flexibility of this approach.

---

Should we respect the OAuth2.0 spec or the OpenID Connect 1.0 spec, since the latter makes some optional fields from the former required?

I don't recall seeing that, do you have an example of this?

Make stronger typed errors, because I noticed a few of them return anyhow::Error, which is fine for a binary but less for a library.

Right, I usually go with thiserror for custom error types

Move some types or create "redundant" types in oauth2-types, e.g. something like mas_axum_utils::client_authorization::Credentials would be useful to make an auth request, although we don't need the methods.

Yeah, some stuff needs to be moved around a bit, it's not always clear where I need to put stuff. For that particular case, this code is really tied to some of Axum's traits, so it would need some refactoring to deal with a http::Request directly

[matrixbot]

This comment was originally posted by @zecakeh at matrix-org/matrix-authentication-service#313 (comment).

Should we respect the OAuth2.0 spec or the OpenID Connect 1.0 spec, since the latter makes some optional fields from the former required?

I don't recall seeing that, do you have an example of this?

Yes, for example the jwks_uri field is optional in RFC8414 but is required in the OpenID Connect Discovery 1.0 spec.

[matrixbot]

This comment was originally posted by @zecakeh at matrix-org/matrix-authentication-service#313 (comment).

I'm closing this, with #347 merged, the types are ready for clients.