Merge pull request #989 from NDLANO/feat/feide-id-jwt-validation

Rewrite Feide auth to use ID tokens

Author: amatho

article-api/src/main/scala/no/ndla/articleapi/ComponentRegistry.scala

@@ -32,7 +32,6 @@ import no.ndla.common.Clock
 import no.ndla.common.util.TraitUtil
 import no.ndla.database.{DBMigrator, DBUtility, DataSource}
 import no.ndla.network.NdlaClient
-import no.ndla.network.clients.rediscache.FeideRedisClient
 import no.ndla.network.tapir.auth.{FeideAuth, NdlaAuth}
 import no.ndla.network.tapir.{
   ErrorHandling,
@@ -62,14 +61,13 @@ class ComponentRegistry(properties: ArticleApiProperties) extends TapirApplicati
   given traitUtil: TraitUtil                                = new TraitUtil
   given articleRepository: ArticleRepository                = new ArticleRepository
   given converterService: ConverterService                  = new ConverterService
-  given redisClient: FeideRedisClient                       = new FeideRedisClient(props.RedisHost, props.RedisPort)
   given ndlaClient: NdlaClient                              = new NdlaClient
   given searchApiClient: SearchApiClient                    = new SearchApiClient(props.SearchApiUrl)
   given feideApiClient: FeideApiClient                      = new FeideApiClient
   given myndlaApiClient: MyNDLAApiClient                    = new MyNDLAApiClient
   implicit val jwsKeySelectorFactory: JwsKeySelectorFactory = DefaultJwsKeySelectorFactory
-  given ndlaAuth: NdlaAuth                                  = NdlaAuth()
-  given feideAuth: FeideAuth                                = FeideAuth()
+  implicit lazy val ndlaAuth: NdlaAuth                      = NdlaAuth()
+  implicit lazy val feideAuth: FeideAuth                    = FeideAuth()
   given frontpageApiClient: FrontpageApiClient              = new FrontpageApiClient
   given imageApiClient: ImageApiClient                      = new ImageApiClient
   given taxonomyApiClient: TaxonomyApiClient                = new TaxonomyApiClient(props.TaxonomyUrl)

article-api/src/main/scala/no/ndla/articleapi/service/ReadService.scala

@@ -31,7 +31,7 @@ import no.ndla.common.model.domain.article.Article
 import no.ndla.common.model.domain.{ArticleType, Availability}
 import no.ndla.database.DBUtility
 import no.ndla.language.Language
-import no.ndla.network.model.{FeideUserWrapper, userOrAccessDenied}
+import no.ndla.network.model.FeideUserWrapper
 import no.ndla.validation.HtmlTagRules.{jsoupDocumentToString, stringToJsoupDocument}
 import org.jsoup.nodes.Element
 import scalikejdbc.DBSession
@@ -72,7 +72,7 @@ class ReadService(using
         case Some(ArticleRow(_, _, _, _, _, Some(article))) if article.availability == Availability.everyone =>
           Cachable.yes(converterService.toApiArticleV2(article, language, fallback))
         case Some(ArticleRow(_, _, _, _, _, Some(article))) =>
-          val feideUser     = feide.flatMap(_.user)
+          val feideUser     = feide.map(_.user)
           val userIsTeacher = feideUser.exists(_.isTeacher)
           article.availability match {
             case Availability.teacher if !userIsTeacher =>
@@ -223,13 +223,7 @@ class ReadService(using
       shouldScroll: Boolean,
       feide: Option[FeideUserWrapper],
   ): Try[Cachable[SearchResult[ArticleSummaryV2DTO]]] = {
-    val availabilities = feide.map(_.userOrAccessDenied) match {
-      case Some(Success(user)) => user.availabilities
-      case None                => Seq.empty
-      case Some(Failure(_))    =>
-        logger.info("User is not authenticated with Feide, assuming non-user")
-        Seq.empty
-    }
+    val availabilities = feide.fold(Seq.empty)(_.user.availabilities)
 
     val settings = query.emptySomeToNone match {
       case Some(q) => SearchSettings(
@@ -275,11 +269,10 @@ class ReadService(using
     else Cachable.yes(result)
   }
 
-  private def getAvailabilityFilter(feide: Option[FeideUserWrapper]): Option[Availability] =
-    feide.userOrAccessDenied match {
-      case Success(user) if user.isTeacher => None
-      case _                               => Some(Availability.everyone)
-    }
+  private def getAvailabilityFilter(feide: Option[FeideUserWrapper]): Option[Availability] = feide match {
+    case Some(f) if f.user.isTeacher => None
+    case _                           => Some(Availability.everyone)
+  }
 
   private def applyAvailabilityFilter(feide: Option[FeideUserWrapper], articles: Seq[Article]): Seq[Article] = {
     val availabilityFilter = getAvailabilityFilter(feide)

article-api/src/test/scala/no/ndla/articleapi/TestEnvironment.scala

@@ -19,7 +19,6 @@ import no.ndla.common.Clock
 import no.ndla.common.util.TraitUtil
 import no.ndla.database.{DBMigrator, DBUtility, DataSource}
 import no.ndla.network.NdlaClient
-import no.ndla.network.clients.rediscache.FeideRedisClient
 import no.ndla.network.clients.{FeideApiClient, MyNDLAApiClient, SearchApiClient, TaxonomyApiClient}
 import no.ndla.network.tapir.*
 import no.ndla.scalatestsuite.DBUtilityStub
@@ -63,7 +62,6 @@ trait TestEnvironment extends MockitoSugar {
   implicit lazy val e4sClient: NdlaE4sClient                       = mock[NdlaE4sClient]
   implicit lazy val searchApiClient: SearchApiClient               = mock[SearchApiClient]
   implicit lazy val feideApiClient: FeideApiClient                 = mock[FeideApiClient]
-  implicit lazy val redisClient: FeideRedisClient                  = mock[FeideRedisClient]
   implicit lazy val frontpageApiClient: FrontpageApiClient         = mock[FrontpageApiClient]
   implicit lazy val imageApiClient: ImageApiClient                 = mock[ImageApiClient]
   implicit lazy val taxonomyApiClient: TaxonomyApiClient           = mock[TaxonomyApiClient]

article-api/src/test/scala/no/ndla/articleapi/service/ReadServiceTest.scala

@@ -20,7 +20,7 @@ import no.ndla.common.model.api.{FrontPageDTO, MenuDTO}
 import no.ndla.common.model.domain.*
 import no.ndla.common.model.domain.myndla.MyNDLAUser
 import no.ndla.network.clients.FeideExtendedUserInfo
-import no.ndla.network.model.FeideUserWrapper
+import no.ndla.network.model.{FeideIdToken, FeideUserWrapper}
 import org.mockito.ArgumentMatchers.{eq as eqTo, *}
 import org.mockito.Mockito.*
 
@@ -165,7 +165,6 @@ class ReadServiceTest extends UnitSuite with TestEnvironment {
   }
 
   test("that getArticlesByIds doesn't perform filter when every article has availability status everyone") {
-    val feideId  = "asd"
     val ids      = List(1L, 2L, 3L)
     val article1 = TestData.sampleDomainArticle.copy(id = Some(1), availability = Availability.everyone)
     val article2 = TestData.sampleDomainArticle.copy(id = Some(2), availability = Availability.everyone)
@@ -179,8 +178,6 @@ class ReadServiceTest extends UnitSuite with TestEnvironment {
       .getArticlesByIds(articleIds = ids, language = "nb", fallback = true, page = 1, pageSize = 10, feide = None)
       .get
     result.length should be(3)
-
-    verify(feideApiClient, times(0)).getFeideExtendedUser(Some(feideId))
   }
 
   test("that getArticlesByIds performs filter and returns articles that can only be seen by teacher") {
@@ -197,7 +194,7 @@ class ReadServiceTest extends UnitSuite with TestEnvironment {
 
     val userMock = mock[MyNDLAUser]
     when(userMock.isTeacher).thenReturn(true)
-    val feideUserInfo = FeideUserWrapper("test-token", Some(userMock))
+    val feideUserInfo = FeideUserWrapper(userMock, mock[FeideIdToken])
 
     val result = readService
       .getArticlesByIds(
@@ -220,7 +217,7 @@ class ReadServiceTest extends UnitSuite with TestEnvironment {
     val article3 = TestData.sampleDomainArticle.copy(id = Some(3), availability = Availability.teacher)
     val userMock = mock[MyNDLAUser]
     when(userMock.isTeacher).thenReturn(false)
-    val feideUserInfo = FeideUserWrapper("test-token", Some(userMock))
+    val feideUserInfo = FeideUserWrapper(userMock, mock[FeideIdToken])
 
     when(articleRepository.withIds(any, any, any)(using any)).thenReturn(
       Success(Seq(toArticleRow(article1), toArticleRow(article2), toArticleRow(article3)))
@@ -241,7 +238,6 @@ class ReadServiceTest extends UnitSuite with TestEnvironment {
   }
 
   test("that getArticlesByIds performs filter if feideAccessToken is not set") {
-    val feideId  = "asd"
     val ids      = List(1L, 2L, 3L)
     val article1 = TestData.sampleDomainArticle.copy(id = Some(1), availability = Availability.everyone)
     val article2 = TestData.sampleDomainArticle.copy(id = Some(2), availability = Availability.everyone)
@@ -257,8 +253,6 @@ class ReadServiceTest extends UnitSuite with TestEnvironment {
       .get
     result.length should be(2)
     result.map(res => res.availability).contains("teacher") should be(false)
-
-    verify(feideApiClient, times(0)).getFeideAccessTokenOrFail(Some(feideId))
   }
 
   test("that getArticlesByIds fails if no ids were given") {

common/src/main/scala/no/ndla/common/configuration/BaseProps.scala

@@ -150,6 +150,11 @@ trait BaseProps extends StrictLogging {
   val ndlaAuth0LegacyIssuer = s"https://$ndlaAuth0LegacyHost/"
   val ndlaAuth0Audience     = "ndla_system"
 
+  val feideIssuer: String           = "https://auth.dataporten.no"
+  val feideAuthorizationUrl: String = s"$feideIssuer/oauth/authorization"
+  val feideTokenUrl: String         = s"$feideIssuer/oauth/token"
+  def feideClientId: Option[String] = propOrNone("FEIDE_CLIENT_ID")
+
   def MAX_SEARCH_THREADS: Int    = intPropOrDefault("MAX_SEARCH_THREADS", 100)
   def SEARCH_INDEX_SHARDS: Int   = intPropOrDefault("SEARCH_INDEX_SHARDS", 1)
   def SEARCH_INDEX_REPLICAS: Int = intPropOrDefault("SEARCH_INDEX_REPLICAS", 1)

image-api/src/main/scala/no/ndla/imageapi/ComponentRegistry.scala

@@ -23,7 +23,6 @@ import no.ndla.imageapi.service.*
 import no.ndla.imageapi.service.search.*
 import no.ndla.network.NdlaClient
 import no.ndla.network.clients.MyNDLAApiClient
-import no.ndla.network.clients.rediscache.FeideRedisClient
 import no.ndla.network.jwt.{DefaultJwsKeySelectorFactory, JwsKeySelectorFactory}
 import no.ndla.network.tapir.auth.NdlaAuth
 import no.ndla.network.tapir.*
@@ -38,7 +37,6 @@ class ComponentRegistry(properties: ImageApiProperties) extends TapirApplication
 
   implicit lazy val s3Client: NdlaS3Client                  = new NdlaS3Client(props.StorageName, props.StorageRegion)
   implicit lazy val cloudFrontClient: NdlaCloudFrontClient  = new NdlaCloudFrontClient
-  given redisClient: FeideRedisClient                       = new FeideRedisClient(props.RedisHost, props.RedisPort)
   given ndlaClient: NdlaClient                              = new NdlaClient
   implicit lazy val e4sClient: NdlaE4sClient                = Elastic4sClientFactory.getClient(props.SearchServer)
   given searchLanguage: SearchLanguage                      = new SearchLanguage
@@ -47,7 +45,7 @@ class ComponentRegistry(properties: ImageApiProperties) extends TapirApplication
   given converterService: ConverterService                  = new ConverterService
   implicit lazy val myndlaApiClient: MyNDLAApiClient        = new MyNDLAApiClient
   implicit val jwsKeySelectorFactory: JwsKeySelectorFactory = DefaultJwsKeySelectorFactory
-  given ndlaAuth: NdlaAuth                                  = NdlaAuth()
+  implicit lazy val ndlaAuth: NdlaAuth                      = NdlaAuth()
   given searchConverterService: SearchConverterService      = new SearchConverterService
   given dbUtility: DBUtility                                = new DBUtility
   given dbImageMetaInformation: DBImageMetaInformation      = new DBImageMetaInformation
@@ -57,7 +55,7 @@ class ComponentRegistry(properties: ImageApiProperties) extends TapirApplication
   implicit lazy val tagIndexService: TagIndexService        = new TagIndexService
   implicit lazy val tagSearchService: TagSearchService      = new TagSearchService
   given validationService: ValidationService                = new ValidationService
-  given bulkUploadStore: BulkUploadStore                    = new BulkUploadStore
+  given bulkUploadStore: BulkUploadStore                    = BulkUploadStore(props.RedisHost, props.RedisPort)
   given readService: ReadService                            = new ReadService
   implicit lazy val imageStorage: ImageStorageService       = new ImageStorageService
   given writeService: WriteService                          = new WriteService

image-api/src/main/scala/no/ndla/imageapi/service/BulkUploadStore.scala

@@ -11,7 +11,7 @@ package no.ndla.imageapi.service
 import com.typesafe.scalalogging.StrictLogging
 import no.ndla.common.CirceUtil
 import no.ndla.imageapi.model.api.bulk.BulkUploadStateDTO
-import no.ndla.network.clients.rediscache.{FeideRedisClient, RedisStoredType}
+import no.ndla.network.clients.rediscache.{RedisStoredType, ScalaJedis}
 
 import java.util.UUID
 import scala.concurrent.duration.{Duration, DurationInt}
@@ -24,9 +24,7 @@ object BulkUploadType extends RedisStoredType {
   val stateField: String           = "state"
 }
 
-class BulkUploadStore(using redisClient: FeideRedisClient) extends StrictLogging {
-  private val jedis = redisClient.jedis
-
+class BulkUploadStore(jedis: ScalaJedis) extends StrictLogging {
   def get(uploadId: UUID): Try[Option[BulkUploadStateDTO]] = jedis
     .hget(BulkUploadType, uploadId.toString, BulkUploadType.stateField)
     .flatMap {
@@ -37,9 +35,13 @@ class BulkUploadStore(using redisClient: FeideRedisClient) extends StrictLogging
   def set(uploadId: UUID, state: BulkUploadStateDTO): Try[Unit] = {
     val json = CirceUtil.toJsonString(state)
     for {
-      newTtl <- jedis.getNewTTL(BulkUploadType, uploadId.toString)
+      newTtl <- jedis.getFieldNewTtl(BulkUploadType, uploadId.toString, BulkUploadType.stateField)
       _      <- jedis.hset(BulkUploadType, uploadId.toString, BulkUploadType.stateField, json)
-      _      <- jedis.expire(BulkUploadType, uploadId.toString, newTtl)
+      _      <- jedis.hexpire(BulkUploadType, uploadId.toString, BulkUploadType.stateField, newTtl)
     } yield ()
   }
 }
+
+object BulkUploadStore {
+  def apply(host: String, port: Int): BulkUploadStore = new BulkUploadStore(ScalaJedis(host, port))
+}

image-api/src/test/scala/no/ndla/imageapi/e2e/BulkUploadTest.scala

@@ -24,10 +24,10 @@ import no.ndla.imageapi.service.ImageStorageService
 import no.ndla.imageapi.service.search.{ImageIndexService, ImageSearchService, TagIndexService, TagSearchService}
 import no.ndla.imageapi.{ComponentRegistry, ImageApiProperties, MainClass, UnitSuite}
 import no.ndla.network.clients.MyNDLAApiClient
-import no.ndla.network.jwt.JwsKeySelectorFactory
+import no.ndla.network.tapir.auth.NdlaAuth
 import no.ndla.scalatestsuite.{DatabaseIntegrationSuite, RedisIntegrationSuite}
 import no.ndla.search.NdlaE4sClient
-import no.ndla.tapirtesting.{NdlaAuthTestTokens, TestJwsKeySelectorFactory}
+import no.ndla.tapirtesting.{NdlaAuthTest, NdlaAuthTestTokens}
 import org.mockito.ArgumentMatchers.any
 import org.mockito.Mockito.{when, withSettings}
 import org.mockito.quality.Strictness
@@ -73,7 +73,7 @@ class BulkUploadTest extends DatabaseIntegrationSuite with RedisIntegrationSuite
         mock[NdlaCloudFrontClient](withSettings.strictness(Strictness.LENIENT))
       override implicit lazy val myndlaApiClient: MyNDLAApiClient =
         mock[MyNDLAApiClient](withSettings.strictness(Strictness.LENIENT))
-      override implicit val jwsKeySelectorFactory: JwsKeySelectorFactory = TestJwsKeySelectorFactory
+      override implicit lazy val ndlaAuth: NdlaAuth = NdlaAuthTest()
 
       override implicit lazy val imageStorage: ImageStorageService = {
         val m = mock[ImageStorageService](withSettings.strictness(Strictness.LENIENT))

image-api/src/test/scala/no/ndla/imageapi/service/BulkUploadStoreTest.scala

@@ -11,7 +11,7 @@ package no.ndla.imageapi.service
 import no.ndla.common.CirceUtil
 import no.ndla.imageapi.{TestEnvironment, UnitSuite}
 import no.ndla.imageapi.model.api.bulk.{BulkUploadItemDTO, BulkUploadItemStatus, BulkUploadStateDTO, BulkUploadStatus}
-import no.ndla.network.clients.rediscache.{FeideRedisClient, RedisStoredType, ScalaJedis}
+import no.ndla.network.clients.rediscache.{RedisStoredType, ScalaJedis}
 import org.mockito.ArgumentMatchers.{any, eq as eqTo}
 import org.mockito.Mockito.{reset, verify, when}
 
@@ -20,13 +20,8 @@ import java.util.concurrent.atomic.AtomicReference
 import scala.util.{Failure, Success}
 
 class BulkUploadStoreTest extends UnitSuite with TestEnvironment {
-
-  given redisClient: FeideRedisClient = mock[FeideRedisClient]
-  val jedis: ScalaJedis               = mock[ScalaJedis]
-  when(redisClient.jedis).thenReturn(jedis)
-
-  override implicit lazy val bulkUploadStore: BulkUploadStore = new BulkUploadStore
-  private val store: BulkUploadStore                          = bulkUploadStore
+  private val jedis: ScalaJedis      = mock[ScalaJedis]
+  private val store: BulkUploadStore = new BulkUploadStore(jedis)
 
   override def beforeEach(): Unit = reset(jedis)
 
@@ -73,10 +68,13 @@ class BulkUploadStoreTest extends UnitSuite with TestEnvironment {
   test("set writes the encoded state and refreshes the TTL using BulkUploadType.cacheTime") {
     val uploadId    = UUID.randomUUID()
     val expectedTtl = BulkUploadType.cacheTime.toSeconds
-    when(jedis.getNewTTL(eqTo(BulkUploadType), eqTo(uploadId.toString))).thenReturn(Success(expectedTtl))
+    when(jedis.getFieldNewTtl(eqTo(BulkUploadType), eqTo(uploadId.toString), eqTo(BulkUploadType.stateField)))
+      .thenReturn(Success(expectedTtl))
     when(jedis.hset(eqTo(BulkUploadType), eqTo(uploadId.toString), eqTo(BulkUploadType.stateField), any[String]))
       .thenReturn(Success(1L))
-    when(jedis.expire(eqTo(BulkUploadType), eqTo(uploadId.toString), eqTo(expectedTtl))).thenReturn(Success(1L))
+    when(
+      jedis.hexpire(eqTo(BulkUploadType), eqTo(uploadId.toString), eqTo(BulkUploadType.stateField), eqTo(expectedTtl))
+    ).thenReturn(Success(1L))
 
     store.set(uploadId, sampleState) should be(Success(()))
 
@@ -86,19 +84,27 @@ class BulkUploadStoreTest extends UnitSuite with TestEnvironment {
       eqTo(BulkUploadType.stateField),
       eqTo(CirceUtil.toJsonString(sampleState)),
     )
-    verify(jedis).expire(eqTo(BulkUploadType), eqTo(uploadId.toString), eqTo(expectedTtl))
+    verify(jedis).hexpire(
+      eqTo(BulkUploadType),
+      eqTo(uploadId.toString),
+      eqTo(BulkUploadType.stateField),
+      eqTo(expectedTtl),
+    )
   }
 
   test("set followed by get round-trips a state through the cache without losing fields") {
     val uploadId     = UUID.randomUUID()
     val capturedJson = new AtomicReference[String]()
-    when(jedis.getNewTTL(eqTo(BulkUploadType), eqTo(uploadId.toString))).thenReturn(Success(1L))
+    when(jedis.getFieldNewTtl(eqTo(BulkUploadType), eqTo(uploadId.toString), eqTo(BulkUploadType.stateField)))
+      .thenReturn(Success(1L))
     when(jedis.hset(eqTo(BulkUploadType), eqTo(uploadId.toString), eqTo(BulkUploadType.stateField), any[String]))
       .thenAnswer { i =>
         capturedJson.set(i.getArgument[String](3))
         Success(1L)
       }
-    when(jedis.expire(any[RedisStoredType], any[String], any[Long])).thenReturn(Success(1L))
+    when(jedis.hexpire(any[RedisStoredType], any[String], eqTo(BulkUploadType.stateField), any[Long])).thenReturn(
+      Success(1L)
+    )
     when(jedis.hget(eqTo(BulkUploadType), eqTo(uploadId.toString), eqTo(BulkUploadType.stateField))).thenAnswer(_ =>
       Success(Option(capturedJson.get()))
     )