Merge pull request #121 from delphi-hub/authTest

Removed user secret from GET endpoints

Author: johannesduesing

OpenAPISpecification.yaml

@@ -959,9 +959,6 @@ definitions:
       userName:
         type: string
         example: Ben
-      secret:
-        type: string
-        example: 0DE19F6FAAFB7CF372172CEA658800999A75DB9E79AF5F378F274E47DF810CEE
       userType:
         type: string
         enum:

src/main/scala/de/upb/cs/swt/delphi/instanceregistry/RequestHandler.scala

@@ -977,14 +977,14 @@ class RequestHandler(configuration: Configuration, authDao: AuthDAO, instanceDao
     * @param user The user to add
     * @return Id assigned to that user
     */
-  def handleAddUser(user: DelphiUser): Try[String] = {
+  def handleAddUser(user: DelphiUser): Try[Long] = {
 
     val noIdUser = DelphiUser(id = None, userName = user.userName, secret = user.secret, userType = user.userType)
 
     authDao.addUser(noIdUser) match {
-      case Success(username) =>
+      case Success(userId) =>
         log.info(s"Successfully handled create user request")
-        Success(username)
+        Success(userId)
       case Failure(x) => Failure(x)
     }
   }

src/main/scala/de/upb/cs/swt/delphi/instanceregistry/authorization/AuthProvider.scala

@@ -53,7 +53,7 @@ class AuthProvider(authDAO: AuthDAO) extends AppLogging {
   private def getSecretForUser(userName: String): Option[String] ={
     val user = authDAO.getUserWithUsername(userName)
     if(user.isDefined){
-      Some(user.get.secret)
+      Some(user.get.secret.get)
     } else {
       None
     }

src/main/scala/de/upb/cs/swt/delphi/instanceregistry/connection/Server.scala

@@ -1191,9 +1191,9 @@ class Server(handler: RequestHandler) extends HttpApp
         try {
           val paramInstance: DelphiUser = UserString.parseJson.convertTo[DelphiUser](authDelphiUserFormat)
           handler.handleAddUser(paramInstance) match {
-            case Success(username) =>
+            case Success(userId) =>
               complete {
-                username
+                userId.toString
               }
             case Failure(ex) =>
               log.error(ex, "Failed to handle create user request.")
@@ -1223,6 +1223,8 @@ class Server(handler: RequestHandler) extends HttpApp
   def allUsers(): server.Route = Route.seal{
     authenticateOAuth2[AccessToken]("Secure Site", handler.authProvider.authenticateOAuthRequire(_, userType = UserType.Admin)) { token =>
       get {
+        log.info("kutta")
+        log.info(handler.getAllUsers().toString())
         complete {
           handler.getAllUsers().toList
         }

src/main/scala/de/upb/cs/swt/delphi/instanceregistry/daos/AuthDAO.scala

@@ -25,7 +25,7 @@ trait AuthDAO {
     * Add user
     * @return
     */
-  def addUser(delphiUser : DelphiUser) : Try[String]
+  def addUser(delphiUser : DelphiUser) : Try[Long]
 
   /**
     * Remove user with username

src/main/scala/de/upb/cs/swt/delphi/instanceregistry/daos/DatabaseAuthDAO.scala

@@ -53,13 +53,13 @@ class DatabaseAuthDAO (configuration : Configuration) extends AuthDAO with AppLo
   {
     if(hasUserWithUsername(userName)) {
       val result = Await.result(dbAuth.run(users.filter(_.userName === userName).result.headOption), Duration.Inf)
-      Some(dataToObjectAuthenticate(result.get._1, result.get._2, result.get._3, result.get._4))
+      Some(dataToObjectAuthenticateWithSecret(result.get._1, result.get._2, Some(result.get._3), result.get._4))
     } else {
       None
     }
   }
 
-  override def addUser(delphiUser : DelphiUser) : Try[String] = {
+  override def addUser(delphiUser : DelphiUser) : Try[Long] = {
     if(hasUserWithUsername(delphiUser.userName)){
       Failure(new RuntimeException(s"username ${delphiUser.userName} is already exist."))
     } else {
@@ -68,11 +68,11 @@ class DatabaseAuthDAO (configuration : Configuration) extends AuthDAO with AppLo
       val secret = delphiUser.secret
       val userType = delphiUser.userType.toString
 
-      val addFuture: Future[Long] = dbAuth.run((users returning users.map(_.id)) += (id, userName, hashString(secret), userType))
+      val addFuture: Future[Long] = dbAuth.run((users returning users.map(_.id)) += (id, userName, hashString(secret.get), userType))
       val userId = Await.result(addFuture, Duration.Inf)
 
       log.info(s"Added user ${delphiUser.userName} with id $userId to database.")
-      Success(userName)
+      Success(userId)
     }
 
   }
@@ -146,7 +146,7 @@ class DatabaseAuthDAO (configuration : Configuration) extends AuthDAO with AppLo
     log.info("Shutdown complete.")
   }
 
-  private def dataToObjectAuthenticate(id:Long, userName: String, secret: String, userType: String): DelphiUser = {
+  private def dataToObjectAuthenticateWithSecret(id:Long, userName: String, secret: Option[String], userType: String): DelphiUser = {
     DelphiUser.apply(Option(id), userName, secret, getDelphiUserTypeFromString(userType))
   }
 
@@ -169,7 +169,7 @@ class DatabaseAuthDAO (configuration : Configuration) extends AuthDAO with AppLo
 
   private def dataToObjectUser(options : Option[(Long, String, String, String)]): DelphiUser ={
     val optionValue = options.get
-    DelphiUser(Some(optionValue._1), optionValue._2, optionValue._3, getUserTypeFromString(optionValue._4))
+    DelphiUser(Some(optionValue._1), optionValue._2, None, getUserTypeFromString(optionValue._4))
   }
 
   private def getUserTypeFromString(userType: String): DelphiUserType ={

src/main/scala/de/upb/cs/swt/delphi/instanceregistry/daos/DynamicAuthDAO.scala

@@ -40,23 +40,23 @@ class DynamicAuthDAO (configuration : Configuration) extends AuthDAO with AppLog
     if(hasUserWithUsername(userName)) {
       val query = users filter {i => i.userName == userName}
       val user  = query.iterator.next()
-      Some(dataToObjectAuthenticate(user.id.get, user.userName, user.secret, user.userType.toString))
+      Some(dataToObjectAuthenticateWithSecret(user.id.get, user.userName, user.secret.get, user.userType.toString))
     } else {
       None
     }
 
   }
 
-  override def addUser(delphiUser : DelphiUser) : Try[String] = {
+  override def addUser(delphiUser : DelphiUser) : Try[Long] = {
     if(hasUserWithUsername(delphiUser.userName)){
       Failure(new RuntimeException(s"username ${delphiUser.userName} is already exist."))
     } else{
       val id = nextId()
-      val newUser = DelphiUser(Some(id), delphiUser.userName, hashString(delphiUser.secret), delphiUser.userType)
+      val newUser = DelphiUser(Some(id), delphiUser.userName, Some(hashString(delphiUser.secret.get)), delphiUser.userType)
       users.add(newUser)
 
       log.info(s"Added user ${newUser.userName} with id ${newUser.id.get} to database.")
-      Success(newUser.userName)
+      Success(id)
     }
 
   }
@@ -76,14 +76,14 @@ class DynamicAuthDAO (configuration : Configuration) extends AuthDAO with AppLog
     if(hasUserWithId(id)) {
       val query = users filter {i => i.id.get == id}
       val result  = query.iterator.next()
-      Some(result)
+      Some(dataToObjectUser(result))
     } else {
       None
     }
   }
 
   override def getAllUser(): List[DelphiUser] = {
-    List() ++ users
+    List() ++ users map dataToObjectUser
   }
 
   override def hasUserWithUsername(username: String) : Boolean = {
@@ -109,10 +109,14 @@ class DynamicAuthDAO (configuration : Configuration) extends AuthDAO with AppLog
     log.info("Shutdown complete dynamic Auth DAO.")
   }
 
-  private def dataToObjectAuthenticate(id:Long, userName: String, secret: String, userType: String): DelphiUser = {
-    DelphiUser.apply(Option(id), userName, secret, getDelphiUserTypeFromString(userType))
+  private def dataToObjectUser(delphiUser : DelphiUser): DelphiUser ={
+
+    DelphiUser(delphiUser.id, delphiUser.userName, None, delphiUser.userType)
   }
 
+  private def dataToObjectAuthenticateWithSecret(id:Long, userName: String, secret: String, userType: String): DelphiUser = {
+    DelphiUser.apply(Option(id), userName, Some(secret), getDelphiUserTypeFromString(userType))
+  }
 
   private[daos] def clearData() : Unit = {
     users.clear()

src/main/scala/de/upb/cs/swt/delphi/instanceregistry/io/swagger/client/model/DelphiUser.scala

@@ -48,7 +48,7 @@ trait UserJsonSupport extends SprayJsonSupport with DefaultJsonProtocol{
   implicit val authDelphiUserFormat: JsonFormat[DelphiUser] = jsonFormat4(DelphiUser)
 }
 
-final case class DelphiUser(id: Option[Long], userName: String, secret: String, userType: DelphiUserType)
+final case class DelphiUser(id: Option[Long], userName: String, secret: Option[String], userType: DelphiUserType)
 
 object DelphiUserEnums {
 

src/test/scala/de/upb/cs/swt/delphi/instanceregistry/connection/ServerTest.scala

@@ -34,8 +34,6 @@ import pdi.jwt.{Jwt, JwtAlgorithm, JwtClaim}
 import spray.json._
 
 import scala.util.{Failure, Success, Try}
-import java.nio.charset.StandardCharsets
-import java.security.MessageDigest
 
 import de.upb.cs.swt.delphi.instanceregistry.io.swagger.client.model.DelphiUserEnums.DelphiUserType
 
@@ -65,7 +63,7 @@ class ServerTest
   private val invalidJsonInstance = validJsonInstance.replace(""""name":"ValidInstance",""", """"name":Invalid", """)
 
   private val validJsonDelphiUser = DelphiUser(id = None,
-                                              userName = "validUser" , secret = "validUser",
+                                              userName = "validUser" , secret = Some("validUser"),
                                               userType = DelphiUserType.Admin).toJson(authDelphiUserFormat
                                               ).toString
   private val invalidTypedJsonDelphiUser = validJsonDelphiUser.replace(""""userType":"User",""", """"userType":"Component",""")
@@ -81,8 +79,8 @@ class ServerTest
   override def beforeAll(): Unit = {
     requestHandler.initialize()
     authDao.initialize()
-    authDao.addUser(DelphiUser(None, "admin" , "admin", DelphiUserType.Admin))
-    authDao.addUser(DelphiUser(None, "user" , "user", DelphiUserType.User))
+    authDao.addUser(DelphiUser(None, "admin" , Some("admin"), DelphiUserType.Admin))
+    authDao.addUser(DelphiUser(None, "user" , Some("user"), DelphiUserType.User))
   }
 
   /**
@@ -214,14 +212,14 @@ class ServerTest
     }
 
     "successfully remove user when everything is valid" in {
-      authDao.addUser(DelphiUser(None, "user3" , "user3", DelphiUserType.User))
+      authDao.addUser(DelphiUser(None, "user3" , Some("user3"), DelphiUserType.User))
       Post("/users/3/remove") ~> addAuthorization("Admin") ~> Route.seal(server.routes) ~> check {
         assert(status === StatusCodes.ACCEPTED)
       }
     }
 
     "not remove user if request is invalid" in {
-      authDao.addUser(DelphiUser(None, "user4" , "user4", DelphiUserType.User))
+      authDao.addUser(DelphiUser(None, "user4" , Some("user4"), DelphiUserType.User))
       //Required type of user authorization needed to create user
       Post("/users/4/remove") ~> addAuthorization("User") ~> Route.seal(server.routes) ~> check {
         assert(status === StatusCodes.UNAUTHORIZED)
@@ -246,7 +244,7 @@ class ServerTest
     }
 
     "successfully get user if user exist" in {
-      authDao.addUser(DelphiUser(None, "user3" , "user3", DelphiUserType.User))
+      authDao.addUser(DelphiUser(None, "user3" , Some("user3"), DelphiUserType.User))
       Get("/users/1") ~> addAuthorization("Admin") ~> Route.seal(server.routes) ~> check {
         assert(status === StatusCodes.OK)
       }

src/test/scala/de/upb/cs/swt/delphi/instanceregistry/daos/DatabaseAuthDAOTest.scala

@@ -64,7 +64,7 @@ class DatabaseAuthDAOTest extends FlatSpec with Matchers with BeforeAndAfterEach
   private def buildUser(id : Int, userName : String = "") : DelphiUser = {
     val userType = if(id == 1) DelphiUserType.Admin else DelphiUserType.User
     val name = if(userName == "") "user" + id else userName
-    DelphiUser(Some(id), name , hashString("123456"), userType)
+    DelphiUser(Some(id), name , Some(hashString("123456")), userType)
   }
 
   private def hashString(secret: String): String = {