Skip to content

Commit

Permalink
#76 rearchitecting suggestions for easier testability of UserSearchSe…
Browse files Browse the repository at this point in the history
…rvice + added some tests (#102)
  • Loading branch information
dk1844 authored Mar 7, 2024
1 parent 826aae2 commit 489f7aa
Show file tree
Hide file tree
Showing 22 changed files with 384 additions and 164 deletions.
1 change: 0 additions & 1 deletion api/src/main/resources/example.application.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ loginsvc:
#poll-time: 5min
#alg-name: "RS256"
config:
some-key: "BETA"
# Generates git.properties file for use on info endpoint.
git-info:
# Generate Git Information on each run of the application.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,28 +22,33 @@ import org.springframework.context.annotation.{Bean, Configuration}
import org.springframework.security.authentication.{AuthenticationManager, AuthenticationProvider}
import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder
import org.springframework.security.config.annotation.web.builders.HttpSecurity
import za.co.absa.loginsvc.rest.config.auth.{ActiveDirectoryLDAPConfig, DynamicAuthOrder, UsersConfig}
import za.co.absa.loginsvc.rest.config.auth.{ActiveDirectoryLDAPConfig, ConfigOrdering, UsersConfig}
import za.co.absa.loginsvc.rest.config.provider.AuthConfigProvider
import za.co.absa.loginsvc.rest.config.validation.ConfigValidationException
import za.co.absa.loginsvc.rest.provider.ConfigUsersAuthenticationProvider
import za.co.absa.loginsvc.rest.provider.ad.ldap.ActiveDirectoryLDAPAuthenticationProvider

import scala.collection.immutable.SortedMap

/**
* This class registers the authManager bean that is responsible for users to be able to "login" using their credentials
* based on the config
* @param authConfigsProvider
*/
@Configuration
class AuthManagerConfig @Autowired()(authConfigProvider: AuthConfigProvider){
class AuthManagerConfig @Autowired()(authConfigsProvider: AuthConfigProvider){

private val usersConfig: Option[UsersConfig] = authConfigProvider.getUsersConfig
private val adLDAPConfig: Option[ActiveDirectoryLDAPConfig] = authConfigProvider.getLdapConfig
private val usersConfig: Option[UsersConfig] = authConfigsProvider.getUsersConfig
private val adLDAPConfig: Option[ActiveDirectoryLDAPConfig] = authConfigsProvider.getLdapConfig

private val logger = LoggerFactory.getLogger(classOf[AuthManagerConfig])

@Bean
def authManager(http: HttpSecurity): AuthenticationManager = {

val authenticationManagerBuilder = http.getSharedObject(classOf[AuthenticationManagerBuilder]).parentAuthenticationManager(null)
val configs: Array[DynamicAuthOrder] = Array(usersConfig, adLDAPConfig).flatten
val orderedProviders = createProviders(configs)
val configs: Array[ConfigOrdering] = Array(usersConfig, adLDAPConfig).flatten
val orderedProviders = createAuthProviders(configs)

if(orderedProviders.isEmpty)
throw ConfigValidationException("No authentication method enabled in config")
Expand All @@ -55,7 +60,7 @@ class AuthManagerConfig @Autowired()(authConfigProvider: AuthConfigProvider){
authenticationManagerBuilder.build
}

private def createProviders(configs: Array[DynamicAuthOrder]): Array[AuthenticationProvider] = {
private def createAuthProviders(configs: Array[ConfigOrdering]): Array[AuthenticationProvider] = {
Array.empty[AuthenticationProvider] ++ configs.filter(_.order > 0).sortBy(_.order)
.map {
case c: UsersConfig => new ConfigUsersAuthenticationProvider(c)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ case class ActiveDirectoryLDAPConfig(domain: String,
order: Int,
serviceAccount: ServiceAccountConfig,
attributes: Option[Map[String, String]])
extends ConfigValidatable with DynamicAuthOrder
extends ConfigValidatable with ConfigOrdering
{

def throwErrors(): Unit =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@
*/

package za.co.absa.loginsvc.rest.config.auth
trait DynamicAuthOrder {
trait ConfigOrdering {
def order : Int
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import za.co.absa.loginsvc.rest.config.validation.ConfigValidationResult.{Config
import za.co.absa.loginsvc.rest.config.validation.{ConfigValidatable, ConfigValidationException, ConfigValidationResult}

case class UsersConfig(knownUsers: Array[UserConfig], order: Int)
extends ConfigValidatable with DynamicAuthOrder {
extends ConfigValidatable with ConfigOrdering {

lazy val knownUsersMap: Map[String, UserConfig] = knownUsers
.map { entry => (entry.username, entry) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,6 @@ class ConfigProvider(@Value("${spring.config.location}") yamlPath: String)
//GitConfig needs to be initialized at startup
getGitConfig

def getBaseConfig : BaseConfig = {
createConfigClass[BaseConfig]("loginsvc.rest.config").
getOrElse(BaseConfig(""))
}

def getJwtKeyConfig : KeyConfig = {
val inMemoryKeyConfig: Option[InMemoryKeyConfig] = createConfigClass[InMemoryKeyConfig]("loginsvc.rest.jwt.generate-in-memory")
val awsSecretsManagerKeyConfig: Option[AwsSecretsManagerKeyConfig] = createConfigClass[AwsSecretsManagerKeyConfig]("loginsvc.rest.jwt.aws-secrets-manager")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import za.co.absa.loginsvc.model.User
import za.co.absa.loginsvc.rest.config.provider.JwtConfigProvider
import za.co.absa.loginsvc.rest.model.{AccessToken, RefreshToken, Token}
import za.co.absa.loginsvc.rest.service.jwt.JWTService.extractUserFrom
import za.co.absa.loginsvc.rest.service.search.AuthSearchService
import za.co.absa.loginsvc.rest.service.search.UserSearchService

import java.security.interfaces.RSAPublicKey
import java.security.{KeyPair, PublicKey}
Expand All @@ -38,7 +38,7 @@ import scala.compat.java8.DurationConverters._
import scala.concurrent.duration.FiniteDuration

@Service
class JWTService @Autowired()(jwtConfigProvider: JwtConfigProvider, authSearchService: AuthSearchService) {
class JWTService @Autowired()(jwtConfigProvider: JwtConfigProvider, authSearchService: UserSearchService) {

private val logger = LoggerFactory.getLogger(classOf[JWTService])
private val scheduler = Executors.newSingleThreadScheduledExecutor(r => {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright 2023 ABSA Group Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package za.co.absa.loginsvc.rest.service.search

import org.springframework.beans.factory.annotation.Autowired
import org.springframework.stereotype.Component
import za.co.absa.loginsvc.rest.config.auth.{ActiveDirectoryLDAPConfig, ConfigOrdering, UsersConfig}
import za.co.absa.loginsvc.rest.config.provider.AuthConfigProvider
import za.co.absa.loginsvc.rest.config.validation.ConfigValidationException


@Component
class DefaultUserRepositories @Autowired()(authConfigProvider: AuthConfigProvider) extends UserRepositories {

private val usersConfig: Option[ConfigOrdering] = authConfigProvider.getUsersConfig
private val adLDAPConfig: Option[ConfigOrdering] = authConfigProvider.getLdapConfig

private val configs: Seq[ConfigOrdering] = Seq(usersConfig, adLDAPConfig).flatten.filter(_.order != 0).sortBy(_.order)

override val orderedProviders: Seq[UserRepository] = createUserRepositories(configs)

if (orderedProviders.isEmpty)
throw ConfigValidationException("No authentication method enabled in config")

private def createUserRepositories(configs: Seq[ConfigOrdering]): Seq[UserRepository] = {
Array.empty[UserRepository] ++ configs.filter(_.order > 0).sortBy(_.order)
.map {
case c: UsersConfig => new UsersFromConfigRepository(c)
case c: ActiveDirectoryLDAPConfig => new LdapUserRepository(c)
case other => throw new IllegalStateException(s"unsupported config $other")
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ import javax.naming.directory.{Attributes, DirContext, SearchControls, SearchRes
import javax.naming.ldap.{Control, InitialLdapContext, PagedResultsControl}
import scala.collection.JavaConverters.enumerationAsScalaIteratorConverter

class LdapSearchProvider(activeDirectoryLDAPConfig: ActiveDirectoryLDAPConfig)
extends AuthSearchProvider {
class LdapUserRepository(activeDirectoryLDAPConfig: ActiveDirectoryLDAPConfig)
extends UserRepository {

private val logger = LoggerFactory.getLogger(classOf[LdapSearchProvider])
private val logger = LoggerFactory.getLogger(classOf[LdapUserRepository])

def searchForUser(username: String): Option[User] = {
logger.info(s"Searching for user in Ldap: $username")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Copyright 2023 ABSA Group Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package za.co.absa.loginsvc.rest.service.search

trait UserRepositories {

def orderedProviders: Seq[UserRepository]
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ package za.co.absa.loginsvc.rest.service.search

import za.co.absa.loginsvc.model.User

trait AuthSearchProvider {
trait UserRepository {
def searchForUser(username: String): Option[User]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright 2023 ABSA Group Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package za.co.absa.loginsvc.rest.service.search

import org.springframework.beans.factory.annotation.Autowired
import org.springframework.stereotype.Service
import za.co.absa.loginsvc.model.User

@Service
class UserSearchService @Autowired()(userRepositories: UserRepositories) {
def searchUser(username: String): User = {
val maybeFoundUser = userRepositories.orderedProviders.toIterator // this makes it lazy
.map(_.searchForUser(username)) // expensive operation done lazily
.dropWhile(_.isEmpty) // keeps searching until the possibly-found-user is not empty or all providers are exhausted

if (maybeFoundUser.hasNext) {
maybeFoundUser.next().getOrElse(throw new IllegalStateException(s"Valid user $maybeFoundUser should be available"))
} else
throw new NoSuchElementException(s"No user found by username $username anywhere")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ import org.slf4j.LoggerFactory
import za.co.absa.loginsvc.model.User
import za.co.absa.loginsvc.rest.config.auth.UsersConfig

class ConfigSearchProvider(usersConfig: UsersConfig)
extends AuthSearchProvider {
class UsersFromConfigRepository(usersConfig: UsersConfig)
extends UserRepository {

private val logger = LoggerFactory.getLogger(classOf[ConfigSearchProvider])
private val logger = LoggerFactory.getLogger(classOf[UsersFromConfigRepository])
def searchForUser(username: String): Option[User] = {
logger.info(s"Searching for user in config: $username")
usersConfig.knownUsersMap.get(username).flatMap { userConfig =>
Expand Down
2 changes: 0 additions & 2 deletions api/src/test/resources/application.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ loginsvc:
refresh-exp-time: 10h
key-rotation-time: 5sec
alg-name: "RS256"
config:
some-key: "BETA"

# Rest Auth Config (AD)
auth:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,6 @@ class ConfigProviderTest extends AnyFlatSpec with Matchers {

private val configProvider : ConfigProvider = new ConfigProvider("api/src/test/resources/application.yaml")

"The baseConfig properties" should "Match" in {
val baseConfig: BaseConfig = configProvider.getBaseConfig
baseConfig.someKey shouldBe "BETA"
}

"The jwtConfig properties" should "Match" in {
val keyConfig: KeyConfig = configProvider.getJwtKeyConfig
keyConfig.algName shouldBe "RS256"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import za.co.absa.loginsvc.model.User
import za.co.absa.loginsvc.rest.config.jwt.{InMemoryKeyConfig, KeyConfig}
import za.co.absa.loginsvc.rest.config.provider.{ConfigProvider, JwtConfigProvider}
import za.co.absa.loginsvc.rest.model.{AccessToken, RefreshToken, Token}
import za.co.absa.loginsvc.rest.service.search.AuthSearchService
import za.co.absa.loginsvc.rest.service.search.{DefaultUserRepositories, UserSearchService}

import java.security.PublicKey
import java.util
Expand All @@ -38,7 +38,7 @@ class JWTServiceTest extends AnyFlatSpec with BeforeAndAfterEach with Matchers {

private val testConfig : ConfigProvider = new ConfigProvider("api/src/test/resources/application.yaml")
private var jwtService: JWTService = _
private var authSearchService: AuthSearchService = _
private var authSearchService: UserSearchService = _

private val userWithoutEmailAndGroups: User = User(
name = "user2",
Expand All @@ -59,7 +59,7 @@ class JWTServiceTest extends AnyFlatSpec with BeforeAndAfterEach with Matchers {
}

override def beforeEach(): Unit = {
authSearchService = new AuthSearchService(testConfig)
authSearchService = new UserSearchService(new DefaultUserRepositories(testConfig))
jwtService = new JWTService(testConfig, authSearchService)
}

Expand Down
Loading

0 comments on commit 489f7aa

Please sign in to comment.