Skip to content

Commit

Permalink
[SHIBUI-1030]
Browse files Browse the repository at this point in the history
Modified the User<->Role relationship, removing Cascade completely.
Added eager fetching to roles w/in users, lazy to users w/in roles.
Modified DevConfig to create roles first, then create users.

Replaced existing email-sending unit test with one that checks the
results from MailHog. It is a bit messy due to all of the junk that ends
up in the email json that Mailhog returns that isn't the same each time.
  • Loading branch information
Bill Smith committed Jan 17, 2019
1 parent e74abe9 commit d5d7ad7
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.ReloadableMetadat
import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository
import edu.internet2.tier.shibboleth.admin.ui.security.model.Role
import edu.internet2.tier.shibboleth.admin.ui.security.model.User
import edu.internet2.tier.shibboleth.admin.ui.security.repository.RoleRepository
import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository
import edu.internet2.tier.shibboleth.admin.util.ModelRepresentationConversions
import org.springframework.context.annotation.Bean
Expand All @@ -24,39 +25,65 @@ import javax.annotation.PostConstruct
@Profile('dev')
class DevConfig {
private final UserRepository adminUserRepository
private final RoleRepository roleRepository

private final MetadataResolverRepository metadataResolverRepository

DevConfig(UserRepository adminUserRepository, MetadataResolverRepository metadataResolverRepository) {
DevConfig(UserRepository adminUserRepository, MetadataResolverRepository metadataResolverRepository, RoleRepository roleRepository) {
this.adminUserRepository = adminUserRepository
this.metadataResolverRepository = metadataResolverRepository
this.roleRepository = roleRepository
}

@Transactional
@PostConstruct
void createDevUsers() {
if (roleRepository.count() == 0) {
def roles = [new Role().with {
name = 'ROLE_ADMIN'
it
}, new Role().with {
name = 'ROLE_USER'
it
}, new Role().with {
name = 'ROLE_NONE'
it
}]
roles.each {
roleRepository.save(it)
}
}
roleRepository.flush()
if (adminUserRepository.count() == 0) {
def users = [new User().with {
username = 'admin'
password = '{noop}adminpass'
firstName = 'Joe'
lastName = 'Doe'
emailAddress = 'joe@institution.edu'
roles.add(new Role(name: 'ROLE_ADMIN'))
roles.add(roleRepository.findByName('ROLE_ADMIN').get())
it
}, new User().with {
username = 'nonadmin'
password = '{noop}nonadminpass'
firstName = 'Peter'
lastName = 'Vandelay'
emailAddress = 'peter@institution.edu'
roles.add(new Role(name: 'ROLE_USER'))
roles.add(roleRepository.findByName('ROLE_USER').get())
it
}, new User().with {
username = 'admin2'
password = '{noop}anotheradmin'
firstName = 'Rand'
lastName = 'al\'Thor'
emailAddress = 'rand@institution.edu'
roles.add(roleRepository.findByName('ROLE_ADMIN').get())
it
}]
users.each {
adminUserRepository.save(it)
}

adminUserRepository.flush()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public Role(String name, int rank) {

//Ignore properties annotation here is to prevent stack overflow recursive error during JSON serialization
@JsonIgnoreProperties("roles")
@ManyToMany(mappedBy = "roles", fetch = FetchType.EAGER)
@ManyToMany(mappedBy = "roles", fetch = FetchType.LAZY)
private Set<User> users = new HashSet<>();

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
import lombok.ToString;
import org.apache.commons.lang.StringUtils;

import javax.persistence.CascadeType;
import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.FetchType;
import javax.persistence.JoinColumn;
import javax.persistence.JoinTable;
import javax.persistence.ManyToMany;
Expand Down Expand Up @@ -50,7 +50,7 @@ public class User extends AbstractAuditable {
private String role;

@JsonIgnore
@ManyToMany(cascade = CascadeType.PERSIST)
@ManyToMany(fetch = FetchType.EAGER)
@JoinTable(name = "user_role", joinColumns = @JoinColumn(name = "user_id"), inverseJoinColumns = @JoinColumn(name = "role_id"))
private Set<Role> roles = new HashSet<>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@
public interface EmailService {
void sendMail(String emailTemplate, String fromAddress, String[] recipients, String subject, Locale locale) throws MessagingException;
void sendNewUserMail(String newUsername) throws MessagingException;
String[] getSystemAdminEmailAddresses();
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ public void sendMail(String emailTemplate, String fromAddress, String[] recipien

public void sendNewUserMail(String newUsername) throws MessagingException {
String subject = String.format("User Access Request for %s", newUsername);
sendMail("new-user", systemEmailAddress, getSystemAdmins(), subject, Locale.getDefault());
sendMail("new-user", systemEmailAddress, getSystemAdminEmailAddresses(), subject, Locale.getDefault());
}

private String[] getSystemAdmins() {
public String[] getSystemAdminEmailAddresses() {
Set<User> systemAdmins = userRepository.findByRoles_Name("ROLE_ADMIN");
if (systemAdmins == null || systemAdmins.size() == 0) {
//TODO: Should this be an exception?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class TestConfiguration {
@Bean
JavaMailSender javaMailSender() {
return new JavaMailSenderImpl().with {
it.host = 'mailhog'
it.host = 'localhost'
it.port = 1025
it
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@ import edu.internet2.tier.shibboleth.admin.ui.configuration.EmailConfiguration
import edu.internet2.tier.shibboleth.admin.ui.configuration.InternationalizationConfiguration
import edu.internet2.tier.shibboleth.admin.ui.configuration.SearchConfiguration
import edu.internet2.tier.shibboleth.admin.ui.configuration.TestConfiguration
import groovy.json.JsonOutput
import groovy.json.JsonSlurper
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.boot.autoconfigure.domain.EntityScan
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest
import org.springframework.boot.test.context.SpringBootTest
import org.springframework.core.env.Environment
import org.springframework.data.jpa.repository.config.EnableJpaRepositories
import org.springframework.test.context.ActiveProfiles
import org.springframework.test.context.ContextConfiguration
Expand All @@ -30,13 +33,38 @@ class EmailServiceImplTests extends Specification {
@Autowired
EmailService emailService

@Autowired
Environment env

JsonSlurper jsonSlurper = new JsonSlurper()

// Ignoring until we can figure out how to get this to pass on Jenkins
@Ignore
def "emailService can successfully send an email"() {
given:

def mailhogHost = 'localhost'
def mailhogPort = '8025'

def newUserName = 'foobar'
def expectedToAddresses = emailService.systemAdminEmailAddresses
def expectedFromAddress = env.getProperty('shibui.mail.system-email-address')
def expectedNewUserEmailSubject = "User Access Request for ${newUserName}"
def expectedTextEmailBody = new File(this.class.getResource('/mail/text/new-user.txt').toURI()).text

when:
emailService.sendNewUserMail("foobar")
def getEmailsURL = new URL("http://${mailhogHost}:${mailhogPort}/api/v2/messages")
def resultJson = jsonSlurper.parse(getEmailsURL)
println(JsonOutput.prettyPrint(JsonOutput.toJson(resultJson)))

then:
noExceptionThrown()
// There's a bunch of other noise in here that changes per email
resultJson.total == 1
expectedToAddresses.size() == resultJson.items[0].To.size()
expectedToAddresses.join(', ') == resultJson.items[0].Content.Headers.To[0]
expectedFromAddress == resultJson.items[0].From.Mailbox + '@' + resultJson.items[0].From.Domain
expectedNewUserEmailSubject == resultJson.items[0].Content.Headers.Subject[0]
resultJson.items[0].Content.Body.contains(expectedTextEmailBody)
}
}

0 comments on commit d5d7ad7

Please sign in to comment.