Skip to content

Commit

Permalink
Distinguish LiftSession uniqueId from underlyingId.
Browse files Browse the repository at this point in the history
What used to be uniqueId is now called underlyingId, to
indicate the fact that it is tied to the underlying host’s session
id. A new uniqueId is introduced, which is secure and unique,
and is not tied to the host id and cannot be used to look the
session up in the future.

The only place that is currently continuing to use uniqueId is
when we emit the id into the page as data-lift-session-id for
cache-busting purposes. The underlying id used to be
emitted into the page, which triggered some small security
concerns, so we switch to this model to be on the safe side.
  • Loading branch information
Shadowfiend committed Dec 11, 2014
1 parent 62d4021 commit f18e340
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 10 deletions.
4 changes: 2 additions & 2 deletions web/webkit/src/main/scala/net/liftweb/http/LiftServlet.scala
Expand Up @@ -653,7 +653,7 @@ class LiftServlet extends Loggable {
case _ => JsCommands(S.noticesToJsCmd :: JsCmds.Noop :: S.jsToAppend).toResponse
}

LiftRules.cometLogger.debug("AJAX Response: " + liftSession.uniqueId + " " + ret)
LiftRules.cometLogger.debug("AJAX Response: " + liftSession.underlyingId + " " + ret)

Full(ret)
} finally {
Expand Down Expand Up @@ -684,7 +684,7 @@ class LiftServlet extends Loggable {
private def handleAjax(liftSession: LiftSession,
requestState: Req): Box[LiftResponse] = {
extractVersions(requestState.path.partPath) { versionInfo =>
LiftRules.cometLogger.debug("AJAX Request: " + liftSession.uniqueId + " " + requestState.params)
LiftRules.cometLogger.debug("AJAX Request: " + liftSession.underlyingId + " " + requestState.params)
tryo {
LiftSession.onBeginServicing.foreach(_(liftSession, requestState))
}
Expand Down
23 changes: 16 additions & 7 deletions web/webkit/src/main/scala/net/liftweb/http/LiftSession.scala
Expand Up @@ -310,7 +310,7 @@ object SessionMaster extends LiftActor with Loggable {
private def lockAndBump(f: => Box[SessionInfo]): Box[LiftSession] = this.synchronized {
f.map {
s =>
nsessions.put(s.session.uniqueId, SessionInfo(s.session, s.userAgent, s.ipAddress, s.requestCnt + 1, millis))
nsessions.put(s.session.underlyingId, SessionInfo(s.session, s.userAgent, s.ipAddress, s.requestCnt + 1, millis))

s.session
}
Expand Down Expand Up @@ -372,7 +372,7 @@ object SessionMaster extends LiftActor with Loggable {
val ses = lockRead(nsessions)
(Box !! ses.get(sessionId)).foreach {
case SessionInfo(s, _, _, _, _) =>
killedSessions.put(s.uniqueId, Helpers.millis)
killedSessions.put(s.underlyingId, Helpers.millis)
s.markedForShutDown_? = true
Schedule.schedule(() => {
try {
Expand Down Expand Up @@ -412,7 +412,7 @@ object SessionMaster extends LiftActor with Loggable {
f(ses, shutDown => {
if (!shutDown.session.markedForShutDown_?) {
shutDown.session.markedForShutDown_? = true
this.sendMsg(RemoveSession(shutDown.session.uniqueId))
this.sendMsg(RemoveSession(shutDown.session.underlyingId))
}
})
} else {
Expand All @@ -424,7 +424,7 @@ object SessionMaster extends LiftActor with Loggable {

this ! RemoveSession(shutDown.
session.
uniqueId)
underlyingId)
}
}
), 0.seconds)
Expand Down Expand Up @@ -595,13 +595,22 @@ private[http] class BooleanThreadGlobal extends ThreadGlobal[Boolean] {
/**
* The LiftSession class containg the session state information
*/
class LiftSession(private[http] val _contextPath: String, val uniqueId: String,
class LiftSession(private[http] val _contextPath: String, val underlyingId: String,
val httpSession: Box[HTTPSession]) extends LiftMerge with Loggable with HowStateful {
def sessionHtmlProperties = LiftRules.htmlProperties.session.is.make openOr LiftRules.htmlProperties.default.is.vend

val requestHtmlProperties: TransientRequestVar[HtmlProperties] =
new TransientRequestVar[HtmlProperties](sessionHtmlProperties(S.request openOr Req.nil)) {}

/**
* The unique id of this session. Can be used to securely and uniquely
* identify the session, unlike underlyingId which is tied to the host
* session id. This id should be secure to emit into a page as needed (for
* example, it is used to provide randomness/cache-busting to the comet
* request path).
*/
val uniqueId: String = nextFuncName

@volatile
private[http] var markedForTermination = false

Expand Down Expand Up @@ -897,7 +906,7 @@ class LiftSession(private[http] val _contextPath: String, val uniqueId: String,
* Destroy this session and the underlying container session.
*/
def destroySession() {
SessionMaster ! RemoveSession(this.uniqueId)
SessionMaster ! RemoveSession(this.underlyingId)

S.request.foreach(_.request.session.terminate)
this.doShutDown()
Expand Down Expand Up @@ -1126,7 +1135,7 @@ class LiftSession(private[http] val _contextPath: String, val uniqueId: String,

_running_? = false

SessionMaster.sendMsg(RemoveSession(this.uniqueId))
SessionMaster.sendMsg(RemoveSession(this.underlyingId))

import scala.collection.JavaConversions._
nasyncComponents.foreach {
Expand Down
Expand Up @@ -28,7 +28,7 @@ class HTTPServletSession(session: HttpSession) extends HTTPSession {

def sessionId: String = session.getId

def link(liftSession: LiftSession) = session.setAttribute(LiftMagicID, SessionToServletBridge(liftSession.uniqueId))
def link(liftSession: LiftSession) = session.setAttribute(LiftMagicID, SessionToServletBridge(liftSession.underlyingId))

def unlink(liftSession: LiftSession) = session.removeAttribute(LiftMagicID)

Expand Down

0 comments on commit f18e340

Please sign in to comment.