Skip to content

Commit

Permalink
reduce number of keycloak calls for performance
Browse files Browse the repository at this point in the history
  • Loading branch information
sealexan committed Nov 18, 2024
1 parent d54add6 commit 0f1bc8a
Show file tree
Hide file tree
Showing 9 changed files with 285 additions and 95 deletions.
7 changes: 4 additions & 3 deletions src/main/kotlin/ch/uzh/ifi/access/config/CacheConfig.kt
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@ class CacheConfig {
fun cacheManager(): CacheManager {
return ConcurrentMapCacheManager(
"calculateAvgTaskPoints",
"getUserByUsername",
"getUserRoles",
"getStudent",
"getStudentWithPoints"
"getStudentWithPoints",
"userRoles",
"usernameForLogin",
"getAllUserIdsFor"
)
}
}
50 changes: 0 additions & 50 deletions src/main/kotlin/ch/uzh/ifi/access/config/SecurityConfig.kt
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
package ch.uzh.ifi.access.config

import ch.uzh.ifi.access.service.CourseService
import ch.uzh.ifi.access.service.RoleService
import io.github.oshai.kotlinlogging.KotlinLogging
import lombok.AllArgsConstructor
import org.keycloak.admin.client.Keycloak
import org.keycloak.admin.client.resource.RealmResource
import org.springframework.context.ApplicationListener
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.core.env.Environment
import org.springframework.security.authentication.event.AuthenticationSuccessEvent
import org.springframework.security.authorization.AuthorityAuthorizationDecision
import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity
import org.springframework.security.config.annotation.web.builders.HttpSecurity
Expand All @@ -23,14 +19,8 @@ import org.springframework.security.oauth2.jwt.Jwt
import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationToken
import org.springframework.security.web.SecurityFilterChain
import org.springframework.security.web.access.intercept.RequestAuthorizationContext
import org.springframework.stereotype.Component
import org.springframework.web.filter.CommonsRequestLoggingFilter
import java.nio.file.Path
import java.time.Duration
import java.time.Instant
import java.time.LocalDate
import java.time.LocalDateTime
import java.time.ZoneId


@AllArgsConstructor
Expand Down Expand Up @@ -143,43 +133,3 @@ class SecurityConfig(private val env: Environment) {
return keycloakClient.realm("access")
}
}

@Component
class AuthenticationSuccessListener(
val courseService: CourseService,
val roleService: RoleService
) : ApplicationListener<AuthenticationSuccessEvent> {

private val logger = KotlinLogging.logger {}

override fun onApplicationEvent(event: AuthenticationSuccessEvent) {
val username = event.authentication.name
logger.debug { "USER [$username] LOGGED IN" }
roleService.getUserRepresentationForUsername(username)?.let { user ->
// TODO: clean up this horrible mess
val currentAttributes = user.attributes ?: mutableMapOf()
if (user.attributes?.containsKey("roles_synced_at") != true) {
logger.debug { "syncing $username to courses for the first time" }
courseService.updateStudentRoles(username)
}
else {
try {
val lastSync = currentAttributes["roles_synced_at"]!!.first()
val lastSyncInstant = LocalDateTime.parse(lastSync)
val now = LocalDateTime.now()
val diff = Duration.between(lastSyncInstant, now).toMinutes()
if (diff > 10) {
logger.debug { "syncing $username to courses after more than 10min" }
courseService.updateStudentRoles(username)
}
else {
logger.debug { "only $diff minutes elapsed since last sync of $username at ${lastSync} (now: $now)" }
}
} catch (e: Exception) {
logger.debug { "problem ($e, ${e.stackTrace}) with sync calculation; syncing $username to courses anyway" }
courseService.updateStudentRoles(username)
}
}
}
}
}
78 changes: 60 additions & 18 deletions src/main/kotlin/ch/uzh/ifi/access/controller/CourseController.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ import ch.uzh.ifi.access.service.CourseServiceForCaching
import ch.uzh.ifi.access.service.RoleService
import io.github.oshai.kotlinlogging.KotlinLogging
import org.springframework.http.HttpStatus
import org.springframework.scheduling.annotation.EnableAsync
import org.springframework.security.access.prepost.PreAuthorize
import org.springframework.security.core.Authentication
import org.springframework.web.bind.annotation.*
import org.springframework.web.server.ResponseStatusException
import java.time.LocalDateTime
import java.util.concurrent.Semaphore


