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.46k stars 587 forks source link

Passing parameters between the login and the callback functions #507

Open JPFrancoia opened 1 year ago

JPFrancoia commented 1 year ago

I'm in a weird situation where when I call the auth_callback, I need to know how the auth flow was initiated:

func Login(c *gin.Context) {
    var loginUrl LoginUrl
    if err := c.BindQuery(&loginUrl); err != nil {
        c.AbortWithStatus(http.StatusInternalServerError)
        return
    }

    var w http.ResponseWriter = c.Writer
    var req *http.Request = c.Request

    logger.Debug("Starting auth flow")
    url, err := gothic.GetAuthURL(c.Writer, c.Request)

    if err != nil {
        c.AbortWithError(http.StatusInternalServerError, err)
    }

    err = gothic.StoreInSession("is_web", strconv.FormatBool(loginUrl.IsWeb), req, w)
    c.Redirect(http.StatusFound, url)
}
func AuthCallback(c *gin.Context) {
    user, err := gothic.CompleteUserAuth(c.Writer, c.Request)
    fmt.Println("user: ", user)
    is_web, _ := gothic.GetFromSession("is_web", c.Request)
    fmt.Println("it's web or not", is_web)
}

Basically when I start the flow, I call mysite.com/auth/provider_name?is_web=true. I'm parsing the is_web parameter in the Login function, and I'm then trying to store it in the gothic session.

Then when AuthCallback is triggered, I'm trying to get is_web back, but the call to gothic.CompleteUserAuth raises an error: you must select a provider. I believe at this point the content of the session is overwritten? Does the call to StoreInSession wipe out the relevant info?

mgerasimchuk commented 1 year ago

Also faced this problem with the same usecase.

I believe at this point the content of the session is overwritten? Does the call to StoreInSession wipe out the relevant info? Yes, you are right, just have a look: https://github.com/markbates/goth/blob/master/gothic/gothic.go#L311-L312

I tried a lot of ways to find the workaround but found nothing. And finally, I found the moment when it possibility had been broken: https://github.com/markbates/goth/pull/217

PS: Looks like it can be reached only with a custom Store

JPFrancoia commented 1 year ago

I ultimately found a solution to this problem. I'm not sure it's pretty though. With OAUth flow, there is a state parameter that is used to prevent attacks by making the url hard to guess: https://auth0.com/docs/secure/attack-protection/state-parameters. It's basically a random string.

I'm kinda hijacking that at the moment:

...
    // Add any data to the state, we can decode this data later in the auth callback.
    // - add is_web (bool) to check if the client is a web browser
    authState := AuthState{IsWeb: loginQuery.IsWeb, Rand: generateRandomState()}
    q.Add("state", authState.ToStateString())

    c.Request.URL.RawQuery = q.Encode()

    logger.Debug("Starting auth flow")

    url, err := gothic.GetAuthURL(c.Writer, c.Request)

    if err != nil {
        c.AbortWithError(http.StatusInternalServerError, err)
    }

    logger.Debug("Auth url generated, redirecting")

    c.Redirect(http.StatusFound, url)

This can be later decoded in the auth callback. From what I understand, the only thing to watch out for is that the state parameter is still random and unguessable, but technically it's possible to shove some custom data in there.

I'd be happy to see a better solution though. Also, if the session approach can be fixed, I'd have one question: how does the session work in the case where my app is deployed on kubernetes with several replicas? Is the session stored in each replica?

mgerasimchuk commented 1 year ago

@JPFrancoia thank you for sharing your workaround, I also thought about it but decided to avoid it, because usually the state it's just a csrf token which is coming from the frontend side, and when we custom this, we can break a default behavior of the frameworks..

But anyway, I think your solution is totally legal(cos you include the random string in your state object too), cos Google, for example, says that the state parameter can be used for saving application custom data

how does the session work in the case where my app is deployed on kubernetes with several replicas?

I think, to cover these needs you should implement your own Store and directly set it this way:

gothic.Store = redisStore

and for sharing data between different PODs, your implementation should be based on not the default cookie store, but on the Redis key-value storage for example.

JPFrancoia commented 1 year ago

Thanks for commenting!

@JPFrancoia thank you for sharing your workaround, I also thought about it but decided to avoid it, because usually the state it's just a csrf token which is coming from the frontend side, and when we custom this, we can break a default behavior of the frameworks..

In my case the state is entirely controlled by the backend, the frontend just triggers the flow but I get your point.

and for sharing data between different PODs, your implementation should be based on not the default cookie store, but on the Redis key-value storage for example.

Yep, makes sense. Another pro for using the state parameter in my case, because then I don't really need the store and I can just rely on the data in the request. Right now I have my backend running with 3 replicas and it works.

TomG-Mona commented 10 months ago

How do you know the state you get back is the state value you sent?

JPFrancoia commented 10 months ago

I don't, but in my case I don't care. I'm just storing the is_web flag in the state. It's used for displaying behaviour. If someone decides to mess around with it, the only thing they'll achieve is getting a bad display