Skip to content

#23 Add: Feature: Track User Account Creation and Last Login Timestamps #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Airat102vb wants to merge 1 commit into
base: main
Choose a base branch
from
Open

#23 Add: Feature: Track User Account Creation and Last Login Timestamps #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/main/java/com/fredmaina/chatapp/Auth/Dtos/LoginRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@

import lombok.Data;

import java.time.Instant;

@Data
public class LoginRequest {
private String username;
private String password;
private final Instant lastLoginAt = Instant.now();
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lastLoginAt field should not be included in the LoginRequest DTO. This is a security and design issue because:

  1. Security: Clients should not be able to control server-side timestamps - this could allow timestamp manipulation
  2. Design: Login timestamps should be set by the server at the time of successful authentication, not by the client
  3. Inconsistency: The handleGoogleOAuth method correctly sets lastLoginAt server-side using Instant.now() (line 207 of AuthService)

Remove this field entirely from LoginRequest. The server should set the timestamp in the login() method using Instant.now().

Suggested change
private final Instant lastLoginAt = Instant.now();

Copilot uses AI. Check for mistakes.
}
5 changes: 5 additions & 0 deletions src/main/java/com/fredmaina/chatapp/Auth/Dtos/UserDto.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import lombok.Getter;
import lombok.Setter;

import java.time.Instant;
import java.util.UUID;

