Fix: Prevent uri-modification on redirect_uri-handling (#28)

* Localhost no trailing slash dump

* dump

* Add a display bound to `redirect_uri` and pass the raw `redirect_uri` into payloads

* Update deny

CHANGELOG.md

@@ -8,8 +8,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 <!-- next-header -->
 ## [x.x.x] - Unreleased
+### Changed
 - Update `http` to 1.x
 
+### Fixed
+- Add `Display` trait-bound to `redirect_uri`s so that they can be passed as-is to the request payload.
+Since these `URI`s are used for an integrity-check, rather than a request-destination, `IntoUri`'s manipulation
+(adding a slash to the end), can make `redirect_uri`'s that should be valid invalid.
+- Don't send a `code_verifier` when refreshing tokens
+
 ## [0.7.0] - 2023-12-19
 ### Changed
 - Change default behaviour back since updating `jsonwebtoken` to `0.9x` to client-based audience validation 

deny.toml

@@ -1,3 +1,4 @@
+[graph]
 targets = [
     { triple = "x86_64-unknown-linux-gnu" },
     { triple = "x86_64-unknown-linux-musl" },
@@ -6,7 +7,6 @@ targets = [
 ]
 
 [advisories]
-unmaintained = "deny"
 ignore = [
 ]
 
@@ -17,12 +17,13 @@ deny = [
     { name = "openssl-sys" },
 ]
 skip = [
+    # jsonwebtoken needs to update base64 internally, it's in progress but no new release
+    { name = "base64", version = "=0.21.7" }
 ]
 skip-tree = [
 ]
 
 [licenses]
-unlicensed = "deny"
 # We want really high confidence when inferring licenses from text
 confidence-threshold = 0.92
 allow = [

examples/embark-pkce.rs

@@ -5,14 +5,15 @@ use http::Request;
 use rand::rngs::ThreadRng;
 use rand::RngCore;
 use reqwest::Url;
+use serde_json::Value;
+use std::collections::HashMap;
 use std::{
     convert::TryInto,
     io::{prelude::*, BufReader},
     net::{TcpListener, TcpStream},
     str,
 };
 use tame_oidc::auth_scheme::{AuthenticationScheme, ClientAuthentication, PkceCredentials};
-use tame_oidc::provider::Claims;
 use tame_oidc::{
     oidc::Token,
     provider::{self, Provider, JWKS},
@@ -28,7 +29,7 @@ fn handle_connection(mut stream: TcpStream) -> Option<String> {
     reader.read_line(&mut request).unwrap();
 
     let query_params = request.split_whitespace().nth(1).unwrap();
-    let url = Url::parse(&format!("http://127.0.0.1:8000{query_params}")).unwrap();
+    let url = Url::parse(&format!("http://localhost:8000{query_params}")).unwrap();
 
     stream.write_all(http_status_ok().as_bytes()).unwrap();
     stream.flush().unwrap();
@@ -95,10 +96,10 @@ async fn main() {
     // Secret is optional in the PKCE flow
     let client_secret = std::env::var("CLIENT_SECRET").ok();
     let client_id = std::env::var("CLIENT_ID").unwrap();
-    let host = "127.0.0.1";
+    let host = "localhost";
     let port = 8000u16;
     // It's very important that this exactly matches where it's provided in other places, protocol and trailing slash all
-    let redirect_uri = format!("http://{host}:{port}/");
+    let redirect_uri = format!("http://{host}:{port}");
 
     // Fetch and instantiate a provider using a `well-known` uri from an issuer
     let request = provider::well_known(&issuer_domain).unwrap();
@@ -117,7 +118,8 @@ response_type=code&\
 client_id={client_id}&\
 redirect_uri={redirect_uri}&\
 state={state_str}&\
-scope=openid+offline",
+access_type=offline&\
+scope=openid+email+profile",
     );
     println!("Authorize at {authorize_url}");
 
@@ -148,7 +150,10 @@ scope=openid+offline",
     let response = http_send(&http_client, request).await;
     let jwks = JWKS::from_response(response).unwrap();
 
-    let token_data = provider::verify_token::<Claims>(&access_token.access_token, &jwks.keys);
+    let token_data = provider::verify_token::<HashMap<String, Value>>(
+        access_token.id_token.as_ref().unwrap(),
+        &jwks.keys,
+    );
     dbg!(&token_data);
     dbg!(&access_token);
     let refresh_token = access_token.refresh_token.unwrap();

src/oidc.rs

@@ -4,6 +4,7 @@ use data_encoding::BASE64;
 use http::header::AUTHORIZATION;
 use http::{header::CONTENT_TYPE, Request, Uri};
 use std::convert::TryInto;
+use std::fmt::Display;
 use std::time::{SystemTime, UNIX_EPOCH};
 use url::form_urlencoded::Serializer;
 
@@ -15,14 +16,16 @@ pub fn authorization_request<ReqUri, RedirectUri>(
 ) -> Result<Request<Vec<u8>>, RequestError>
 where
     ReqUri: TryInto<Uri>,
-    RedirectUri: TryInto<Uri>,
+    RedirectUri: TryInto<Uri> + Display,
 {
     let request_builder = Request::builder()
         .method("POST")
         .uri(into_uri(uri)?)
         .header(CONTENT_TYPE, "application/x-www-form-urlencoded");
     let mut serializer = Serializer::new(String::new());
-    serializer.append_pair("redirect_uri", &into_uri(redirect_uri)?.to_string());
+    let raw_redirect_uri = redirect_uri.to_string();
+    into_uri(redirect_uri)?;
+    serializer.append_pair("redirect_uri", &raw_redirect_uri);
     serializer.append_pair("grant_type", "authorization_code");
     serializer.append_pair("response_type", "code");
     serializer.append_pair(
@@ -77,7 +80,7 @@ struct TokenExchangeResponse {
     /// A JSON Web Token that contains information about an authentication event
     /// and claims about the authenticated user.
     id_token: Option<String>,
-    /// An opaque refresh token. This is returned if the offline_access scope is
+    /// An opaque refresh token. This is returned if the `offline_access` scope is
     /// granted.
     refresh_token: Option<String>,
 }
@@ -98,7 +101,7 @@ pub struct Token {
     /// A JSON Web Token that contains information about an authentication event
     /// and claims about the authenticated user.
     pub id_token: Option<String>,
-    /// An opaque refresh token. This is returned if the offline_access scope is
+    /// An opaque refresh token. This is returned if the `offline_access` scope is
     /// granted.
     pub refresh_token: Option<String>,
 }
@@ -155,10 +158,12 @@ pub fn exchange_token_request<ReqUri, RedirectUri>(
 ) -> Result<Request<Vec<u8>>, RequestError>
 where
     ReqUri: TryInto<Uri>,
-    RedirectUri: TryInto<Uri>,
+    RedirectUri: TryInto<Uri> + Display,
 {
     let mut serializer = Serializer::new(String::new());
-    serializer.append_pair("redirect_uri", &into_uri(redirect_uri)?.to_string());
+    let raw_redirect_uri = redirect_uri.to_string();
+    into_uri(redirect_uri)?;
+    serializer.append_pair("redirect_uri", &raw_redirect_uri);
     serializer.append_pair("grant_type", "authorization_code");
     serializer.append_pair("code", auth_code);
     let request_builder = Request::builder()
@@ -265,11 +270,7 @@ where
             if let Some(client_secret) = &pkce.client_secret {
                 partial.append_pair("client_secret", client_secret);
             }
-            request_builder.body(Vec::from(
-                partial
-                    .append_pair("code_verifier", &pkce.code_verifier)
-                    .finish(),
-            ))
+            request_builder.body(Vec::from(partial.finish()))
         }
     }?;
     Ok(body)

src/provider.rs

@@ -9,6 +9,7 @@ use jsonwebtoken::{decode, Algorithm, DecodingKey, TokenData, Validation};
 use serde::de::DeserializeOwned;
 use serde::{Deserialize, Serialize};
 use std::convert::TryInto;
+use std::fmt::Display;
 
 #[derive(Deserialize, Debug)]
 pub struct Provider {
@@ -47,7 +48,7 @@ impl Provider {
         scopes: &Option<Vec<String>>,
     ) -> Result<Request<Vec<u8>>, RequestError>
     where
-        RedirectUri: TryInto<Uri>,
+        RedirectUri: TryInto<Uri> + Display,
     {
         authorization_request(&self.authorization_endpoint, redirect_uri, auth, scopes)
     }
@@ -59,7 +60,7 @@ impl Provider {
         auth_code: &str,
     ) -> Result<Request<Vec<u8>>, RequestError>
     where
-        RedirectUri: TryInto<Uri>,
+        RedirectUri: TryInto<Uri> + Display,
     {
         exchange_token_request(&self.token_endpoint, redirect_uri, auth, auth_code)
     }