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

State parameter on the recieveRedirect of theOAauth2Authorizer class breaks sign ins? #213

Closed
ezhes opened this issue Apr 15, 2018 · 6 comments

Comments

@ezhes
Copy link
Contributor

ezhes commented Apr 15, 2018

So I'm not entirely sure when this issue came up but it hit me recently. When the receiveRedirect(_ url... function gets the callback I'm sending from the AppDelegate, Reddift silently ignores the request because of this check:

&& state == currentState

I've removed it, seemingly to no maleffect, and am able to sign in properly again. I can't seem to nail down the cause since the state data is sent with every account, not just my 2FA main account. I also can't say for sure when this changed since it only affects newly authorized accounts, not token refreshes.

@sonsongithub
Copy link
Owner

I've never tried 2FA authorization.
Maybe, just as you said, this problem is caused by 2FA....?

@ezhes
Copy link
Contributor Author

ezhes commented Apr 15, 2018

The problem though is still there with all my non-2FA accounts. Is there a reason why you're checking state in this function? Reddit seems to be handing a state value back now on oauth login while the app is expecting there to be none.

@sonsongithub
Copy link
Owner

I'm so sorry, I don't remember the details......
Your suggest is removing the code to check state?

if let code = parameters["code"], let state = parameters["state"] {
    if code.characters.count > 0  {
        do {
            try OAuth2Token.getOAuth2Token(code, completion: completion)
            return true
        } catch {
            print(error)
            return false
        }
    }
}
return false

@ezhes
Copy link
Contributor Author

ezhes commented Apr 16, 2018

Yeah that's what I patched on my fork. ezhes@d1bfb91

As I understand it state is the CSRF token but I'm not entirely sure how we're getting away with not saving it. Are we not required to use CSRF tokens since we're authorizing as an app and not using the standard reddit interface?

@sonsongithub
Copy link
Owner

I got it.
I think it's not required, too.
Please, send a pull request if you have enough time to do it.
I want to merge your nice job into my codes.
Thank you.

@ezhes
Copy link
Contributor Author

ezhes commented Apr 21, 2018

I've submitted a pull. I've fixed this on my branch so I think this can be closed just fine.

@ezhes ezhes closed this as completed Apr 21, 2018
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

No branches or pull requests

2 participants