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

[feature] Add configuration to csrf to be able to change default /csrfTo... #2367

Merged
merged 12 commits into from
Mar 17, 2015

Conversation

konstantinzolotarev
Copy link
Member

...ken route #2366

});
}
// Create a route path for getting _csrf parameter
var csrfRoute = {};
csrfRoute[sails.config.csrf.route] = {target: csrfToken, cors: {origin: sails.config.csrf.origin, credentials:true}};
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make this line more readable please (indent)?

@loicsaintroch
Copy link
Contributor

Looks good to me. Can you write a test for this new feature please? Then, I will merge it.

@mikermcneil
Copy link
Member

@konstantinzolotarev @loicsaintroch Closing for now- please feel free to reopen w/ test(s)

@mikermcneil mikermcneil closed this Dec 3, 2014
@konstantinzolotarev
Copy link
Member Author

@mikermcneil @loicsaintroch Seems that I couldn't open this pull request and update it. Should I create a new one or you will open this one ?

@loicsaintroch loicsaintroch reopened this Dec 6, 2014
@konstantinzolotarev
Copy link
Member Author

@mikermcneil @sgress454 @loicsaintroch Might be we could include this enchantment into new sails version ?

@loicsaintroch
Copy link
Contributor

Looks good to me. What do you think @mikermcneil @sgress454 ?

@mikermcneil
Copy link
Member

@konstantinzolotarev @loicsaintroch thanks! Looks like it needs a rebase (but also travis is being crazy) If travis calms down (tests pass + merge can be performed automatically), can merge this afternoon

@mikermcneil
Copy link
Member

@konstantinzolotarev please see #2367 (comment) -- this refactor introduces potential entanglement issues

@konstantinzolotarev
Copy link
Member Author

@mikermcneil Sorry for long response. Was in a trip.
Fixed.

@loicsaintroch
Copy link
Contributor

Looks good to me. Ok to merge this feature @mikermcneil @sgress454 ?

@CWyrtzen
Copy link

Bump~

@mikermcneil
Copy link
Member

@konstantinzolotarev thanks! If you wouldn't mind rebasing so we can auto-merge, we can go ahead and do that.

@konstantinzolotarev
Copy link
Member Author

@mikermcneil done

loicsaintroch pushed a commit that referenced this pull request Mar 17, 2015
Add configuration to csrf to be able to change default /csrfToken
@loicsaintroch loicsaintroch merged commit f5cf006 into balderdashy:master Mar 17, 2015
@loicsaintroch
Copy link
Contributor

Thank you very much for your contribution @konstantinzolotarev, we appreciate your support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants