From 2ee969466ed26c32641d56104209364d93aad386 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Mon, 22 May 2023 13:54:25 -0700 Subject: [PATCH] SHIBUI-2568 Adding tracking of logins for use in telemetry reporting --- .../ui/security/model/UserLoginRecord.java | 34 ++++++++++++ .../repository/UserLoginRecordRepository.java | 10 ++++ .../ui/security/service/UserService.java | 25 +++++++++ .../springsecurity/AdminUserService.java | 1 + .../admin/ui/AbstractBaseDataJpaTest.groovy | 4 ++ .../security/service/UserServiceTests.groovy | 38 ++++++++++++++ .../unicon/shibui/pac4j/AddNewUserFilter.java | 2 + .../shibui/pac4j/AddNewUserFilterTests.groovy | 52 +++++++++++++++++-- 8 files changed, 163 insertions(+), 3 deletions(-) create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/UserLoginRecord.java create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/UserLoginRecordRepository.java diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/UserLoginRecord.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/UserLoginRecord.java new file mode 100644 index 000000000..75e5fa728 --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/UserLoginRecord.java @@ -0,0 +1,34 @@ +package edu.internet2.tier.shibboleth.admin.ui.security.model; + +import lombok.Data; + +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.Table; +import javax.persistence.UniqueConstraint; +import java.util.Date; + +@Data +@Entity +@Table(uniqueConstraints = { @UniqueConstraint(columnNames = { "username", "login" }) }) +public class UserLoginRecord { + @Id + @GeneratedValue + private Long id; + + private String username; + + private String login; + + private Date loginDate; + + public UserLoginRecord() { + } + + public UserLoginRecord(String username, Date loginDate, String formattedDate) { + this.username = username; + this.login = formattedDate; + this.loginDate = loginDate; + } +} \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/UserLoginRecordRepository.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/UserLoginRecordRepository.java new file mode 100644 index 000000000..b5819bcd5 --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/UserLoginRecordRepository.java @@ -0,0 +1,10 @@ +package edu.internet2.tier.shibboleth.admin.ui.security.repository; + +import edu.internet2.tier.shibboleth.admin.ui.security.model.UserLoginRecord; +import org.springframework.data.jpa.repository.JpaRepository; + +import java.util.Optional; + +public interface UserLoginRecordRepository extends JpaRepository { + Optional findByUsernameAndLogin(String username, String formattedDate); +} \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java index 429dfa6c2..82429ab26 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java @@ -10,8 +10,10 @@ import edu.internet2.tier.shibboleth.admin.ui.security.model.Ownership; 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.model.UserLoginRecord; import edu.internet2.tier.shibboleth.admin.ui.security.repository.OwnershipRepository; import edu.internet2.tier.shibboleth.admin.ui.security.repository.RoleRepository; +import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserLoginRecordRepository; import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository; import lombok.NoArgsConstructor; import org.apache.commons.lang.StringUtils; @@ -21,7 +23,9 @@ import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import java.text.SimpleDateFormat; import java.util.ArrayList; +import java.util.Date; import java.util.HashSet; import java.util.List; import java.util.Optional; @@ -34,6 +38,8 @@ @Service @NoArgsConstructor public class UserService { + private final static SimpleDateFormat DATE_FORMAT = new SimpleDateFormat("dd-MM-yyyy"); + @Autowired private IGroupService groupService; @@ -43,6 +49,9 @@ public class UserService { @Autowired private RoleRepository roleRepository; + @Autowired + private UserLoginRecordRepository userLoginRecordRepository; + @Autowired private UserRepository userRepository; @@ -139,6 +148,8 @@ public Set getUserRoles(String username) { HashSet result = new HashSet<>(); user.ifPresent(value -> value.getRoles().forEach(role -> result.add(role.getName()))); return result; + + } // @TODO - probably delegate this out to something plugable at some point @@ -236,4 +247,18 @@ public void updateUserRole(User user) { public boolean currentUserCanEnable() { return getCurrentUser().getRole().equals("ROLE_ENABLE"); } + + /** + * Ensure there exists a login record for this username and today's date + * @param username + */ + public void updateLoginRecord(String username) { + Date current = new Date(); + String formattedDate = DATE_FORMAT.format(current); + + if (userLoginRecordRepository.findByUsernameAndLogin(username,formattedDate).isEmpty()) { + UserLoginRecord ulr = new UserLoginRecord(username, current, formattedDate); + userLoginRecordRepository.saveAndFlush(ulr); + } + } } \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/springsecurity/AdminUserService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/springsecurity/AdminUserService.java index 34e6ffd06..c7da5f668 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/springsecurity/AdminUserService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/springsecurity/AdminUserService.java @@ -44,6 +44,7 @@ public UserDetails loadUserByUsername(String username) throws UsernameNotFoundEx throw new UsernameNotFoundException(String.format("No roles are defined for user [%s]", username)); } + userService.updateLoginRecord(username); return new org.springframework.security.core.userdetails.User(user.getUsername(), user.getPassword(), grantedAuthorities); } } \ No newline at end of file diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/AbstractBaseDataJpaTest.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/AbstractBaseDataJpaTest.groovy index fda2f6b8c..8db64d5f8 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/AbstractBaseDataJpaTest.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/AbstractBaseDataJpaTest.groovy @@ -10,6 +10,7 @@ import edu.internet2.tier.shibboleth.admin.ui.security.repository.ApproversRepos import edu.internet2.tier.shibboleth.admin.ui.security.repository.GroupsRepository import edu.internet2.tier.shibboleth.admin.ui.security.repository.OwnershipRepository import edu.internet2.tier.shibboleth.admin.ui.security.repository.RoleRepository +import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserLoginRecordRepository import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository import edu.internet2.tier.shibboleth.admin.ui.security.service.GroupServiceForTesting import edu.internet2.tier.shibboleth.admin.ui.security.service.IGroupService @@ -63,6 +64,9 @@ abstract class AbstractBaseDataJpaTest extends Specification implements ResetsDa @Autowired RoleRepository roleRepository + @Autowired + UserLoginRecordRepository userLoginRecordRepository + @Autowired UserRepository userRepository diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/UserServiceTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/UserServiceTests.groovy index 73c56aa91..df99bf0b3 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/UserServiceTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/UserServiceTests.groovy @@ -5,12 +5,16 @@ import edu.internet2.tier.shibboleth.admin.ui.security.model.Group import edu.internet2.tier.shibboleth.admin.ui.security.model.Ownership 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.model.UserLoginRecord + +import java.text.SimpleDateFormat class UserServiceTests extends AbstractBaseDataJpaTest { Role userRole def setup() { userRole = roleRepository.findByName("ROLE_USER").get() + userLoginRecordRepository.deleteAll() } protected createAdminUser() { @@ -163,4 +167,38 @@ class UserServiceTests extends AbstractBaseDataJpaTest { then: gbUpdated.ownedItems.size() == 1 } + + def "when user login - ensure record is created"() { + given: + SimpleDateFormat DATE_FORMAT = new SimpleDateFormat("dd-MM-yyyy") + + Date current = new Date(); + String formattedDate = DATE_FORMAT.format(current) + UserLoginRecord ulr = new UserLoginRecord("username", current, formattedDate) + + expect: + userLoginRecordRepository.findByUsernameAndLogin("username", formattedDate).isEmpty() + userLoginRecordRepository.count() == 0 + + when: + userService.updateLoginRecord("username") + + then: + userLoginRecordRepository.findByUsernameAndLogin("username", formattedDate).isPresent() + userLoginRecordRepository.count() == 1 + + when: 'try adding again should not change result' + userService.updateLoginRecord("username") + + then: + userLoginRecordRepository.findByUsernameAndLogin("username", formattedDate).isPresent() + userLoginRecordRepository.count() == 1 + + when: + userService.updateLoginRecord("username2") + + then: + userLoginRecordRepository.findByUsernameAndLogin("username2", formattedDate).isPresent() + userLoginRecordRepository.count() == 2 + } } \ No newline at end of file diff --git a/pac4j-module/src/main/java/net/unicon/shibui/pac4j/AddNewUserFilter.java b/pac4j-module/src/main/java/net/unicon/shibui/pac4j/AddNewUserFilter.java index b837fb081..d9365ac06 100644 --- a/pac4j-module/src/main/java/net/unicon/shibui/pac4j/AddNewUserFilter.java +++ b/pac4j-module/src/main/java/net/unicon/shibui/pac4j/AddNewUserFilter.java @@ -135,6 +135,8 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha if (user.getRole().equals(ROLE_NONE)) { ((HttpServletResponse) response).sendRedirect("/unsecured/error.html"); } else { + // User exists or has been created and has a role so we can continue on. + userService.updateLoginRecord(username); chain.doFilter(request, response); // else, user is in the system already, carry on } } diff --git a/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterTests.groovy b/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterTests.groovy index d58ecbd4c..2097ccaa1 100644 --- a/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterTests.groovy +++ b/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterTests.groovy @@ -1,16 +1,16 @@ package net.unicon.shibui.pac4j +import edu.internet2.tier.shibboleth.admin.ui.security.model.Group 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.OwnershipRepository import edu.internet2.tier.shibboleth.admin.ui.security.repository.RoleRepository +import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserLoginRecordRepository import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository import edu.internet2.tier.shibboleth.admin.ui.security.service.GroupServiceForTesting +import edu.internet2.tier.shibboleth.admin.ui.security.service.IRolesService import edu.internet2.tier.shibboleth.admin.ui.security.service.RolesServiceImpl import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService -import jakarta.servlet.FilterChain -import jakarta.servlet.http.HttpServletRequest -import jakarta.servlet.http.HttpServletResponse import org.pac4j.core.matching.matcher.PathMatcher import org.pac4j.saml.profile.SAML2Profile import org.springframework.beans.factory.annotation.Autowired @@ -27,6 +27,11 @@ import org.springframework.transaction.annotation.Transactional import spock.lang.Specification import spock.lang.Subject +import javax.servlet.FilterChain +import javax.servlet.http.HttpServletRequest +import javax.servlet.http.HttpServletResponse +import java.text.SimpleDateFormat + @DataJpaTest @ContextConfiguration(classes=[Pac4JTestingConfig]) @EnableJpaRepositories(basePackages = ["edu.internet2.tier.shibboleth.admin.ui"]) @@ -52,6 +57,9 @@ class AddNewUserFilterTests extends Specification { @Autowired Pac4jConfigurationProperties pac4jConfigurationProperties + @Autowired + UserLoginRecordRepository userLoginRecordRepository + @Autowired UserRepository userRepository @@ -99,6 +107,44 @@ class AddNewUserFilterTests extends Specification { roles.each { roleRepository.save(it) } + + Group gb = new Group() + gb.setResourceId("testingGroupBBB") + gb.setName("Group BBB") + gb.setValidationRegex("^(?:https?:\\/\\/)?(?:[^.]+\\.)?shib\\.org(\\/.*)?\$") + gb = groupService.createGroup(gb) + + Optional userRole = roleRepository.findByName("ROLE_USER") + User user = new User(username: "someUser", roles:[userRole.get()], password: "foo") + user.setGroup(gb) + userService.save(user) + } + + def "user logged in"() { + given: + ['Username': 'someUser', + 'FirstName': 'Some', + 'LastName': 'User', + 'Email': 'someUser@institution.edu'].each { key, value -> + saml2Profile.getAttribute(profileMapping."get${key}"()) >> [value] + } + saml2Profile.getUsername() >> "someUser" + + SimpleDateFormat DATE_FORMAT = new SimpleDateFormat("dd-MM-yyyy") + + Date current = new Date(); + String formattedDate = DATE_FORMAT.format(current) + + expect: + userLoginRecordRepository.findByUsernameAndLogin("someUser", formattedDate).isEmpty() + userLoginRecordRepository.count() == 0 + + when: + addNewUserFilter.doFilter(request, response, chain) + + then: + userLoginRecordRepository.findByUsernameAndLogin("someUser", formattedDate).isPresent() + userLoginRecordRepository.count() == 1 } def "new user created"() {