Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,27 @@ public UserController(UserService userService) {

// ——— Admin-only endpoints ———

// return all users sorted by id (ascending)
// return all users sorted by id (ascending), emails masked
@GetMapping
@PreAuthorize("hasRole('ADMIN')")
public List<UserResponse> getAllUsers() {
log.debug("Retrieving all users");
List<UserResponse> users = userService.findAll(Sort.by(Sort.Direction.ASC, "id"))
.stream()
.map(UserResponse::fromEntity)
.map(UserResponse::fromEntityMasked)
.collect(Collectors.toList());
log.debug("Retrieved {} users", users.size());
return users;
}

@GetMapping("/{id}")
@PreAuthorize("hasRole('ADMIN')")
public ResponseEntity<UserResponse> getUserById(@PathVariable Long id) {
Optional<User> opt = userService.findById(id);
if (opt.isEmpty()) return ResponseEntity.notFound().build();
return ResponseEntity.ok(UserResponse.fromEntity(opt.get()));
}

/**
* Creates a new user with validation.
*
Expand Down Expand Up @@ -150,7 +158,7 @@ public ResponseEntity<?> updateUser(@PathVariable Long id,
}

// Check if email is being changed to a different email
if (!request.getEmail().equals(existing.getEmail())) {
if (!request.getEmail().equalsIgnoreCase(existing.getEmail())) {
// Check if the new email is already taken by another user
if (userService.emailExists(request.getEmail())) {
log.warn("User update failed - email already exists: {}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.empress.usermanagementapi.entity.Role;
import com.empress.usermanagementapi.entity.User;
import com.empress.usermanagementapi.util.LoggingUtil;

/**
* Data Transfer Object for User responses in REST API.
Expand All @@ -19,9 +20,6 @@ public class UserResponse {
public UserResponse() {
}

/**
* Create a UserResponse from a User entity.
*/
public static UserResponse fromEntity(User user) {
UserResponse response = new UserResponse();
response.setId(user.getId());
Expand All @@ -32,6 +30,12 @@ public static UserResponse fromEntity(User user) {
return response;
}

public static UserResponse fromEntityMasked(User user) {
UserResponse response = fromEntity(user);
response.setEmail(LoggingUtil.maskEmail(user.getEmail()));
return response;
}

// Getters and setters

public Long getId() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ public interface UserRepository extends JpaRepository<User, Long> {
Optional<User> findByEmail(String email);
Optional<User> findByUsernameAndEmail(String username, String email);
User findByUsername(String username);
boolean existsByEmailIgnoreCase(String email);
boolean existsByEmail(String email);
boolean existsByUsername(String username);
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public UserService(UserRepository userRepo, PasswordEncoder passwordEncoder) {
* Check if an email is already in use.
*/
public boolean emailExists(String email) {
return userRepo.existsByEmail(email);
return userRepo.existsByEmailIgnoreCase(email);
}

/**
Expand Down
54 changes: 33 additions & 21 deletions src/main/resources/templates/admin.html
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ <h6 class="alert-heading">Health Details</h6>
const API = '';
let editingUserId = null; // when set, the Create button acts as Update
let editingUserName = null; // for heading text
let pendingEditId = null; // guards against stale out-of-order fetch responses

async function authFetch(path, opts = {}) {
opts.credentials = 'include';
Expand Down Expand Up @@ -376,7 +377,10 @@ <h6 class="alert-heading">Health Details</h6>
tr.innerHTML = `
<td>${u.id}</td>
<td class="user-username cell-like" data-id="${u.id}">${u.username}</td>
<td class="user-email cell-like" data-id="${u.id}">${u.email}</td>
<td class="user-email cell-like" data-id="${u.id}">
<span id="email-display-${u.id}">${u.email}</span>
<button class="btn btn-link p-0 ms-1 reveal-email" data-id="${u.id}" style="font-size:0.72rem;vertical-align:baseline;opacity:0.6">reveal</button>
</td>
<td class="user-role cell-like" data-id="${u.id}">${u.role}</td>
<td class="d-flex gap-2">
<button class="btn btn-sm btn-primary edit-in-form" data-id="${u.id}">Edit</button>
Expand All @@ -392,14 +396,20 @@ <h6 class="alert-heading">Health Details</h6>
}

function attachHandlers(cachedUsers) {
// Edit → copy row values into the bottom form and highlight row
// Edit → fetch full user detail (unmasked email) then populate form
document.querySelectorAll('.edit-in-form').forEach(btn => {
btn.onclick = () => {
btn.onclick = async () => {
const id = +btn.dataset.id;
const u = cachedUsers.find(x => x.id === id);
if (!u) return;
pendingEditId = id;
let u;
try {
u = await authFetch(`/users/${id}`, { method: 'GET' });

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Ignore stale edit-detail responses

Prevent an earlier edit request from overwriting a later selection. If an admin clicks Edit for user A and then user B before A's request completes, and B's response arrives first, A's later response repopulates the form and resets editingUserId to A; the next submission can therefore modify the wrong account. Track the latest requested ID, disable competing edits, or abort superseded requests before applying the response.

Useful? React with 👍 / 👎.

} catch (err) {
if (pendingEditId === id) alert('Could not load user: ' + err.message);
return;
}
if (pendingEditId !== id) return; // superseded by a later edit click

// copy values to form (ONLY editable place)
document.getElementById('newUsername').value = u.username;
document.getElementById('newEmail').value = u.email;
document.getElementById('newRole').value = u.role;
Expand All @@ -410,12 +420,25 @@ <h6 class="alert-heading">Health Details</h6>
setFormMode('update');
highlightRow(id);

// bring form into view
document.getElementById('createForm')
.scrollIntoView({behavior:'smooth', block:'center'});
};
});

// Reveal → fetch full user detail and replace masked email in table
document.querySelectorAll('.reveal-email').forEach(btn => {
btn.onclick = async () => {
const id = +btn.dataset.id;
try {
const u = await authFetch(`/users/${id}`, { method: 'GET' });
document.getElementById(`email-display-${id}`).textContent = u.email;
btn.remove();
} catch (err) {
alert('Could not reveal email: ' + err.message);
}
};
});

// Delete
document.querySelectorAll('.delete-btn').forEach(btn => {
btn.onclick = async () => {
Expand Down Expand Up @@ -519,30 +542,19 @@ <h6 class="alert-heading">Health Details</h6>

try {
const users = await authFetch('/users', { method: 'GET' });
const lowerU = username.toLowerCase(), lowerE = email.toLowerCase();
const lowerU = username.toLowerCase();

// exclude record being edited from duplicate checks
// username duplicate check (emails are masked in list, so email check is server-side)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve case-insensitive duplicate email checks

Keep a case-insensitive duplicate check rather than relying solely on UserService.emailExists(): that method delegates to the case-sensitive existsByEmail, and PostgreSQL's ordinary text uniqueness is also case-sensitive. When alice@example.com already exists, the admin form can now create alice@EXAMPLE.COM, even though the removed client check treated those addresses as duplicates and both deliver to the same domain/mailbox.

Useful? React with 👍 / 👎.

const usernameTaken = users.some(u =>
(editingUserId ? u.id !== editingUserId : true) &&
(u.username||'').toLowerCase() === lowerU
);
const emailTaken = users.some(u =>
(editingUserId ? u.id !== editingUserId : true) &&
(u.email||'').toLowerCase() === lowerE
);

let blocked = false;
if (usernameTaken){
usernameError.textContent = 'Username already exists.';
usernameError.classList.remove('d-none');
blocked = true;
}
if (emailTaken){
emailError.textContent = 'Email already in use.';
emailError.classList.remove('d-none');
blocked = true;
return;
}
if (blocked) return;

if (editingUserId){ // update existing user
const payload = { username, email, role };
Expand Down
Loading