Skip to content
This repository has been archived by the owner on Jun 27, 2023. It is now read-only.

Allow storing state in the database. #8

Open
frederikcreemers opened this issue Jun 24, 2018 · 4 comments
Open

Allow storing state in the database. #8

frederikcreemers opened this issue Jun 24, 2018 · 4 comments

Comments

@frederikcreemers
Copy link

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.

@danilopolani
Copy link
Owner

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

@frederikcreemers
Copy link
Author

When nnYou're doing Dispatch.Handle, it'd hnave to get the gocial corresponding to the given state, right?

@danilopolani
Copy link
Owner

danilopolani commented Jun 25, 2018

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

@gadelkareem
Copy link

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants