Skip to content

Commit

Permalink
Pivotal ID # 185110100: Case Insensitive Submission Ownership (#722)
Browse files Browse the repository at this point in the history
* Pivotal ID # 185110100: Case Insensitive Submission Ownership

- Ensure user email is always lowercase on registration
- Remove unnecessary data classes
  • Loading branch information
jhoanmanuelms authored Jul 5, 2023
1 parent 5f3a6f7 commit 7d9afb2
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 33 deletions.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,10 @@ class RegisterRequest(
@JsonProperty("recaptcha2-response")
val captcha: String? = null
)

class CheckUserRequest(
@field:Email(message = "The provided email is not valid")
val userEmail: String,

val userName: String,
)
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class SecurityQueryService(

private fun createUserInactive(email: String, username: String): DbUser {
val user = DbUser(
email = email,
email = email.lowercase(),
fullName = username,
secret = securityUtil.newKey(),
storageMode = securityProperties.filesProperties.defaultMode,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ open class SecurityService(
}

private fun asUser(registerRequest: RegisterRequest) = DbUser(
email = registerRequest.email,
email = registerRequest.email.lowercase(),
fullName = registerRequest.name,
orcid = registerRequest.orcid,
secret = securityUtil.newKey(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,26 @@ package ac.uk.ebi.biostd.itest.test.security
import ac.uk.ebi.biostd.client.exception.WebClientException
import ac.uk.ebi.biostd.client.integration.web.SecurityWebClient
import ac.uk.ebi.biostd.itest.entities.TestUser
import ac.uk.ebi.biostd.persistence.repositories.UserDataRepository
import ebi.ac.uk.api.security.CheckUserRequest
import ebi.ac.uk.api.security.LoginRequest
import ebi.ac.uk.api.security.RegisterRequest
import org.assertj.core.api.Assertions.assertThat
import org.assertj.core.api.Assertions.assertThatExceptionOfType
import org.junit.jupiter.api.BeforeAll
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.boot.test.context.SpringBootTest
import org.springframework.boot.test.web.server.LocalServerPort
import org.springframework.test.context.junit.jupiter.SpringExtension
import kotlin.test.assertNotNull

@ExtendWith(SpringExtension::class)
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
class SecurityApiTest(
@LocalServerPort val serverPort: Int,
@Autowired private val userDataRepository: UserDataRepository,
) {
private lateinit var webClient: SecurityWebClient

Expand Down Expand Up @@ -49,6 +54,26 @@ class SecurityApiTest(
.withMessageContaining("Authentication Error: Invalid email address or password")
}

@Test
fun `22-4 case insensitive user registration`() {
val request = RegisterRequest("User", "Case-Insensitive@mail.org", "123")
webClient.registerUser(request)

val user = userDataRepository.findByEmailAndActive("Case-Insensitive@mail.org", true)
assertNotNull(user)
assertThat(user.email).isEqualTo("case-insensitive@mail.org")
}

@Test
fun `22-5 case insensitive inactive registration`() {
val request = CheckUserRequest("Case-Insensitive-Inactive@mail.org", "User")
webClient.checkUser(request)

val user = userDataRepository.findByEmailAndActive("Case-Insensitive-Inactive@mail.org", false)
assertNotNull(user)
assertThat(user.email).isEqualTo("case-insensitive-inactive@mail.org")
}

object NewUser : TestUser {
override val username = "New User"
override val email = "new-biostudies-mgmt@ebi.ac.uk"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,13 @@
| SecurityApiTest | 22-1 | register with invalid email | Shows registration user behaviour |
| SecurityApiTest | 22-2 | register when activation is not enable | |
| SecurityApiTest | 22-3 | login when inactive | |
| SecurityApiTest | 22-4 | case insensitive user registration | |
| SecurityApiTest | 22-5 | case insensitive inactive registration | |
| SubmissionDraftListApiTest | 23-1 | get draft by key | Shows how to get drafts, paginated or not. |
| SubmissionDraftListApiTest | 23-2 | get drafts without pagination | |
| SubmissionDraftListApiTest | 23-3 | get drafts with pagination | |
| UserGroupsApiTest | 24-1 | get user groups | Shows belonging behaviour of users in groups, and exceptions |
| UserGroupsApiTest | 24-2 | trying to add a user to unexisting group | |
| UserGroupsApiTest | 24-2 | trying to add a user to un-existing group | |
| UserGroupsApiTest | 24-3 | trying to add a user that does not exist | |
| UserGroupsApiTest | 24-4 | trying to add a user by regularUser | |
| SubmissionRefreshApiTest | 25-1 | Refresh when submission title is updated | |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ class SecurityResource(

@PostMapping(value = ["/check-registration"])
@ResponseBody
fun checkUser(@Valid @RequestBody register: CheckUserRequest): SecurityUser =
securityQueryService.getOrCreateInactive(register.userEmail, register.userName)
fun checkUser(@Valid @RequestBody register: CheckUserRequest): SecurityUser {
return securityQueryService.getOrCreateInactive(register.userEmail, register.userName)
}

@PostMapping(value = ["/signin", "/login"])
@ResponseBody
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
package ac.uk.ebi.biostd.submission.domain.helpers

import ac.uk.ebi.biostd.submission.web.model.OnBehalfRequest
import ebi.ac.uk.api.security.GetOrRegisterUserRequest
import ebi.ac.uk.base.orFalse
import ebi.ac.uk.security.integration.components.ISecurityQueryService
import ebi.ac.uk.security.integration.model.api.SecurityUser

class OnBehalfUtils(
private val securityQueryService: ISecurityQueryService,
) {
fun getOnBehalfUser(onBehalfRequest: OnBehalfRequest): SecurityUser {
val request = onBehalfRequest.asRegisterRequest()
return if (request.register) registerInactive(request) else securityQueryService.getUser(request.userEmail)
fun getOnBehalfUser(request: OnBehalfRequest): SecurityUser {
val register = request.register.orFalse()
return if (register) registerInactive(request) else securityQueryService.getUser(request.userEmail)
}

private fun registerInactive(registerRequest: GetOrRegisterUserRequest): SecurityUser {
private fun registerInactive(registerRequest: OnBehalfRequest): SecurityUser {
requireNotNull(registerRequest.userName) { "A valid user name must be provided for registration" }
return securityQueryService.getOrCreateInactive(registerRequest.userEmail, registerRequest.userName!!)
return securityQueryService.getOrCreateInactive(registerRequest.userEmail, registerRequest.userName)
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package ac.uk.ebi.biostd.submission.web.model

import ac.uk.ebi.biostd.integration.SubFormat
import ebi.ac.uk.api.security.GetOrRegisterUserRequest
import ebi.ac.uk.base.orFalse
import ebi.ac.uk.extended.model.ExtAttributeDetail
import ebi.ac.uk.extended.model.StorageMode
import ebi.ac.uk.io.sources.PreferredSource
Expand All @@ -11,13 +9,10 @@ import ebi.ac.uk.security.integration.model.api.SecurityUser
import java.io.File

class OnBehalfRequest(
private val userEmail: String,
private val userName: String?,
private val register: Boolean?,
) {
fun asRegisterRequest(): GetOrRegisterUserRequest =
GetOrRegisterUserRequest(register.orFalse(), userEmail, userName)
}
val userEmail: String,
val userName: String?,
val register: Boolean?,
)

data class SubmissionRequestParameters(
val preferredSources: List<PreferredSource> = emptyList(),
Expand Down

0 comments on commit 7d9afb2

Please sign in to comment.