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.

Sign up for GitHub

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

Jump to bottom

added serde to grant types #162

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

added serde to grant types #162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
550f004
added serde to grant types
Wicpar Apr 11, 2023
ccc912a
added serde to chrono dependency
Wicpar Apr 11, 2023
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
2 changes: 1 addition & 1 deletion oxide-auth/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ autoexamples = false

[dependencies]
base64 = "0.13"
chrono = { version = "0.4", default-features = false, features = ["clock"] }
chrono = { version = "0.4", default-features = false, features = ["clock", "serde"] }
hmac = "0.12.0"
once_cell = "1.3.1"
serde = { version = "1.0", features = ["derive"] }
Expand Down
8 changes: 5 additions & 3 deletions oxide-auth/src/primitives/grant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::collections::HashMap;
use std::collections::hash_map::Iter;
use std::rc::Rc;
use std::sync::Arc;
use serde_derive::{Deserialize, Serialize};

/// Provides a name registry for extensions.
pub trait GrantExtension {
Expand All @@ -24,7 +25,8 @@ pub trait GrantExtension {
///
/// Some extensions have semantics where the presence alone is the stored data, so storing data
/// is optional and storing no data is distinct from not attaching any extension instance at all.
#[derive(Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(tag = "type")]
Comment on lines +28 to +29
Copy link
Owner

Choose a reason for hiding this comment

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

For this type, there's a security concern when serializing Private since this may leak information rather unwittingly. I suppose it's okay overall but may be a paper cut when using the library. Just as a smoke-test, are there any alternatives that mitigate or avoid the risk?

Copy link
Author

Choose a reason for hiding this comment

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

Indeed that is an issue. As a quick fix we can ignore it during serialization, but if we rely on serialization to store them on the server like with sqlx json it will work improperly and since that is my usecase for the serialization i kept it in.
Ideally it would serialize, but there would be a function to sanitize it before sending it to a user.

It's a double edged sword but i would err on the safer side with a serde skip indeed

pub enum Value {
/// An extension that the token owner is allowed to read and interpret.
Public(Option<String>),
Expand All @@ -39,7 +41,7 @@ pub enum Value {
///
/// This also serves as a clean interface for both frontend and backend to reliably and
/// conveniently manipulate or query the stored data sets.
#[derive(Clone, Default, Debug, PartialEq, Eq)]
#[derive(Clone, Default, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub struct Extensions {
extensions: HashMap<String, Value>,
}
Expand All @@ -48,7 +50,7 @@ pub struct Extensions {
///
/// This can be stored in a database without worrying about lifetimes or shared across thread
/// boundaries. A reference to this can be converted to a purely referential `GrantRef`.
#[derive(Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub struct Grant {
/// Identifies the owner of the resource.
pub owner_id: String,
Expand Down