oauth flow /authorize and /token endpoints by Sanket-Singhvi · Pull Request #115 · patr-cloud/patr

Sanket-Singhvi · 2025-12-06T12:04:10Z

No description provided.

rakshith-ravi · 2025-12-06T17:08:51Z

Cargo.toml

 ts-rs                          = { default-features = false, version = "11" }
 typed-builder                  = { default-features = false, version = "0.23" }
 url                            = { default-features = false, version = "2" }
+urlencoding                    = { default-features = false, version = "2" }


serde_qs is already there. Can't we use that?

rakshith-ravi · 2025-12-06T17:09:42Z

models/src/error.rs

+			Self::InvalidResponseType => {
+				"The response type provided in the OAuth2.1 authorize request is not supported"
+			}
+			Self::InvalidGrantType => {
+				"The grant type provided in the OAuth2.1 token request is not supported"
+			}
+			Self::InvalidAuthorizationCode => {
+				"The authorization code provided in the OAuth2.1 token request is invalid"
+			}


OAuthInvalidResponseType

OAuthInvalidGrantType

OAuthInvalidAuthorizationCode

rakshith-ravi · 2025-12-06T17:10:02Z

models/src/error.rs

@@ -137,6 +144,8 @@ impl ErrorType {
 			Self::RoleInUse => StatusCode::CONFLICT,
 			Self::RunnerAlreadyConnected => StatusCode::CONFLICT,
 			Self::InvalidRunnerMode => StatusCode::FORBIDDEN,
+			Self::InvalidGrantType => StatusCode::BAD_REQUEST,
+			Self::InvalidAuthorizationCode => StatusCode::BAD_REQUEST,


Are these validated with the OAuth spec?

rakshith-ravi · 2025-12-06T17:10:22Z

models/src/api/auth/oauth/authorize.rs

 	}
 );
+
+macros::declare_api_endpoint!(


Separate file

rakshith-ravi · 2025-12-06T17:11:37Z

api/src/routes/api.patr.cloud/auth/oauth/authorize.rs

+		.into_result()
+}
+
+pub async fn authorize_post(


Separate file

rakshith-ravi · 2025-12-06T17:14:17Z

api/src/routes/api.patr.cloud/auth/oauth/authorize.rs

+	let success = argon2::Argon2::new_with_secret(
+		config.password_pepper.as_ref(),
+		Algorithm::Argon2id,
+		Version::V0x13,
+		constants::HASHING_PARAMS,
+	)
+	.inspect_err(|err| {
+		error!("Error creating Argon2: `{}`", err);
+	})
+	.map_err(ErrorType::server_error)?
+	.verify_password(
+		password.as_bytes(),
+		&PasswordHash::new(&user_data.password).map_err(ErrorType::server_error)?,
+	)
+	.inspect_err(|err| {
+		info!("Error verifying password: `{}`", err);
+	})
+	.is_ok();


What's the point of this? The whole reason of OAuth is to not give the password to the client

rakshith-ravi · 2025-12-06T17:14:38Z

api/src/routes/api.patr.cloud/auth/oauth/authorize.rs

+
+	// Store the authorization code and its metadata in the redis with an expiration
+	// time.
+	let key = format!("auth_code:{}", authorization_code);


This should be in the redis::keys module

rakshith-ravi · 2025-12-06T17:15:16Z

api/src/routes/api.patr.cloud/auth/oauth/authorize.rs

+		})
+		.map_err(ErrorType::server_error)?;
+
+	let redirect_uri_str = redirect_uri.as_deref().unwrap_or("");


Again, this should come from the DB ideally

rakshith-ravi · 2025-12-06T17:16:21Z

api/src/routes/api.patr.cloud/auth/oauth/token.rs

+			}
+		} else {
+			if code_verifier != auth_code_data.code_challenge {
+				return Err(ErrorType::InvalidGrantType);


Idk if this is the right error here

rakshith-ravi · 2025-12-06T17:16:33Z

api/src/routes/api.patr.cloud/auth/oauth/token.rs

+		}
+	} else if grant_type == OAuthTokenGrantType::RefreshToken {
+		// Handle refresh token grant type
+		let key = format!("refresh_token:{}", refresh_token.unwrap());


keys in a separate module

…l encoding

rakshith-ravi · 2025-12-10T12:08:00Z

api/src/routes/api.patr.cloud/auth/oauth/authorize_post.rs

+		"Generated authorization code `{}` for client_id `{}`",
+		authorization_code, client_id
+	);
+	let metadata = json!({


Make a struct out of this

rakshith-ravi · 2025-12-10T12:08:10Z

api/src/routes/api.patr.cloud/auth/oauth/login.rs

+use crate::prelude::*;
+
+#[derive(Serialize)]
+struct RedirectQueryParams {


Document what this is

rakshith-ravi · 2025-12-10T12:08:26Z

api/src/routes/api.patr.cloud/auth/oauth/login.rs

+		ErrorType::server_error(e)
+	})?;
+
+	let redirect_url = if let Some(uri) = redirect_uri {


This should be validated with the DB

rakshith-ravi · 2025-12-10T12:08:55Z

api/src/routes/api.patr.cloud/auth/oauth/token.rs

 use crate::prelude::*;

+#[derive(Debug, Clone, Serialize, Deserialize)]
+struct AuthCodeData {


We already have this na?

rakshith-ravi · 2025-12-10T12:09:24Z

api/Cargo.toml

 tracing-opentelemetry          = { workspace = true, features = ["default"] }
 tracing-subscriber             = { workspace = true, features = ["default"] }
 typed-builder                  = { workspace = true, features = [] }
+urlencoding                    = { workspace = true, features = [] }


rakshith-ravi · 2025-12-10T12:09:44Z

models/src/api/auth/oauth/login.rs

+   /// - PKCE challenge (if any)
+   ///
+   /// Then backend generates an authorization code and redirects.
+   OAuthAuthorizePost,


Better name

rakshith-ravi · 2025-12-10T12:13:45Z

models/src/api/auth/oauth/login.rs

+   POST "/auth/oauth/authorize",
+   request = {
+   /// The client ID of the OAuth client
+   pub client_id: String,


Client secret needs to be there

rakshith-ravi · 2025-12-10T12:18:27Z

api/src/redis/keys.rs

 	format!("registryBlobUploadPart:{}", session_id)
 }
+/// The prefix used for authorization_code for oauth
+pub fn oauth_authorization_code_prefix(authorization_code: &str) -> String {


Strong types internally

…e database - Update `authorize` endpoint to validate client secret and redirect URI.

oauth flow /authorize and /token endpoints

90c6ea9

rakshith-ravi requested changes Dec 6, 2025

View reviewed changes

redis keys added, made separate files for routes used serde_qs for ur…

ff2939c

…l encoding

rakshith-ravi requested changes Dec 10, 2025

View reviewed changes

Sanket-Singhvi added 2 commits December 16, 2025 18:40

-Add initialize_client function to set up OAuth client tables in th…

2819d3f

…e database - Update `authorize` endpoint to validate client secret and redirect URI.

Add OAuth login endpoint model

5ad290f

Conversation

Sanket-Singhvi commented Dec 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants