Skip to content

Commit 388afdc

Browse files
kanikabansal-juspaykanikabansal-juspay
committed
fix: address comments
1 parent 994c64e commit 388afdc

File tree

8 files changed

+129
-51
lines changed

8 files changed

+129
-51
lines changed

‎config/deployments/production.toml‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -208,19 +208,6 @@ base_url = "https://live.hyperswitch.io"
208208
force_two_factor_auth = true
209209
force_cookies = false
210210

211-
[oidc.key.k1]
212-
kid = ""
213-
private_key = ""
214-
215-
[oidc.key.k2]
216-
kid = ""
217-
private_key = ""
218-
219-
[oidc.client.c1]
220-
client_id = ""
221-
client_secret = ""
222-
redirect_uri = ""
223-
224211
[frm]
225212
enabled = false
226213

‎crates/api_models/src/oidc.rs‎

Lines changed: 52 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use std::str::FromStr;
22

33
use common_utils::{events::ApiEventMetric, pii};
4+
use masking::Secret;
45
use serde::{Deserialize, Serialize};
56
use utoipa::ToSchema;
67

@@ -23,56 +24,58 @@ const CLAIMS_SUPPORTED: &[Claim] = &[
2324
];
2425

2526
/// OIDC Response Type
26-
#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)]
27+
#[derive(Copy, Clone, Debug, serde::Serialize, serde::Deserialize)]
2728
#[serde(rename_all = "snake_case")]
2829
pub enum ResponseType {
2930
Code,
3031
}
3132

3233
/// OIDC Response Mode
33-
#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)]
34+
#[derive(Copy, Clone, Debug, serde::Serialize, serde::Deserialize)]
3435
#[serde(rename_all = "snake_case")]
3536
pub enum ResponseMode {
3637
Query,
3738
}
3839

3940
/// OIDC Subject Type
40-
#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)]
41+
#[derive(Copy, Clone, Debug, serde::Serialize, serde::Deserialize)]
4142
#[serde(rename_all = "snake_case")]
4243
pub enum SubjectType {
4344
Public,
4445
}
4546

4647
/// OIDC Signing Algorithm
47-
#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)]
48+
#[derive(Copy, Clone, Debug, serde::Serialize, serde::Deserialize)]
4849
pub enum SigningAlgorithm {
4950
#[serde(rename = "RS256")]
5051
Rs256,
5152
}
5253

5354
/// JWK Key Type
54-
#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)]
55+
#[derive(Copy, Clone, Debug, serde::Serialize, serde::Deserialize)]
5556
pub enum KeyType {
5657
#[serde(rename = "RSA")]
5758
Rsa,
5859
}
5960

6061
/// JWK Key Use
61-
#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)]
62+
#[derive(Copy, Clone, Debug, serde::Serialize, serde::Deserialize)]
6263
#[serde(rename_all = "snake_case")]
6364
pub enum KeyUse {
6465
Sig,
6566
}
6667

6768
/// OIDC Grant Type
68-
#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)]
69+
#[derive(Copy, Clone, Debug, serde::Serialize, serde::Deserialize)]
6970
#[serde(rename_all = "snake_case")]
7071
pub enum GrantType {
7172
AuthorizationCode,
7273
}
7374

7475
/// OIDC Scope
75-
#[derive(Clone, Debug, PartialEq, serde::Serialize, serde::Deserialize, strum::EnumString)]
76+
#[derive(
77+
Copy, Clone, Debug, PartialEq, serde::Serialize, serde::Deserialize, strum::EnumString,
78+
)]
7679
#[serde(rename_all = "snake_case")]
7780
#[strum(serialize_all = "snake_case")]
7881
pub enum Scope {
@@ -81,14 +84,14 @@ pub enum Scope {
8184
}
8285

8386
/// OIDC Token Endpoint Authentication Method
84-
#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)]
87+
#[derive(Copy, Clone, Debug, serde::Serialize, serde::Deserialize)]
8588
#[serde(rename_all = "snake_case")]
8689
pub enum TokenAuthMethod {
8790
ClientSecretBasic,
8891
}
8992

9093
/// OIDC Claim
91-
#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)]
94+
#[derive(Copy, Clone, Debug, serde::Serialize, serde::Deserialize)]
9295
#[serde(rename_all = "snake_case")]
9396
pub enum Claim {
9497
Aud,
@@ -101,7 +104,7 @@ pub enum Claim {
101104
}
102105

103106
/// OIDC Authorization Error as per RFC 6749
104-
#[derive(Clone, Debug, serde::Serialize, serde::Deserialize, strum::Display)]
107+
#[derive(Copy, Clone, Debug, serde::Serialize, serde::Deserialize, strum::Display)]
105108
#[serde(rename_all = "snake_case")]
106109
#[strum(serialize_all = "snake_case")]
107110
pub enum OidcAuthorizationError {
@@ -115,7 +118,7 @@ pub enum OidcAuthorizationError {
115118
}
116119

117120
/// OIDC Token Error as per RFC 6749
118-
#[derive(Clone, Debug, serde::Serialize, serde::Deserialize, strum::Display)]
121+
#[derive(Copy, Clone, Debug, serde::Serialize, serde::Deserialize, strum::Display)]
119122
#[serde(rename_all = "snake_case")]
120123
#[strum(serialize_all = "snake_case")]
121124
pub enum OidcTokenError {
@@ -320,7 +323,8 @@ pub struct OidcTokenRequest {
320323
#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)]
321324
pub struct OidcTokenResponse {
322325
/// ID Token value associated with the authenticated session
323-
pub id_token: String,
326+
#[schema(value_type = String)]
327+
pub id_token: Secret<String>,
324328

325329
/// OAuth 2.0 Token Type value
326330
#[schema(example = "Bearer")]
@@ -331,9 +335,38 @@ pub struct OidcTokenResponse {
331335
pub expires_in: u64,
332336
}
333337

334-
impl ApiEventMetric for OidcDiscoveryResponse {}
335-
impl ApiEventMetric for JwksResponse {}
336-
impl ApiEventMetric for OidcAuthorizeQuery {}
337-
impl ApiEventMetric for AuthCodeData {}
338-
impl ApiEventMetric for OidcTokenRequest {}
339-
impl ApiEventMetric for OidcTokenResponse {}
338+
impl ApiEventMetric for OidcDiscoveryResponse {
339+
fn get_api_event_type(&self) -> Option<common_utils::events::ApiEventsType> {
340+
Some(common_utils::events::ApiEventsType::Oidc)
341+
}
342+
}
343+
344+
impl ApiEventMetric for JwksResponse {
345+
fn get_api_event_type(&self) -> Option<common_utils::events::ApiEventsType> {
346+
Some(common_utils::events::ApiEventsType::Oidc)
347+
}
348+
}
349+
350+
impl ApiEventMetric for OidcAuthorizeQuery {
351+
fn get_api_event_type(&self) -> Option<common_utils::events::ApiEventsType> {
352+
Some(common_utils::events::ApiEventsType::Oidc)
353+
}
354+
}
355+
356+
impl ApiEventMetric for AuthCodeData {
357+
fn get_api_event_type(&self) -> Option<common_utils::events::ApiEventsType> {
358+
Some(common_utils::events::ApiEventsType::Oidc)
359+
}
360+
}
361+
362+
impl ApiEventMetric for OidcTokenRequest {
363+
fn get_api_event_type(&self) -> Option<common_utils::events::ApiEventsType> {
364+
Some(common_utils::events::ApiEventsType::Oidc)
365+
}
366+
}
367+
368+
impl ApiEventMetric for OidcTokenResponse {
369+
fn get_api_event_type(&self) -> Option<common_utils::events::ApiEventsType> {
370+
Some(common_utils::events::ApiEventsType::Oidc)
371+
}
372+
}

‎crates/common_utils/src/events.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ pub enum ApiEventsType {
140140
},
141141
ThreeDsDecisionRule,
142142
Chat,
143+
Oidc,
143144
}
144145

145146
impl ApiEventMetric for serde_json::Value {}

‎crates/hyperswitch_domain_models/src/errors/api_error_response.rs‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use http::StatusCode;
77

88
use crate::router_data;
99

10-
#[derive(Clone, Debug, serde::Serialize)]
10+
#[derive(Copy, Clone, Debug, serde::Serialize)]
1111
#[serde(rename_all = "snake_case")]
1212
pub enum ErrorType {
1313
InvalidRequestError,
@@ -227,11 +227,6 @@ pub enum ApiErrorResponse {
227227
message = "Access forbidden, invalid JWT token was used"
228228
)]
229229
InvalidJwtToken,
230-
#[error(
231-
error_type = ErrorType::InvalidRequestError, code = "IR_17_1",
232-
message = "Access forbidden, invalid Basic authentication credentials"
233-
)]
234-
InvalidBasicAuth,
235230
#[error(
236231
error_type = ErrorType::InvalidRequestError, code = "IR_18",
237232
message = "{message}",
@@ -315,12 +310,17 @@ pub enum ApiErrorResponse {
315310
ConnectedAccountAuthNotSupported,
316311
#[error(error_type = ErrorType::InvalidRequestError, code = "IR_50", message = "Invalid connected account operation")]
317312
InvalidConnectedOperation,
318-
#[error(error_type = ErrorType::InvalidRequestError, code = "IR_48", message = "{error}: {description}")]
313+
#[error(
314+
error_type = ErrorType::InvalidRequestError, code = "IR_51",
315+
message = "Access forbidden, invalid Basic authentication credentials"
316+
)]
317+
InvalidBasicAuth,
318+
#[error(error_type = ErrorType::InvalidRequestError, code = "IR_52", message = "{error}: {description}")]
319319
OidcAuthorizationError {
320320
error: OidcAuthorizationError,
321321
description: String,
322322
},
323-
#[error(error_type = ErrorType::InvalidRequestError, code = "IR_49", message = "{error}: {description}")]
323+
#[error(error_type = ErrorType::InvalidRequestError, code = "IR_53", message = "{error}: {description}")]
324324
OidcTokenError {
325325
error: OidcTokenError,
326326
description: String,
@@ -616,7 +616,7 @@ impl ErrorSwitch<api_models::errors::types::ApiErrorResponse> for ApiErrorRespon
616616
AER::BadRequest(ApiError::new("IR", 16, message.to_string(), None))
617617
}
618618
Self::InvalidJwtToken => AER::Unauthorized(ApiError::new("IR", 17, "Access forbidden, invalid JWT token was used", None)),
619-
Self::InvalidBasicAuth => AER::Unauthorized(ApiError::new("IR", 171, "Access forbidden, invalid Basic authentication credentials", None)),
619+
Self::InvalidBasicAuth => AER::Unauthorized(ApiError::new("IR", 51, "Access forbidden, invalid Basic authentication credentials", None)),
620620
Self::GenericUnauthorized { message } => {
621621
AER::Unauthorized(ApiError::new("IR", 18, message.to_string(), None))
622622
},
@@ -750,10 +750,10 @@ impl ErrorSwitch<api_models::errors::types::ApiErrorResponse> for ApiErrorRespon
750750
AER::BadRequest(ApiError::new("CE", 9, format!("Subscription operation: {operation} failed with connector"), None))
751751
}
752752
Self::OidcAuthorizationError { error, description } => {
753-
AER::BadRequest(ApiError::new("IR", 48, format!("{}: {}", error, description), None))
753+
AER::BadRequest(ApiError::new("IR", 52, format!("{}: {}", error, description), None))
754754
}
755755
Self::OidcTokenError { error, description } => {
756-
AER::BadRequest(ApiError::new("IR", 49, format!("{}: {}", error, description), None))
756+
AER::BadRequest(ApiError::new("IR", 53, format!("{}: {}", error, description), None))
757757
}
758758
}
759759
}

