Skip to content

Commit

Permalink
fixed issue with gothic not handling multiple logins correctly
Browse files Browse the repository at this point in the history
  • Loading branch information
@markbates
markbates committed Sep 21, 2017
1 parent c130f2f commit 9885e53
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion gothic/gothic.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ 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)

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 Down Expand Up @@ -180,7 +182,8 @@ var CompleteUserAuth = func(res http.ResponseWriter, req *http.Request) (goth.Us
return goth.User{}, err
}

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

// validateState ensures that the state token param from the original
Expand Down

2 comments on commit 9885e53

@neumayer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain the problem that's being solved here?
I'm not entirely sure I understand it :-)
Is this about users having sessions with multiple oauth providers at the same time? Wouldn't that be solved by removing all sessions with providers which are not x, when completing the flow for provider x?

After this change, we immediately remove the session with the provider at login. No cookies are left. No session is left. In terms of examples: If one logs in in the example main.go, login does fetch userinfo, create a session, and immediately deletes that session. Refreshing the page will already leave a user without session.

Is this the intention (it means any type of session management is left to the user, basically they'd need to overwrite the CompleteUserAuth function)? Can you clarify this?

@markbates
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The session here is an internal session for goth to use to store information it needs during the auth phase. It was never to be used by the end user for their applications session. The problem this fixed was that the goth session was being kept around and was causing problems if someone tried to login in with one provider and then another.

Session manangement for your application should be done by you in your application. Goth doesn’t manage that.

Please sign in to comment.