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

Add csrf cookie for login flow submission #2454

Merged
merged 2 commits into from May 13, 2022

Conversation

vizv
Copy link
Contributor

@vizv vizv commented May 9, 2022

Related issue(s)

#2003 (comment) doesn't work without allowing SDK pass a CSRF cookie, however I only fixed endpoint for login, but they are the similar issues.

Checklist

  • I have read the contributing guidelines.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • I am following the
    contributing code guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security. vulnerability, I
    confirm that I got green light (please contact
    security@ory.sh) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

Further Comments

I can understand it's not recommend to use Go SDK to proxy the request, and login flow submit HTTP API is not stable yet, so feel free to reject this PR. However please remove browser login flow for SDK since it's broken right now (without passing CSRF cookie, the CSRF token will be rejected anyway).

@vizv vizv requested review from aeneasr and zepatrik as code owners May 9, 2022 14:21
@CLAassistant
Copy link

CLAassistant commented May 9, 2022

CLA assistant check
All committers have signed the CLA.

@vizv
Copy link
Contributor Author

vizv commented May 9, 2022

I have tested my change works after regenerated SDK for go, by forward cookie with csrf_token_ prefix. I'm able to login using Go SDK with browser flow.

@aeneasr
Copy link
Member

aeneasr commented May 13, 2022

Thank you very much, I added the rest of them here:

#2467

:)

@aeneasr aeneasr merged commit 2bffee8 into ory:master May 13, 2022
aeneasr added a commit that referenced this pull request May 13, 2022
aeneasr added a commit that referenced this pull request May 13, 2022
aeneasr added a commit that referenced this pull request May 13, 2022
@vizv vizv deleted the add-csrf-cookie-for-login-flow-submission branch May 13, 2022 16:37
@vinckr
Copy link
Member

vinckr commented May 20, 2022

Hello @vizv
Congrats on merging your first PR in Ory 🎉 !
Your contribution will soon be helping secure millions of identities around the globe 🌏.
As a small token of appreciation we send all our first time contributors a gift package to welcome them to the community.
Please drop me an email and I will forward you the form to claim your Ory swag!

peturgeorgievv pushed a commit to senteca/kratos-fork that referenced this pull request Jun 30, 2023
peturgeorgievv pushed a commit to senteca/kratos-fork that referenced this pull request Jun 30, 2023
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.

None yet

4 participants