From 5e450a3d5c2b72c857909e8103b3e8824223e22b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Nevyho=C5=A1t=C4=9Bn=C3=BD?= Date: Sun, 10 Mar 2024 16:39:46 +0100 Subject: [PATCH] Fix personal token auth for pagination Previously the AuthHeaderLayer added the auth header only when the authority was empty (i.e., base URI for GitHub was supposed to be used). However, the page URIs (e.g., next page) returned by GitHub API contains the absolute URL including the hostname. Because of this, the following code didn't work for resources that needed authentication (e.g., private repos) ``` let page = /* get a page of something using octocrab */ // This fails with NOT_FOUND octocrab.get_page(&page.next).await; ``` This fix adds the base URI to the AuthHeaderLayer so that it can check whether a request is destined for GitHub (either the hostname is empty or it is equal to the base URI). --- src/lib.rs | 4 ++-- src/service/middleware/auth_header.rs | 15 ++++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 7b614d06..85340d0e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -741,9 +741,9 @@ impl OctocrabBuilder .clone() .unwrap_or_else(|| Uri::from_str(GITHUB_BASE_URI).unwrap()); - let client = BaseUriLayer::new(uri).layer(client); + let client = BaseUriLayer::new(uri.clone()).layer(client); - let client = AuthHeaderLayer::new(auth_header).layer(client); + let client = AuthHeaderLayer::new(auth_header, uri).layer(client); Ok(Octocrab::new(client, auth_state)) } diff --git a/src/service/middleware/auth_header.rs b/src/service/middleware/auth_header.rs index e25b2fb3..deec1034 100644 --- a/src/service/middleware/auth_header.rs +++ b/src/service/middleware/auth_header.rs @@ -1,18 +1,20 @@ use std::sync::Arc; -use http::{header::AUTHORIZATION, request::Request, HeaderValue}; +use http::{header::AUTHORIZATION, request::Request, HeaderValue, Uri}; use tower::{Layer, Service}; #[derive(Clone)] /// Layer that adds the authentication header to github-bound requests pub struct AuthHeaderLayer { pub(crate) auth_header: Arc>, + base_uri: Uri, } impl AuthHeaderLayer { - pub fn new(auth_header: Option) -> Self { + pub fn new(auth_header: Option, base_uri: Uri) -> Self { AuthHeaderLayer { auth_header: Arc::new(auth_header), + base_uri, } } } @@ -24,6 +26,7 @@ impl Layer for AuthHeaderLayer { AuthHeader { inner, auth_header: self.auth_header.clone(), + base_uri: self.base_uri.clone(), } } } @@ -33,6 +36,7 @@ impl Layer for AuthHeaderLayer { pub struct AuthHeader { inner: S, pub(crate) auth_header: Arc>, + base_uri: Uri, } impl Service> for AuthHeader @@ -51,11 +55,12 @@ where } fn call(&mut self, mut req: Request) -> Self::Future { - // Only set the auth_header if the authority (host) is empty (destined for - // GitHub). Otherwise, leave it off as we could have been redirected + // Only set the auth_header if the authority (host) is destined for + // GitHub. Otherwise, leave it off as we could have been redirected // away from GitHub (via follow_location_to_data()), and we don't // want to give our credentials to third-party services. - if req.uri().authority().is_none() { + let authority = req.uri().authority(); + if authority.is_none() || authority == self.base_uri.authority() { if let Some(auth_header) = &*self.auth_header { req.headers_mut().append(AUTHORIZATION, auth_header.clone()); }