markbates / goth

Package goth provides a simple, clean, and idiomatic way to write authentication packages for Go web applications.
https://blog.gobuffalo.io/goth-needs-a-new-maintainer-626cd47ca37b
MIT License
5.2k stars 566 forks source link

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

Open JonPichel opened 6 months ago

JonPichel commented 6 months ago

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.

ctrl-alt-lulz commented 4 months ago

this lib is fucking trash...

yaronius commented 4 months ago

@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 commented 4 months ago

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.

Disservin commented 2 weeks ago

@tmstorm hey ; ) do you mind sharing how your solution for using redis instead of gorilla looks like?

tmstorm commented 2 weeks ago

@Disservin Sure, I wrote a decent sized auth.go for this. I'll try and give you just the relevant code. I am sure there is a better way to do it but I wanted more control over how the auth flowed.

I started by writing my own BeginAuth function. What I changed in there were the calls to SetState and how the session was stored for callback.

func BeginAuth(c *gin.Context) {
...
    state, token, err := SetState(c)
    if err != nil {
        c.JSON(http.StatusBadRequest, gin.H{
            "error": "Bad Request",
        })
        log.Printf("[AUTH] Error setting state: %v", err)
    }
...

    // Store the initialized session with token as the key for later.
    err = cache.Mem(CacheNum, token).Write([]byte(sess.Marshal()), false)
    if err != nil {
        log.Printf("[AUTH] Error storing state: %v", err)
        return
    }
...
}

Then I created my own SetState, GetState, and ValidateState functions.

// ValidateState checks if the return state matches what was sent
func ValidateState(c *gin.Context) (string, error) {
    storedSess := GetState(c)

    s, err := cache.Mem(CacheNum, storedSess.Token).Read()
    if err != nil {
        return "", err
    }

    cache.Mem(CacheNum, storedSess.Token).Remove()
    return string(s), nil
}

// SetState should create a new state for the authentication.
// It should set the return URL for redirect later and a random Session string to validate when the user returns from authentication.
func SetState(c *gin.Context) (string, string, error) {
    // Set the state to the return URL for callback and redirect later
    randToken, err := helpers.RandString(16)
    if err != nil {
        c.Header("Status", fmt.Sprint(http.StatusBadRequest))
        log.Printf("[AUTH] Error creating random token: %v", err)
        return "", "", err
    }

    returnURL := c.Query("return_url")
    if returnURL == "" {
        err = errors.New("no return url provided")
        return "", "", err
    }

    s := State{
        Token:     randToken,
        ReturnURL: returnURL,
    }

    jsonState, err := json.Marshal(s)
    if err != nil {
        c.Header("Status", fmt.Sprint(http.StatusBadRequest))
        log.Printf("[AUTH] Error marshaling state: %v", err)
        return "", "", err
    }
    return base64.StdEncoding.EncodeToString(jsonState), s.Token, nil
}

// GetState retrieves the state for verification after the provider returns the user.
func GetState(c *gin.Context) State {
    var s State
    params := c.Request.URL.Query()
    if params.Encode() == "" && c.Request.Method == http.MethodPost {
        state := c.Request.FormValue("state")
        rawState, _ := base64.StdEncoding.DecodeString(state)
        err := json.Unmarshal(rawState, &s)
        if err != nil {
            c.JSON(http.StatusUnauthorized, gin.H{
                "error": "Not Authorized",
            })
            return State{}
        }
        return s
    }

    state := params.Get("state")
    rawSate, _ := base64.StdEncoding.DecodeString(state)
    err := json.Unmarshal(rawSate, &s)
    if err != nil {
        c.JSON(http.StatusUnauthorized, gin.H{
            "error": "Not Authorized",
        })
        return State{}
    }

    return s
}

I then created a CompleteAuth.

func CompleteAuth(c *gin.Context, prov string) (goth.User, string, error) {
...
    value, err := ValidateState(c)
    if err != nil {
        return goth.User{}, "", err
    }
...
    // get the saved return url from the state for redirect
    ret := GetState(c)

        return user, ret.ReturnURL, nil
}

I have a callback endpoint that handles the completion of the auth.

func CallBack(c *gin.Context) {
...
// Attempt to complete user authentication if not print error to log and send json error as response.
        user, _, err := CompleteAuth(c, prov)
        if err != nil {
            log.Printf("[GOTH] Error in getting user: %s", err)
            c.Abort()
            c.JSON(http.StatusUnauthorized, gin.H{
                "error": err,
            })
            return
        }

        // Get new user UUID
        id := uuid.NewString()

        // Initiate new user
        newUser := User{
            UserID:       user.UserID,
            FirstName:    user.FirstName,
            LastName:     user.LastName,
            Email:        user.Email,
            Provider:     user.Provider,
            AccessToken:  user.AccessToken,
            RefreshToken: user.RefreshToken,
            TokenExp:     user.ExpiresAt,
        }

        // Save user session in redis
        err = session.Client().SetUser(c, id, newUser)
        if err != nil {
            log.Printf("[REDIS] Cannot save user: %v", err)
            c.Abort()
            c.JSON(http.StatusUnauthorized, gin.H{
                "error": err,
            })
            return
        }

        // Once login is complete send user UUID as JSON response.
        c.JSON(http.StatusOK, id)
}

Finally I created a function to handle the logout.

func Logout(c *gin.Context) {
    uuid := c.Query("uuid")
    u := User{}

    data, err := session.Client().GetUser(c, uuid)
    if err != nil {
        log.Printf("[AUTH] Unable to delete session: %v", err)
        c.JSON(http.StatusBadRequest, gin.H{
            "error": err,
        })
        return
    }

    err = json.Unmarshal(data, &u)
    if err != nil {
        log.Printf("[AUTH] cannot unmarshal user: %v", err)
        c.JSON(http.StatusBadRequest, gin.H{
            "error": err,
        })
        return
    }

    err = session.Client().DeleteUser(c.Request.Context(), uuid)
    if err != nil {
        log.Printf("[AUTH] Unable to delete session: %v", err)
        c.JSON(http.StatusBadRequest, gin.H{
            "error": err,
        })
        return
    }

    retURI := c.Request.URL.Query().Get("redirect_uri")

    logoutURI, _ := url.Parse(EntraLogoutURI)

    q := logoutURI.Query()
    q.Set("post_logout_redirect_uri", retURI)

    logoutURI.RawQuery = q.Encode()
    c.Redirect(http.StatusTemporaryRedirect, logoutURI.String())
}

I also have a few other functions that handle the auth on each protected endpoint and to persist sessions without user interaction if still valid.

Where I am calling things like session.Client().SetUser() I created a session.go that handles communication directly with the redis database. This way I can manage and access the sessions outside of goth if needed.

I am also using github.com/google/uuid to create a UUID for the user sessions.

Hopefully this helps. If you would like to see the session.go I would be happy to share that as well.

Disservin commented 2 weeks ago

Thanks, but I now think this all a bit of an overkill solution... as stated in https://github.com/markbates/goth/issues/270, where they already a) stated that the persistent login doesnt work anymore and b) you should roll your own. Which means... just use the simple gorilla session store for gothic and after retrieving the user information, simply create your own session cookie (and store it i.e. in redis) and use this for authentication with your API and call BeginAuthHandler when you need to login. Which makes me wonder what is the current purpose of CompleteUserAuth ?? apart from the case inside the auth callback