diff --git a/Library-Management-System-Version-2/src/main/java/app/Application.java b/Library-Management-System-Version-2/src/main/java/app/Application.java index ab7f527..bd7eca5 100644 --- a/Library-Management-System-Version-2/src/main/java/app/Application.java +++ b/Library-Management-System-Version-2/src/main/java/app/Application.java @@ -8,9 +8,6 @@ @SpringBootApplication @EnableFeignClients @EnableScheduling -// The catalogue seed runs off the main thread: stocking the shelves from a public catalogue must -// not hold up serving requests. -/** Entry point for the library backend. */ @EnableAsync public class Application { public static void main(String[] args) { diff --git a/Library-Management-System-Version-2/src/main/java/app/adapters/input/rest/GlobalExceptionHandler.java b/Library-Management-System-Version-2/src/main/java/app/adapters/input/rest/GlobalExceptionHandler.java index 1ef7d1b..0b552ab 100644 --- a/Library-Management-System-Version-2/src/main/java/app/adapters/input/rest/GlobalExceptionHandler.java +++ b/Library-Management-System-Version-2/src/main/java/app/adapters/input/rest/GlobalExceptionHandler.java @@ -3,7 +3,9 @@ import app.infrastructure.exceptions.BorrowNotAllowedException; import app.infrastructure.exceptions.ResourceNotFoundException; import jakarta.persistence.EntityNotFoundException; +import lombok.extern.slf4j.Slf4j; import org.springframework.http.HttpStatus; +import org.springframework.web.servlet.resource.NoResourceFoundException; import org.springframework.http.ResponseEntity; import org.springframework.security.authentication.BadCredentialsException; import org.springframework.web.bind.MethodArgumentNotValidException; @@ -17,6 +19,7 @@ /** Turns exceptions into the JSON error bodies the API promises. */ @ControllerAdvice +@Slf4j public class GlobalExceptionHandler { /** Matching the base type, not each subclass, is what stops a new one falling through as a 500. */ @@ -36,10 +39,26 @@ private Map messageBody(Exception ex, String fallback) { return Map.of("message", ex.getMessage() == null ? fallback : ex.getMessage()); } + /** + * A path that matches nothing is a 404, not a server error. The catch-all below would otherwise + * turn every mistyped URL - and every disabled endpoint, such as Swagger in production - into a + * 500, which reads as "we broke" rather than "that is not here". + */ + @ExceptionHandler(NoResourceFoundException.class) + public ResponseEntity> handleNoResource(NoResourceFoundException ex) { + return ResponseEntity.status(HttpStatus.NOT_FOUND) + .body(Map.of("message", "We could not find what you were looking for.")); + } + + /** + * The last resort. The detail goes to the log, not to the caller: an exception message can + * carry a query, a file path or a class name, and none of that is the client's business. + */ @ExceptionHandler(Exception.class) - public ResponseEntity handleGenericException(Exception ex) { - return new ResponseEntity<>("An unexpected error occurred: " + ex.getMessage(), - HttpStatus.INTERNAL_SERVER_ERROR); + public ResponseEntity> handleGenericException(Exception ex) { + log.error("Unhandled exception", ex); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body(Map.of("message", "Something went wrong on our side.")); } @ExceptionHandler(MethodArgumentNotValidException.class) public ResponseEntity> handleValidationExceptions(MethodArgumentNotValidException ex) { diff --git a/Library-Management-System-Version-2/src/main/java/app/adapters/input/rest/LoginController.java b/Library-Management-System-Version-2/src/main/java/app/adapters/input/rest/LoginController.java index bcb05a7..fbf137a 100644 --- a/Library-Management-System-Version-2/src/main/java/app/adapters/input/rest/LoginController.java +++ b/Library-Management-System-Version-2/src/main/java/app/adapters/input/rest/LoginController.java @@ -5,8 +5,11 @@ import app.domain.dto.ChangePasswordRequest; import app.domain.model.AccountCredentials; import app.domain.services.JwtService; +import app.domain.services.TokenRevocationService; +import app.domain.services.LoginAttemptService; import jakarta.persistence.EntityNotFoundException; import jakarta.validation.Valid; +import jakarta.servlet.http.HttpServletRequest; import lombok.RequiredArgsConstructor; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; @@ -14,6 +17,7 @@ import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; +import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.validation.BindingResult; @@ -40,14 +44,34 @@ public class LoginController { private final JwtService jwtService; private final UserRepository userRepository; private final PasswordEncoder passwordEncoder; + private final LoginAttemptService loginAttempts; + private final TokenRevocationService revocationService; @PostMapping("/login") public ResponseEntity> getToken(@RequestBody AccountCredentials credentials) { - Authentication auth = authenticationManager.authenticate( - new UsernamePasswordAuthenticationToken( - credentials.getUsername(), credentials.getPassword() - ) - ); + String username = credentials.getUsername(); + + // Refuse before touching the password: guessing has to cost time, or the only thing + // protecting an account is how good its password is. + if (loginAttempts.isLockedOut(username)) { + long seconds = loginAttempts.secondsRemaining(username); + return ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS) + .header(HttpHeaders.RETRY_AFTER, String.valueOf(seconds)) + .body(Map.of("message", "Too many sign-in attempts. Try again in " + + Math.max(1, seconds / 60) + " minute(s).")); + } + + Authentication auth; + try { + auth = authenticationManager.authenticate( + new UsernamePasswordAuthenticationToken(username, credentials.getPassword()) + ); + } catch (AuthenticationException e) { + loginAttempts.recordFailure(username); + throw e; + } + + loginAttempts.recordSuccess(username); String jwt = jwtService.getToken(auth.getName(), roleOf(auth)); @@ -61,6 +85,19 @@ public ResponseEntity> getToken(@RequestBody AccountCredenti .body(body); } + /** + * Signs the caller out and refuses their token from now on. + * + *

