From c5acd8e748896b13359e6b213ae6abbcb60d472b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Mili=C4=87?= Date: Fri, 15 Mar 2024 09:08:48 +0100 Subject: [PATCH 01/10] docfix/Tweak function name and improve a comment --- .../src/main/scala/code/api/util/ConsentUtil.scala | 2 +- obp-api/src/main/scala/code/views/MapperViews.scala | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/obp-api/src/main/scala/code/api/util/ConsentUtil.scala b/obp-api/src/main/scala/code/api/util/ConsentUtil.scala index b8a17d49e..aaa2b75f6 100644 --- a/obp-api/src/main/scala/code/api/util/ConsentUtil.scala +++ b/obp-api/src/main/scala/code/api/util/ConsentUtil.scala @@ -415,7 +415,7 @@ object Consent { // 1. Get or Create a User getOrCreateUser(consent.sub, consent.iss, Some(consent.toConsent().consentId), None, None) map { case (Full(user), newUser) => - // 2. Assign entitlements to the User + // 2. Assign entitlements (Roles) to the User addEntitlements(user, consent) match { case Full(user) => // 3. Copy Auth Context to the User diff --git a/obp-api/src/main/scala/code/views/MapperViews.scala b/obp-api/src/main/scala/code/views/MapperViews.scala index 9b6c46cc7..aa2e6980c 100644 --- a/obp-api/src/main/scala/code/views/MapperViews.scala +++ b/obp-api/src/main/scala/code/views/MapperViews.scala @@ -90,14 +90,14 @@ object MapperViews extends Views with MdcLoggable { Full(Permission(user, getViewsForUser(user))) } // This is an idempotent function - private def getOrGrantAccessToCustomView(user: User, viewDefinition: View, bankId: String, accountId: String): Box[View] = { + private def getOrGrantAccessToViewCommon(user: User, viewDefinition: View, bankId: String, accountId: String): Box[View] = { if (AccountAccess.findByUniqueIndex( BankId(bankId), AccountId(accountId), viewDefinition.viewId, user.userPrimaryKey, ALL_CONSUMERS).isEmpty) { - logger.debug(s"getOrGrantAccessToCustomView AccountAccess.create" + + logger.debug(s"getOrGrantAccessToViewCommon AccountAccess.create" + s"user(UserId(${user.userId}), ViewId(${viewDefinition.viewId.value}), bankId($bankId), accountId($accountId), consumerId($ALL_CONSUMERS)") // SQL Insert AccountAccessList val saved = AccountAccess.create. @@ -115,13 +115,13 @@ object MapperViews extends Views with MdcLoggable { Empty ~> APIFailure("Server error adding permission", 500) //TODO: move message + code logic to api level } } else { - logger.debug(s"getOrGrantAccessToCustomView AccountAccess is already existing (UserId(${user.userId}), ViewId(${viewDefinition.viewId.value}), bankId($bankId), accountId($accountId))") + logger.debug(s"getOrGrantAccessToViewCommon AccountAccess is already existing (UserId(${user.userId}), ViewId(${viewDefinition.viewId.value}), bankId($bankId), accountId($accountId))") Full(viewDefinition) } //accountAccess already exists, no need to create one } // This is an idempotent function private def getOrGrantAccessToSystemView(bankId: BankId, accountId: AccountId, user: User, view: View): Box[View] = { - getOrGrantAccessToCustomView(user, view, bankId.value, accountId.value) + getOrGrantAccessToViewCommon(user, view, bankId.value, accountId.value) } // TODO Accept the whole view as a parameter so we don't have to select it here. def grantAccessToCustomView(viewIdBankIdAccountId: ViewIdBankIdAccountId, user: User): Box[View] = { @@ -136,7 +136,7 @@ object MapperViews extends Views with MdcLoggable { if(v.isPublic && !allowPublicViews) return Failure(PublicViewsNotAllowedOnThisInstance) // SQL Select Count AccountAccessList where // This is idempotent - getOrGrantAccessToCustomView(user, v, viewIdBankIdAccountId.bankId.value, viewIdBankIdAccountId.accountId.value) //accountAccess already exists, no need to create one + getOrGrantAccessToViewCommon(user, v, viewIdBankIdAccountId.bankId.value, viewIdBankIdAccountId.accountId.value) //accountAccess already exists, no need to create one } case _ => { Empty ~> APIFailure(s"View $viewIdBankIdAccountId. not found", 404) //TODO: move message + code logic to api level @@ -168,7 +168,7 @@ object MapperViews extends Views with MdcLoggable { val viewDefinition = v._1 val viewIdBankIdAccountId = v._2 // This is idempotent - getOrGrantAccessToCustomView(user, viewDefinition, viewIdBankIdAccountId.bankId.value, viewIdBankIdAccountId.accountId.value) + getOrGrantAccessToViewCommon(user, viewDefinition, viewIdBankIdAccountId.bankId.value, viewIdBankIdAccountId.accountId.value) }) Full(viewDefinitions.map(_._1)) } From eb8d39e2814b3e864d2805796d5877301cbc08c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Mili=C4=87?= Date: Mon, 18 Mar 2024 11:31:38 +0100 Subject: [PATCH 02/10] docfix/Fix typos --- obp-api/src/main/resources/props/sample.props.template | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/obp-api/src/main/resources/props/sample.props.template b/obp-api/src/main/resources/props/sample.props.template index 1a8952ea3..2ea77fb3d 100644 --- a/obp-api/src/main/resources/props/sample.props.template +++ b/obp-api/src/main/resources/props/sample.props.template @@ -1178,12 +1178,12 @@ webui_developer_user_invitation_email_html_text=\ # List of countries where consent is not required for the collection of personal data personal_data_collection_consent_country_waiver_list = Austria, Belgium, Bulgaria, Croatia, Republic of Cyprus, Czech Republic, Denmark, Estonia, Finland, France, Germany, Greece, Hungary, Ireland, Italy, Latvia, Lithuania, Luxembourg, Malta, Netherlands, Poland, Portugal, Romania, Slovakia, Slovenia, Spain, Sweden, England, Scotland, Wales, Northern Ireland -# Sngle Sign On/Off +# Single Sign On/Off # sso.enabled=false # Local identity provider url # it defaults to the hostname props value -# local_identity_provider=strongly recomended to use top level domain name so that all nodes in the cluster share same provider name +# local_identity_provider=strongly recommended to use top level domain name so that all nodes in the cluster share same provider name # enable dynamic code sandbox, default is false, this will make sandbox works for code running in Future, will make performance lower than disable From 94a59193c073a8f5dee91425f2954fb6da75b83e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Mili=C4=87?= Date: Mon, 18 Mar 2024 15:08:13 +0100 Subject: [PATCH 03/10] bugfix/Function findUserByUsernameLocally MUST use composite key (username, provider) --- .../code/model/dataAccess/AuthUser.scala | 34 ++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/obp-api/src/main/scala/code/model/dataAccess/AuthUser.scala b/obp-api/src/main/scala/code/model/dataAccess/AuthUser.scala index e08a215ac..6d1fc7c9a 100644 --- a/obp-api/src/main/scala/code/model/dataAccess/AuthUser.scala +++ b/obp-api/src/main/scala/code/model/dataAccess/AuthUser.scala @@ -1544,12 +1544,38 @@ def restoreSomeSessions(): Unit = { false } } + + /* + ┌────────────┐ + │FIND A USER │ + │AT MAPPER DB│ + └──────┬─────┘ + ___________▽___________ ┌────────────────────────┐ + ╱ props: ╲ │FIND USER BY COMPOSITE │ + ╱ local_identity_provider ╲______________________│KEY (username, │ + ╲ ╱yes │local_identity_provider)│ + ╲_______________________╱ └────────────┬───────────┘ + │no │ + ___▽____ ┌────────────────────────┐ │ + ╱ props: ╲ │FIND USER BY COMPOSITE │ │ + ╱ hostname ╲___│KEY (username, hostname)│ │ + ╲ ╱yes└────────────┬───────────┘ │ + ╲________╱ │ │ + │no │ │ + ┌──▽──┐ │ │ + │ERROR│ │ │ + └─────┘ │ │ + └──────┬──────────────────┘ + ┌────▽────┐ + │BOX[USER]│ + └─────────┘ + */ /** - * Find the authUser by author user name(authUser and resourceUser are the same). - * Only search for the local database. - */ + * Find the authUser by author user name(authUser and resourceUser are the same). + * Only search for the local database + */ def findUserByUsernameLocally(name: String): Box[TheUserType] = { - find(By(this.username, name)) + find(By(this.username, name), By(this.provider, Constant.localIdentityProvider)) } def passwordResetUrl(name: String, email: String, userId: String): String = { From ffb9919e947ca1342e45d5623afd433d4df8aca2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Mili=C4=87?= Date: Tue, 19 Mar 2024 11:21:51 +0100 Subject: [PATCH 04/10] refactor/Reduce use of the function findAuthUserByUsernameLocally --- .../src/main/scala/code/api/directlogin.scala | 2 +- .../bankconnectors/LocalMappedConnector.scala | 4 +- .../code/model/dataAccess/AuthUser.scala | 146 +++++++++--------- .../test/scala/code/api/DirectLoginTest.scala | 2 +- 4 files changed, 79 insertions(+), 75 deletions(-) diff --git a/obp-api/src/main/scala/code/api/directlogin.scala b/obp-api/src/main/scala/code/api/directlogin.scala index 62b45943c..576ce7511 100644 --- a/obp-api/src/main/scala/code/api/directlogin.scala +++ b/obp-api/src/main/scala/code/api/directlogin.scala @@ -114,7 +114,7 @@ object DirectLogin extends RestHelper with MdcLoggable { def grantEntitlementsToUseDynamicEndpointsInSpacesInDirectLogin(userId:Long) = { try { val resourceUser = UserX.findByResourceUserId(userId).openOrThrowException(s"$InvalidDirectLoginParameters can not find the resourceUser!") - val authUser = AuthUser.findUserByUsernameLocally(resourceUser.name).openOrThrowException(s"$InvalidDirectLoginParameters can not find the auth user!") + val authUser = AuthUser.findAuthUserByPrimaryKey(resourceUser.userPrimaryKey.value).openOrThrowException(s"$InvalidDirectLoginParameters can not find the auth user!") AuthUser.grantEntitlementsToUseDynamicEndpointsInSpaces(authUser) AuthUser.grantEmailDomainEntitlementsToUser(authUser) // User init actions diff --git a/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnector.scala b/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnector.scala index 70d8be346..631bac4fd 100644 --- a/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnector.scala +++ b/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnector.scala @@ -52,7 +52,7 @@ import code.metadata.transactionimages.TransactionImages import code.metadata.wheretags.WhereTags import code.metrics.MappedMetric import code.model._ -import code.model.dataAccess.AuthUser.findUserByUsernameLocally +import code.model.dataAccess.AuthUser.findAuthUserByUsernameLocally import code.model.dataAccess._ import code.productAttributeattribute.MappedProductAttribute import code.productattribute.ProductAttributeX @@ -5793,7 +5793,7 @@ object LocalMappedConnector extends Connector with MdcLoggable { //NOTE: this method is not for mapped connector, we put it here for the star default implementation. // : we call that method only when we set external authentication and provider is not OBP-API override def checkExternalUserExists(username: String, callContext: Option[CallContext]): Box[InboundExternalUser] = { - findUserByUsernameLocally(username).map( user => + findAuthUserByUsernameLocally(username).map(user => InboundExternalUser(aud = "", exp = "", iat = "", diff --git a/obp-api/src/main/scala/code/model/dataAccess/AuthUser.scala b/obp-api/src/main/scala/code/model/dataAccess/AuthUser.scala index 6d1fc7c9a..38e785b22 100644 --- a/obp-api/src/main/scala/code/model/dataAccess/AuthUser.scala +++ b/obp-api/src/main/scala/code/model/dataAccess/AuthUser.scala @@ -591,7 +591,7 @@ import net.liftweb.util.Helpers._ * Overridden to use the hostname set in the props file */ override def sendPasswordReset(name: String) { - findUserByUsernameLocally(name).toList ::: findUsersByEmailLocally(name) map { + findAuthUserByUsernameLocally(name).toList ::: findUsersByEmailLocally(name) map { // reason of case parameter name is "u" instead of "user": trait AuthUser have constant mumber name is "user" // So if the follow case paramter name is "user" will cause compile warnings case u if u.validated_? => @@ -653,14 +653,14 @@ import net.liftweb.util.Helpers._ generateValidationEmailBodies(user, resetLink) ::: (bccEmail.toList.map(BCC(_))) :_* ) } - + def grantDefaultEntitlementsToAuthUser(user: TheUserType) = { tryo{getResourceUserByUsername(user.getProvider(), user.username.get).head.userId} match { case Full(userId)=>APIUtil.grantDefaultEntitlementsToNewUser(userId) case _ => logger.error("Can not getResourceUserByUsername here, so it breaks the grantDefaultEntitlementsToNewUser process.") } } - + override def validateUser(id: String): NodeSeq = findUserByUniqueId(id) match { case Full(user) if !user.validated_? => user.setValidated(true).resetUniqueId().save @@ -668,7 +668,7 @@ import net.liftweb.util.Helpers._ logUserIn(user, () => { S.notice(S.?("account.validated")) APIUtil.getPropsValue("user_account_validated_redirect_url") match { - case Full(redirectUrl) => + case Full(redirectUrl) => logger.debug(s"user_account_validated_redirect_url = $redirectUrl") S.redirectTo(redirectUrl) case _ => @@ -679,7 +679,7 @@ import net.liftweb.util.Helpers._ case _ => S.error(S.?("invalid.validation.link")); S.redirectTo(homePage) } - + override def actionsAfterSignup(theUser: TheUserType, func: () => Nothing): Nothing = { theUser.setValidated(skipEmailValidation).resetUniqueId() theUser.save @@ -737,7 +737,7 @@ import net.liftweb.util.Helpers._ scala.xml.Unparsed(s"""$agreeTermsHtml""") } } - + def agreePrivacyPolicy = { val webUi = new WebUI val privacyPolicyCheckboxText = Helper.i18n("privacy_policy_checkbox_text", Some("I agree to the above Privacy Policy")) @@ -755,7 +755,7 @@ import net.liftweb.util.Helpers._ |
""".stripMargin scala.xml.Unparsed(agreePrivacyPolicy) - } + } def enableDisableSignUpButton = { val javaScriptCode = """