Show last_logon_time in userinfo response

- Made last_logon_time nullable so avoid displaying erroneous timestamps in the userinfo response. - last_logon_time now defaults to null in the database - Add LAST_LOGON_TIME to token ClaimConstants [#136373683] https://www.pivotaltracker.com/story/show/136373683 Signed-off-by: Bharath Sekar <bharath.sekar@ge.com>
cloudfoundry · Jan 4, 2017 · 9542d8a · 9542d8a
1 parent 62cbf33
commit 9542d8a
Show file tree

Hide file tree

Showing 15 changed files with 66 additions and 27 deletions.
diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/account/UserInfoResponse.java b/model/src/main/java/org/cloudfoundry/identity/uaa/account/UserInfoResponse.java
@@ -38,6 +38,7 @@
 import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.EMAIL;
 import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.FAMILY_NAME;
 import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.GIVEN_NAME;
+import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.LAST_LOGON_TIME;
 import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.NAME;
 import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.PHONE_NUMBER;
 import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.SUB;
@@ -134,9 +135,18 @@ public Object getAttributeValue(String name) {
     }
 
     public void addAttributes(MultiValueMap<String,Object> attr) {
-        ofNullable(attr).orElse(EMPTY_MAP).entrySet().stream().forEach(e -> setAttributeValues(e.getKey(), e.getValue()));
+        ofNullable(attr).orElse(EMPTY_MAP).entrySet().stream().forEach(
+            e -> setAttributeValues(e.getKey(), e.getValue())
+        );
     }
 
+    public Long getLastLogonSuccess() {
+        return (Long) getAttributeValue(LAST_LOGON_TIME);
+    }
+
+    public void setLastLogonSuccess(Long lastLogonSuccess) {
+        setAttributeValue(LAST_LOGON_TIME, lastLogonSuccess);
+    }
 
 
     public static class UserInfoResponseSerializer extends JsonSerializer<UserInfoResponse> {
@@ -168,6 +178,10 @@ public void serialize(UserInfoResponse object, JsonGenerator gen, SerializerProv
                         }
                         break;
                     }
+                    case LAST_LOGON_TIME:
+                        gen.writeFieldName(key);
+                        gen.writeObject(value.get(0));
+                        break;
                     //multi value fields
                     default:
                         gen.writeFieldName(key);
@@ -205,6 +219,9 @@ public UserInfoResponse deserialize(JsonParser p, DeserializationContext ctxt) t
                         response.setAttributeValue(key, value);
                         break;
                     }
