Add support for session authentication #935
Conversation
ezekg
force-pushed
the
feature/sessions
branch
5 times, most recently
from
db47748 to
8c1314a
Compare
ezekg
force-pushed
the
feature/sessions
branch
15 times, most recently
from
06bba15 to
c7b1233
Compare
ezekg
force-pushed
the
feature/sessions
branch
from
c7b1233 to
1ec62df
Compare
| if token.save | ||
| cookies.encrypted[:session_id] = { value: session.id, httponly: true, secure: true, same_site: :none, expires: session.expiry, domain: Keygen::DOMAIN } |
There was a problem hiding this comment.
What if we're generating a token on behalf of another object, but don't want to clobber our session? e.g. an admin generating an environment token for an integration. I guess use the object's tokens relationship instead of the root? i.e. POST /v1/environments/<environment>/tokens instead of /v1/tokens?
There was a problem hiding this comment.
Or we could start a session only for :password authn?
| include Environmental | ||
| include Accountable | ||
|
|
||
| belongs_to :token |
There was a problem hiding this comment.
How will this work with SSO, i.e. not tied to a token? Maybe the token association should be optional?
There was a problem hiding this comment.
But then we'd need to expose a sessions resource so that they can be revoked.
May be easier to simply generate a token as part of the SSO flow.
ezekg
force-pushed
the
feature/sessions
branch
3 times, most recently
from
c292f5a to
79995e3
Compare
ezekg
force-pushed
the
feature/sessions
branch
from
79995e3 to
0277298
Compare
ezekg
force-pushed
the
feature/sessions
branch
from
44973b7 to
480f68f
Compare
ezekg
force-pushed
the
feature/sessions
branch
from
ea53052 to
0ec7a19
Compare
ezekg
force-pushed
the
feature/sessions
branch
from
c133b6f to
2f4b2ce
Compare
ezekg
force-pushed
the
feature/sessions
branch
from
bc0ba01 to
03c3d56
Compare
ezekg
force-pushed
the
feature/sessions
branch
from
03c3d56 to
91d6845
Compare
| @@ -139,6 +154,11 @@ def revoke | |||
| resource: token, | |||
| ) | |||
|
|
|||
| # expire session if we're revoking the current token | |||
| if token == current_token | |||
| cookies.delete(:session_id, domain: Keygen::DOMAIN, same_site: :none) | |||
There was a problem hiding this comment.
Is all this really needed? Invalid sessions will be auto-expired after unsuccessful authn.
| @@ -127,6 +137,11 @@ def regenerate | |||
| resource: token, | |||
| ) | |||
|
|
|||
| # expire session if we're regenerating the current token | |||
| if token == current_token | |||
| cookies.delete(:session_id, domain: Keygen::DOMAIN, same_site: :none) | |||
There was a problem hiding this comment.
Do we really want to expire the current session when a token is regenerated?
Using token.regenerate!(except: Current.session) may be better?
There was a problem hiding this comment.
Or should we revoke all sessions and generate a new session entirely? I lean towards that.
There was a problem hiding this comment.
regenerate! can return the new session.
ezekg
force-pushed
the
feature/sessions
branch
2 times, most recently
from
2710894 to
311cc7e
Compare
ezekg
force-pushed
the
feature/sessions
branch
from
311cc7e to
4cbfc03
Compare
Groundwork for #409 and Portal.