goth icon indicating copy to clipboard operation
goth copied to clipboard

No documentation on why there is a defer logout

Open lil5 opened this issue 1 year ago • 5 comments

https://github.com/markbates/goth/blob/f4685f5f6edf65de920b6c6c03fc1ffabcb88e68/gothic/gothic.go#L180

func CompleteUserAuth In this function, why is there a defer to Logout?


I've been trying to figure out why my use of gothic is returning a cookie (_gothic_session) with an expires of "1970-01-01T00:00:01.000Z".

lil5 avatar Apr 16 '24 19:04 lil5

I was thinking the same thing and ended up copying the whole function and commenting the defer Logout() line. I also moved the validateState(...) check after FetchUser(...) to let CompleteUserAuth(...) work for already logged-in users like in the main example.

Here's the code:

var CompleteUserAuthNoLogout = func(res http.ResponseWriter, req *http.Request) (goth.User, error) {
	providerName, err := gothic.GetProviderName(req)
	if err != nil {
		return goth.User{}, err
	}

	provider, err := goth.GetProvider(providerName)
	if err != nil {
		return goth.User{}, err
	}

	value, err := gothic.GetFromSession(providerName, req)
	if err != nil {
		return goth.User{}, err
	}
	//HACK: Removed defer gothic.Logout(res, req)
	sess, err := provider.UnmarshalSession(value)
	if err != nil {
		return goth.User{}, err
	}

	user, err := provider.FetchUser(sess)
	if err == nil {
		// user can be found with existing session data
		return user, err
	}

	// HACK: validateState only after FetchUser fails
	err = validateState(req, sess)
	if err != nil {
		return goth.User{}, err
	}

	params := req.URL.Query()
	if params.Encode() == "" && req.Method == "POST" {
		_ = req.ParseForm()
		params = req.Form
	}

	// get new token and retry fetch
	_, err = sess.Authorize(provider, params)
	if err != nil {
		return goth.User{}, err
	}

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

	if err != nil {
		return goth.User{}, err
	}

	gu, err := provider.FetchUser(sess)
	return gu, err
}

func validateState(req *http.Request, sess goth.Session) error {
	rawAuthURL, err := sess.GetAuthURL()
	if err != nil {
		return err
	}

	authURL, err := url.Parse(rawAuthURL)
	if err != nil {
		return err
	}

	reqState := gothic.GetState(req)

	originalState := authURL.Query().Get("state")
	if originalState != "" && (originalState != reqState) {
		return errors.New("state token mismatch")
	}
	return nil
}

Am I doing something wrong? Should I submit this as a PR?

yeicor avatar May 16 '24 16:05 yeicor

Since gothic uses Gorilla Sessions by default it creates a state cookie when starting authentication to verify on callback. The defer logout is meant to delete the state cookie after auth is completed. StoreInSession doesn't really work the way I would like it to. For this reason I am managing the state and session on my own. See this comment I made previously.

I also made my own CompleteUserAuth since I do not want to use the Gorilla Sessions. Since CompleteUserAuth is a var its very easy to make your own function and use that instead.

I also made a function to check the sessions validity and attempt to silently get a new access token on expiration. This is called every time an API call happens.

tmstorm avatar May 16 '24 20:05 tmstorm

The defer logout makes the whole standard flow of trying to reauth obsolete.

// try to get the user without re-authenticating
  if gothUser, err := gothic.CompleteUserAuth(res, req); err == nil { //this will always be FALSE
  } else {
	  gothic.BeginAuthHandler(res, req)
  }

Doesn't make sense to call defer logout.

jessedegans avatar Sep 11 '24 13:09 jessedegans