Skip to content

[Feature] detect and "logout" on old csrf token #11182

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

Closed
6543 opened this issue Apr 22, 2020 · 12 comments
Closed

[Feature] detect and "logout" on old csrf token #11182

6543 opened this issue Apr 22, 2020 · 12 comments
Labels
issue/stale type/feature Completely new functionality. Can only be merged if feature freeze is not active.

Comments

@6543
Copy link
Member

6543 commented Apr 22, 2020

currently if the csrf token becomes infalide you still can act as if you where valid logedin.
If you however triger a POST request by changing setings etc ... you get a csrf token infalide error.

What it should do instead: logout the user

Gitea Version: 1.12.0+dev-225-ge0bf844b0

@6543 6543 mentioned this issue Apr 22, 2020
6 tasks
@choucavalier
Copy link

Thanks, this would lead to a better UX indeed 🙏

@zeripath
Copy link
Contributor

I think logging out would be a bit aggressive here - and could lead to denial of service.

It might be that issuing a redirect would be more correct however we'd need to look for best practices for csrf tokens (my googlefu is failing me here).

If we do want to redirect - we will probably have to change the csrf library to give us the original request back as I think you need that to send a redirect.

@choucavalier
Copy link

Side note: this happened to me as I was trying to submit a review on a PR. I spent time writing the review and lost my content when I tried to submit it and got this error. In terms of UX that's pretty bad.

@jolheiser
Copy link
Member

I also had limited luck when searching this, but I'd imagine a redirect would be preferable if the session itself is still valid.

I think in terms of general usage a redirect would be fine, however when it comes to submitting forms and being able to "save" your work, perhaps it's a good tie in to #11004?

@3F
Copy link

3F commented Apr 22, 2020

@6543,

Just to be clear, I am not against the way for invalidating token. But I was starting with describing my issue with your Gitea for Firefox #4311 (comment)

Where I had sometimes the csrf problem for every ~5 minutes or so. And for the just received "caching problem of Chrome - just delete your cache" [?] I just offer my thoughts that this is not a good idea if I will login again every ~5 minutes. Why I can't just say it? I don't understand.

@zeripath,

What a rude? You're personally insulted me with this!

I think logging out would be a bit aggressive here
... It might be that issuing a redirect would be more correct

This is what I'm talking about.

I am grateful to all the volunteers! I am also open source developer, I know what is this. So please keep calm when your users just asking and sharing their opinions (which by the way we are also spending time for configuring and integrating environment with your products, trusting you)

With regards,

@choucavalier
Copy link

@3F the way you write your messages generates very negative vibes. Apparently I'm not the only one who noticed this. Maybe you should work on your way of communicating your opinions about things. We should all try to be the nicest contributors to work with. :)

@3F
Copy link

3F commented Apr 22, 2020

@tgy,

For my I'd prefer spend ~3 sec for just ... That was sarcasm. Do I need more :) smile for it?

But here it is. "sarcasm" was started just because we just received "caching problem of Chrome - just delete your cache". hmm, well, ok. thank you. Right? So I don't see the problems since it was started not by me.

As I said, I just offer my thoughts for the problem that affects for all Gitea users! Nothing more.

@6543
Copy link
Member Author

6543 commented Apr 22, 2020

@3F:
I just read Chrome ... on the original issue (witch was closed) and I know this issue when restarting different gitea versions while working on gitea so my first thought was ... this bad cache thing ... :(

If the csrf error happens regulary, this is sure an issue and is anoying!

In generel we should somehow imprufe wrong csrf request handeling ...

@3F
Copy link

3F commented Apr 22, 2020

I just read Chrome ...

And for some reason everyone attacked me just when I started clarify the case for the issue.

@6543,

Nothing worry if you're missing something!
You're doing an amazing job! Thank you!

To the team, please be friendly to users too. I was personally not starting anything offensive at all. Just re-read my comments again. Thus, I was very offended by the words of @zeripath

If the csrf error happens regulary, this is sure an issue and is anoying!

Well, I'm constantly getting this starting with 1.10.3 (when I started use gitea). Currently, I am on 1.11.4. After receiving a new reply to the subscribed issue, I just decided to report about problems for Firefox too.

In generel we should somehow imprufe wrong csrf request handeling ...

Maybe just automate the requesting by mentioned combination as a temporary solution (or some option in admin panel)? It can be really easy to implement. And this could be a good option at least before a normal solution.

What do you think? Thanks!

@silverwind
Copy link
Member

silverwind commented Apr 23, 2020

IMHO, CRSF tokens should just be replaced with SameSite attribute, see #11188.

@techknowlogick techknowlogick added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Apr 23, 2020
@stale
Copy link

stale bot commented Aug 1, 2020

This issue has been automatically marked as stale because it has not had recent activity. I am here to help clear issues left open even if solved or waiting for more insight. This issue will be closed if no further activity occurs during the next 2 weeks. If the issue is still valid just add a comment to keep it alive. Thank you for your contributions.

@stale stale bot added the issue/stale label Aug 1, 2020
@6543
Copy link
Member Author

6543 commented Aug 1, 2020

I'll close this if fafour of #11188

@6543 6543 closed this as completed Aug 1, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/stale type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

No branches or pull requests

7 participants