@RestController
Expand Down Expand Up @@ -75,21 +78,56 @@ class WebhooksController(

@RestController
@RequestMapping("/courses")
@EnableAsync
class CourseController (
private val courseService: CourseService,
private val courseServiceForCaching: CourseServiceForCaching,
private val roleService: RoleService
)
{

private val logger = KotlinLogging.logger {}

@PostMapping("/{course}/pull")
@PreAuthorize("hasRole(#course+'-supervisor')")
fun updateCourse(@PathVariable course: String?): String? {
return courseService.updateCourse(course!!).slug
}

private val semaphore = Semaphore(1)
private fun updateRoles(authentication: Authentication) {
val username = authentication.name
try {
semaphore.acquire()
roleService.getUserRepresentationForUsername(username)?.let { user ->
val attributes = user.attributes ?: mutableMapOf()
if (attributes["roles_synced_at"] == null) {
user.attributes = attributes
roleService.getUserResourceById(user.id).update(user)
val searchNames = buildList {
add(user.username)
user.email?.let { add(it) }
attributes["swissEduIDLinkedAffiliationMail"]?.let { addAll(it) }
attributes["swissEduIDAssociatedMail"]?.let { addAll(it) }
}
val roles = courseService.getUserRoles(searchNames)
roleService.setFirstLoginRoles(user, roles)
logger.debug { "Enrolled first-time user $username in their courses" }
attributes["roles_synced_at"] = listOf(LocalDateTime.now().toString())
}
}
} catch (e: Exception) {
logger.error { "Error looking up user $username: ${e.message}" }
throw e
} finally {
semaphore.release()
logger.debug { "Released semaphore (${semaphore.queueLength} waiting, ${semaphore.availablePermits()} available) for user lookup: $username" }
}
}

@GetMapping("")
fun getCourses(): List<CourseOverview> {
fun getCourses(authentication: Authentication): List<CourseOverview> {
updateRoles(authentication)
return courseService.getCoursesOverview()
}

Expand Down Expand Up @@ -148,28 +186,32 @@ class CourseController (
.filter { it.email != null && it.firstName != null && it.lastName != null }
}

@PostMapping("/{course}/participants")
fun registerParticipants(@PathVariable course: String, @RequestBody students: List<String>) {
// set list of course students
courseService.setStudents(course, students)
// update keycloak roles
roleService.updateStudentRoles(course, students.toSet(),
Role.STUDENT.withCourse(course))

//@Transactional
private fun assignRoles(slug: String, loginNames: List<String>, role: Role) {
val assessment = courseService.getCourseBySlug(slug)
// Saves the list of usernames in the database
val (remove, add) = courseService.setRoleUsers(assessment, loginNames, role)
// Grants the correct role to any existing users in usernames
// This is one of two ways a keycloak user can receive/lose a role, the other way is on first login.
roleService.setRoleUsers(assessment, remove, add, role)
//return courseService.getAssessmentDetailsBySlug(slug)
}

@PostMapping("/{course}/assistants")
//@PreAuthorize("hasRole('supervisor')")
fun registerAssistants(@PathVariable course: String, @RequestBody assistants: List<String>) {
// set list of course students
courseService.setAssistants(course, assistants)
// update keycloak roles
roleService.updateStudentRoles(course, assistants.toSet(),
Role.ASSISTANT.withCourse(course))
//@PreAuthorize("hasAuthority('API_KEY') or hasRole('owner')")
fun registerAssistants(@PathVariable course: String, @RequestBody usernames: List<String>) {
return assignRoles(course, usernames, Role.ASSISTANT)
}

@PostMapping("/{course}/participants")
fun registerParticipants(@PathVariable course: String, @RequestBody participants: List<String>) {
return assignRoles(course, participants, Role.STUDENT)
}

@GetMapping("/{course}/participants/{participant}")
fun getCourseProgress(@PathVariable course: String, @PathVariable participant: String): CourseProgressDTO? {
val user = roleService.getUserByUsername(participant)?:
val user = roleService.getUserRepresentationForUsername(participant)?:
throw ResponseStatusException(HttpStatus.NOT_FOUND, "No participant $participant")
return courseService.getCourseProgress(course, user.username)
}
Expand All @@ -180,7 +222,7 @@ class CourseController (
@PathVariable assignment: String,
@PathVariable participant: String
): AssignmentProgressDTO? {
val user = roleService.getUserByUsername(participant)?:
val user = roleService.getUserRepresentationForUsername(participant)?:
throw ResponseStatusException(HttpStatus.NOT_FOUND, "No participant $participant")
return courseService.getAssignmentProgress(course, assignment, user.username)
}
Expand All @@ -190,7 +232,7 @@ class CourseController (
@PathVariable course: String, @PathVariable assignment: String,
@PathVariable task: String, @PathVariable participant: String
): TaskProgressDTO? {
val user = roleService.getUserByUsername(participant)?:
val user = roleService.getUserRepresentationForUsername(participant)?:
throw ResponseStatusException(HttpStatus.NOT_FOUND, "No participant $participant")
return courseService.getTaskProgress(course, assignment, task, user.username)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ interface AssignmentOverview {
val countDown: List<Timer?>?
val isPastDue: Boolean
val isActive: Boolean
@get:Value("#{@courseService.calculateAssignmentMaxPoints(target.tasks, null)}")
@get:Value("#{@courseService.calculateAssignmentMaxPoints(target.tasks}")
val maxPoints: Double?

@get:Value("#{@courseService.calculateAssignmentPoints(target.tasks, null)}")
@get:Value("#{@courseService.calculateAssignmentPoints(target.tasks)}")
val points: Double?

@get:Value("#{target.tasks.size()}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ interface AssignmentWorkspace {
@get:Value("#{@courseService.calculateAssignmentMaxPoints(target.tasks, null)}")
val maxPoints: Double?

@get:Value("#{@courseService.calculateAssignmentPoints(target.tasks, null)}")
@get:Value("#{@courseService.calculateAssignmentPoints(target.tasks)}")
val points: Double?

@get:Value("#{@courseService.enabledTasksOnly(target.tasks)}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ interface TaskOverview {
@get:Value("#{@courseService.calculateAvgTaskPoints(target.slug)}")
val avgPoints: Double?

@get:Value("#{@courseService.calculateTaskPoints(target.id, target.userId)}")
@get:Value("#{@courseService.calculateTaskPoints(target.id, {target.userId})}")
val points: Double?

@get:Value("#{@courseService.getRemainingAttempts(target.id, target.userId, target.maxAttempts)}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,8 @@ interface CourseRepository : JpaRepository<Course?, Long?> {
)
fun getTotalPoints(courseSlug: String, userIds: Array<String>): Double?

// Bypasses role restrictions, use only if preventing leaks by other means.
// Necessary for retrieving user roles upon first login.
fun findAllUnrestrictedByDeletedFalse(): List<Course>

}
Loading

0 comments on commit 0f1bc8a

Please sign in to comment.