From 012bd1af921551a17720fdfb4c7cffdaea9d20e4 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Mon, 2 Mar 2026 13:56:39 +0000 Subject: [PATCH] hardening (#5807) Co-authored-by: Claude Haiku 4.5 --- app/common/build.gradle | 2 + .../software/common/service/TaskManager.java | 20 +++ app/core/build.gradle | 5 +- .../common/controller/JobController.java | 38 ++++-- .../common/controller/JobControllerTest.java | 32 +++++ app/proprietary/build.gradle | 3 +- .../api/ProprietaryUIDataController.java | 37 +++++- .../api/AdminSettingsController.java | 29 ++++- .../controller/api/AuthController.java | 2 +- .../controller/api/InviteLinkController.java | 34 +++-- .../controller/api/UserController.java | 2 +- .../proprietary/security/model/User.java | 2 + .../security/model/dto/AdminUserSummary.java | 72 +++++++++++ .../api/AuthControllerLoginTest.java | 2 +- .../api/InviteLinkControllerTest.java | 6 +- .../public/locales/en-GB/translation.toml | 1 + .../components/shared/EditableSecretField.tsx | 118 ++++++++++++++++++ .../config/configSections/ProviderCard.tsx | 6 +- .../configSections/AdminDatabaseSection.tsx | 18 ++- .../configSections/AdminMailSection.tsx | 18 +-- .../shared/config/configSections/ApiKeys.tsx | 26 +++- 21 files changed, 407 insertions(+), 66 deletions(-) create mode 100644 app/proprietary/src/main/java/stirling/software/proprietary/security/model/dto/AdminUserSummary.java create mode 100644 frontend/src/core/components/shared/EditableSecretField.tsx diff --git a/app/common/build.gradle b/app/common/build.gradle index 4c9e1b855a..e19b36d466 100644 --- a/app/common/build.gradle +++ b/app/common/build.gradle @@ -5,6 +5,7 @@ bootRun { spotless { java { target 'src/**/java/**/*.java' + targetExclude 'src/main/java/org/apache/**' googleJavaFormat(googleJavaFormatVersion).aosp().reorderImports(false) importOrder("java", "javax", "org", "com", "net", "io", "jakarta", "lombok", "me", "stirling") @@ -26,6 +27,7 @@ spotless { } } dependencies { + api 'com.google.guava:guava:33.4.8-jre' api 'org.springframework.boot:spring-boot-starter-webmvc' api 'org.springframework.boot:spring-boot-starter-aspectj' api 'com.googlecode.owasp-java-html-sanitizer:owasp-java-html-sanitizer:20260102.1' diff --git a/app/common/src/main/java/stirling/software/common/service/TaskManager.java b/app/common/src/main/java/stirling/software/common/service/TaskManager.java index 2ee8807946..4af6a8e145 100644 --- a/app/common/src/main/java/stirling/software/common/service/TaskManager.java +++ b/app/common/src/main/java/stirling/software/common/service/TaskManager.java @@ -467,4 +467,24 @@ public class TaskManager { } return null; } + + /** + * Find the job key that owns a given file ID. + * + * @param fileId file identifier to look up + * @return scoped job key if found, otherwise null + */ + public String findJobKeyByFileId(String fileId) { + for (Map.Entry entry : jobResults.entrySet()) { + JobResult jobResult = entry.getValue(); + if (jobResult.hasFiles()) { + for (ResultFile resultFile : jobResult.getAllResultFiles()) { + if (fileId.equals(resultFile.getFileId())) { + return entry.getKey(); + } + } + } + } + return null; + } } diff --git a/app/core/build.gradle b/app/core/build.gradle index ae2faf31e4..9e3ea9c2a8 100644 --- a/app/core/build.gradle +++ b/app/core/build.gradle @@ -12,11 +12,10 @@ configurations { spotless { java { target 'src/**/java/**/*.java' - targetExclude 'src/main/resources/static/**' + targetExclude 'src/main/resources/static/**', 'src/main/java/org/apache/**' googleJavaFormat(googleJavaFormatVersion).aosp().reorderImports(false) importOrder("java", "javax", "org", "com", "net", "io", "jakarta", "lombok", "me", "stirling") - toggleOffOn() trimTrailingWhitespace() leadingTabsToSpaces() endWithNewline() @@ -91,7 +90,7 @@ dependencies { exclude group: 'org.bouncycastle', module: 'bcprov-jdk15on' exclude group: 'com.google.code.gson', module: 'gson' } - // CVE-2022-25647: Explicit gson 2.8.9 to prevent unsafe deserialization (tabula would pull 2.8.7) + // CVE-2022-25647: Explicit gson 2.13.2 to prevent unsafe deserialization (tabula would pull 2.8.7) implementation 'com.google.code.gson:gson:2.13.2' implementation 'org.apache.pdfbox:jbig2-imageio:3.0.4' implementation 'com.opencsv:opencsv:5.12.0' // https://mvnrepository.com/artifact/com.opencsv/opencsv diff --git a/app/core/src/main/java/stirling/software/common/controller/JobController.java b/app/core/src/main/java/stirling/software/common/controller/JobController.java index c0c57d92a0..7bd81741ed 100644 --- a/app/core/src/main/java/stirling/software/common/controller/JobController.java +++ b/app/core/src/main/java/stirling/software/common/controller/JobController.java @@ -262,17 +262,30 @@ public class JobController { @Operation(summary = "Get file metadata") public ResponseEntity getFileMetadata(@PathVariable("fileId") String fileId) { try { - // Verify file exists - if (!fileStorage.fileExists(fileId)) { + String jobKey = taskManager.findJobKeyByFileId(fileId); + if (jobKey == null) { return ResponseEntity.notFound().build(); } + if (!validateJobAccess(jobKey)) { + log.warn("Unauthorized attempt to access file metadata: {}", fileId); + return ResponseEntity.status(403) + .body(Map.of("message", "You are not authorized to access this file")); + } + // Find the file metadata from any job that contains this file ResultFile resultFile = taskManager.findResultFileByFileId(fileId); if (resultFile != null) { return ResponseEntity.ok(resultFile); - } else { + } + + if (!isSecurityEnabled()) { + // Backwards compatibility when ownership service is unavailable + if (!fileStorage.fileExists(fileId)) { + return ResponseEntity.notFound().build(); + } + // File exists but no metadata found, get basic info efficiently long fileSize = fileStorage.getFileSize(fileId); return ResponseEntity.ok( @@ -286,6 +299,8 @@ public class JobController { "fileSize", fileSize)); } + + return ResponseEntity.notFound().build(); } catch (Exception e) { log.error("Error retrieving file metadata {}: {}", fileId, e.getMessage(), e); return ResponseEntity.internalServerError() @@ -303,11 +318,17 @@ public class JobController { @Operation(summary = "Download a file") public ResponseEntity downloadFile(@PathVariable("fileId") String fileId) { try { - // Verify file exists - if (!fileStorage.fileExists(fileId)) { + String jobKey = taskManager.findJobKeyByFileId(fileId); + if (jobKey == null) { return ResponseEntity.notFound().build(); } + if (!validateJobAccess(jobKey)) { + log.warn("Unauthorized attempt to download file: {}", fileId); + return ResponseEntity.status(403) + .body(Map.of("message", "You are not authorized to access this file")); + } + // Retrieve file content byte[] fileContent = fileStorage.retrieveBytes(fileId); @@ -327,11 +348,14 @@ public class JobController { .body(fileContent); } catch (Exception e) { log.error("Error retrieving file {}: {}", fileId, e.getMessage(), e); - return ResponseEntity.internalServerError() - .body("Error retrieving file: " + e.getMessage()); + return ResponseEntity.internalServerError().body("Error retrieving file"); } } + private boolean isSecurityEnabled() { + return jobOwnershipService != null; + } + /** * Create Content-Disposition header with UTF-8 filename support * diff --git a/app/core/src/test/java/stirling/software/common/controller/JobControllerTest.java b/app/core/src/test/java/stirling/software/common/controller/JobControllerTest.java index 4b7c216fb2..1388abeae6 100644 --- a/app/core/src/test/java/stirling/software/common/controller/JobControllerTest.java +++ b/app/core/src/test/java/stirling/software/common/controller/JobControllerTest.java @@ -15,11 +15,13 @@ import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.mock.web.MockHttpSession; +import org.springframework.test.util.ReflectionTestUtils; import jakarta.servlet.http.HttpServletRequest; import stirling.software.common.model.job.JobResult; import stirling.software.common.service.FileStorage; +import stirling.software.common.service.JobOwnershipService; import stirling.software.common.service.JobQueue; import stirling.software.common.service.TaskManager; @@ -33,6 +35,8 @@ class JobControllerTest { @Mock private HttpServletRequest request; + @Mock private JobOwnershipService jobOwnershipService; + private MockHttpSession session; @InjectMocks private JobController controller; @@ -404,4 +408,32 @@ class JobControllerTest { verify(taskManager).setError(jobId, "Job was cancelled by user"); } + + @Test + void testDownloadFile_ForbiddenWhenFileOwnedByAnotherUser() throws Exception { + String fileId = "file-id"; + + ReflectionTestUtils.setField(controller, "jobOwnershipService", jobOwnershipService); + when(taskManager.findJobKeyByFileId(fileId)).thenReturn("other-user:job-id"); + when(jobOwnershipService.validateJobAccess("other-user:job-id")).thenReturn(false); + + ResponseEntity response = controller.downloadFile(fileId); + + assertEquals(HttpStatus.FORBIDDEN, response.getStatusCode()); + verify(fileStorage, never()).retrieveBytes(eq(fileId)); + } + + @Test + void testGetFileMetadata_ForbiddenWhenFileOwnedByAnotherUser() throws Exception { + String fileId = "file-id"; + + ReflectionTestUtils.setField(controller, "jobOwnershipService", jobOwnershipService); + when(taskManager.findJobKeyByFileId(fileId)).thenReturn("other-user:job-id"); + when(jobOwnershipService.validateJobAccess("other-user:job-id")).thenReturn(false); + + ResponseEntity response = controller.getFileMetadata(fileId); + + assertEquals(HttpStatus.FORBIDDEN, response.getStatusCode()); + verify(fileStorage, never()).getFileSize(eq(fileId)); + } } diff --git a/app/proprietary/build.gradle b/app/proprietary/build.gradle index bd175abdec..507b0852b9 100644 --- a/app/proprietary/build.gradle +++ b/app/proprietary/build.gradle @@ -14,10 +14,10 @@ bootRun { spotless { java { target 'src/**/java/**/*.java' + targetExclude 'src/main/java/org/apache/**' googleJavaFormat(googleJavaFormatVersion).aosp().reorderImports(false) importOrder("java", "javax", "org", "com", "net", "io", "jakarta", "lombok", "me", "stirling") - toggleOffOn() trimTrailingWhitespace() leadingTabsToSpaces() endWithNewline() @@ -37,6 +37,7 @@ spotless { } dependencies { implementation project(':common') + api 'com.google.guava:guava:33.4.8-jre' api 'org.springframework:spring-jdbc' api 'org.springframework:spring-webmvc' diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/ProprietaryUIDataController.java b/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/ProprietaryUIDataController.java index d41116a6bf..0417134644 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/ProprietaryUIDataController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/ProprietaryUIDataController.java @@ -43,6 +43,7 @@ import stirling.software.proprietary.security.database.repository.UserRepository import stirling.software.proprietary.security.model.Authority; import stirling.software.proprietary.security.model.SessionEntity; import stirling.software.proprietary.security.model.User; +import stirling.software.proprietary.security.model.dto.AdminUserSummary; import stirling.software.proprietary.security.repository.TeamRepository; import stirling.software.proprietary.security.saml2.CustomSaml2AuthenticatedPrincipal; import stirling.software.proprietary.security.service.DatabaseService; @@ -357,8 +358,12 @@ public class ProprietaryUIDataController { int licenseMaxUsers = licenseSettingsService.getSettings().getLicenseMaxUsers(); boolean premiumEnabled = applicationProperties.getPremium().isEnabled(); + // Convert User entities to AdminUserSummary DTOs to exclude sensitive fields + List userSummaries = + sortedUsers.stream().map(this::convertUserToSummary).toList(); + AdminSettingsData data = new AdminSettingsData(); - data.setUsers(sortedUsers); + data.setUsers(userSummaries); data.setCurrentUsername(authentication.getName()); data.setRoleDetails(roleDetails); data.setUserSessions(userSessions); @@ -518,6 +523,34 @@ public class ProprietaryUIDataController { return ResponseEntity.ok(data); } + /** + * Convert User entity to AdminUserSummary DTO, excluding sensitive fields like password and + * apiKey. + */ + private AdminUserSummary convertUserToSummary(User user) { + AdminUserSummary summary = new AdminUserSummary(); + summary.setId(user.getId()); + summary.setUsername(user.getUsername()); + summary.setEmail(user.getUsername()); // Use username as email for consistency + summary.setRoleName(user.getRoleName()); + summary.setRolesAsString(user.getRolesAsString()); + summary.setEnabled(user.isEnabled()); + summary.setIsFirstLogin(user.isFirstLogin()); + summary.setAuthenticationType(user.getAuthenticationType()); + summary.setCreatedAt(user.getCreatedAt()); + summary.setUpdatedAt(user.getUpdatedAt()); + + // Map team if present + if (user.getTeam() != null) { + AdminUserSummary.TeamSummary teamSummary = new AdminUserSummary.TeamSummary(); + teamSummary.setId(user.getTeam().getId()); + teamSummary.setName(user.getTeam().getName()); + summary.setTeam(teamSummary); + } + + return summary; + } + // Data classes @Data public static class AuditDashboardData { @@ -544,7 +577,7 @@ public class ProprietaryUIDataController { @Data public static class AdminSettingsData { - private List users; + private List users; private String currentUsername; private Map roleDetails; private Map userSessions; diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java index 8ad14287da..b3dff95243 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java @@ -196,8 +196,9 @@ public class AdminSettingsController { log.info("Admin updating setting: {} = {}", key, value); GeneralUtils.saveKeyToSettings(key, value); - // Track this as a pending change - pendingChanges.put(key, value); + // Track this as a pending change (convert null to empty string for + // ConcurrentHashMap) + pendingChanges.put(key, value != null ? value : ""); updatedCount++; } @@ -268,6 +269,9 @@ public class AdminSettingsController { } } + // Mask sensitive fields before returning to frontend + sectionMap = maskSensitiveFields(sectionMap); + log.debug( "Admin requested settings section: {} (includePending={})", sectionName, @@ -404,6 +408,13 @@ public class AdminSettingsController { return ResponseEntity.badRequest() .body("Setting key not found: " + HtmlUtils.htmlEscape(key)); } + + // Mask sensitive values before returning + String keyName = key.contains(".") ? key.substring(key.lastIndexOf(".") + 1) : key; + if (isSensitiveFieldWithPath(keyName, key)) { + value = createMaskedValue(value); + } + log.debug("Admin requested setting: {}", key); return ResponseEntity.ok(new SettingValueResponse(key, value)); } catch (IllegalArgumentException e) { @@ -441,6 +452,20 @@ public class AdminSettingsController { } Object value = request.getValue(); + + // Prevent saving masked values for sensitive fields to avoid data loss + if ("********".equals(value)) { + String keyName = key.contains(".") ? key.substring(key.lastIndexOf(".") + 1) : key; + if (isSensitiveFieldWithPath(keyName, key)) { + log.warn( + "Admin attempted to save masked value for sensitive field: {}. This operation is blocked to prevent data loss.", + key); + return ResponseEntity.badRequest() + .body( + "Cannot save masked values for sensitive settings. Please provide the actual value."); + } + } + log.info("Admin updating single setting: {} = {}", key, value); GeneralUtils.saveKeyToSettings(key, value); diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AuthController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AuthController.java index 62b5f55477..eae9e5d144 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AuthController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AuthController.java @@ -123,7 +123,7 @@ public class AuthController { log.warn("Invalid password for user: {} from IP: {}", username, ip); loginAttemptService.loginFailed(username); return ResponseEntity.status(HttpStatus.UNAUTHORIZED) - .body(Map.of("error", "Invalid credentials")); + .body(Map.of("error", "Invalid username or password")); } if (!user.isEnabled()) { diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/InviteLinkController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/InviteLinkController.java index 58c40cdb00..fc82c6e8c3 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/InviteLinkController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/InviteLinkController.java @@ -358,27 +358,23 @@ public class InviteLinkController { Optional inviteOpt = inviteTokenRepository.findByToken(token); if (inviteOpt.isEmpty()) { - return ResponseEntity.status(HttpStatus.NOT_FOUND) - .body(Map.of("error", "Invalid invite link")); + return invalidInviteResponse(); } InviteToken invite = inviteOpt.get(); if (invite.isUsed()) { - return ResponseEntity.status(HttpStatus.GONE) - .body(Map.of("error", "This invite link has already been used")); + return invalidInviteResponse(); } if (invite.isExpired()) { - return ResponseEntity.status(HttpStatus.GONE) - .body(Map.of("error", "This invite link has expired")); + return invalidInviteResponse(); } // Check if user already exists (only if email is pre-set) if (invite.getEmail() != null && userService.usernameExistsIgnoreCase(invite.getEmail())) { - return ResponseEntity.status(HttpStatus.CONFLICT) - .body(Map.of("error", "User already exists")); + return invalidInviteResponse(); } Map response = new HashMap<>(); @@ -391,8 +387,7 @@ public class InviteLinkController { } catch (Exception e) { log.error("Failed to validate invite token: {}", e.getMessage(), e); - return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body(Map.of("error", "Failed to validate invite link")); + return invalidInviteResponse(); } } @@ -419,20 +414,17 @@ public class InviteLinkController { Optional inviteOpt = inviteTokenRepository.findByToken(token); if (inviteOpt.isEmpty()) { - return ResponseEntity.status(HttpStatus.NOT_FOUND) - .body(Map.of("error", "Invalid invite link")); + return invalidInviteResponse(); } InviteToken invite = inviteOpt.get(); if (invite.isUsed()) { - return ResponseEntity.status(HttpStatus.GONE) - .body(Map.of("error", "This invite link has already been used")); + return invalidInviteResponse(); } if (invite.isExpired()) { - return ResponseEntity.status(HttpStatus.GONE) - .body(Map.of("error", "This invite link has expired")); + return invalidInviteResponse(); } // Determine the email to use @@ -455,8 +447,7 @@ public class InviteLinkController { // Check if user already exists if (userService.usernameExistsIgnoreCase(effectiveEmail)) { - return ResponseEntity.status(HttpStatus.CONFLICT) - .body(Map.of("error", "User already exists")); + return invalidInviteResponse(); } // Create the user account @@ -484,7 +475,12 @@ public class InviteLinkController { } catch (Exception e) { log.error("Failed to accept invite: {}", e.getMessage(), e); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body(Map.of("error", "Failed to create account: " + e.getMessage())); + .body(Map.of("error", "Failed to create account")); } } + + private ResponseEntity> invalidInviteResponse() { + return ResponseEntity.status(HttpStatus.NOT_FOUND) + .body(Map.of("error", "Invalid invite link")); + } } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/UserController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/UserController.java index 3687029bae..cde7b0e17e 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/UserController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/UserController.java @@ -942,7 +942,7 @@ public class UserController { public ResponseEntity completeInitialSetup() { try { String username = userService.getCurrentUsername(); - if (username == null) { + if (username == null || "anonymousUser".equalsIgnoreCase(username)) { return ResponseEntity.status(HttpStatus.UNAUTHORIZED) .body("User not authenticated"); } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/model/User.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/model/User.java index 59ac1d2fa4..943be47e8b 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/model/User.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/model/User.java @@ -46,9 +46,11 @@ public class User implements UserDetails, Serializable { private String username; @Column(name = "password") + @JsonIgnore private String password; @Column(name = "apiKey") + @JsonIgnore private String apiKey; @Column(name = "enabled") diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/model/dto/AdminUserSummary.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/model/dto/AdminUserSummary.java new file mode 100644 index 0000000000..2a3e7d2af6 --- /dev/null +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/model/dto/AdminUserSummary.java @@ -0,0 +1,72 @@ +package stirling.software.proprietary.security.model.dto; + +import java.time.LocalDateTime; + +import com.fasterxml.jackson.annotation.JsonInclude; + +import io.swagger.v3.oas.annotations.media.Schema; + +import lombok.AllArgsConstructor; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; + +/** + * Data Transfer Object for admin user listings. Contains only the fields needed by the admin UI for + * displaying user information. Excludes sensitive fields like password and apiKey. + */ +@Getter +@Setter +@NoArgsConstructor +@AllArgsConstructor +@JsonInclude(JsonInclude.Include.NON_NULL) +@Schema(description = "Admin user summary for listing in admin UI - excludes sensitive fields") +public class AdminUserSummary { + + @Schema(description = "User ID") + private Long id; + + @Schema(description = "Username/login identifier") + private String username; + + @Schema(description = "User email address") + private String email; + + @Schema(description = "Role name (translation key like 'adminUserSettings.admin')") + private String roleName; + + @Schema(description = "Role identifier (e.g., 'ROLE_ADMIN')") + private String rolesAsString; + + @Schema(description = "Whether user account is enabled") + private boolean enabled; + + @Schema(description = "Whether this is the user's first login") + private Boolean isFirstLogin; + + @Schema(description = "Authentication type (WEB, OAUTH2, SAML2)") + private String authenticationType; + + @Schema(description = "Team membership (if any)") + private TeamSummary team; + + @Schema(description = "User account creation timestamp") + private LocalDateTime createdAt; + + @Schema(description = "User account last update timestamp") + private LocalDateTime updatedAt; + + /** Minimal Team DTO for admin user listings */ + @Getter + @Setter + @NoArgsConstructor + @AllArgsConstructor + @Schema(description = "Team summary (id and name only)") + public static class TeamSummary { + @Schema(description = "Team ID") + private Long id; + + @Schema(description = "Team name") + private String name; + } +} diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/security/controller/api/AuthControllerLoginTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/security/controller/api/AuthControllerLoginTest.java index 6b89e91e78..cf92ef5f3e 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/security/controller/api/AuthControllerLoginTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/security/controller/api/AuthControllerLoginTest.java @@ -149,7 +149,7 @@ class AuthControllerLoginTest { .contentType(MediaType.APPLICATION_JSON) .content(objectMapper.writeValueAsString(payload))) .andExpect(status().isUnauthorized()) - .andExpect(jsonPath("$.error").value("Invalid credentials")); + .andExpect(jsonPath("$.error").value("Invalid username or password")); verify(loginAttemptService).loginFailed("user@example.com"); } diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/security/controller/api/InviteLinkControllerTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/security/controller/api/InviteLinkControllerTest.java index c3eafe4241..2687969d8e 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/security/controller/api/InviteLinkControllerTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/security/controller/api/InviteLinkControllerTest.java @@ -125,7 +125,7 @@ class InviteLinkControllerTest { } @Test - void validateInviteTokenReturnsGoneWhenExpired() throws Exception { + void validateInviteTokenReturnsNotFoundWhenExpired() throws Exception { InviteToken expired = new InviteToken(); expired.setToken("abc"); expired.setExpiresAt(LocalDateTime.now().minusHours(1)); @@ -133,8 +133,8 @@ class InviteLinkControllerTest { when(inviteTokenRepository.findByToken("abc")).thenReturn(Optional.of(expired)); mockMvc.perform(get("/api/v1/invite/validate/abc")) - .andExpect(status().isGone()) - .andExpect(jsonPath("$.error").value("This invite link has expired")); + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.error").value("Invalid invite link")); } @Test diff --git a/frontend/public/locales/en-GB/translation.toml b/frontend/public/locales/en-GB/translation.toml index b7ba6db491..8347b501bc 100644 --- a/frontend/public/locales/en-GB/translation.toml +++ b/frontend/public/locales/en-GB/translation.toml @@ -105,6 +105,7 @@ save = "Save" saveToBrowser = "Save to Browser" saveUnavailable = "Save unavailable for this item" seeDockerHub = "See Docker Hub" +editSecret = "Edit this value" selectFillter = "-- Select --" size = "Size" sponsor = "Sponsor" diff --git a/frontend/src/core/components/shared/EditableSecretField.tsx b/frontend/src/core/components/shared/EditableSecretField.tsx new file mode 100644 index 0000000000..dfd3da6458 --- /dev/null +++ b/frontend/src/core/components/shared/EditableSecretField.tsx @@ -0,0 +1,118 @@ +import { useState, useRef, useEffect } from 'react'; +import { PasswordInput, Group, ActionIcon, Tooltip, TextInput } from '@mantine/core'; +import { useTranslation } from 'react-i18next'; +import LocalIcon from '@app/components/shared/LocalIcon'; + +interface EditableSecretFieldProps { + label?: string; + description?: string; + value: string; + onChange: (value: string) => void; + placeholder?: string; + disabled?: boolean; + error?: string; +} + +/** + * Component for editing sensitive fields (passwords, API keys, secrets). + * + * UX: + * - Normal password input in all scenarios EXCEPT when value is masked (********) + * - When backend returns masked value (********): Shows read-only display + Edit button + * - Click Edit to change the masked value + */ +export default function EditableSecretField({ + label, + description, + value, + onChange, + placeholder = 'Enter value', + disabled = false, + error, +}: EditableSecretFieldProps) { + const { t } = useTranslation(); + const [isEditing, setIsEditing] = useState(false); + const [tempValue, setTempValue] = useState(''); + const inputRef = useRef(null); + + const isMasked = value === '********'; + + useEffect(() => { + if (isEditing && inputRef.current) { + inputRef.current.focus(); + } + }, [isEditing]); + + const handleEdit = () => { + setTempValue(''); + setIsEditing(true); + }; + + const handleCancel = () => { + setTempValue(''); + setIsEditing(false); + }; + + const handleSave = () => { + if (tempValue.trim() !== '') { + onChange(tempValue); + } + setTempValue(''); + setIsEditing(false); + }; + + return ( +
+ {label && } + {description &&

{description}

} + + {isMasked && !isEditing ? ( + // Masked value from backend: show display + Edit button + + + + + + + + + ) : isEditing ? ( + // Edit mode: normal password input + setTempValue(e.currentTarget.value)} + placeholder={placeholder} + disabled={disabled} + error={error} + autoComplete="new-password" + onBlur={handleSave} + onKeyDown={(e) => { + if (e.key === 'Escape') handleCancel(); + }} + /> + ) : ( + // Normal password input: empty or user typing + onChange(e.currentTarget.value)} + placeholder={placeholder} + disabled={disabled} + error={error} + autoComplete="new-password" + /> + )} +
+ ); +} diff --git a/frontend/src/core/components/shared/config/configSections/ProviderCard.tsx b/frontend/src/core/components/shared/config/configSections/ProviderCard.tsx index 710ae71dbf..7877a53f09 100644 --- a/frontend/src/core/components/shared/config/configSections/ProviderCard.tsx +++ b/frontend/src/core/components/shared/config/configSections/ProviderCard.tsx @@ -9,12 +9,12 @@ import { TextInput, Textarea, Switch, - PasswordInput, NumberInput, TagsInput, } from '@mantine/core'; import { useTranslation } from 'react-i18next'; import LocalIcon from '@app/components/shared/LocalIcon'; +import EditableSecretField from '@app/components/shared/EditableSecretField'; import { Provider, ProviderField } from '@app/components/shared/config/configSections/providerDefinitions'; interface ProviderCardProps { @@ -94,13 +94,13 @@ export default function ProviderCard({ case 'password': return ( - handleFieldChange(field.key, e.target.value)} + onChange={(newValue) => handleFieldChange(field.key, newValue)} disabled={disabled} /> ); diff --git a/frontend/src/proprietary/components/shared/config/configSections/AdminDatabaseSection.tsx b/frontend/src/proprietary/components/shared/config/configSections/AdminDatabaseSection.tsx index d4655c9147..2bbe61972a 100644 --- a/frontend/src/proprietary/components/shared/config/configSections/AdminDatabaseSection.tsx +++ b/frontend/src/proprietary/components/shared/config/configSections/AdminDatabaseSection.tsx @@ -10,7 +10,6 @@ import { Loader, Group, TextInput, - PasswordInput, Select, Badge, Table, @@ -29,6 +28,7 @@ import { useAdminSettings } from "@app/hooks/useAdminSettings"; import PendingBadge from "@app/components/shared/config/PendingBadge"; import { useLoginRequired } from "@app/hooks/useLoginRequired"; import LoginRequiredBanner from "@app/components/shared/config/LoginRequiredBanner"; +import EditableSecretField from "@app/components/shared/EditableSecretField"; import apiClient from "@app/services/apiClient"; import LocalIcon from "@app/components/shared/LocalIcon"; import databaseManagementService, { DatabaseBackupFile } from "@app/services/databaseManagementService"; @@ -515,17 +515,15 @@ export default function AdminDatabaseSection() {
- - {t("admin.settings.database.password.label", "Password")} - - - } + + {t("admin.settings.database.password.label", "Password")} + + + setSettings({ ...settings, password: e.target.value })} - placeholder="••••••••" + onChange={(value) => setSettings({ ...settings, password: value })} + placeholder="Enter database password" disabled={!loginEnabled} />
diff --git a/frontend/src/proprietary/components/shared/config/configSections/AdminMailSection.tsx b/frontend/src/proprietary/components/shared/config/configSections/AdminMailSection.tsx index e5ae992e19..7a1d99ed03 100644 --- a/frontend/src/proprietary/components/shared/config/configSections/AdminMailSection.tsx +++ b/frontend/src/proprietary/components/shared/config/configSections/AdminMailSection.tsx @@ -1,12 +1,13 @@ import { useEffect } from 'react'; import { useTranslation } from 'react-i18next'; import { useNavigate } from 'react-router-dom'; -import { TextInput, NumberInput, Switch, Button, Stack, Paper, Text, Loader, Group, PasswordInput, Anchor } from '@mantine/core'; +import { TextInput, NumberInput, Switch, Button, Stack, Paper, Text, Loader, Group, Anchor } from '@mantine/core'; import { alert } from '@app/components/toast'; import RestartConfirmationModal from '@app/components/shared/config/RestartConfirmationModal'; import { useRestartServer } from '@app/components/shared/config/useRestartServer'; import { useAdminSettings } from '@app/hooks/useAdminSettings'; import PendingBadge from '@app/components/shared/config/PendingBadge'; +import EditableSecretField from '@app/components/shared/EditableSecretField'; import apiClient from '@app/services/apiClient'; interface MailSettingsData { @@ -174,16 +175,15 @@ export default function AdminMailSection() {
- - {t('admin.settings.mail.password.label', 'SMTP Password')} - - - } + + {t('admin.settings.mail.password.label', 'SMTP Password')} + + + setSettings({ ...settings, password: e.target.value })} + onChange={(value) => setSettings({ ...settings, password: value })} + placeholder="Enter SMTP password" />
diff --git a/frontend/src/proprietary/components/shared/config/configSections/ApiKeys.tsx b/frontend/src/proprietary/components/shared/config/configSections/ApiKeys.tsx index fcc9108875..e1b1a53530 100644 --- a/frontend/src/proprietary/components/shared/config/configSections/ApiKeys.tsx +++ b/frontend/src/proprietary/components/shared/config/configSections/ApiKeys.tsx @@ -18,11 +18,29 @@ export default function ApiKeys() { const copy = async (text: string, tag: string) => { try { - await navigator.clipboard.writeText(text); - setCopied(tag); - setTimeout(() => setCopied(null), 1600); + // Try modern Clipboard API first (requires HTTPS) + if (navigator.clipboard?.writeText) { + await navigator.clipboard.writeText(text); + setCopied(tag); + setTimeout(() => setCopied(null), 1600); + } else { + // Fallback for HTTP: use old execCommand method + const textarea = document.createElement('textarea'); + textarea.value = text; + textarea.style.position = 'fixed'; + textarea.style.opacity = '0'; + document.body.appendChild(textarea); + textarea.select(); + + if (document.execCommand('copy')) { + setCopied(tag); + setTimeout(() => setCopied(null), 1600); + } + + document.body.removeChild(textarea); + } } catch (e) { - console.error(e); + console.error('Failed to copy:', e); } };