‎crates/router/src/configs/secrets_transformers.rs‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,50 @@ impl SecretsHandler for settings::NetworkTokenizationService {
351351
}
352352
}
353353

354+
#[async_trait::async_trait]
355+
impl SecretsHandler for settings::OidcSettings {
356+
async fn convert_to_raw_secret(
357+
value: SecretStateContainer<Self, SecuredSecret>,
358+
secret_management_client: &dyn SecretManagementInterface,
359+
) -> CustomResult<SecretStateContainer<Self, RawSecret>, SecretsManagementError> {
360+
let oidc_settings = value.get_inner();
361+
362+
let mut decrypted_keys = std::collections::HashMap::new();
363+
for (key_id, oidc_key) in &oidc_settings.key {
364+
let private_key = secret_management_client
365+
.get_secret(oidc_key.private_key.clone())
366+
.await?;
367+
decrypted_keys.insert(
368+
key_id.clone(),
369+
settings::OidcKey {
370+
kid: oidc_key.kid.clone(),
371+
private_key,
372+
},
373+
);
374+
}
375+
376+
let mut decrypted_clients = std::collections::HashMap::new();
377+
for (client_key, oidc_client) in &oidc_settings.client {
378+
let client_secret = secret_management_client
379+
.get_secret(oidc_client.client_secret.clone())
380+
.await?;
381+
decrypted_clients.insert(
382+
client_key.clone(),
383+
settings::OidcClient {
384+
client_id: oidc_client.client_id.clone(),
385+
client_secret,
386+
redirect_uri: oidc_client.redirect_uri.clone(),
387+
},
388+
);
389+
}
390+
391+
Ok(value.transition_state(|_| Self {
392+
key: decrypted_keys,
393+
client: decrypted_clients,
394+
}))
395+
}
396+
}
397+
354398
/// # Panics
355399
///
356400
/// Will panic even if kms decryption fails for at least one field
@@ -487,6 +531,11 @@ pub(crate) async fn fetch_raw_secrets(
487531
.await
488532
.expect("Failed to decrypt superposition config");
489533

534+
#[allow(clippy::expect_used)]
535+
let oidc = settings::OidcSettings::convert_to_raw_secret(conf.oidc, secret_management_client)
536+
.await
537+
.expect("Failed to decrypt oidc configs");
538+
490539
Settings {
491540
server: conf.server,
492541
chat,
@@ -524,7 +573,7 @@ pub(crate) async fn fetch_raw_secrets(
524573
#[cfg(feature = "email")]
525574
email: conf.email,
526575
user: conf.user,
527-
oidc: conf.oidc,
576+
oidc,
528577
mandates: conf.mandates,
529578
zero_mandates: conf.zero_mandates,
530579
network_transaction_id_supported_connectors: conf

‎crates/router/src/configs/settings.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ pub struct Settings<S: SecretState> {
107107
#[cfg(feature = "email")]
108108
pub email: EmailSettings,
109109
pub user: UserSettings,
110-
pub oidc: OidcSettings,
110+
pub oidc: SecretStateContainer<OidcSettings, S>,
111111
pub crm: CrmManagerConfig,
112112
pub cors: CorsSettings,
113113
pub mandates: Mandates,

‎crates/router/src/services/authentication.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,7 @@ impl BasicAuthProvider for OidcAuthProvider {
442442
let client = session
443443
.conf
444444
.oidc
445+
.get_inner()
445446
.get_client(identifier)
446447
.ok_or(errors::ApiErrorResponse::InvalidBasicAuth)?;
447448

‎crates/router/src/services/oidc_provider.rs‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ static CACHED_JWKS: OnceCell<JwksResponse> = OnceCell::new();
4343
/// Build JWKS response with public keys (all keys for token validation)
4444
pub async fn get_jwks(state: SessionState) -> RouterResponse<JwksResponse> {
4545
let jwks_response = CACHED_JWKS.get_or_try_init(|| {
46-
let oidc_keys = state.conf.oidc.get_all_keys();
46+
let oidc_keys = state.conf.oidc.get_inner().get_all_keys();
4747
let mut keys = Vec::new();
4848

4949
for key_config in oidc_keys {
@@ -97,6 +97,7 @@ pub fn validate_authorize_params(
9797
let client = state
9898
.conf
9999
.oidc
100+
.get_inner()
100101
.get_client(&payload.client_id)
101102
.ok_or_else(|| {
102103
report!(ApiErrorResponse::OidcAuthorizationError {
@@ -248,6 +249,7 @@ fn validate_token_request(
248249
let registered_client = state
249250
.conf
250251
.oidc
252+
.get_inner()
251253
.get_client(request_client_id)
252254
.ok_or_else(|| {
253255
report!(ApiErrorResponse::OidcTokenError {
@@ -335,10 +337,15 @@ async fn generate_id_token(
335337
state: &SessionState,
336338
auth_code_data: &AuthCodeData,
337339
) -> error_stack::Result<String, ApiErrorResponse> {
338-
let signing_key = state.conf.oidc.get_signing_key().ok_or_else(|| {
339-
report!(ApiErrorResponse::InternalServerError)
340-
.attach_printable("No signing key configured for OIDC")
341-
})?;
340+
let signing_key = state
341+
.conf
342+
.oidc
343+
.get_inner()
344+
.get_signing_key()
345+
.ok_or_else(|| {
346+
report!(ApiErrorResponse::InternalServerError)
347+
.attach_printable("No signing key configured for OIDC")
348+
})?;
342349

343350
let now = SystemTime::now()
344351
.duration_since(UNIX_EPOCH)
@@ -407,7 +414,7 @@ pub async fn process_token_request(
407414
let id_token = generate_id_token(&state, &auth_code_data).await?;
408415

409416
Ok(ApplicationResponse::Json(OidcTokenResponse {
410-
id_token,
417+
id_token: id_token.into(),
411418
token_type: TOKEN_TYPE_BEARER.to_string(),
412419
expires_in: ID_TOKEN_TTL_IN_SECS,
413420
}))

0 commit comments

Comments
 (0)