From 1837b0c3584a05c7141bd59aab33938a1d25d1c6 Mon Sep 17 00:00:00 2001 From: Walker Crouse Date: Thu, 5 Jan 2017 15:21:42 -0500 Subject: [PATCH] Improve SSO security Signed-off-by: Walker Crouse --- app/controllers/Actions.scala | 27 ++++++++++++++++++++--- app/controllers/BaseController.scala | 4 ++++ app/controllers/Users.scala | 18 ++++++++++----- app/db/impl/schema.scala | 11 ++++++++- app/db/impl/service/OreModelConfig.scala | 4 +++- app/db/impl/service/OreModelService.scala | 1 + app/db/impl/table/ModelKeys.scala | 5 ++++- app/models/user/SignOn.scala | 26 ++++++++++++++++++++++ build.sbt | 4 ++-- conf/evolutions/default/63.sql | 12 ++++++++++ conf/logback.xml | 1 + 11 files changed, 99 insertions(+), 14 deletions(-) create mode 100644 app/models/user/SignOn.scala create mode 100644 conf/evolutions/default/63.sql diff --git a/app/controllers/Actions.scala b/app/controllers/Actions.scala index 67532f351..ae0e0d1eb 100755 --- a/app/controllers/Actions.scala +++ b/app/controllers/Actions.scala @@ -1,9 +1,13 @@ package controllers +import java.util.Date + import controllers.Requests._ +import db.impl.OrePostgresDriver.api._ +import db.access.ModelAccess import db.impl.access.{OrganizationBase, ProjectBase, UserBase} import models.project.Project -import models.user.User +import models.user.{SignOn, User} import ore.permission.scope.GlobalScope import ore.permission.{EditSettings, HideProjects, Permission} import org.spongepowered.play.security.SingleSignOnConsumer @@ -23,6 +27,7 @@ trait Actions extends Calls with ActionHelpers { val projects: ProjectBase val organizations: OrganizationBase val sso: SingleSignOnConsumer + val signOns: ModelAccess[SignOn] val PermsLogger = play.api.Logger("Permissions") @@ -180,6 +185,22 @@ trait Actions extends Calls with ActionHelpers { } + /** + * Returns true and marks the nonce as used if the specified nonce has not + * been used, has not exired. + * + * @param nonce Nonce to check + * @return True if valid + */ + def isNonceValid(nonce: String): Boolean = this.signOns.find(_.nonce === nonce).exists { signOn => + if (signOn.isCompleted || new Date().getTime - signOn.createdAt.get.getTime > 600000) + false + else { + signOn.setCompleted() + true + } + } + // Implementation private def userLock(redirect: Call) = new ActionFilter[AuthRequest] { @@ -196,11 +217,11 @@ trait Actions extends Calls with ActionHelpers { if (sso.isEmpty || sig.isEmpty) Some(Unauthorized) else { - Actions.this.sso.authenticate(sso.get, sig.get) match { + Actions.this.sso.authenticate(sso.get, sig.get)(isNonceValid) match { case None => Some(Unauthorized) case Some(spongeUser) => - if (spongeUser.username.equals(request.user.username)) + if (spongeUser.id == request.user.id.get) None else Some(Unauthorized) diff --git a/app/controllers/BaseController.scala b/app/controllers/BaseController.scala index c40f87ed6..6505f5169 100755 --- a/app/controllers/BaseController.scala +++ b/app/controllers/BaseController.scala @@ -1,9 +1,11 @@ package controllers import db.ModelService +import db.access.ModelAccess import db.impl.VersionTable import db.impl.access.{OrganizationBase, ProjectBase, UserBase} import models.project.{Project, Version} +import models.user.SignOn import ore.{OreConfig, OreEnv} import org.spongepowered.play.security.SingleSignOnConsumer import play.api.i18n.I18nSupport @@ -25,6 +27,8 @@ abstract class BaseController(implicit val env: OreEnv, implicit override val projects: ProjectBase = this.service.getModelBase(classOf[ProjectBase]) implicit override val organizations: OrganizationBase = this.service.getModelBase(classOf[OrganizationBase]) + override val signOns: ModelAccess[SignOn] = this.service.access[SignOn](classOf[SignOn]) + /** * Executes the given function with the specified result or returns a * NotFound if not found. diff --git a/app/controllers/Users.scala b/app/controllers/Users.scala index a8c8686c1..d23186cc4 100755 --- a/app/controllers/Users.scala +++ b/app/controllers/Users.scala @@ -1,5 +1,6 @@ package controllers +import java.util.Date import javax.inject.Inject import db.ModelService @@ -8,7 +9,7 @@ import db.impl.access.UserBase.ORDERING_PROJECTS import discourse.OreDiscourseApi import form.OreForms import models.user.role.RoleModel -import models.user.{Notification, User} +import models.user.{Notification, SignOn, User} import ore.rest.OreWrites import ore.user.notification.InviteFilters.InviteFilter import ore.user.notification.NotificationFilters.NotificationFilter @@ -45,7 +46,9 @@ class Users @Inject()(fakeUser: FakeUser, * @return Logged in page */ def signUp() = Action { implicit request => - redirectToSso(this.sso.getSignupUrl(this.baseUrl + "/login")) + val nonce = SingleSignOnConsumer.nonce + this.signOns.add(SignOn(nonce = nonce)) + redirectToSso(this.sso.getSignupUrl(this.baseUrl + "/login", nonce)) } /** @@ -62,11 +65,12 @@ class Users @Inject()(fakeUser: FakeUser, this.users.getOrCreate(this.fakeUser) this.redirectBack(returnPath.getOrElse(request.path), this.fakeUser) } else if (sso.isEmpty || sig.isEmpty) { - // Check if forums are available and redirect to login if so - redirectToSso(this.sso.getLoginUrl(this.baseUrl + "/login")) + val nonce = SingleSignOnConsumer.nonce + this.signOns.add(SignOn(nonce = nonce)) + redirectToSso(this.sso.getLoginUrl(this.baseUrl + "/login", nonce)) } else { // Redirected from SpongeSSO, decode SSO payload and convert to Ore user - this.sso.authenticate(sso.get, sig.get) match { + this.sso.authenticate(sso.get, sig.get)(isNonceValid) match { case None => Redirect(ShowHome).withError("error.loginFailed") case Some(spongeUser) => @@ -85,7 +89,9 @@ class Users @Inject()(fakeUser: FakeUser, * @return Redirect to verification */ def verify(returnPath: Option[String]) = Authenticated { implicit request => - redirectToSso(this.sso.getVerifyUrl(this.baseUrl + returnPath.getOrElse("/"))) + val nonce = SingleSignOnConsumer.nonce + this.signOns.add(SignOn(nonce = nonce)) + redirectToSso(this.sso.getVerifyUrl(this.baseUrl + returnPath.getOrElse("/"), nonce)) } private def redirectToSso(url: String): Result = { diff --git a/app/db/impl/schema.scala b/app/db/impl/schema.scala index 6eccc35a3..e1c4b3698 100755 --- a/app/db/impl/schema.scala +++ b/app/db/impl/schema.scala @@ -9,7 +9,7 @@ import db.table.{AssociativeTable, ModelTable, NameColumn} import models.project._ import models.statistic.{ProjectView, VersionDownload} import models.user.role.{OrganizationRole, ProjectRole, RoleModel} -import models.user.{Notification, Organization, User, Session => DbSession} +import models.user.{Notification, Organization, SignOn, User, Session => DbSession} import ore.Colors.Color import ore.permission.role.RoleTypes.RoleType import ore.project.Categories.Category @@ -175,6 +175,15 @@ class SessionTable(tag: Tag) extends ModelTable[DbSession](tag, "user_sessions") } +class SignOnTable(tag: Tag) extends ModelTable[SignOn](tag, "sign_ons") { + + def nonce = column[String]("nonce") + def isCompleted = column[Boolean]("is_completed") + + def * = (id.?, createdAt.?, nonce, isCompleted) <> (SignOn.tupled, SignOn.unapply) + +} + class OrganizationTable(tag: Tag) extends ModelTable[Organization](tag, "organizations") with NameColumn[Organization] { override def id = column[Int]("id", O.PrimaryKey) diff --git a/app/db/impl/service/OreModelConfig.scala b/app/db/impl/service/OreModelConfig.scala index 26712b33c..3cf48863a 100644 --- a/app/db/impl/service/OreModelConfig.scala +++ b/app/db/impl/service/OreModelConfig.scala @@ -8,7 +8,7 @@ import db.{ModelSchema, ModelService} import models.project._ import models.statistic.{ProjectView, VersionDownload} import models.user.role.{OrganizationRole, ProjectRole} -import models.user.{Notification, Organization, User} +import models.user.{Notification, Organization, SignOn, User} trait OreModelConfig extends ModelService with OreDBOs { @@ -56,6 +56,8 @@ trait OreModelConfig extends ModelService with OreDBOs { val SessionSchema = new ModelSchema[models.user.Session](this, classOf[models.user.Session], TableQuery[SessionTable]) + val SignOnSchema = new ModelSchema[SignOn](this, classOf[SignOn], TableQuery[SignOnTable]) + val ProjectRolesSchema = new ModelSchema[ProjectRole](this, classOf[ProjectRole], TableQuery[ProjectRoleTable]) val ProjectSchema = new ProjectSchema(this, Users) diff --git a/app/db/impl/service/OreModelService.scala b/app/db/impl/service/OreModelService.scala index 9e9f2a179..9ed91f5b8 100644 --- a/app/db/impl/service/OreModelService.scala +++ b/app/db/impl/service/OreModelService.scala @@ -52,6 +52,7 @@ class OreModelService @Inject()(override val env: OreEnv, // Register model schemas registerSchema(UserSchema) registerSchema(SessionSchema) + registerSchema(SignOnSchema) registerSchema(ProjectRolesSchema) registerSchema(ProjectSchema) registerSchema(ProjectSettingsSchema) diff --git a/app/db/impl/table/ModelKeys.scala b/app/db/impl/table/ModelKeys.scala index d811100c6..61e9d1a8c 100755 --- a/app/db/impl/table/ModelKeys.scala +++ b/app/db/impl/table/ModelKeys.scala @@ -7,7 +7,7 @@ import db.table.key._ import models.project._ import models.statistic.StatEntry import models.user.role.RoleModel -import models.user.{Notification, User} +import models.user.{Notification, SignOn, User} import ore.Colors.Color import ore.permission.role.RoleTypes.RoleType import ore.project.Categories.Category @@ -58,6 +58,9 @@ object ModelKeys { val AvatarUrl = new StringKey[User](_.avatarUrl, _.avatarUrl.orNull) val ReadPrompts = new Key[User, List[Prompt]](_.readPrompts, _.readPrompts.toList) + // SignOn + val IsCompleted = new BooleanKey[SignOn](_.isCompleted, _.isCompleted) + // Version val IsReviewed = new BooleanKey[Version](_.isReviewed, _.isReviewed) val ChannelId = new IntKey[Version](_.channelId, _.channelId) diff --git a/app/models/user/SignOn.scala b/app/models/user/SignOn.scala new file mode 100644 index 000000000..6c4d43604 --- /dev/null +++ b/app/models/user/SignOn.scala @@ -0,0 +1,26 @@ +package models.user + +import java.sql.Timestamp + +import db.impl.SignOnTable +import db.impl.model.OreModel +import db.impl.table.ModelKeys._ + +case class SignOn(override val id: Option[Int] = None, + override val createdAt: Option[Timestamp] = None, + nonce: String, + var _isCompleted: Boolean = false) extends OreModel(id, createdAt) { + + override type M = SignOn + override type T = SignOnTable + + def isCompleted: Boolean = this._isCompleted + + def setCompleted(completed: Boolean = true) = Defined { + this._isCompleted = completed + update(IsCompleted) + } + + override def copyWith(id: Option[Int], theTime: Option[Timestamp]) = this.copy(id = id, createdAt = theTime) + +} diff --git a/build.sbt b/build.sbt index 7f1f742b6..5a6ba7650 100755 --- a/build.sbt +++ b/build.sbt @@ -1,6 +1,6 @@ name := "ore" -version := "1.1.9" +version := "1.1.10" lazy val `ore` = (project in file(".")).enablePlugins(PlayScala) @@ -20,7 +20,7 @@ resolvers ++= Seq( ) libraryDependencies ++= Seq( - "org.spongepowered" % "sponge-play" % "1.0.0-SNAPSHOT", + "org.spongepowered" % "sponge-play" % "1.0.1-SNAPSHOT", "org.spongepowered" % "play-discourse" % "1.0.0-SNAPSHOT", "org.spongepowered" % "plugin-meta" % "0.2", "com.typesafe.play" %% "play-slick" % "2.0.0", diff --git a/conf/evolutions/default/63.sql b/conf/evolutions/default/63.sql new file mode 100644 index 000000000..867f16d55 --- /dev/null +++ b/conf/evolutions/default/63.sql @@ -0,0 +1,12 @@ +# --- !Ups + +create table sign_ons( + id bigserial not null primary key, + created_at timestamp not null, + nonce varchar(255) not null unique, + is_completed boolean not null default false +); + +# --- !Downs + +drop table sign_ons; diff --git a/conf/logback.xml b/conf/logback.xml index 2328c4ba4..1d6cc7ba9 100644 --- a/conf/logback.xml +++ b/conf/logback.xml @@ -41,5 +41,6 @@ +