Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: flatten (de)serialization of custom user claims #1159

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions examples/demo/src/controllers/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ async fn register(
let jwt_secret = ctx.config.get_jwt_config()?;

let token = user
.generate_jwt(&jwt_secret.secret, &jwt_secret.expiration)
.generate_jwt(&jwt_secret.secret, jwt_secret.expiration)
.or_else(|_| unauthorized("unauthorized!"))?;
format::json(UserSession::new(&user, &token))
}
Expand Down Expand Up @@ -130,7 +130,7 @@ async fn login(State(ctx): State<AppContext>, Json(params): Json<LoginParams>) -
let jwt_secret = ctx.config.get_jwt_config()?;

let token = user
.generate_jwt(&jwt_secret.secret, &jwt_secret.expiration)
.generate_jwt(&jwt_secret.secret, jwt_secret.expiration)
.or_else(|_| unauthorized("unauthorized!"))?;

format::json(UserSession::new(&user, &token))
Expand Down
12 changes: 5 additions & 7 deletions examples/demo/src/models/users.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use async_trait::async_trait;
use chrono::offset::Local;
use loco_rs::{auth::jwt, hash, prelude::*};
use serde::{Deserialize, Serialize};
use serde_json::json;
use serde_json::Map;
use uuid::Uuid;

