Allow storing state in the database.
If I understand the codebase correctly, the Dispatcher keeps a map from state to Gocial instance. This means that:
- If there are multiple servers handling requests, the same state user must always be routed to the same server
- i the process needs to restart, the state is lost and any authentication flows in progress will fail.
What I'd propose, is that we implement a way to serialise Gocials to a []byte, either using json or gob encoding, and then add an interface like this:
type GocialStore interface{
SaveGocial(state string, gocial []byte) error
GetGocialByState(state string) ([]byte, error)
}
which we can pass to NewDispatcher. We could provide a basic implementation that uses the current mechanism that stores them in a map.
Hi,
you're right about the issues, I was reading about Gob and it's fine to return a []byte, just to know, how would you save it in your DB?
Regarding GocialStore, won't be better different methods like Gocial.SaveGocial and Gocial.GetByState? Or do you want to use them like "hooks", so Gocial automatically invokes SaveGocial in the Handle method? Because SaveGocial makes sense as hook, but I can't find where GetGocialByState should be invoked
When nnYou're doing Dispatch.Handle, it'd hnave to get the gocial corresponding to the given state, right?
Right, you have only the state (by query params), the user and the token (returned by Handle) but not the "instance". Since I suppose you need the state + gocial right after New() (when performing the redirect), we could create something like this:
import (
...
)
var gocial = gocialite.NewDispatcher()
gocial.setHooks(gocialite.Hooks{
AfterStateCreated: func(state, instance) { // state string, instance Gocial
// Here you can encode instance with gob, or maybe we can add a third parameter where you get the instance already encoded in []byte
},
})
In this case, AfterStateCreated is invoked in the New() method before the return.
Then, before the Handle method, you could use:
gocial.LoadState(state, instance) // state string, instance Gocial
// Handle callback and check for errors
user, token, err := gocial.Handle(state, code)
if err != nil {
c.Writer.Write([]byte("Error: " + err.Error()))
return
}
Or
gocial.LoadStateEncoded(state, bytes) // state string, bytes []byte
// Handle callback and check for errors
user, token, err := gocial.Handle(state, code)
if err != nil {
c.Writer.Write([]byte("Error: " + err.Error()))
return
}
These functions will basically do (more or less) what New() does:
func (d *Dispatcher) LoadState(state string, instance *Gocial) {
d.mu.Lock()
defer d.mu.Unlock()
d.g[state] = instance
}
Tell me if I wrote some bullshit or if I didn't understand something
I am also interested in this since the current implementation is not scalable. If we are running few instances infront of a load balancer, there will be no way to get back the correct state. We need to find a way to store in a cache server like Redis for ex. Here is a lib I created that works with different cache backend https://github.com/gadelkareem/cachita and I think it could help.