+                    case LAST_LOGON_TIME:
+                        response.setAttributeValue(key, (Long) ((Integer) value).longValue());
+                        break;
                     //multi value fields
                     default:
                         if (value instanceof List) {

diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/ClaimConstants.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/ClaimConstants.java
@@ -52,4 +52,5 @@ public class ClaimConstants {
     public static final String EXTERNAL_ATTR = "ext_attr";
     public static final String AMR = "amr";
     public static final String ACR = "acr";
+    public static final String LAST_LOGON_TIME = "last_logon_time";
 }
diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/account/UserInfoResponseJsonTests.java b/model/src/test/java/org/cloudfoundry/identity/uaa/account/UserInfoResponseJsonTests.java
@@ -23,6 +23,7 @@
 import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.EMAIL;
 import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.FAMILY_NAME;
 import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.GIVEN_NAME;
+import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.LAST_LOGON_TIME;
 import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.NAME;
 import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.PHONE_NUMBER;
 import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.SUB;
@@ -46,7 +47,8 @@ public class UserInfoResponseJsonTests {
         "  \"number\": 123,\n" +
         "  \"origin\": \"uaa\",\n" +
         "  \"zid\": \"uaa\",\n" +
-        "  \"single_value\": \"value3\"\n" +
+        "  \"single_value\": \"value3\",\n" +
+        "  \"last_logon_time\": 1000\n" +
         "}";
 
     @Test
@@ -76,6 +78,9 @@ public void deserializeTest() {
 
         assertEquals("olds", response.getAttributeValue(USER_NAME));
         assertEquals("olds", response.getUsername());
+
+        assertEquals(1000L, response.getAttributeValue(LAST_LOGON_TIME));
+
     }
 
     @Test
@@ -84,4 +89,4 @@ public void serializeTest() {
         json = JsonUtils.writeValueAsString(response);
         deserializeTest();
     }
-}
+}
diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/account/UserInfoEndpoint.java b/server/src/main/java/org/cloudfoundry/identity/uaa/account/UserInfoEndpoint.java
@@ -75,6 +75,7 @@ protected UserInfoResponse getResponse(UaaPrincipal principal, boolean addCustom
         response.setEmail(user.getEmail());
         response.setPhoneNumber(user.getPhoneNumber());
         response.setSub(user.getId());
+        response.setLastLogonSuccess(user.getLastLogonTime());
 
         if (addCustomAttributes) {
             UserInfo info = userDatabase.getUserInfo(user.getId());

diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/user/UaaUser.java b/server/src/main/java/org/cloudfoundry/identity/uaa/user/UaaUser.java
@@ -54,7 +54,7 @@ public class UaaUser {
     private final Date passwordLastModified;
 
     private final String phoneNumber;
-    private long lastLogonTime;
+    private Long lastLogonTime;
 
     public String getZoneId() {
         return zoneId;
@@ -356,11 +356,11 @@ public void setPasswordChangeRequired(boolean passwordChangeRequired) {
         this.passwordChangeRequired = passwordChangeRequired;
     }
 
-    public long getLastLogonTime() {
+    public Long getLastLogonTime() {
         return lastLogonTime;
     }
 
-    public void setLastLogonTime(long lastLogonTime) {
+    public void setLastLogonTime(Long lastLogonTime) {
         this.lastLogonTime = lastLogonTime;
     }
 }
diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/user/UaaUserPrototype.java b/server/src/main/java/org/cloudfoundry/identity/uaa/user/UaaUserPrototype.java
@@ -55,7 +55,7 @@ public final class UaaUserPrototype {
 
     private boolean passwordChangeRequired;
 
-    private long lastLogonTime;
+    private Long lastLogonTime;
 
     public String getId() {
         return id;
@@ -217,11 +217,11 @@ public UaaUserPrototype withPasswordChangeRequired(boolean requiresPasswordChang
         return this;
     }
 
-    public long getLastLogonTime() {
+    public Long getLastLogonTime() {
         return lastLogonTime;
     }
 
-    public UaaUserPrototype withLastLogonSuccess(long lastLogonTime) {
+    public UaaUserPrototype withLastLogonSuccess(Long lastLogonTime) {
         this.lastLogonTime = lastLogonTime;
         return this;
     }

diff --git a/...ain/resources/org/cloudfoundry/identity/uaa/db/hsqldb/V3_10_1__Add_Last_Logon_To_User.sql b/...ain/resources/org/cloudfoundry/identity/uaa/db/hsqldb/V3_10_1__Add_Last_Logon_To_User.sql
@@ -1 +1 @@
-ALTER TABLE users ADD COLUMN last_logon_success_time BIGINT DEFAULT 0 NOT NULL;
+ALTER TABLE users ADD COLUMN last_logon_success_time BIGINT DEFAULT NULL;
diff --git a/...main/resources/org/cloudfoundry/identity/uaa/db/mysql/V3_10_1__Add_Last_Logon_To_User.sql b/...main/resources/org/cloudfoundry/identity/uaa/db/mysql/V3_10_1__Add_Last_Logon_To_User.sql
@@ -1 +1 @@
-ALTER TABLE users ADD COLUMN last_logon_success_time BIGINT DEFAULT 0 NOT NULL;
+ALTER TABLE users ADD COLUMN last_logon_success_time BIGINT DEFAULT NULL;
diff --git a/...resources/org/cloudfoundry/identity/uaa/db/postgresql/V3_10_1__Add_Last_Logon_To_User.sql b/...resources/org/cloudfoundry/identity/uaa/db/postgresql/V3_10_1__Add_Last_Logon_To_User.sql
@@ -1 +1 @@
-ALTER TABLE users ADD COLUMN last_logon_success_time BIGINT DEFAULT 0 NOT NULL;
+ALTER TABLE users ADD COLUMN last_logon_success_time BIGINT DEFAULT NULL;
diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/openid/UserInfoEndpointTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/openid/UserInfoEndpointTests.java
@@ -75,10 +75,10 @@ public class UserInfoEndpointTests {
                     .withVerified(false)
                     .withZoneId(IdentityZoneHolder.get().getId())
                     .withSalt("12345")
-                    .withPasswordLastModified(new Date()));
+                    .withPasswordLastModified(new Date())
+                    .withLastLogonSuccess(1000L));
     private InMemoryUaaUserDatabase userDatabase = new InMemoryUaaUserDatabase(Collections.singleton(user));
     private UserInfo info;
-    private UserInfo stored;
     private OAuth2Request request;
 
     public UserInfoEndpointTests() {
@@ -87,27 +87,42 @@ public UserInfoEndpointTests() {
 
     @Before
     public void setup() {
-        MultiValueMap<String, String> customattributes = new LinkedMultiValueMap<>();
-        customattributes.put(MULTI_VALUE, Arrays.asList("value1", "value2"));
-        customattributes.add(SINGLE_VALUE, "value3");
-        info = new UserInfo(customattributes);
-        stored = userDatabase.storeUserInfo(ID, info);
+        MultiValueMap<String, String> customAttributes = new LinkedMultiValueMap<>();
+        customAttributes.put(MULTI_VALUE, Arrays.asList("value1", "value2"));
+        customAttributes.add(SINGLE_VALUE, "value3");
+        info = new UserInfo(customAttributes);
+        userDatabase.storeUserInfo(ID, info);
     }
 
     @Test
     public void testSunnyDay() {
         UaaUser user = userDatabase.retrieveUserByName("olds", OriginKeys.UAA);
         UaaAuthentication authentication = UaaAuthenticationTestFactory.getAuthentication(user.getId(), "olds",
                         "olds@vmware.com", new HashSet<>(Arrays.asList("openid")));
+
         UserInfoResponse map = endpoint.loginInfo(new OAuth2Authentication(createOauthRequest(Arrays.asList("openid")), authentication));
+
         assertEquals("olds", map.getUsername());
         assertEquals("Dale Olds", map.getFullName());
         assertEquals("olds@vmware.com", map.getEmail());
         assertEquals("8505551234", map.getPhoneNumber());
+        assertEquals(1000, (long) map.getLastLogonSuccess());
         assertEquals(user.getId(), map.getSub());
         assertNull(map.getAttributeValue(USER_ATTRIBUTES));
     }
 
+    @Test
+    public void testSunnyDay_whenLastLogonNull_displaysNull() {
+        user.setLastLogonTime(null);
+        UaaUser user = userDatabase.retrieveUserByName("olds", OriginKeys.UAA);
+        UaaAuthentication authentication = UaaAuthenticationTestFactory.getAuthentication(user.getId(), "olds",
+            "olds@vmware.com", new HashSet<>(Arrays.asList("openid")));
+
+        UserInfoResponse map = endpoint.loginInfo(new OAuth2Authentication(createOauthRequest(Arrays.asList("openid")), authentication));
+
+        assertNull(map.getLastLogonSuccess());
+    }
+
     @Test
     public void testSunnyDay_WithCustomAttributes() {
         UaaUser user = userDatabase.retrieveUserByName("olds", OriginKeys.UAA);

diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/user/InMemoryUaaUserDatabaseTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/user/InMemoryUaaUserDatabaseTests.java
@@ -10,6 +10,7 @@
 import java.util.Date;
 
 import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 
@@ -85,6 +86,6 @@ public void testUpdateUser() throws Exception {
     public void updateLastLogonTime() {
         db.updateLastLogonTime("test-id");
         UaaUser uaaUser = db.retrieveUserById("test-id");
-        assertNotEquals(uaaUser.getLastLogonTime(), 0L);
+        assertNotNull(uaaUser.getLastLogonTime());
     }
 }
diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabaseTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabaseTests.java
@@ -316,12 +316,12 @@ public void testUpdateLastLogonTime() {
         when(timeService.getCurrentTimeMillis()).thenReturn(1000L);
         db.updateLastLogonTime(JOE_ID);
         UaaUser joe = db.retrieveUserById(JOE_ID);
-        assertEquals(joe.getLastLogonTime(), 1000L);
+        assertEquals((long) joe.getLastLogonTime(), 1000L);
 
         when(timeService.getCurrentTimeMillis()).thenReturn(2000L);
         db.updateLastLogonTime(JOE_ID);
         joe = db.retrieveUserById(JOE_ID);
-        assertEquals(joe.getLastLogonTime(), 2000L);
+        assertEquals((long) joe.getLastLogonTime(), 2000L);
     }
 
     @Test(expected = UsernameNotFoundException.class)

diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/DefaultConfigurationTestSuite.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/DefaultConfigurationTestSuite.java
@@ -55,7 +55,6 @@ public static void setUpContextVoid() throws Exception {
         setUpContext();
     }
     public static XmlWebApplicationContext setUpContext() throws Exception {
-        //clearDatabase();
         webApplicationContext = new XmlWebApplicationContext();
         MockEnvironment mockEnvironment = getMockEnvironment();
         webApplicationContext.setEnvironment(mockEnvironment);

diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/UserInfoEndpointDocs.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/UserInfoEndpointDocs.java
@@ -12,15 +12,12 @@
 
 import static org.cloudfoundry.identity.uaa.mock.util.MockMvcUtils.utils;
 import static org.cloudfoundry.identity.uaa.test.SnippetUtils.fieldWithPath;
-import static org.junit.Assert.assertEquals;
 import static org.springframework.restdocs.headers.HeaderDocumentation.headerWithName;
 import static org.springframework.restdocs.headers.HeaderDocumentation.requestHeaders;
 import static org.springframework.restdocs.mockmvc.MockMvcRestDocumentation.document;
 import static org.springframework.restdocs.operation.preprocess.Preprocessors.preprocessResponse;
 import static org.springframework.restdocs.operation.preprocess.Preprocessors.prettyPrint;
 import static org.springframework.restdocs.payload.PayloadDocumentation.responseFields;
-import static org.springframework.restdocs.request.RequestDocumentation.pathParameters;
-import static org.springframework.restdocs.request.RequestDocumentation.requestParameters;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
@@ -70,7 +67,8 @@ public void test_Get_UserInfo() throws Exception {
       fieldWithPath("given_name").description("The user's first name."),
       fieldWithPath("family_name").description("The user's last name."),
       fieldWithPath("name").description("A map with the user's first name and last name."),
-      fieldWithPath("phone_number").description("The user's phone number")
+      fieldWithPath("phone_number").description("The user's phone number."),
+      fieldWithPath("last_logon_time").description("The unix epoch timestamp of when the user last authenticated.")
       );
 
     getMockMvc().perform(

diff --git a/.../test/java/org/cloudfoundry/identity/uaa/scim/endpoints/UserInfoEndpointMockMvcTests.java b/.../test/java/org/cloudfoundry/identity/uaa/scim/endpoints/UserInfoEndpointMockMvcTests.java
@@ -30,6 +30,7 @@
 
 import static org.cloudfoundry.identity.uaa.mock.util.MockMvcUtils.utils;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
@@ -92,6 +93,7 @@ public void get_user_info() throws Exception {
         assertEquals(user.getUserName(), map.get("user_name"));
         assertEquals(user.getFamilyName(), map.get("family_name"));
         assertEquals(user.getGivenName(), map.get("given_name"));
+        assertTrue(System.currentTimeMillis()/1000 - ((long) map.get("last_logon_time"))/1000 <= 5);
     }