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

Logging in without reauthentication in example code doesn't work #534

Open
JonPichel opened this issue Dec 25, 2023 · 3 comments
Open

Logging in without reauthentication in example code doesn't work #534

JonPichel opened this issue Dec 25, 2023 · 3 comments

Comments

@JonPichel
Copy link

I've tried running the example in https://github.com/markbates/goth/blob/master/examples/main.go.

After successfully logging in with google, I expected the session to remember the previous authentication, so that when I go try to log in with google again I skip the logging flow. I expected this because of this part of the code:

	p.Get("/auth/{provider}", func(res http.ResponseWriter, req *http.Request) {
		// try to get the user without re-authenticating
		if gothUser, err := gothic.CompleteUserAuth(res, req); err == nil {
			t, _ := template.New("foo").Parse(userTemplate)
			t.Execute(res, gothUser)
		} else {
			gothic.BeginAuthHandler(res, req)
		}
	})

However, when I go to /auth/google again, the CompleteUserAuth fails with this error: could not find a matching session for this request

What could be the problem? I am running the example without changing anything.

@JonPichel JonPichel changed the title Logging in without reauthentication in example code doesn't work Secure cookie enabled causes authentication on Chromium to fail Dec 25, 2023
@JonPichel JonPichel changed the title Secure cookie enabled causes authentication on Chromium to fail Logging in without reauthentication in example code doesn't work Dec 25, 2023
@ctrl-alt-lulz
Copy link

this lib is fucking trash...

@yaronius
Copy link
Contributor

@JonPichel I managed to reproduce the issue you are describing. The cause of it seems to be inside gothic.CompleteUserAuth(res, req) implementation. And I was able to "fix" it by commenting out some code 😅 One thing that looks odd to me is this line

defer Logout(res, req)

I wonder why we need to logout there, it removes the active session 🤔
The other issue that was breaking sessions is the state check that is needed for callback but not needed in regular session fetch. So another couple of lines to comment out

err = validateState(req, sess)
if err != nil {
	return goth.User{}, err
}

and you are good to go 🙂 But, of course, ideally there should be a PR to fix this logic there (I can see that gothic hasn’t been touched for 2 years already). Anyone willing to contribute? 😉

@tmstorm
Copy link

tmstorm commented Feb 19, 2024

I had a similar issue and from what I could tell the reason you are logged out here is the current stored session only stores the state to compare with the state returned after the user is logged in with the provider. The issue with this is that the logout is deferred then later down on this line the session is updated. Which means the new session is stored and then immediately deleted at the end of the function.

        err = StoreInSession(providerName, sess.Marshal(), req, res)

My initial work around was what @yaronius suggested but I didn't like changing the code in the lib. So ultimately what I ended up doing was making my own CompleteUserAuth functions since it is a var in the lib and can whatever you would like.

var CompleteUserAuth = func(res http.ResponseWriter, req *http.Request) (goth.User, error) {

I am also not using gorilla sessions and instead storing my sessions in a redis database. It would be nice to have sessions decoupled from this lib or made into an interface as others have mentioned #512 and #422.

There is also an issue with StoreInSession where storing in a session actually makes a new one and will deleted the current session causing the session to fail entirely #527.

// StoreInSession stores a specified key/value pair in the session.
func StoreInSession(key string, value string, req *http.Request, res http.ResponseWriter) error {
        session, _ := Store.New(req, SessionName)


        if err := updateSessionValue(session, key, value); err != nil {
                return err
        }


        return session.Save(req, res)
}

I do really like this lib and have managed to get it working with my current project after using my own CompleteUserAuth and State management. The huge number of providers this lib offers makes it hard to pass up. It would be nice to have a maintainer to get this up to speed.

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

4 participants