AngellusMortis / django_microsoft_auth

Simple app to enable Microsoft Account, Office 365 and Xbox Live authentcation as a Django authentcation backend.
MIT License
137 stars 84 forks source link

Extend CSRF token validity to nearly a day, and use simpler error message #464

Closed mnelson4 closed 7 months ago

mnelson4 commented 2 years ago

Fix for issue AngellusMortis#420 by improving wording and extending CSRF validity timeframe.

I'm using these changes as-is on a live site.

AngellusMortis commented 2 years ago

Thanks for the PR! I really do appreciate even if everything else is going to sound a bit harsh.

OAuth state is not CSRF. Wordpress does not have an OAuth provider out of the box so it is not a valid comparison (nor should Worldpress ever really be used for an example of "secure").

The OAuth state should only be valid for as long as it takes the user to login. 5 minutes is plenty long enough to log in. As you said though it sounds like users are leaving the page open for an extended period of time rather than actually logging in. So, I think the solution is that we need to not generate the state value until they have the intent to login. I think this is already possible based on another PR that was submitted a while ago.

We should instead change the URL that gets put into the frontend to to-auth-redirect/ instead. This is a URL that just redirects to the authorization URL. This will make it so the state is not generated until the user goes to log with Microsoft's OAuth service.

If you still think 5 minutes is not long enough to complete the login flow after initializing it, I think adding a configuration setting for the state should be added instead of increasing the default to super long value.

Some code references:

mnelson4 commented 2 years ago

Ya only generating the oauth state just before redirecting the user would be a better solution, in which case just a few seconds would usually be long enough (so a configuration settings probably won’t be necessary). As a WordPress plugin author and contributor, the comment about WordPress not being secure wasn’t was a little hurtful. But I do see you’re point about WordPress not being a valid comparison because this is oauth

AngellusMortis commented 2 years ago

the comment about WordPress not being secure wasn’t was a little hurtful

I do have to apologize for that. I just have never had the best experience with WordPress and well, it does not have the best track record on CVEs.