Open cKellyDesign opened 5 years ago
@cKellyDesign in my view the Gatekeeper component is not really concerned with where you keep the access token once it gets it for you.
In fact, the way it works right now is that it requires for you to specify a reducer that you want your token put in, which by default is the SessionReducer but whoever is using Gatekeeper can provide any other compatible reducer.
Therefore I would say that Gatekeeper is not the place to implement persistence.
What we need to do, in my view, is create a way to persist the data stored in the session reducer. My vote would be for this to be a new package in this repo.
Do you agree?
Ah I see, yes I agree. Even the name gatekeeper
suggests it's just the start, and not necessarily responsible for what happens once through the gate!
Persisting the data stored in the session reducer sounds good, having some new package to handle this sounds like the right approach so it's not coupled with either GateKeeper
or SessionReducer
.
@moshthepitt I agree that we can/should have the actual implementation (methods) of persistence built out in a new package, though I'm trying to understand where those methods should be called. I've been trying to get my head around the relation between gatekeeper
, session-reducer
, connected-private-route
, and the client (in this case reveal-frontend
) and am trying to understand the best way to move forward.
Since session-reducer
is a shared dependency of the rest (and its state is what's being persisted), what do you think of the business logic (aka @onaio/cookies
for CRUDing cookies) being called from the session-reducer actionCreators? I think this approach aligns with redux best practices by keeping non-deterministic code out of the Redux reducer.
This approach should standup to asynchronous means of persisting the data as well.
Thoughts?
@cKellyDesign sorry only just gotten to reply today. I agree completely. we should still use the session-reducer
store. I think what we want to do is to add a utility to persist and sync the session-reducer
reducer to cookies as securely as possible.
I think @KipSigei has done something similar for Gisida, but using local storage (I think one of our conclusions in https://github.com/onaio/reveal-frontend/issues/39 was to not use Localstorage for this), which I believe we can re-use for our purposes in this new package.
Addressing onaio/reveal-frontend#39, gatekeeper should store temporary credentials on the client in order to persist oAuth2 sessions without requiring reauthorization.
In alignment with Ona's current strategy, whatever method we use to store the creds should automatically self-expire. Ideally there could be functionality to warn the user of impending token expiration, giving them an opportunity to refresh the token without being logged out.