Spring's own logout handler clears the session, which a stateless token never had. Without + * this the token in the browser stays valid until it expires, so "sign out" would only mean the + * client agreeing to forget it. + */ + @PostMapping("/revoke") + public ResponseEntity> revoke(HttpServletRequest request) { + revocationService.revoke(jwtService.getClaims(request)); + return ResponseEntity.ok(Map.of("message", "Signed out.")); + } + /** Lets the client check on start-up whether its stored token is still valid, and who it belongs to. */ @GetMapping("/me") public ResponseEntity> currentUser(Authentication auth) { diff --git a/Library-Management-System-Version-2/src/main/java/app/domain/services/JwtService.java b/Library-Management-System-Version-2/src/main/java/app/domain/services/JwtService.java index dad3d12..af0b58e 100644 --- a/Library-Management-System-Version-2/src/main/java/app/domain/services/JwtService.java +++ b/Library-Management-System-Version-2/src/main/java/app/domain/services/JwtService.java @@ -14,6 +14,7 @@ import javax.crypto.SecretKey; import java.nio.charset.StandardCharsets; import java.util.Date; +import java.util.UUID; /** Issues and verifies the tokens the API authenticates with. */ @Component @@ -62,8 +63,11 @@ public JwtService(@Value("${library.jwt.secret:}") String secret) { */ public String getToken(String username, String role) { return Jwts.builder() + // An id, so a token can be named in the revocation list when its owner signs out. + .id(UUID.randomUUID().toString()) .subject(username) .claim(ROLE_CLAIM, role) + .issuedAt(new Date()) .expiration(new Date(System.currentTimeMillis() + EXPIRATION_TIME)) .signWith(key) .compact(); diff --git a/Library-Management-System-Version-2/src/main/java/app/domain/services/LoginAttemptService.java b/Library-Management-System-Version-2/src/main/java/app/domain/services/LoginAttemptService.java new file mode 100644 index 0000000..c797171 --- /dev/null +++ b/Library-Management-System-Version-2/src/main/java/app/domain/services/LoginAttemptService.java @@ -0,0 +1,90 @@ +package app.domain.services; + +import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.stereotype.Service; + +import java.time.Duration; +import java.time.Instant; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Counts failed sign-ins and locks an account out for a while once there have been too many. + * + *

Without this a password can be guessed at network speed, which makes the strength of the + * password the only thing standing in the way. Attempts are counted per username rather than per + * address: an attacker can change address far more easily than they can change whose account they + * are trying to open. + * + *

Held in memory, so the count resets when the application does. That is a deliberate limit + * rather than an oversight - surviving a restart means a shared store, and the point here is to + * turn an instant guessing loop into a slow one. + */ +@Service +@Slf4j +public class LoginAttemptService { + + private record Attempts(int count, Instant firstFailure, Instant lockedUntil) { + } + + private final Map attempts = new ConcurrentHashMap<>(); + + @Value("${library.login.max-attempts:5}") + private int maxAttempts; + + @Value("${library.login.lockout:PT15M}") + private Duration lockout; + + /** How long attempts are remembered for; a slow trickle of failures should not add up forever. */ + @Value("${library.login.window:PT15M}") + private Duration window; + + public boolean isLockedOut(String username) { + Attempts current = attempts.get(key(username)); + if (current == null || current.lockedUntil() == null) { + return false; + } + if (Instant.now().isAfter(current.lockedUntil())) { + attempts.remove(key(username)); + return false; + } + return true; + } + + /** Seconds left on the lockout, for telling the caller when to come back. */ + public long secondsRemaining(String username) { + Attempts current = attempts.get(key(username)); + if (current == null || current.lockedUntil() == null) { + return 0; + } + return Math.max(0, Duration.between(Instant.now(), current.lockedUntil()).toSeconds()); + } + + public void recordFailure(String username) { + String key = key(username); + Instant now = Instant.now(); + + attempts.compute(key, (ignored, current) -> { + if (current == null || now.isAfter(current.firstFailure().plus(window))) { + return new Attempts(1, now, null); + } + + int count = current.count() + 1; + if (count >= maxAttempts) { + log.warn("Sign-in locked for '{}' after {} failed attempts.", username, count); + return new Attempts(count, current.firstFailure(), now.plus(lockout)); + } + return new Attempts(count, current.firstFailure(), null); + }); + } + + /** A success clears the slate, so a typo before a correct password costs nothing. */ + public void recordSuccess(String username) { + attempts.remove(key(username)); + } + + private static String key(String username) { + return username == null ? "" : username.toLowerCase(java.util.Locale.ROOT); + } +} diff --git a/Library-Management-System-Version-2/src/main/java/app/domain/services/TokenRevocationService.java b/Library-Management-System-Version-2/src/main/java/app/domain/services/TokenRevocationService.java new file mode 100644 index 0000000..d54ace6 --- /dev/null +++ b/Library-Management-System-Version-2/src/main/java/app/domain/services/TokenRevocationService.java @@ -0,0 +1,59 @@ +package app.domain.services; + +import io.jsonwebtoken.Claims; +import org.springframework.stereotype.Service; + +import java.time.Instant; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Remembers tokens that have been signed out, so a bearer token stops working the moment its owner + * says so rather than when it happens to expire. + * + *

A signed JWT is valid until its expiry by design: nothing about it is looked up, which is what + * makes it cheap. The cost is that signing out can only ever be a client-side gesture unless the + * server keeps a list like this one, so a copied token would keep working for the rest of the day. + * + *

Entries are dropped once the token would have expired anyway, so the list stays the size of + * however many people signed out recently. It is in memory: a restart forgets it, but a restart + * also mints a new signing key unless one is configured, which invalidates everything regardless. + */ +@Service +public class TokenRevocationService { + + /** Token id to the moment it expires, after which remembering it serves no purpose. */ + private final Map revoked = new ConcurrentHashMap<>(); + + public void revoke(Claims claims) { + if (claims == null) { + return; + } + String id = claims.getId(); + if (id == null) { + return; + } + purgeExpired(); + revoked.put(id, claims.getExpiration() == null ? Instant.now() : claims.getExpiration().toInstant()); + } + + public boolean isRevoked(Claims claims) { + if (claims == null || claims.getId() == null) { + return false; + } + Instant expiry = revoked.get(claims.getId()); + if (expiry == null) { + return false; + } + if (Instant.now().isAfter(expiry)) { + revoked.remove(claims.getId()); + return false; + } + return true; + } + + private void purgeExpired() { + Instant now = Instant.now(); + revoked.entrySet().removeIf(entry -> now.isAfter(entry.getValue())); + } +} diff --git a/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/database/DataInitializer.java b/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/database/DataInitializer.java index 63696aa..43d3118 100644 --- a/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/database/DataInitializer.java +++ b/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/database/DataInitializer.java @@ -9,42 +9,96 @@ import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.stereotype.Component; +import java.security.SecureRandom; +import java.util.Base64; +import java.util.Optional; + /** * Creates the bootstrap administrator, the one account self-registration cannot produce. * - *

Credentials come from configuration; set {@code LIBRARY_ADMIN_PASSWORD} outside local runs. + *

A configured {@code library.admin.password} is authoritative and is reapplied on every + * start-up. That matters now the database is file-backed: the account outlives the process, so + * creating it only when missing would mean setting the variable later had no effect at all. + * + *

With nothing configured a password is generated and written to the log, so there is always a + * way in without a well-known one being baked into a public repository. */ @Component @Slf4j public class DataInitializer { - private static final String DEFAULT_PASSWORD = "admin"; + private static final SecureRandom RANDOM = new SecureRandom(); @Value("${library.admin.username:admin}") private String adminUsername; - @Value("${library.admin.password:" + DEFAULT_PASSWORD + "}") + @Value("${library.admin.password:}") private String adminPassword; @Bean public CommandLineRunner initDatabase(UserRepository repository, PasswordEncoder passwordEncoder) { return args -> { - if (adminPassword == null || adminPassword.isBlank()) { - log.warn("library.admin.password is blank - no administrator account was created."); - return; - } - if (repository.findByUsername(adminUsername).isPresent()) { + Optional existing = repository.findByUsername(adminUsername); + boolean configured = adminPassword != null && !adminPassword.isBlank(); + + if (existing.isPresent()) { + reconcile(existing.get(), configured, repository, passwordEncoder); return; } - repository.save(new UserEntity(adminUsername, passwordEncoder.encode(adminPassword), "ADMIN")); + String password = configured ? adminPassword : generatePassword(); + repository.save(new UserEntity(adminUsername, passwordEncoder.encode(password), "ADMIN")); - if (DEFAULT_PASSWORD.equals(adminPassword)) { - log.warn("Administrator '{}' created with the default password. " - + "Set LIBRARY_ADMIN_PASSWORD before deploying this anywhere.", adminUsername); + if (configured) { + log.info("Administrator '{}' created from configuration.", adminUsername); } else { - log.info("Administrator '{}' created.", adminUsername); + announceGenerated(password); } }; } + + /** + * Brings a stored account back in line with configuration. + * + *

Only when a password is configured: a generated one must not be reapplied, or it would + * change on every restart and lock out whoever had just been told it. + */ + private void reconcile(UserEntity admin, boolean configured, + UserRepository repository, PasswordEncoder passwordEncoder) { + if (!configured) { + log.info("Administrator '{}' already exists. Set LIBRARY_ADMIN_PASSWORD to choose its " + + "password; the stored one is left alone.", adminUsername); + return; + } + + if (passwordEncoder.matches(adminPassword, admin.getPassword())) { + return; + } + + admin.setPassword(passwordEncoder.encode(adminPassword)); + repository.save(admin); + log.info("Administrator '{}' password reset to the configured one.", adminUsername); + } + + private static String generatePassword() { + byte[] bytes = new byte[12]; + RANDOM.nextBytes(bytes); + return Base64.getUrlEncoder().withoutPadding().encodeToString(bytes); + } + + private void announceGenerated(String password) { + // Deliberately loud, and the only place this is ever readable: the alternative is either a + // password everyone knows or no way to sign in at all. + log.warn(""" + + ---------------------------------------------------------------- + No library.admin.password was set, so one has been generated: + + username: {} + password: {} + + It is only shown here. Set LIBRARY_ADMIN_PASSWORD to choose one. + ---------------------------------------------------------------- + """, adminUsername, password); + } } diff --git a/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/security/AuthenticationFilter.java b/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/security/AuthenticationFilter.java index b282b71..8184530 100644 --- a/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/security/AuthenticationFilter.java +++ b/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/security/AuthenticationFilter.java @@ -1,6 +1,7 @@ package app.infrastructure.config.security; import app.domain.services.JwtService; +import app.domain.services.TokenRevocationService; import io.jsonwebtoken.Claims; import jakarta.servlet.FilterChain; import jakarta.servlet.ServletException; @@ -27,6 +28,7 @@ public class AuthenticationFilter extends OncePerRequestFilter { private final JwtService jwtService; + private final TokenRevocationService revocationService; @Override protected void doFilterInternal(@NonNull HttpServletRequest request, @@ -37,7 +39,9 @@ protected void doFilterInternal(@NonNull HttpServletRequest request, if (SecurityContextHolder.getContext().getAuthentication() == null) { Claims claims = jwtService.getClaims(request); - if (claims != null) { + // A signed token stays valid until it expires, so signing out only means anything if + // the server refuses it from then on. + if (claims != null && !revocationService.isRevoked(claims)) { // Spring Security matches hasRole("ADMIN") against the authority "ROLE_ADMIN". List authorities = List.of(new SimpleGrantedAuthority("ROLE_" + jwtService.getRole(claims))); diff --git a/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/security/SecurityConfig.java b/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/security/SecurityConfig.java index 982736e..ff27481 100644 --- a/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/security/SecurityConfig.java +++ b/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/security/SecurityConfig.java @@ -20,6 +20,7 @@ import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.web.SecurityFilterChain; +import org.springframework.security.web.header.writers.ReferrerPolicyHeaderWriter; import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.cors.CorsConfigurationSource; import org.springframework.web.cors.UrlBasedCorsConfigurationSource; @@ -92,13 +93,43 @@ public CorsConfigurationSource corsConfigurationSource() { return source; } + /** + * Response headers that limit what a browser will do with our pages. + * + *

The token lives in localStorage, so any script that runs on the page can read it. The + * content security policy is what makes that unlikely: injected script has nowhere to load from + * and no inline execution. HSTS matters once a proxy terminates TLS - it stops the next visit + * being made over plain HTTP in the first place. + */ + private static void hardenHeaders(HeadersConfigurer headers) { + headers + .contentSecurityPolicy(csp -> csp.policyDirectives(String.join("; ", + "default-src 'self'", + // The catalogue covers come from Open Library; data: covers inline SVG. + "img-src 'self' data: https://covers.openlibrary.org", + "script-src 'self'", + "style-src 'self' 'unsafe-inline'", + "frame-ancestors 'none'", + "base-uri 'self'", + "form-action 'self'"))) + .referrerPolicy(referrer -> referrer.policy( + ReferrerPolicyHeaderWriter.ReferrerPolicy.SAME_ORIGIN)) + .httpStrictTransportSecurity(hsts -> hsts + .includeSubDomains(true) + .maxAgeInSeconds(31536000)); + } + @Bean @Order(1) public SecurityFilterChain apiSecurityFilterChain(HttpSecurity http) throws Exception { return http .securityMatcher(API_PATHS) .cors(Customizer.withDefaults()) + // Safe to disable only because this chain is stateless and authenticates from a + // header: a browser does not attach an Authorization header to a cross-site form + // post, which is what CSRF relies on. .csrf(AbstractHttpConfigurer::disable) + .headers(SecurityConfig::hardenHeaders) .authorizeHttpRequests(auth -> auth .requestMatchers(HttpMethod.POST, "/api/login", "/api/register").permitAll() .requestMatchers("/api/logout").permitAll() @@ -160,7 +191,11 @@ public SecurityFilterChain webSecurityFilterChain(HttpSecurity http) throws Exce .deleteCookies("JSESSIONID") .permitAll() ) - .headers(headers -> headers.frameOptions(HeadersConfigurer.FrameOptionsConfig::disable)) + .headers(headers -> { + hardenHeaders(headers); + // The H2 console renders in a frame; it only exists under the dev profile. + headers.frameOptions(HeadersConfigurer.FrameOptionsConfig::disable); + }) // Form login needs a session to remember who signed in. .sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.IF_REQUIRED)) .build(); diff --git a/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/startup/DevProfileWarning.java b/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/startup/DevProfileWarning.java new file mode 100644 index 0000000..757cc77 --- /dev/null +++ b/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/startup/DevProfileWarning.java @@ -0,0 +1,33 @@ +package app.infrastructure.config.startup; + +import lombok.extern.slf4j.Slf4j; +import org.springframework.boot.context.event.ApplicationReadyEvent; +import org.springframework.context.annotation.Profile; +import org.springframework.context.event.EventListener; +import org.springframework.stereotype.Component; + +/** + * Warns about the dev profile is active. + */ +@Component +@Profile("dev") +@Slf4j +public class DevProfileWarning { + + @EventListener(ApplicationReadyEvent.class) + public void warn() { + log.warn(""" + + ################################################################ + Running with the 'dev' profile: + + - administrator password is 'admin' + - the JWT signing key is the one committed to this repository + - the H2 console answers without authentication + - Swagger UI publishes the whole API + + Never run this profile anywhere reachable from the internet. + ################################################################ + """); + } +} diff --git a/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/swagger/OpenApiConfig.java b/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/swagger/OpenApiConfig.java index b1e7e02..030f0fa 100644 --- a/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/swagger/OpenApiConfig.java +++ b/Library-Management-System-Version-2/src/main/java/app/infrastructure/config/swagger/OpenApiConfig.java @@ -9,7 +9,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -/** Describes the API for Swagger UI, including the bearer token its secured endpoints need. */ +/** Configures the OpenAPI documentation. */ @Configuration public class OpenApiConfig { @@ -20,8 +20,6 @@ public OpenAPI libraryOpenApi() { return new OpenAPI() .info(apiInfo()) .components(new Components().addSecuritySchemes(BEARER_SCHEME, bearerScheme())) - // Applied globally so "Authorize" in Swagger UI reaches every secured endpoint; - // without it each call answers 401 and the page is of no use for trying anything. .addSecurityItem(new SecurityRequirement().addList(BEARER_SCHEME)); } diff --git a/Library-Management-System-Version-2/src/main/resources/application-dev.properties b/Library-Management-System-Version-2/src/main/resources/application-dev.properties index 3a092dd..f05f5c6 100644 --- a/Library-Management-System-Version-2/src/main/resources/application-dev.properties +++ b/Library-Management-System-Version-2/src/main/resources/application-dev.properties @@ -14,3 +14,7 @@ library.jwt.secret=local-development-only-signing-key-change-me # In-memory locally: a fresh database each run is what makes the JSON fixture reproducible, and # DatabaseSeeder would otherwise add another copy of it on every start. spring.datasource.url=jdbc:h2:mem:library_ms + +# Swagger UI, which the default configuration leaves off. +springdoc.api-docs.enabled=true +springdoc.swagger-ui.enabled=true diff --git a/Library-Management-System-Version-2/src/main/resources/application.properties b/Library-Management-System-Version-2/src/main/resources/application.properties index 415a11d..23ec8c4 100644 --- a/Library-Management-System-Version-2/src/main/resources/application.properties +++ b/Library-Management-System-Version-2/src/main/resources/application.properties @@ -13,7 +13,6 @@ server.port=9092 spring.datasource.url=${LIBRARY_DB_URL:jdbc:h2:file:./data/library_ms} spring.datasource.driverClassName=org.h2.Driver spring.jpa.database-platform=org.hibernate.dialect.H2Dialect -spring.jpa.hibernate.ddl-auto=update spring.jpa.open-in-view=false # Properties for the H2 console @@ -104,6 +103,20 @@ spring.devtools.restart.quiet-period=1s # Reloads the browser tab serving the Thymeleaf pages. The React app has its own HMR via Vite. spring.devtools.livereload.enabled=true +# API documentation. Off by default: /v3/api-docs describes every endpoint, its parameters and its +# shapes to anyone who asks, which is a map of the application handed to a stranger. The dev profile +# turns it back on - springdoc itself warns about leaving it enabled in production. +springdoc.api-docs.enabled=false +springdoc.swagger-ui.enabled=false + +# Trust the proxy's X-Forwarded-* headers. The application speaks plain HTTP and expects TLS to be +# terminated in front of it; without this it builds URLs from its own scheme and reports http. +server.forward-headers-strategy=framework + +# Hibernate is allowed to alter the schema here because there are no migrations. Set it to +# 'validate' and introduce Flyway or Liquibase before a database you care about depends on it. +spring.jpa.hibernate.ddl-auto=${LIBRARY_DDL_AUTO:update} + # Logging configuration logging.level.org.hibernate.persister.entity=ERROR logging.level.org = WARN diff --git a/Library-Management-System-Version-2/src/test/java/app/adapters/input/GlobalExceptionHandlerTest.java b/Library-Management-System-Version-2/src/test/java/app/adapters/input/GlobalExceptionHandlerTest.java index ee801f0..439b3d9 100644 --- a/Library-Management-System-Version-2/src/test/java/app/adapters/input/GlobalExceptionHandlerTest.java +++ b/Library-Management-System-Version-2/src/test/java/app/adapters/input/GlobalExceptionHandlerTest.java @@ -49,12 +49,17 @@ void testHandleNotFoundWithoutMessage() { assertThat(response.getBody()).containsEntry("message", "Not found"); } + /** + * The message is deliberately generic. An exception's own text can carry a query, a file path + * or a class name, and the caller has no business seeing any of it - the log does. + */ @Test void testHandleGenericException() { - Exception exception = new Exception("Something went wrong"); - ResponseEntity response = globalExceptionHandler.handleGenericException(exception); + Exception exception = new Exception("Table CUSTOMERS not found in schema PUBLIC"); + ResponseEntity> response = globalExceptionHandler.handleGenericException(exception); assertThat(response.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); - assertThat(response.getBody()).contains("An unexpected error occurred: Something went wrong"); + assertThat(response.getBody()).containsEntry("message", "Something went wrong on our side."); + assertThat(response.getBody().toString()).doesNotContain("CUSTOMERS"); } } \ No newline at end of file diff --git a/Library-Management-System-Version-2/src/test/java/app/adapters/input/SecurityHardeningTestIT.java b/Library-Management-System-Version-2/src/test/java/app/adapters/input/SecurityHardeningTestIT.java new file mode 100644 index 0000000..0d264b8 --- /dev/null +++ b/Library-Management-System-Version-2/src/test/java/app/adapters/input/SecurityHardeningTestIT.java @@ -0,0 +1,135 @@ +package app.adapters.input; + +import app.domain.model.AccountCredentials; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.http.MediaType; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.web.servlet.MockMvc; + +import static org.hamcrest.Matchers.containsString; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +/** The protections that stop a token or a password being worn down by repetition. */ +@SpringBootTest +@AutoConfigureMockMvc +@TestPropertySource(properties = { + "library.admin.password=known-admin-password", + "library.login.max-attempts=3", + "library.login.lockout=PT5M", +}) +@Tag("integration") +class SecurityHardeningTestIT { + + @Autowired + private MockMvc mockMvc; + @Autowired + private ObjectMapper objectMapper; + + private String loginBody(String username, String password) throws Exception { + AccountCredentials credentials = new AccountCredentials(); + credentials.setUsername(username); + credentials.setPassword(password); + return objectMapper.writeValueAsString(credentials); + } + + private String tokenFor(String username, String password) throws Exception { + String response = mockMvc.perform(post("/api/login") + .contentType(MediaType.APPLICATION_JSON) + .content(loginBody(username, password))) + .andExpect(status().isOk()) + .andReturn().getResponse().getContentAsString(); + return objectMapper.readTree(response).get("token").asText(); + } + + /** Without a limit the password is the only obstacle, and it can be tried at network speed. */ + @Test + void locksTheAccountAfterRepeatedFailures() throws Exception { + String username = "lockme"; + + for (int attempt = 0; attempt < 3; attempt++) { + mockMvc.perform(post("/api/login") + .contentType(MediaType.APPLICATION_JSON) + .content(loginBody(username, "wrong"))) + .andExpect(status().isUnauthorized()); + } + + mockMvc.perform(post("/api/login") + .contentType(MediaType.APPLICATION_JSON) + .content(loginBody(username, "wrong"))) + .andExpect(status().isTooManyRequests()) + .andExpect(header().exists("Retry-After")); + } + + /** A lockout is per account, or one attacker could shut the whole library out. */ + @Test + void lockingOneAccountLeavesAnotherAlone() throws Exception { + for (int attempt = 0; attempt < 4; attempt++) { + mockMvc.perform(post("/api/login") + .contentType(MediaType.APPLICATION_JSON) + .content(loginBody("victim", "wrong"))); + } + + mockMvc.perform(post("/api/login") + .contentType(MediaType.APPLICATION_JSON) + .content(loginBody("admin", "known-admin-password"))) + .andExpect(status().isOk()); + } + + /** Signing out has to mean something server-side, or a copied token works until it expires. */ + @Test + void aRevokedTokenStopsWorking() throws Exception { + String token = tokenFor("admin", "known-admin-password"); + + mockMvc.perform(get("/api/me").header("Authorization", token)) + .andExpect(status().isOk()); + + mockMvc.perform(post("/api/revoke").header("Authorization", token)) + .andExpect(status().isOk()); + + mockMvc.perform(get("/api/me").header("Authorization", token)) + .andExpect(status().isUnauthorized()); + } + + /** A second token must survive the first one being revoked. */ + @Test + void revokingOneTokenLeavesAnotherValid() throws Exception { + String first = tokenFor("admin", "known-admin-password"); + String second = tokenFor("admin", "known-admin-password"); + + mockMvc.perform(post("/api/revoke").header("Authorization", first)).andExpect(status().isOk()); + + mockMvc.perform(get("/api/me").header("Authorization", second)).andExpect(status().isOk()); + } + + @Test + void sendsAContentSecurityPolicy() throws Exception { + mockMvc.perform(post("/api/login") + .contentType(MediaType.APPLICATION_JSON) + .content(loginBody("admin", "known-admin-password"))) + .andExpect(header().string("Content-Security-Policy", containsString("default-src 'self'"))) + .andExpect(header().string("Content-Security-Policy", containsString("frame-ancestors 'none'"))) + .andExpect(header().string("Referrer-Policy", containsString("same-origin"))); + } + + /** + * An unknown path is not a server error, and must not describe our internals. + * + *

Signed in on purpose: an unknown path under /api is refused by the filter chain as a 401 + * before any handler sees it, which is right - this checks what happens once past that. + */ + @Test + void answersAnUnknownPathWithNotFound() throws Exception { + String token = tokenFor("admin", "known-admin-password"); + + mockMvc.perform(get("/api/there-is-nothing-here").header("Authorization", token)) + .andExpect(status().isNotFound()); + } +} diff --git a/Library-Management-System-Version-2/src/test/java/app/support/TestStateResetListener.java b/Library-Management-System-Version-2/src/test/java/app/support/TestStateResetListener.java index 81fc0fa..afcbac2 100644 --- a/Library-Management-System-Version-2/src/test/java/app/support/TestStateResetListener.java +++ b/Library-Management-System-Version-2/src/test/java/app/support/TestStateResetListener.java @@ -8,6 +8,7 @@ import app.infrastructure.config.database.DatabaseSeeder; import org.springframework.cache.Cache; import org.springframework.cache.CacheManager; +import org.springframework.boot.CommandLineRunner; import org.springframework.context.ApplicationContext; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.test.context.TestContext; @@ -63,6 +64,7 @@ public void beforeTestMethod(TestContext testContext) throws Exception { clearCaches(context); wipe(context); seeder.run(); + restoreAdministrator(context); } private static void clearCaches(ApplicationContext context) { @@ -79,6 +81,24 @@ private static void clearCaches(ApplicationContext context) { } } + /** + * The wipe takes the accounts with it, and DatabaseSeeder only stocks books and members - so + * without this no test can sign in as the administrator, which is how the admin endpoints are + * reached. Re-running the bootstrap runner recreates it from the same configuration. + */ + private static void restoreAdministrator(ApplicationContext context) throws Exception { + CommandLineRunner bootstrap = + (CommandLineRunner) context.getBeanProvider(CommandLineRunner.class) + .stream() + .filter(bean -> bean.getClass().getName().contains("DataInitializer")) + .findFirst() + .orElse(null); + + if (bootstrap != null) { + bootstrap.run(); + } + } + /** Children before parents: a transaction points at a book and a customer. */ private static void wipe(ApplicationContext context) { deleteAll(context, TransactionRepository.class); diff --git a/frontend/src/api/client.test.ts b/frontend/src/api/client.test.ts index d9117cb..3959a32 100644 --- a/frontend/src/api/client.test.ts +++ b/frontend/src/api/client.test.ts @@ -109,6 +109,41 @@ describe('authentication', () => { expect(getToken()).toBeNull() }) + /** + * A wrong password is not an expired session. Reporting it as one sends the reader looking for a + * session that never existed, and bounced them to the page they were already on. + */ + it('reports a rejected sign-in as bad credentials, not an expired session', async () => { + fetchMock.mockResolvedValue(respond(401, 'Invalid credentials', 'text/plain')) + + const error = (await api.post('/api/login', { username: 'admin', password: 'nope' }) + .catch((e: unknown) => e)) as ApiError + + expect(error.message).toBe('Invalid credentials') + expect(error.message).not.toMatch(/session/i) + expect(error.name).not.toBe('UnauthorizedError') + }) + + it('keeps whatever token exists when a sign-in is refused', async () => { + setToken('Bearer someone-elses-session') + fetchMock.mockResolvedValue(respond(401, 'Invalid credentials', 'text/plain')) + + await expect(api.post('/api/login', {})).rejects.toBeInstanceOf(ApiError) + + // Nothing was signed out: the failed attempt was not this session's. + expect(getToken()).toBe('Bearer someone-elses-session') + }) + + it('still treats a 401 elsewhere as an expired session', async () => { + setToken('Bearer stale') + fetchMock.mockResolvedValue(respond(401, '')) + + const error = (await api.get('/books/paginated').catch((e: unknown) => e)) as Error + + expect(error.name).toBe('UnauthorizedError') + expect(getToken()).toBeNull() + }) + /** 403 means the token is fine but the door is closed - signing the user out would be wrong. */ it('keeps the token on 403', async () => { setToken('Bearer good-token') diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 73d839a..70d4450 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -16,6 +16,9 @@ const BASE = import.meta.env.VITE_API_BASE_URL ?? '/backend' */ export const DEMO_MODE = import.meta.env.VITE_DEMO === 'true' +/** Where a 401 means "those credentials are wrong", not "your session ended". */ +const SIGN_IN_PATHS = ['/api/login', '/api/register'] + const TOKEN_KEY = 'library.jwt' const SESSION_KEY = 'library.session' @@ -115,8 +118,16 @@ async function request(path: string, init: RequestInit = {}): Promise { } if (response.status === 401) { + const message = await readError(response) + + // Signing in or registering is not a session expiring - there is no session yet - so say what + // actually happened and leave the caller on the page instead of bouncing it to /login. + if (SIGN_IN_PATHS.some((candidate) => path.startsWith(candidate))) { + throw new ApiError(401, message) + } + clearToken() - throw new UnauthorizedError() + throw new UnauthorizedError(message) } if (response.status === 403) { diff --git a/frontend/src/api/demo/backend.ts b/frontend/src/api/demo/backend.ts index 1660d80..b894745 100644 --- a/frontend/src/api/demo/backend.ts +++ b/frontend/src/api/demo/backend.ts @@ -68,17 +68,36 @@ const hydrate = (t: Transaction): Transaction => ({ book: helpers.book(t.bookId), }) -/** Stands in for the Open Library search, so Discover has something to show. */ +/** + * Stands in for the Open Library search. These are real books with real ISBNs, so the cover lookup + * resolves against Open Library exactly as it does with the backend behind it - an invented ISBN + * would leave every card blank. + */ +const CATALOGUE: Omit[] = [ + { title: 'Nineteen Eighty-Four', isbn: '9780451524935', publicationYear: 1949, authors: ['George Orwell'], coverId: null }, + { title: 'Brave New World', isbn: '9780060850524', publicationYear: 1932, authors: ['Aldous Huxley'], coverId: null }, + { title: 'Frankenstein', isbn: '9780486282114', publicationYear: 1818, authors: ['Mary Shelley'], coverId: null }, + { title: 'Pride and Prejudice', isbn: '9780141439518', publicationYear: 1813, authors: ['Jane Austen'], coverId: null }, + { title: 'Moby-Dick', isbn: '9780142437247', publicationYear: 1851, authors: ['Herman Melville'], coverId: null }, + { title: 'The Great Gatsby', isbn: '9780743273565', publicationYear: 1925, authors: ['F. Scott Fitzgerald'], coverId: null }, + { title: 'Crime and Punishment', isbn: '9780140449136', publicationYear: 1866, authors: ['Fyodor Dostoevsky'], coverId: null }, + { title: 'The Hobbit', isbn: '9780547928227', publicationYear: 1937, authors: ['J. R. R. Tolkien'], coverId: null }, + { title: 'Jane Eyre', isbn: '9780141441146', publicationYear: 1847, authors: ['Charlotte Bronte'], coverId: null }, + { title: 'The Odyssey', isbn: '9780140268867', publicationYear: -700, authors: ['Homer'], coverId: null }, + { title: 'Slaughterhouse-Five', isbn: '9780385333849', publicationYear: 1969, authors: ['Kurt Vonnegut'], coverId: null }, + { title: 'The Handmaid’s Tale', isbn: '9780385490818', publicationYear: 1985, authors: ['Margaret Atwood'], coverId: null }, +] + function catalogue(query: string): CatalogCandidate[] { - const q = query.trim() + const q = query.trim().toLowerCase() if (!q) return [] - const held = new Set(db().books.map((b) => b.isbn)) - const n = q.length - return [ - { title: `${q} and Other Essays`, isbn: `978-1-000-${(n * 7919) % 100000}-0`, publicationYear: 2015, authors: ['A. Writer'], coverId: null }, - { title: `The Book of ${q}`, isbn: `978-1-001-${(n * 104729) % 100000}-1`, publicationYear: 2001, authors: ['B. Author'], coverId: null }, - { title: `${q}: A History`, isbn: `978-1-002-${(n * 1299709) % 100000}-2`, publicationYear: 1994, authors: ['C. Historian'], coverId: null }, - ].map((c) => ({ ...c, stocked: held.has(c.isbn) })) + const held = new Set(db().books.map((b) => b.isbn.replace(/[^0-9Xx]/g, ''))) + return CATALOGUE.filter( + (c) => + c.title.toLowerCase().includes(q) || + c.authors.some((a) => a.toLowerCase().includes(q)) || + c.isbn.includes(q), + ).map((c) => ({ ...c, stocked: held.has(c.isbn) })) } export function handle(method: string, path: string, body: unknown, auth: string | null): unknown { @@ -95,7 +114,7 @@ export function handle(method: string, path: string, body: unknown, auth: string const user = state.users.find( (u) => u.username === str('username') && u.password === str('password'), ) - if (!user) throw new DemoHttpError(401, 'Those credentials were not recognised.') + if (!user) throw new DemoHttpError(401, 'Invalid credentials') return { message: 'Signed in.', token: `${TOKEN_PREFIX}${encodeURIComponent(user.username)}`, @@ -276,12 +295,55 @@ export function handle(method: string, path: string, body: unknown, auth: string requireAdmin(auth) const customer = helpers.customer(seg[1]) if (!customer) throw new DemoHttpError(404, 'We could not find that member.') + // Wrapped in {message, data}, which is what customersApi.byId unwraps. return { - ...customer, - transactions: state.transactions - .filter((t) => t.customerId === customer.customerId) - .map(hydrate), + message: 'Member found.', + data: { + ...customer, + transactions: state.transactions + .filter((t) => t.customerId === customer.customerId) + .map(hydrate), + }, + } + } + + if (method === 'POST' && rawPath === '/customers') { + requireAdmin(auth) + const name = str('name').trim() + if (!name) throw new DemoHttpError(400, 'A name is required.') + const customer: Customer = { + customerId: helpers.uuid(), + name, + email: str('email'), + privileges: payload.privileges !== false, + } + state.customers.push(customer) + save() + return customer + } + + if (method === 'PUT' && seg[0] === 'customers' && seg[2] === 'privileges') { + requireAdmin(auth) + const customer = helpers.customer(seg[1]) + if (!customer) throw new DemoHttpError(404, 'We could not find that member.') + // The endpoint takes a bare JSON boolean, not an object. + customer.privileges = body === true + save() + return 'Customer privileges updated successfully!' + } + + if (method === 'DELETE' && seg[0] === 'customers' && seg.length === 2) { + requireAdmin(auth) + const customer = helpers.customer(seg[1]) + if (!customer) throw new DemoHttpError(404, 'We could not find that member.') + if (helpers.activeLoansFor(customer.customerId).length) { + throw new DemoHttpError(400, 'That member still has books out.') } + state.customers = state.customers.filter((c) => c.customerId !== customer.customerId) + state.users = state.users.filter((u) => u.customerId !== customer.customerId) + state.transactions = state.transactions.filter((t) => t.customerId !== customer.customerId) + save() + return 'Customer successfully deleted!' } // ---- loans --------------------------------------------------------------------------------- diff --git a/frontend/src/api/services.ts b/frontend/src/api/services.ts index 0aff0a5..2689b81 100644 --- a/frontend/src/api/services.ts +++ b/frontend/src/api/services.ts @@ -146,6 +146,19 @@ export const customersApi = { api.getPage( `/customers/search?query=${encodeURIComponent(query)}&page=${page}&size=${size}&sortBy=${sortBy}`, ), + + /** + * Adds a membership. It is not a sign-in account: registration is what pairs a username and + * password with a membership, and there is no endpoint for an administrator to do that. + */ + create: (member: { name: string; email: string; privileges: boolean }) => + api.post('/customers', member), + + /** Suspending borrowing rather than removing the member, which keeps their history. */ + setPrivileges: (id: string, privileges: boolean) => + api.put(`/customers/${id}/privileges`, privileges), + + remove: (id: string) => api.delete(`/customers/${id}`), } export const transactionsApi = { diff --git a/frontend/src/components/CustomerForm.tsx b/frontend/src/components/CustomerForm.tsx new file mode 100644 index 0000000..2e9927d --- /dev/null +++ b/frontend/src/components/CustomerForm.tsx @@ -0,0 +1,100 @@ +import { useState } from 'react' +import type { FormEvent } from 'react' +import { customersApi } from '../api/services' +import { Modal } from './Modal' + +interface CustomerFormProps { + onClose: () => void + onSaved: () => void +} + +/** + * Adds a membership. + * + *

Deliberately does not ask for a password: this creates the membership a loan is recorded + * against, not a sign-in account. Registration is what pairs the two, and the backend has no + * endpoint for an administrator to do it on someone's behalf. + */ +export function CustomerForm({ onClose, onSaved }: CustomerFormProps) { + const [name, setName] = useState('') + const [email, setEmail] = useState('') + const [privileges, setPrivileges] = useState(true) + const [error, setError] = useState(null) + const [busy, setBusy] = useState(false) + + async function handleSubmit(event: FormEvent) { + event.preventDefault() + setError(null) + setBusy(true) + try { + await customersApi.create({ name: name.trim(), email: email.trim(), privileges }) + onSaved() + } catch (err) { + setError(err instanceof Error ? err.message : 'That did not work') + } finally { + setBusy(false) + } + } + + return ( + + + + + } + > +

+ {error && ( +

+ {error} +

+ )} + +
+ + setName(event.target.value)} + required + autoFocus + /> +
+ +
+ + setEmail(event.target.value)} + required + /> +
+ + + +

+ This creates a membership only. To sign in, they register an account themselves and it is + matched to this record. +

+ +
+ + ) +} diff --git a/frontend/src/components/PasswordField.tsx b/frontend/src/components/PasswordField.tsx new file mode 100644 index 0000000..d465960 --- /dev/null +++ b/frontend/src/components/PasswordField.tsx @@ -0,0 +1,105 @@ +import { useId, useState } from 'react' + +interface PasswordFieldProps { + label: string + value: string + onChange: (value: string) => void + /** Overrides the generated one where a stable id is needed, e.g. for a test or a label. */ + id?: string + autoComplete?: string + required?: boolean + autoFocus?: boolean + help?: string +} + +/** + * A password box with a reveal toggle. + * + *

Typing a password blind is how a typo becomes a failed sign-in that reads like a forgotten + * password. The toggle only changes the input's type, so the value is never handled separately. + */ +export function PasswordField({ + label, + value, + onChange, + id, + autoComplete, + required, + autoFocus, + help, +}: PasswordFieldProps) { + const generatedId = useId() + const fieldId = id ?? generatedId + const [visible, setVisible] = useState(false) + + return ( +

+ + +
+ onChange(event.target.value)} + autoComplete={autoComplete} + required={required} + autoFocus={autoFocus} + /> + + +
+ + {help && {help}} +
+ ) +} + +function EyeIcon() { + return ( + + ) +} + +function EyeOffIcon() { + return ( + + ) +} diff --git a/frontend/src/pages/CustomersPage.tsx b/frontend/src/pages/CustomersPage.tsx index 58e18c4..a3db5d7 100644 --- a/frontend/src/pages/CustomersPage.tsx +++ b/frontend/src/pages/CustomersPage.tsx @@ -3,18 +3,44 @@ import { customersApi } from '../api/services' import type { Customer, Page } from '../types/domain' import { useApiCall } from '../hooks/useApiCall' import { CustomerDetail } from '../components/CustomerDetail' +import { CustomerForm } from '../components/CustomerForm' import { EmptyState, Pagination, SearchBox, SkeletonRows } from '../components/TableStates' const PAGE_SIZE = 10 -const COLUMNS = 3 +const COLUMNS = 4 export function CustomersPage() { const [term, setTerm] = useState('') const [query, setQuery] = useState('') const [page, setPage] = useState(0) const [selectedId, setSelectedId] = useState(null) + const [adding, setAdding] = useState(false) + const [reloadKey, setReloadKey] = useState(0) + const [busyId, setBusyId] = useState(null) + const [actionError, setActionError] = useState(null) const { data, error, loading, run } = useApiCall>() + const refresh = () => setReloadKey((key) => key + 1) + + async function act(customer: Customer, action: () => Promise) { + setBusyId(customer.customerId) + setActionError(null) + try { + await action() + refresh() + } catch (err) { + setActionError(err instanceof Error ? err.message : 'That did not work') + } finally { + setBusyId(null) + } + } + + async function remove(customer: Customer) { + // Deleting takes their loan history with them, so it is worth a confirmation. + if (!window.confirm(`Remove ${customer.name}? Their borrowing history goes too.`)) return + await act(customer, () => customersApi.remove(customer.customerId)) + } + useEffect(() => { const timer = setTimeout(() => setQuery(term.trim()), 300) return () => clearTimeout(timer) @@ -30,7 +56,7 @@ export function CustomersPage() { ? customersApi.search(query, page, PAGE_SIZE) : customersApi.paginated(page, PAGE_SIZE), ) - }, [query, page, run]) + }, [query, page, reloadKey, run]) const customers = data?.data ?? [] const showSkeleton = loading && !data @@ -45,14 +71,19 @@ export function CustomersPage() {

Everyone who holds a library account. Visible to administrators.

- {data?.totalItems != null && `${data.totalItems} total`} +
+ {data?.totalItems != null && `${data.totalItems} total`} + +
- {error && ( + {(error || actionError) && (

- {error} + {error ?? actionError}

)} @@ -64,6 +95,7 @@ export function CustomersPage() { Name Email Borrowing privileges + @@ -90,6 +122,28 @@ export function CustomersPage() { {customer.privileges ? 'Active' : 'Suspended'} + event.stopPropagation()}> + + + ))} @@ -111,6 +165,16 @@ export function CustomersPage() { onChange={setPage} /> + {adding && ( + setAdding(false)} + onSaved={() => { + setAdding(false) + refresh() + }} + /> + )} + {selectedId && ( setSelectedId(null)} /> )} diff --git a/frontend/src/pages/DiscoverPage.tsx b/frontend/src/pages/DiscoverPage.tsx index 282827f..e57fad4 100644 --- a/frontend/src/pages/DiscoverPage.tsx +++ b/frontend/src/pages/DiscoverPage.tsx @@ -12,6 +12,39 @@ const PAGE_SIZE = 20 * The rest of the world's books, not the library's. Anything found here can be put on the shelves * by whoever wants to read it - the catalogue belongs to the members, not to the desk. */ +/** + * A cover, addressed by cover id where the catalogue gave one and by ISBN otherwise - the search + * endpoint often omits `cover_i` for editions that do have a cover. Falls back to the placeholder + * rather than a broken image when neither resolves. + */ +function BookCover({ coverId, isbn }: { coverId: number | null; isbn: string }) { + const sources = [ + coverId ? `https://covers.openlibrary.org/b/id/${coverId}-M.jpg` : null, + isbn ? `https://covers.openlibrary.org/b/isbn/${isbn.replace(/[^0-9Xx]/g, '')}-M.jpg` : null, + ].filter((url): url is string => Boolean(url)) + + const [attempt, setAttempt] = useState(0) + + if (attempt >= sources.length) { + return