From 6f2eccf1a469bd0833b294ced1aade6f65704117 Mon Sep 17 00:00:00 2001 From: jahnestacado Date: Wed, 15 Mar 2023 13:17:32 +0100 Subject: [PATCH] Now getUserAccountByName returns an optional result since the entry cannot be present in the db --- .../sts/service/db/dao/STSUserDAOItTest.scala | 61 ++++++++++++------- .../com/ing/wbaa/rokku/sts/api/NpaApi.scala | 7 ++- .../sts/service/UserTokenDbService.scala | 6 +- .../rokku/sts/service/db/dao/STSUserDAO.scala | 8 +-- .../ing/wbaa/rokku/sts/api/NpaApiTest.scala | 8 +-- .../sts/service/UserTokenDbServiceTest.scala | 11 ++-- 6 files changed, 62 insertions(+), 39 deletions(-) diff --git a/src/it/scala/com/ing/wbaa/rokku/sts/service/db/dao/STSUserDAOItTest.scala b/src/it/scala/com/ing/wbaa/rokku/sts/service/db/dao/STSUserDAOItTest.scala index f385d89..de8f8a1 100644 --- a/src/it/scala/com/ing/wbaa/rokku/sts/service/db/dao/STSUserDAOItTest.scala +++ b/src/it/scala/com/ing/wbaa/rokku/sts/service/db/dao/STSUserDAOItTest.scala @@ -61,7 +61,10 @@ class STSUserDAOItTest extends AsyncWordSpec val testObject = new TestObject insertAwsCredentials(testObject.userName, testObject.cred, isNPA = false).flatMap { r => assert(r) - getUserAccountByName(testObject.userName).map { case (UserAccount(_, c, _, _, _)) => assert(c.contains(testObject.cred)) } + getUserAccountByName(testObject.userName).map { + case (Some(UserAccount(_, c, _, _, _))) => assert(c.contains(testObject.cred)) + case _ => assert(false) + } getUserAccountByAccessKey(testObject.cred.accessKey).map { u => assert(u.isDefined) val userAccount = u.get @@ -79,15 +82,19 @@ class STSUserDAOItTest extends AsyncWordSpec val newCred = generateAwsCredential insertAwsCredentials(testObject.userName, testObject.cred, isNPA = false).flatMap { inserted => - getUserAccountByName(testObject.userName).map { case (UserAccount(_, c, _, _, _)) => - assert(c.contains(testObject.cred)) - assert(inserted) + getUserAccountByName(testObject.userName).map { + case (Some(UserAccount(_, c, _, _, _))) => + assert(c.contains(testObject.cred)) + assert(inserted) + case _ => assert(false) } insertAwsCredentials(testObject.userName, newCred, isNPA = false).flatMap(inserted => - getUserAccountByName(testObject.userName).map { case (UserAccount(_, c, _, _, _)) => - assert(c.contains(testObject.cred)) - assert(!inserted) + getUserAccountByName(testObject.userName).map { + case (Some(UserAccount(_, c, _, _, _))) => + assert(c.contains(testObject.cred)) + assert(!inserted) + case _ => assert(false) } ) } @@ -98,16 +105,19 @@ class STSUserDAOItTest extends AsyncWordSpec val testObject = new TestObject insertAwsCredentials(testObject.userName, testObject.cred, isNPA = false).flatMap { inserted => - getUserAccountByName(testObject.userName).map { case (UserAccount(_, c, _, _, _)) => - assert(c.contains(testObject.cred)) - assert(inserted) + getUserAccountByName(testObject.userName).map { + case (Some(UserAccount(_, c, _, _, _))) => + assert(c.contains(testObject.cred)) + assert(inserted) + case _ => assert(false) } val anotherTestObject = new TestObject insertAwsCredentials(anotherTestObject.userName, testObject.cred, isNPA = false).flatMap(inserted => - getUserAccountByName(anotherTestObject.userName).map { case (UserAccount(_, c, _, _, _)) => - assert(c.isEmpty) - assert(!inserted) + getUserAccountByName(anotherTestObject.userName).map { + case (None) => + assert(!inserted) + case _ => assert(false) } ) } @@ -141,17 +151,20 @@ class STSUserDAOItTest extends AsyncWordSpec "exists" in { val testObject = new TestObject insertAwsCredentials(testObject.userName, testObject.cred, isNPA = false).flatMap { _ => - getUserAccountByName(testObject.userName).map { case (UserAccount(_, o, _, _, _)) => - assert(o.isDefined) - assert(o.get.accessKey == testObject.cred.accessKey) - assert(o.get.secretKey == testObject.cred.secretKey) + getUserAccountByName(testObject.userName).map { + case (Some(UserAccount(_, Some(creds), _, _, _))) => + assert(creds.accessKey == testObject.cred.accessKey) + assert(creds.secretKey == testObject.cred.secretKey) + case _ => assert(false) } } } "doesn't exist" in { - getUserAccountByName(Username("DOESNTEXIST")).map { case (UserAccount(_, o, _, _, _)) => - assert(o.isEmpty) + getUserAccountByName(Username("DOESNTEXIST")).map { + case (None) => + assert(true) + case _ => assert(false) } } } @@ -248,7 +261,10 @@ class STSUserDAOItTest extends AsyncWordSpec insertAwsCredentials(newUser, newCred, isNPA = false).map(r => assert(r)).flatMap { _ => setAccountStatus(newUser, false).flatMap { _ => - getUserAccountByName(newUser).map { case (UserAccount(_, _, AccountStatus(isEnabled), _, _)) => assert(!isEnabled) } + getUserAccountByName(newUser).map { + case (Some(UserAccount(_, _, AccountStatus(isEnabled), _, _))) => assert(!isEnabled) + case _ => assert(false) + } getUserAccountByAccessKey(testObject.cred.accessKey).map { u => assert(u.isDefined) val userAccount = u.get @@ -260,7 +276,10 @@ class STSUserDAOItTest extends AsyncWordSpec } setAccountStatus(newUser, true).flatMap { _ => - getUserAccountByName(newUser).map { case (UserAccount(_, _, AccountStatus(isEnabled), _, _)) => assert(isEnabled) } + getUserAccountByName(newUser).map { + case (Some(UserAccount(_, _, AccountStatus(isEnabled), _, _))) => assert(isEnabled) + case _ => assert(false) + } getUserAccountByAccessKey(testObject.cred.accessKey).map { u => assert(u.isDefined) val userAccount = u.get diff --git a/src/main/scala/com/ing/wbaa/rokku/sts/api/NpaApi.scala b/src/main/scala/com/ing/wbaa/rokku/sts/api/NpaApi.scala index d3711b1..963234e 100644 --- a/src/main/scala/com/ing/wbaa/rokku/sts/api/NpaApi.scala +++ b/src/main/scala/com/ing/wbaa/rokku/sts/api/NpaApi.scala @@ -43,7 +43,7 @@ trait NpaApi extends LazyLogging with Encryption with JwtToken with TokenGenerat protected[this] def verifyAuthenticationToken(token: BearerToken): Option[AuthenticationUserInfo] - protected[this] def getUserAccountByName(userName: Username): Future[UserAccount] + protected[this] def getUserAccountByName(userName: Username): Future[Option[UserAccount]] protected[this] def registerNpaUser(userName: Username): Future[AwsCredential] @@ -79,11 +79,12 @@ trait NpaApi extends LazyLogging with Encryption with JwtToken with TokenGenerat val npaAccount = keycloakUserInfo.userName authorizeNpa(keycloakUserInfo, keycloakSettings.npaRole) { onComplete(getUserAccountByName(npaAccount)) { - case Success(UserAccount(_, None, _, _, _)) => + // case Success(UserAccount(_, None, _, _, _)) => + case Success(None | Some(UserAccount(_, None, _, _, _))) => val errMsg = s"No credentials were found for user '${npaAccount.value}'" logger.info(errMsg) complete(StatusCodes.NotFound -> errMsg) - case Success(UserAccount(_, Some(awsCredential), AccountStatus(isEnabled), NPA(isNpa), _)) => + case Success(Some(UserAccount(_, Some(awsCredential), AccountStatus(isEnabled), NPA(isNpa), _))) => if (isEnabled && isNpa) { complete(NpaAwsCredentialResponse(awsCredential.accessKey.value, awsCredential.secretKey.value)) } else { diff --git a/src/main/scala/com/ing/wbaa/rokku/sts/service/UserTokenDbService.scala b/src/main/scala/com/ing/wbaa/rokku/sts/service/UserTokenDbService.scala index e5c4823..7ea9a73 100644 --- a/src/main/scala/com/ing/wbaa/rokku/sts/service/UserTokenDbService.scala +++ b/src/main/scala/com/ing/wbaa/rokku/sts/service/UserTokenDbService.scala @@ -15,7 +15,7 @@ trait UserTokenDbService extends LazyLogging with TokenGeneration { implicit protected[this] def executionContext: ExecutionContext - protected[this] def getUserAccountByName(username: Username): Future[UserAccount] + protected[this] def getUserAccountByName(username: Username): Future[Option[UserAccount]] protected[this] def getUserAccountByAccessKey(awsAccessKey: AwsAccessKey): Future[Option[UserAccount]] @@ -193,14 +193,14 @@ trait UserTokenDbService extends LazyLogging with TokenGeneration { private[this] def getOrGenerateAwsCredentialWithStatus(userName: Username): Future[(AwsCredential, AccountStatus)] = getUserAccountByName(userName) .flatMap { - case (UserAccount(_, Some(awsCredential), AccountStatus(isEnabled), _, _)) => + case (Some(UserAccount(_, Some(awsCredential), AccountStatus(isEnabled), _, _))) => if (isEnabled) { Future.successful((awsCredential, AccountStatus(isEnabled))) } else { logger.info(s"User account disabled for ${awsCredential.accessKey}") Future.successful((awsCredential, AccountStatus(isEnabled))) } - case (UserAccount(_, None, _, _, _)) => getNewAwsCredential(userName).map(c => (c, AccountStatus(true))) + case (None | Some(UserAccount(_, None, _, _, _))) => getNewAwsCredential(userName).map(c => (c, AccountStatus(true))) } private[this] def getNewAwsCredential(userName: Username): Future[AwsCredential] = { diff --git a/src/main/scala/com/ing/wbaa/rokku/sts/service/db/dao/STSUserDAO.scala b/src/main/scala/com/ing/wbaa/rokku/sts/service/db/dao/STSUserDAO.scala index e9c1ddf..cc20dff 100644 --- a/src/main/scala/com/ing/wbaa/rokku/sts/service/db/dao/STSUserDAO.scala +++ b/src/main/scala/com/ing/wbaa/rokku/sts/service/db/dao/STSUserDAO.scala @@ -33,8 +33,8 @@ trait STSUserDAO extends LazyLogging with Encryption with Redis with RedisModel * * @param userName The username to search an entry against */ - def getUserAccountByName(username: Username): Future[(UserAccount)] = - withRedisPool[UserAccount] { + def getUserAccountByName(username: Username): Future[Option[UserAccount]] = + withRedisPool[Option[UserAccount]] { client => { Future { @@ -57,8 +57,8 @@ trait STSUserDAO extends LazyLogging with Encryption with Redis with RedisModel groups, ) - userAccount - } else UserAccount(username, None, AccountStatus(false), NPA(false), Set.empty[UserGroup]) + Some(userAccount) + } else None } match { case Success(r) => r case Failure(ex) => diff --git a/src/test/scala/com/ing/wbaa/rokku/sts/api/NpaApiTest.scala b/src/test/scala/com/ing/wbaa/rokku/sts/api/NpaApiTest.scala index 46d3676..40f5022 100644 --- a/src/test/scala/com/ing/wbaa/rokku/sts/api/NpaApiTest.scala +++ b/src/test/scala/com/ing/wbaa/rokku/sts/api/NpaApiTest.scala @@ -54,10 +54,10 @@ class NpaApiTest extends AnyWordSpec case _ => Future.failed(new RuntimeException("Unexpected call of function registerNpaUser")) } - protected[this] def getUserAccountByName(userName: Username): Future[UserAccount] = userName.value match { - case `npaUser` => Future.successful(UserAccount(userName, Some(npaAwsCredential), AccountStatus(true), NPA(true), Set())) - case `nonNpaUser` => Future.successful(UserAccount(userName, Some(npaAwsCredential), AccountStatus(true), NPA(false), Set())) - case `disabledNpaUser` => Future.successful(UserAccount(userName, Some(npaAwsCredential), AccountStatus(false), NPA(true), Set())) + protected[this] def getUserAccountByName(userName: Username): Future[Option[UserAccount]] = userName.value match { + case `npaUser` => Future.successful(Some(UserAccount(userName, Some(npaAwsCredential), AccountStatus(true), NPA(true), Set()))) + case `nonNpaUser` => Future.successful(Some(UserAccount(userName, Some(npaAwsCredential), AccountStatus(true), NPA(false), Set()))) + case `disabledNpaUser` => Future.successful(Some(UserAccount(userName, Some(npaAwsCredential), AccountStatus(false), NPA(true), Set()))) case _ => Future.failed(new RuntimeException("Unexpected call of function getUserAccountByName")) } diff --git a/src/test/scala/com/ing/wbaa/rokku/sts/service/UserTokenDbServiceTest.scala b/src/test/scala/com/ing/wbaa/rokku/sts/service/UserTokenDbServiceTest.scala index 38f913c..b5d354e 100644 --- a/src/test/scala/com/ing/wbaa/rokku/sts/service/UserTokenDbServiceTest.scala +++ b/src/test/scala/com/ing/wbaa/rokku/sts/service/UserTokenDbServiceTest.scala @@ -13,6 +13,7 @@ import scala.concurrent.ExecutionContext import scala.concurrent.Future import scala.concurrent.duration.Duration import scala.util.Random +import scala.collection.immutable.Set class UserTokenDbServiceTest extends AsyncWordSpec with Diagrams { @@ -21,8 +22,8 @@ class UserTokenDbServiceTest extends AsyncWordSpec with Diagrams { override implicit def executionContext: ExecutionContext = testSystem.dispatcher override protected[this] def stsSettings: StsSettings = StsSettings(testSystem) - override protected[this] def getUserAccountByName(userName: Username): Future[UserAccount] = - Future.successful(UserAccount(userName, Some(AwsCredential(AwsAccessKey("a"), AwsSecretKey("s"))), AccountStatus(true), NPA(false), scala.collection.immutable.Set.empty[UserGroup])) + override protected[this] def getUserAccountByName(userName: Username): Future[Option[UserAccount]] = + Future.successful(Some(UserAccount(userName, Some(AwsCredential(AwsAccessKey("a"), AwsSecretKey("s"))), AccountStatus(true), NPA(false), Set.empty[UserGroup]))) override protected[this] def getUserAccountByAccessKey(awsAccessKey: AwsAccessKey): Future[Option[UserAccount]] = Future.successful(Some(UserAccount(Username("u"), Some(AwsCredential(AwsAccessKey("a"), AwsSecretKey("s"))), AccountStatus(true), NPA(false), Set(UserGroup("testGroup"))))) @@ -74,7 +75,8 @@ class UserTokenDbServiceTest extends AsyncWordSpec with Diagrams { "are new credentials" in { val testObject = new TestObject new UserTokenDbServiceTest { - override protected[this] def getUserAccountByName(userName: Username): Future[UserAccount] = Future.successful(UserAccount(userName, None, AccountStatus(false), NPA(false), Set())) + override protected[this] def getUserAccountByName(userName: Username): Future[Option[UserAccount]] = + Future.successful(Some(UserAccount(userName, None, AccountStatus(false), NPA(false), Set()))) }.getAwsCredentialWithToken(testObject.userName, Set.empty[UserGroup], Some(testObject.duration)).map { c => assertExpirationValid(c.session.expiration, testObject.duration) } @@ -84,7 +86,8 @@ class UserTokenDbServiceTest extends AsyncWordSpec with Diagrams { val testObject = new TestObject val utdst = new UserTokenDbServiceTest { - override protected[this] def getUserAccountByName(userName: Username): Future[UserAccount] = Future.successful(UserAccount(userName, None, AccountStatus(false), NPA(false), Set())) + override protected[this] def getUserAccountByName(userName: Username): Future[Option[UserAccount]] = + Future.successful(Some(UserAccount(userName, None, AccountStatus(false), NPA(false), Set()))) override protected[this] def insertAwsCredentials(username: Username, awsCredential: AwsCredential, isNpa: Boolean): Future[Boolean] = Future.successful(false)