@Getter
Expand All @@ -18,6 +19,8 @@ public class UserDto {
private String email;
private String username;
private Role role;
private Instant createdAt;
private Instant lastLoginAt;

public UserDto(User user) {
this.userId = user.getId();
Expand All @@ -26,5 +29,7 @@ public UserDto(User user) {
this.email = user.getEmail();
this.username = user.getUsername();
this.role = user.getRole();
this.createdAt = user.getCreatedAt();
this.lastLoginAt = user.getLastLoginAt();
}
}
9 changes: 9 additions & 0 deletions src/main/java/com/fredmaina/chatapp/Auth/Models/User.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
import jakarta.persistence.*;
import lombok.Getter;
import lombok.Setter;
import org.hibernate.annotations.CreationTimestamp;

import java.time.Instant;
import java.util.UUID;

@Entity
Expand Down Expand Up @@ -37,5 +39,12 @@ public class User {
@Enumerated(EnumType.STRING)
private Role role;

@CreationTimestamp
@Column(name = "created_at", nullable = false, updatable = false)
private Instant createdAt;

@Column(name = "last_login_at", nullable = false)
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lastLoginAt field is marked as nullable = false, but this is incorrect because:

  1. Users won't have a last login timestamp at the moment of registration (only after their first login)
  2. The database migration on line 2 of V8 correctly makes this column nullable
  3. Existing users from OAuth registration don't have this field set until they login

Change this to nullable = true to match the database schema and the actual behavior:

@Column(name = "last_login_at", nullable = true)
Suggested change
@Column(name = "last_login_at", nullable = false)
@Column(name = "last_login_at", nullable = true)

Copilot uses AI. Check for mistakes.
private Instant lastLoginAt;

private boolean verified;
}
17 changes: 11 additions & 6 deletions src/main/java/com/fredmaina/chatapp/Auth/controllers/AuthController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package com.fredmaina.chatapp.Auth.controllers;


import com.fredmaina.chatapp.Auth.Dtos.*;
import com.fredmaina.chatapp.Auth.Dtos.AuthResponse;
import com.fredmaina.chatapp.Auth.Dtos.GoogleOAuthRequest;
import com.fredmaina.chatapp.Auth.Dtos.LoginRequest;
import com.fredmaina.chatapp.Auth.Dtos.SignUpRequest;
import com.fredmaina.chatapp.Auth.Models.User;
import com.fredmaina.chatapp.Auth.Repositories.UserRepository;
import com.fredmaina.chatapp.Auth.services.AuthService;
Expand Down Expand Up @@ -32,7 +35,7 @@ public class AuthController {
@PostMapping("/login")
public ResponseEntity<AuthResponse> login(@RequestBody LoginRequest loginRequest) {
AuthResponse authResponse = authService.login(loginRequest);
if(authResponse.isSuccess()){
if (authResponse.isSuccess()) {
return ResponseEntity.ok(authResponse);
}
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(authResponse);
Expand All @@ -42,14 +45,15 @@ public ResponseEntity<AuthResponse> login(@RequestBody LoginRequest loginRequest
@PostMapping("/register")
public ResponseEntity<AuthResponse> register(@Valid @RequestBody SignUpRequest signUpRequest) {
AuthResponse authResponse = authService.signUp(signUpRequest);
if(authResponse.isSuccess()){
if (authResponse.isSuccess()) {
return ResponseEntity.status(HttpStatus.CREATED).body(authResponse);
}
if ("Username already exists (case-insensitive)".equals(authResponse.getMessage()) || "Email already exists".equals(authResponse.getMessage())) {
return ResponseEntity.status(HttpStatus.CONFLICT).body(authResponse);
}
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(authResponse); // General bad request for other issues
}

@PostMapping("/oauth/google")
public ResponseEntity<?> googleOAuth(@RequestBody GoogleOAuthRequest request) {
AuthResponse response = authService.handleGoogleOAuth(request.getCode(), request.getRedirectUri());
Expand Down Expand Up @@ -81,8 +85,9 @@ public ResponseEntity<AuthResponse> me(@RequestHeader("Authorization") String au
.user(user)
.build());
}

@PostMapping("/set-username")
public ResponseEntity<AuthResponse> setUsername(@RequestBody Map<String,String> map) {
public ResponseEntity<AuthResponse> setUsername(@RequestBody Map<String, String> map) {
String email = map.get("email");
String username = map.get("username");

Expand All @@ -91,8 +96,8 @@ public ResponseEntity<AuthResponse> setUsername(@RequestBody Map<String,String>
.body(AuthResponse.builder().success(false).message("Email and username are required.").build());
}

AuthResponse authResponse = authService.setUsername(email,username);
if(authResponse.isSuccess()){
AuthResponse authResponse = authService.setUsername(email, username);
if (authResponse.isSuccess()) {
return ResponseEntity.ok(authResponse);
}
// Distinguish between user not found and username taken
Expand Down
16 changes: 11 additions & 5 deletions src/main/java/com/fredmaina/chatapp/Auth/services/AuthService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@


import com.fredmaina.chatapp.Auth.Dtos.AuthResponse;
import com.fredmaina.chatapp.Auth.Dtos.GoogleOAuthRequest;
import com.fredmaina.chatapp.Auth.Dtos.LoginRequest;
import com.fredmaina.chatapp.Auth.Dtos.SignUpRequest;
import com.fredmaina.chatapp.Auth.Models.Role;
Expand All @@ -15,14 +14,13 @@
import org.springframework.cache.annotation.Cacheable;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.http.*;
import org.springframework.security.authentication.AuthenticationManager;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.stereotype.Service;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.web.client.RestTemplate;

import java.util.HashMap;
import java.time.Instant;
import java.util.Map;
import java.util.Optional;

Expand All @@ -37,7 +35,7 @@ public class AuthService {
final RestTemplate restTemplate;

@Autowired
AuthService (UserRepository userRepository, PasswordEncoder passwordEncoder, GoogleOAuthProperties googleOAuthProperties,RestTemplate restTemplate,JWTService jwtService) {
AuthService(UserRepository userRepository, PasswordEncoder passwordEncoder, GoogleOAuthProperties googleOAuthProperties, RestTemplate restTemplate, JWTService jwtService) {
this.userRepository = userRepository;
this.passwordEncoder = passwordEncoder;
this.googleOAuthProperties = googleOAuthProperties;
Expand Down Expand Up @@ -133,6 +131,10 @@ public AuthResponse login(LoginRequest loginRequest) {
authResponse.setMessage("Login successful");
authResponse.setUser(user);
authResponse.setToken(token);

user.setLastLoginAt(loginRequest.getLastLoginAt());
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line uses the timestamp from the client's LoginRequest, which is a security issue and incorrect design. The server should set the timestamp at the time of successful login, not trust the client's timestamp.

Change this to:

user.setLastLoginAt(Instant.now());

This matches the correct implementation in handleGoogleOAuth() at line 207.

Suggested change
user.setLastLoginAt(loginRequest.getLastLoginAt());
user.setLastLoginAt(Instant.now());

Copilot uses AI. Check for mistakes.
userRepository.save(user);
Comment on lines +135 to +136
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new lastLoginAt timestamp functionality lacks test coverage. Consider adding test cases to verify:

  1. That lastLoginAt is correctly set when a user logs in successfully
  2. That lastLoginAt is persisted to the database (verify userRepository.save() is called after setting the timestamp)

The existing test testLogin_success() should be updated to verify this behavior.

Copilot uses AI. Check for mistakes.

return authResponse;
}

Expand Down Expand Up @@ -202,6 +204,9 @@ public AuthResponse handleGoogleOAuth(String code, String redirectUri) {

String token = jwtService.generateToken(user.getEmail());

user.setLastLoginAt(Instant.now());
userRepository.save(user);
Comment on lines +207 to +208
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lastLoginAt timestamp functionality in the OAuth flow lacks test coverage. Consider adding test cases to verify that lastLoginAt is correctly set and persisted during OAuth login.

Copilot uses AI. Check for mistakes.

return AuthResponse.builder()
.success(true)
.message("OAuth login successful")
Expand Down Expand Up @@ -253,9 +258,10 @@ public AuthResponse setUsername(String email, String username) {
.success(false)
.build();
}

@Cacheable(value = "usernameCheck", key = "#username != null ? #username.toLowerCase() : 'null'")
public Boolean checkUsernameExists(String username) {
log.info("checking if username: {} exists",username);
log.info("checking if username: {} exists", username);
if (username == null) return false;
return userRepository.existsByUsernameIgnoreCase(username);
}
Expand Down
2 changes: 2 additions & 0 deletions src/main/resources/db/migration/V8__Alter_users_table_to_add_register_login_times.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE users ADD COLUMN created_at TIMESTAMP WITH TIME ZONE NOT NULL;
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The created_at column is marked as NOT NULL, but this migration will fail if there are any existing users in the database because they won't have a value for this column. Consider either:

  1. Adding a DEFAULT NOW() or DEFAULT CURRENT_TIMESTAMP clause, or
  2. Making the column nullable initially, then backfilling data and adding the NOT NULL constraint in a subsequent step, or
  3. Setting a specific timestamp as the default value for existing records

Example: ALTER TABLE users ADD COLUMN created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT NOW();

Suggested change
ALTER TABLE users ADD COLUMN created_at TIMESTAMP WITH TIME ZONE NOT NULL;
ALTER TABLE users ADD COLUMN created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT NOW();

Copilot uses AI. Check for mistakes.
ALTER TABLE users ADD COLUMN last_login_at TIMESTAMP WITH TIME ZONE;
7 changes: 4 additions & 3 deletions src/test/java/com/fredmaina/chatapp/Auth/Services/AuthServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
import com.fredmaina.chatapp.Auth.Dtos.AuthResponse;
import com.fredmaina.chatapp.Auth.Dtos.LoginRequest;
import com.fredmaina.chatapp.Auth.Dtos.SignUpRequest;
import com.fredmaina.chatapp.Auth.Models.Role;
import com.fredmaina.chatapp.Auth.Models.User;
import com.fredmaina.chatapp.Auth.Repositories.UserRepository;
import com.fredmaina.chatapp.Auth.services.AuthService;
import com.fredmaina.chatapp.Auth.services.JWTService;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.*;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.security.crypto.password.PasswordEncoder;

Expand Down Expand Up @@ -106,7 +107,7 @@ void testLogin_success() {
user.setPassword("encodedPassword");
user.setEmail("fred@example.com");

when(userRepository.findByUsername("fredmaina123")).thenReturn(Optional.of(user));
when(userRepository.findByUsernameIgnoreCase("fredmaina123")).thenReturn(Optional.of(user));
when(passwordEncoder.matches("mypassword", "encodedPassword")).thenReturn(true);
when(jwtService.generateToken(user.getEmail())).thenReturn("jwt-token");

Expand Down