Skip to content

Commit

Permalink
fixed multiple logins
Browse files Browse the repository at this point in the history
  • Loading branch information
markbates committed Dec 21, 2017
1 parent 86ca064 commit 78433fe
Showing 1 changed file with 4 additions and 10 deletions.
14 changes: 4 additions & 10 deletions gothic/gothic.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ as either "provider" or ":provider".
See https://github.com/markbates/goth/examples/main.go to see this in action.
*/
var CompleteUserAuth = func(res http.ResponseWriter, req *http.Request) (goth.User, error) {
defer Logout(res, req)

This comment has been minimized.

Copy link
@darh

darh Feb 2, 2018

@markbates why was this moved up to again (after being moved down to UnmarshalSession with commit 86ca064 in Oct 2017?

It makes my setup unusable. Might be that I'm force-feeding it with gin but if I move the deferred logout down it works.

It causes (when it at the first line of the func) 2 cookies to be sent (both named _gothic_session) in the same response:

Set-Cookie:_gothic_session=MTUxNzYwNDY2NnxEd...UFBQUJQLUNBQUE9fGo4CUZuEk-09kEzuQZm6WwOPwyEo_7kCkj; Path=/auth; Expires=Thu, 01 Jan 1970 00:00:01 GMT; Max-Age=0; HttpOnly
Set-Cookie:_gothic_session=MTUxNzYwNDY2NnxEd...QUFfXzhCQUFEX181UnJsMXBmQVFBQXxGQaGkYhhUxbyJlYb_EqKJThpVNTq1=; Path=/auth; Expires=Thu, 01 Jan 1970 00:00:01 GMT; Max-Age=0; HttpOnly 

(first one is 185 chars long and the 2nd one is 805 chars long)

Looks like this confuses my browser (chrome, firefox).

This comment has been minimized.

Copy link
@markbates

markbates Feb 2, 2018

Author Owner

I've explained this is several places, it was moved up because when it was moved down it caused errors when trying to use multiple types of authentication providers in one application.

I have never had any problems with cookie store with this line where it is, but have nothing but problems with the line moved down. When it's moved down it does not clean up after itself if there is an error.

My suggestion, to everyone who has asked about this, is to submit a patch that fixes both problems, not just one.

I'm happy to have someone else fix it, but I can't accept a patch that breaks what I consider to be working functionality.

I'm sorry it's not the answer you're probably looking for, but it has been explained multiple times and no one has yet to write the patch that solves all the problems.

This comment has been minimized.

Copy link
@darh

darh Feb 2, 2018

Damn, didn't notice that this was discussed before. Apologies.

I have the case of multiple providers (fb & g+) and if I run into any problems with defer moved down, I'll do something about it.

if !keySet && defaultStore == Store {
fmt.Println("goth/gothic: no SESSION_SECRET environment variable is set. The default cookie store is not available and any calls will fail. Ignore this warning if you are using a different store.")
}
Expand All @@ -166,8 +167,6 @@ var CompleteUserAuth = func(res http.ResponseWriter, req *http.Request) (goth.Us
return goth.User{}, err
}

defer Logout(res, req)

sess, err := provider.UnmarshalSession(value)
if err != nil {
return goth.User{}, err
Expand Down Expand Up @@ -222,12 +221,7 @@ func validateState(req *http.Request, sess goth.Session) error {

// Logout invalidates a user session.
func Logout(res http.ResponseWriter, req *http.Request) error {
providerName, err := GetProviderName(req)
if err != nil {
return err
}

session, err := Store.Get(req, providerName+SessionName)
session, err := Store.Get(req, SessionName)
if err != nil {
return err
}
Expand Down Expand Up @@ -287,15 +281,15 @@ func getProviderName(req *http.Request) (string, error) {
}

func storeInSession(key string, value string, req *http.Request, res http.ResponseWriter) error {
session, _ := Store.Get(req, key+SessionName)
session, _ := Store.Get(req, SessionName)

session.Values[key] = value

return session.Save(req, res)
}

func getFromSession(key string, req *http.Request) (string, error) {
session, _ := Store.Get(req, key+SessionName)
session, _ := Store.Get(req, SessionName)
value := session.Values[key]
if value == nil {
return "", errors.New("could not find a matching session for this request")
Expand Down

0 comments on commit 78433fe

Please sign in to comment.