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

HMAC added #32

Merged
Merged

Conversation

Aditya-PS-05
Copy link
Contributor

closes #21

@@ -188,6 +196,30 @@ pub async fn login_handler(
})
}

fn generate_secure_token() -> (String, String) {
Copy link
Owner

Choose a reason for hiding this comment

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

I think it will be better if you create a new file in /utils for functions related to hmac.
This file should only contain routes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohh okay,
Is the overall functionality of the code, I written is good ?

Copy link
Owner

Choose a reason for hiding this comment

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

It seems fine!!. I have got to test it. If you happen to test the route, provide us with details

use rand::Rng;
use base64::{Engine as _, engine::general_purpose};

const HMAC_SECRET: &[u8] = b"your-secret-key-here";
Copy link
Owner

Choose a reason for hiding this comment

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

Store secrets in src/utils/constants and then import them here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh sorry for blunder

@Sidharth-Singh10
Copy link
Owner

run cargo fmt before each commit.

Copy link
Collaborator

@ShashwatAgrawal20 ShashwatAgrawal20 left a comment

Choose a reason for hiding this comment

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

idk this seems useless

Cargo.toml Outdated
@@ -25,6 +25,7 @@ serde = "1.0.197"
serde_json = "1.0.114"
chrono = "0.4.38"
sea-orm = { version = "1.0.0-rc.5", features = [ "sqlx-postgres", "runtime-tokio-rustls", "macros" ] }
sqlx = { version = "0.5", features = [ "postgres", "runtime-tokio-rustls" ] }
Copy link
Collaborator

Choose a reason for hiding this comment

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

btw, why are you adding sqlx as a dependency tho?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was trying to setup the db, and on cli, I don't know why but sqlx error was coming so I added it.
I forgot to review. I will remove it.

@Aditya-PS-05
Copy link
Contributor Author

@ShashwatAgrawal20 ,
required changes done.

Copy link
Collaborator

@ShashwatAgrawal20 ShashwatAgrawal20 left a comment

Choose a reason for hiding this comment

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

LGTM!

shipit

Copy link

@Wahid7852 Wahid7852 left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me. SHIP IT!

@ShashwatAgrawal20
Copy link
Collaborator

@Sidharth-Singh10

@Sidharth-Singh10 Sidharth-Singh10 merged commit 1e94dcd into Sidharth-Singh10:main Oct 9, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Security Issue: Token for Password Reset is Not Hashed Before Storing in Database
4 participants