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

Bug/added csrf protection #28

Merged
merged 3 commits into from
Oct 2, 2023
Merged

Bug/added csrf protection #28

merged 3 commits into from
Oct 2, 2023

Conversation

1-ankush-1
Copy link
Contributor

@1-ankush-1 1-ankush-1 commented Oct 2, 2023

issue ref - #22
commit ref - c2ebe95 153044b e4a30a8

Changes

  • Added the csrfProtection inside the app.js file.
+ const csrfProtection = csrf({ cookie: true });
+ app.use(csrfProtection);
  • Added the csrfProtection middleware between the routes that are getting the csrf token from forms.
  • updated the logout function so it will destroy the session after logout.

Suggestion

  • There is no home for home path .so create one so user can access that after login.
  • Add a setup.md file so anyone can contribute easily.
  • You can put the config.json info inside the .env file.

@Artlfmj
Copy link
Owner

Artlfmj commented Oct 2, 2023

Have you tested your changes on the forms to check they work

@1-ankush-1
Copy link
Contributor Author

Have you tested your changes on the forms to check they work

yup i did . just add a Home file too so it will display the home page data after login.

@Artlfmj
Copy link
Owner

Artlfmj commented Oct 2, 2023

Ok, you can create a new pr for your other suggestions :)

@Artlfmj
Copy link
Owner

Artlfmj commented Oct 2, 2023

Could you resolve conflicts?

@1-ankush-1
Copy link
Contributor Author

Ok, you can create a new pr for your other suggestions :)

you should create seprate issue for them as well.

@1-ankush-1
Copy link
Contributor Author

Could you resolve conflicts?

wait give me 2 min

@Artlfmj
Copy link
Owner

Artlfmj commented Oct 2, 2023

You can go ahead and create the issue, I will assign you once you create it

@1-ankush-1
Copy link
Contributor Author

You can go ahead and create the issue, I will assign you once you create it

  • resolved the conflict

@1-ankush-1
Copy link
Contributor Author

You can go ahead and create the issue, I will assign you once you create it

hey i am not clear about the requirments for the Home file. you should create the issue for Home and explain what feature you would like to add in Home.

@Artlfmj Artlfmj merged commit bd3494c into Artlfmj:main Oct 2, 2023
3 checks passed
@Artlfmj
Copy link
Owner

Artlfmj commented Oct 2, 2023

Will say home later today

@Artlfmj
Copy link
Owner

Artlfmj commented Oct 2, 2023

@1-ankush-1 checked this, you did not add csrf verification on form submit

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.

2 participants