pub use super::_entities::users::{self, ActiveModel, Entity, Model};
Expand Down Expand Up @@ -216,12 +216,10 @@ impl super::_entities::users::Model {
/// # Errors
///
/// when could not convert user claims to jwt token
pub fn generate_jwt(&self, secret: &str, expiration: &u64) -> ModelResult<String> {
Ok(jwt::JWT::new(secret).generate_token(
expiration,
self.pid.to_string(),
Some(json!({"Roll": "Administrator"})),
)?)
pub fn generate_jwt(&self, secret: &str, expiration: u64) -> ModelResult<String> {
let mut claims = Map::new();
claims.insert("Roll".to_string(), "Administrator".into());
kaplanelad marked this conversation as resolved.
Show resolved Hide resolved
Ok(jwt::JWT::new(secret).generate_token(expiration, self.pid.to_string(), claims)?)
}
}

Expand Down
117 changes: 102 additions & 15 deletions src/auth/jwt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,28 @@
//!
//! This module provides functionality for working with JSON Web Tokens (JWTs)
//! and password hashing.

use jsonwebtoken::{
decode, encode, errors::Result as JWTResult, get_current_timestamp, Algorithm, DecodingKey,
EncodingKey, Header, TokenData, Validation,
};
use serde::{Deserialize, Serialize};
use serde_json::Value;
use serde_json::{Map, Value};

/// Represents the default JWT algorithm used by the [`JWT`] struct.
const JWT_ALGORITHM: Algorithm = Algorithm::HS512;

/// Represents the claims associated with a user JWT.
#[derive(Debug, Serialize, Deserialize)]
#[derive(Debug, Serialize, Deserialize, Eq, PartialEq)]
jorgehermo9 marked this conversation as resolved.
Show resolved Hide resolved
jorgehermo9 marked this conversation as resolved.
Show resolved Hide resolved
pub struct UserClaims {
pub pid: String,
exp: u64,
pub claims: Option<Value>,
#[serde(default, flatten)]
// TODO: should we wrap this in an Option? `Option<Map<String, Value>>`
// so we can use `auth::jwt::JWT::new("PqRwLF2rhHe8J22oBeHy").generate_token(&604800, "PID".to_string(), None);
// TODO: serde_json::Map or std::collections::HashMap?
// TODO: is it ok to use a generic Map<String, Value> here? Or should we let the user specify their desired typed claim and
jorgehermo9 marked this conversation as resolved.
Show resolved Hide resolved
// use generics to serialize/deserialize it?
pub claims: Map<String, Value>,
jorgehermo9 marked this conversation as resolved.
Show resolved Hide resolved
}

/// Represents the JWT configuration and operations.
Expand Down Expand Up @@ -61,17 +66,18 @@ impl JWT {
///
/// # Example
/// ```rust
/// use serde_json::Map;
jorgehermo9 marked this conversation as resolved.
Show resolved Hide resolved
/// use loco_rs::auth;
///
/// auth::jwt::JWT::new("PqRwLF2rhHe8J22oBeHy").generate_token(&604800, "PID".to_string(), None);
/// auth::jwt::JWT::new("PqRwLF2rhHe8J22oBeHy").generate_token(604800, "PID".to_string(), Map::new());
/// ```
pub fn generate_token(
&self,
expiration: &u64,
expiration: u64,
jorgehermo9 marked this conversation as resolved.
Show resolved Hide resolved
pid: String,
claims: Option<Value>,
claims: Map<String, Value>,
) -> JWTResult<String> {
let exp = get_current_timestamp().saturating_add(*expiration);
let exp = get_current_timestamp().saturating_add(expiration);

let claims = UserClaims { pid, exp, claims };

Expand Down Expand Up @@ -119,18 +125,27 @@ mod tests {
use super::*;

#[rstest]
#[case("valid token", 60, None)]
#[case("token expired", 1, None)]
#[case("valid token and custom claims", 60, Some(json!({})))]
#[tokio::test]
async fn can_generate_token(
#[case("valid token", 60, json!({}))]
#[case("token expired", 1, json!({}))]
#[case("valid token and custom string claims", 60, json!({ "custom": "claim",}))]
jorgehermo9 marked this conversation as resolved.
Show resolved Hide resolved
#[case("valid token and custom boolean claims",60, json!({ "custom": true,}))]
#[case("valid token and custom number claims",60, json!({ "custom": 123,}))]
#[case("valid token and custom nested claims",60, json!({ "level1": { "level2": { "level3": "claim" } } }))]
#[case("valid token and custom array claims",60, json!({ "array": [1, 2, 3] }))]
#[case("valid token and custom nested array claims",60, json!({ "level1": { "level2": { "level3": [1, 2, 3] } } }))]
fn can_generate_token(
#[case] test_name: &str,
#[case] expiration: u64,
#[case] claims: Option<Value>,
#[case] json_claims: Value,
) {
let claims = json_claims
.as_object()
.expect("case input claims must be an object")
.clone();
let jwt = JWT::new("PqRwLF2rhHe8J22oBeHy");

let token = jwt
.generate_token(&expiration, "pid".to_string(), claims)
.generate_token(expiration, "pid".to_string(), claims)
.unwrap();

std::thread::sleep(std::time::Duration::from_secs(3));
Expand All @@ -140,4 +155,76 @@ mod tests {
assert_debug_snapshot!(test_name, jwt.validate(&token));
});
}

#[rstest]
#[case::without_custom_claims(json!({}))]
#[case::with_custom_string_claims(json!({ "custom": "claim",}))]
#[case::with_custom_boolean_claims(json!({ "custom": true,}))]
#[case::with_custom_number_claims(json!({ "custom": 123,}))]
#[case::with_custom_nested_claims(json!({ "level1": { "level2": { "level3": "claim" } } }))]
#[case::with_custom_array_claims(json!({ "array": [1, 2, 3] }))]
#[case::with_custom_nested_array_claims(json!({ "level1": { "level2": { "level3": [1, 2, 3] } } }))]
// we use `Value` to reduce code duplicity in the case inputs
fn serialize_user_claims(#[case] json_claims: Value) {
let claims = json_claims
.as_object()
.expect("case input claims must be an object")
.clone();
let input_user_claims = UserClaims {
pid: "pid".to_string(),
exp: 60,
claims: claims.clone(),
};

let mut expected_claim = Map::new();
expected_claim.insert("pid".to_string(), "pid".into());
expected_claim.insert("exp".to_string(), 60.into());
// we add the claims in a flattened way
expected_claim.extend(claims);
let expected_value = Value::from(expected_claim);

// We check between `Value` instead of `String` to avoid key ordering issues when serializing.
// It is because `expected_value` has all the keys in alphabetical order, as the `Value` serialization ensures that.
// But when serializing `input_user_claims`, first the `pid` and `exp` fields are serialized (in that order),
// and then the claims are serialized in alfabetic order. So, the resulting JSON string from the `input_user_claims` serialization
// may have the `pid` and `exp` fields unordered which differs from the `Value` serialization.
assert_eq!(
expected_value,
serde_json::to_value(&input_user_claims).unwrap()
);
}

#[rstest]
#[case::without_custom_claims(json!({}))]
#[case::with_custom_string_claims(json!({ "custom": "claim",}))]
#[case::with_custom_boolean_claims(json!({ "custom": true,}))]
#[case::with_custom_number_claims(json!({ "custom": 123,}))]
#[case::with_custom_nested_claims(json!({ "level1": { "level2": { "level3": "claim" } } }))]
#[case::with_custom_array_claims(json!({ "array": [1, 2, 3] }))]
#[case::with_custom_nested_array_claims(json!({ "level1": { "level2": { "level3": [1, 2, 3] } } }))]
// we use `Value` to reduce code duplicity in the case inputs
fn deserialize_user_claims(#[case] json_claims: Value) {
let claims = json_claims
.as_object()
.expect("case input claims must be an object")
.clone();

let mut input_claims = Map::new();
input_claims.insert("pid".to_string(), "pid".into());
input_claims.insert("exp".to_string(), 60.into());
// we add the claims in a flattened way
input_claims.extend(claims.clone());
let input_json = Value::from(input_claims).to_string();

let expected_user_claims = UserClaims {
pid: "pid".to_string(),
exp: 60,
claims,
};

assert_eq!(
expected_user_claims,
serde_json::from_str(&input_json).unwrap()
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
---
source: src/auth/jwt.rs
expression: jwt.validate(&token)
---
Ok(
TokenData {
header: Header {
typ: Some(
"JWT",
),
alg: HS512,
cty: None,
jku: None,
jwk: None,
kid: None,
x5u: None,
x5c: None,
x5t: None,
x5t_s256: None,
},
claims: UserClaims {
pid: "pid",
exp: EXP,
claims: {
"array": Array [
Number(1),
Number(2),
Number(3),
],
},
},
},
)
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
---
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this snapshot file's diff is marked as "renamed file", but actually I deleted the old valid token and custom claims snapshot, as it was unreferenced, and added a new valid token and custom boolean claims snapshot, such as the other from this PR.

source: src/auth/jwt.rs
assertion_line: 133
expression: jwt.validate(&token)
---
Ok(
Expand All @@ -22,9 +21,9 @@ Ok(
claims: UserClaims {
pid: "pid",
exp: EXP,
claims: Some(
Object {},
),
claims: {
"custom": Bool(true),
},
},
},
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
---
source: src/auth/jwt.rs
expression: jwt.validate(&token)
---
Ok(
TokenData {
header: Header {
typ: Some(
"JWT",
),
alg: HS512,
cty: None,
jku: None,
jwk: None,
kid: None,
x5u: None,
x5c: None,
x5t: None,
x5t_s256: None,
},
claims: UserClaims {
pid: "pid",
exp: EXP,
claims: {
"level1": Object {
"level2": Object {
"level3": Array [
Number(1),
Number(2),
Number(3),
],
},
},
},
},
},
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
---
source: src/auth/jwt.rs
expression: jwt.validate(&token)
---
Ok(
TokenData {
header: Header {
typ: Some(
"JWT",
),
alg: HS512,
cty: None,
jku: None,
jwk: None,
kid: None,
x5u: None,
x5c: None,
x5t: None,
x5t_s256: None,
},
claims: UserClaims {
pid: "pid",
exp: EXP,
claims: {
"level1": Object {
"level2": Object {
"level3": String("claim"),
},
},
},
},
},
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
---
source: src/auth/jwt.rs
assertion_line: 155
expression: jwt.validate(&token)
---
Ok(
TokenData {
header: Header {
typ: Some(
"JWT",
),
alg: HS512,
cty: None,
jku: None,
jwk: None,
kid: None,
x5u: None,
x5c: None,
x5t: None,
x5t_s256: None,
},
claims: UserClaims {
pid: "pid",
exp: EXP,
claims: {
"custom": Number(123),
},
},
},
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
source: src/auth/jwt.rs
expression: jwt.validate(&token)
---
Ok(
TokenData {
header: Header {
typ: Some(
"JWT",
),
alg: HS512,
cty: None,
jku: None,
jwk: None,
kid: None,
x5u: None,
x5c: None,
x5t: None,
x5t_s256: None,
},
claims: UserClaims {
pid: "pid",
exp: EXP,
claims: {
"custom": String("claim"),
},
},
},
)
Loading
Loading