diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/SkillsFacade.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/SkillsFacade.java new file mode 100644 index 0000000000..951cfafced --- /dev/null +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/SkillsFacade.java @@ -0,0 +1,119 @@ +package uk.ac.cam.cl.dtg.isaac.api; + +import com.google.inject.Inject; +import io.swagger.v3.oas.annotations.tags.Tag; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import uk.ac.cam.cl.dtg.isaac.api.managers.InvalidAnvilMarkingRequestException; +import uk.ac.cam.cl.dtg.isaac.api.managers.SkillsManager; +import uk.ac.cam.cl.dtg.isaac.dos.content.AnvilApp; +import uk.ac.cam.cl.dtg.isaac.dos.content.Content; +import uk.ac.cam.cl.dtg.isaac.dto.AnvilMarkingRequestDTO; +import uk.ac.cam.cl.dtg.isaac.dto.AnvilPayloadDTO; +import uk.ac.cam.cl.dtg.isaac.dto.SegueErrorResponse; +import uk.ac.cam.cl.dtg.isaac.dto.users.RegisteredUserDTO; +import uk.ac.cam.cl.dtg.segue.api.managers.UserAccountManager; +import uk.ac.cam.cl.dtg.segue.auth.exceptions.NoUserLoggedInException; +import uk.ac.cam.cl.dtg.segue.dao.ILogManager; +import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; +import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; +import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; +import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.Context; +import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.core.Response.Status; + +/** Skills Facade, supports interaction related to the Isaac Skill Practice apps. */ +@Path("/skills") +@Tag(name = "SkillsFacade", description = "/skills") +public class SkillsFacade extends AbstractIsaacFacade { + private static final Logger log = LoggerFactory.getLogger(SkillsFacade.class); + + private final UserAccountManager userManager; + private final GitContentManager contentManager; + private final SkillsManager skillsManager; + + /** + * Constructor. + */ + @Inject + public SkillsFacade(final AbstractConfigLoader properties, final UserAccountManager userManager, + final ILogManager logManager, final GitContentManager contentManager, + final SkillsManager skillsManager) { + super(properties, logManager); + this.userManager = userManager; + this.contentManager = contentManager; + this.skillsManager = skillsManager; + } + + /** + * Endpoint that records when a user has answered a question. + * + * @param request + * - the servlet request so we can find out if it is a known user. + * @param appId + * - the app that the attempt belongs to + */ + @POST + @Path("/{appId}/answer") + @Consumes(MediaType.APPLICATION_JSON) + @Produces(MediaType.APPLICATION_JSON) + public Response answerQuestion(@Context final HttpServletRequest request, + @PathParam("appId") final String appId, + final AnvilMarkingRequestDTO markingRequest) { + try { + RegisteredUserDTO currentUser = userManager.getCurrentRegisteredUser(request); + + if (!(hasAnvilApp(this.contentManager.getContentDOById(appId)))) { + var error = new SegueErrorResponse(Status.NOT_FOUND, "No app found for given id: " + appId); + log.warn(error.getErrorMessage()); + return error.toResponse(); + } + + if (!skillsManager.isHmacValid(markingRequest)) { + var error = new SegueErrorResponse(Status.BAD_REQUEST, "Invalid HMAC signature"); + log.warn(error.getErrorMessage()); + return error.toResponse(); + } + + AnvilPayloadDTO payloadDTO = skillsManager.parsePayload( + markingRequest.getPayload(), currentUser.getId(), appId); + skillsManager.recordAttempt(payloadDTO); + + return Response.ok(payloadDTO).build(); + } catch (final NoUserLoggedInException e) { + return SegueErrorResponse.getNotLoggedInResponse(); + } catch (final InvalidAnvilMarkingRequestException e) { + var error = new SegueErrorResponse(Status.BAD_REQUEST, e.getMessage()); + log.warn(error.getErrorMessage() + ", " + e.getDetailedProblem()); + return error.toResponse(); + } catch (final SegueDatabaseException e) { + if ("Duplicate attempt ID".equals(e.getMessage())) { + var error = new SegueErrorResponse(Status.CONFLICT, "Duplicate attempt ID"); + log.warn(error.getErrorMessage()); + return error.toResponse(); + } + var error = new SegueErrorResponse(Status.INTERNAL_SERVER_ERROR, "Something went wrong"); + log.error(error.getErrorMessage()); + return error.toResponse(); + } catch (final ContentManagerException e) { + var error = new SegueErrorResponse(Status.NOT_FOUND, "Error locating the version requested", e); + log.error(error.getErrorMessage(), e); + return error.toResponse(); + } + } + + private boolean hasAnvilApp(final Content content) { + return content instanceof AnvilApp + || (content != null + && content.getChildren().stream().anyMatch(c -> c instanceof Content cc && hasAnvilApp(cc))); + } +} diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/InvalidAnvilMarkingRequestException.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/InvalidAnvilMarkingRequestException.java new file mode 100644 index 0000000000..6be145d8f5 --- /dev/null +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/InvalidAnvilMarkingRequestException.java @@ -0,0 +1,19 @@ +package uk.ac.cam.cl.dtg.isaac.api.managers; + +/** + * An exception that indicates an invalid marking response was received from the external marking server. + */ +public class InvalidAnvilMarkingRequestException extends Exception { + private final String detailedProblem; + + /** Constructor with message, detailed problem.*/ + public InvalidAnvilMarkingRequestException(final String message, final String detailedProblem) { + super(message); + this.detailedProblem = detailedProblem; + } + + /** Get detailed problem.*/ + public String getDetailedProblem() { + return detailedProblem; + } +} diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/SkillsManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/SkillsManager.java new file mode 100644 index 0000000000..352e367829 --- /dev/null +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/SkillsManager.java @@ -0,0 +1,89 @@ +package uk.ac.cam.cl.dtg.isaac.api.managers; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.inject.Inject; +import uk.ac.cam.cl.dtg.isaac.dto.AnvilMarkingRequestDTO; +import uk.ac.cam.cl.dtg.isaac.dto.AnvilPayloadDTO; +import uk.ac.cam.cl.dtg.isaac.quiz.ISkillsAttemptManager; +import uk.ac.cam.cl.dtg.segue.api.managers.UserAuthenticationManager; +import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; +import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; + +import java.util.Date; + +import static uk.ac.cam.cl.dtg.segue.api.Constants.*; + +/** + * Manager for Isaac Skills Practice app interactions. + */ +public class SkillsManager { + private static final ObjectMapper objectMapper = new ObjectMapper(); + + private final String hmacSecret; + private final ISkillsAttemptManager skillsAttemptManager; + + /** + * Constructor. + * + * @param properties - application configuration + * @param skillsAttemptManager - persistence manager for skills attempts + */ + @Inject + public SkillsManager(final AbstractConfigLoader properties, final ISkillsAttemptManager skillsAttemptManager) { + this.hmacSecret = properties.getProperty(SKILLS_HMAC_SECRET); + this.skillsAttemptManager = skillsAttemptManager; + } + + /** + * Verifies that the HMAC in the DTO matches the expected signature of the payload. + * + * @param dto - the parsed marking request + * @return whether the hmac was valid + */ + public boolean isHmacValid(final AnvilMarkingRequestDTO dto) { + String expected = UserAuthenticationManager.calculateHMAC(hmacSecret, dto.getPayload()); + return expected.equals(dto.getHmac()); + } + + /** + * Validates the content of the signed payload string. + * + * @param payloadStr - the payload string from the marking request + * @param userId - the ID of the currently authenticated user + * @param appId - the app ID from the URL, which must match the payload's skill_id + * @throws InvalidAnvilMarkingRequestException if the payload is malformed or any validation fails + */ + public AnvilPayloadDTO parsePayload( + final String payloadStr, final long userId, final String appId + ) throws InvalidAnvilMarkingRequestException { + try { + if (payloadStr.length() > 10 * 1024) { + throw new InvalidAnvilMarkingRequestException("Payload too large", null); + } + + AnvilPayloadDTO dto = objectMapper.readValue(payloadStr, AnvilPayloadDTO.class); + if (dto.getUserId() != userId) { + throw new InvalidAnvilMarkingRequestException("Payload user_id does not match session", null); + } + if (dto.getTimestamp().before(new Date(System.currentTimeMillis() - 300_000L))) { + throw new InvalidAnvilMarkingRequestException("Payload timestamp is outside the allowed window", null); + } + if (!dto.getSkillId().equals(appId)) { + throw new InvalidAnvilMarkingRequestException("Payload skill_id does not match app", null); + } + return dto; + } catch (final JsonProcessingException e) { + throw new InvalidAnvilMarkingRequestException("Invalid payload", e.getMessage()); + } + } + + /** + * Records a validated skills attempt. + * + * @param attempt - the validated payload DTO + */ + public void recordAttempt(final AnvilPayloadDTO attempt) throws SegueDatabaseException { + skillsAttemptManager.registerSkillsAttempt(attempt); + } +} diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/configuration/IsaacApplicationRegister.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/configuration/IsaacApplicationRegister.java index a540be9203..5bcee613a1 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/configuration/IsaacApplicationRegister.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/configuration/IsaacApplicationRegister.java @@ -34,6 +34,7 @@ import uk.ac.cam.cl.dtg.isaac.api.IsaacController; import uk.ac.cam.cl.dtg.isaac.api.PagesFacade; import uk.ac.cam.cl.dtg.isaac.api.QuizFacade; +import uk.ac.cam.cl.dtg.isaac.api.SkillsFacade; import uk.ac.cam.cl.dtg.segue.api.AdminFacade; import uk.ac.cam.cl.dtg.segue.api.AuthenticationFacade; import uk.ac.cam.cl.dtg.segue.api.AuthorisationFacade; @@ -123,6 +124,9 @@ public final Set getSingletons() { this.singletons.add(injector.getInstance(EmailFacade.class)); this.singletons.add(injector.getInstance(QuizFacade.class)); this.singletons.add(injector.getInstance(BookmarksFacade.class)); + if (injector.getInstance(AbstractConfigLoader.class).getProperty(SKILLS_HMAC_SECRET) != null) { + this.singletons.add(injector.getInstance(SkillsFacade.class)); + } // initialise filters this.singletons.add(injector.getInstance(PerformanceMonitor.class)); diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/PgSkillsAttemptManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/PgSkillsAttemptManager.java new file mode 100644 index 0000000000..540343074a --- /dev/null +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/PgSkillsAttemptManager.java @@ -0,0 +1,46 @@ +package uk.ac.cam.cl.dtg.isaac.dao; + +import com.google.inject.Inject; +import uk.ac.cam.cl.dtg.isaac.dto.AnvilPayloadDTO; +import uk.ac.cam.cl.dtg.isaac.quiz.ISkillsAttemptManager; +import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; +import uk.ac.cam.cl.dtg.segue.database.PostgresSqlDb; + +import java.sql.SQLException; +import java.sql.Timestamp; + +/** PostgreSQL-backed persistence for Anvil skills question attempts. */ +public class PgSkillsAttemptManager implements ISkillsAttemptManager { + private final PostgresSqlDb database; + + @Inject + public PgSkillsAttemptManager(final PostgresSqlDb database) { + this.database = database; + } + + @Override + public void registerSkillsAttempt(final AnvilPayloadDTO attempt) throws SegueDatabaseException { + try (var conn = database.getDatabaseConnection(); + var pst = conn.prepareStatement(""" + INSERT INTO skills_question_attempts ( + id, user_id, skill_assignment_id, skill_id, subskill_id, question, question_attempt, marks, + timestamp + ) VALUES (?, ?, ?, ?, ?, (?::jsonb), (?::jsonb), ?, ?)""")) { + pst.setObject(1, attempt.getId()); + pst.setLong(2, attempt.getUserId()); + pst.setString(3, attempt.getSkillAssignmentId()); + pst.setString(4, attempt.getSkillId()); + pst.setString(5, attempt.getSubskillId()); + pst.setString(6, attempt.getQuestion().toString()); + pst.setString(7, attempt.getQuestionAttempt().toString()); + pst.setInt(8, (Integer) attempt.getMarks()); + pst.setTimestamp(9, new Timestamp(attempt.getTimestamp().getTime())); + pst.executeUpdate(); + } catch (final SQLException e) { + if ("23505".equals(e.getSQLState())) { + throw new SegueDatabaseException("Duplicate attempt ID"); + } + throw new SegueDatabaseException("Something went wrong saving the attempt."); + } + } +} diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/AnvilMarkingRequestDTO.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/AnvilMarkingRequestDTO.java new file mode 100644 index 0000000000..cee248527a --- /dev/null +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/AnvilMarkingRequestDTO.java @@ -0,0 +1,44 @@ +package uk.ac.cam.cl.dtg.isaac.dto; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; + +/** + * DTO representing the request received from the external Anvil marking server. + */ +public class AnvilMarkingRequestDTO { + private final String payload; + private final String hmac; + + /** + * Constructor. + * + * @param payload - the signed payload from the marking server + * @param hmac - the HMAC-SHA256 hex digest authenticating the payload + */ + @JsonCreator + public AnvilMarkingRequestDTO( + @JsonProperty(value = "payload", required = true) final String payload, + @JsonProperty(value = "hmac", required = true) final String hmac) { + this.payload = payload; + this.hmac = hmac; + } + + /** + * Gets the payload. + * + * @return the signed payload string + */ + public String getPayload() { + return payload; + } + + /** + * Gets the HMAC signature. + * + * @return the HMAC-SHA256 hex digest + */ + public String getHmac() { + return hmac; + } +} diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/AnvilPayloadDTO.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/AnvilPayloadDTO.java new file mode 100644 index 0000000000..20a64973fe --- /dev/null +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/AnvilPayloadDTO.java @@ -0,0 +1,109 @@ +package uk.ac.cam.cl.dtg.isaac.dto; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSetter; +import com.fasterxml.jackson.annotation.Nulls; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.PropertyNamingStrategies; +import com.fasterxml.jackson.databind.annotation.JsonNaming; +import org.apache.commons.lang3.Validate; + +import java.util.Date; +import java.util.UUID; +import java.util.stream.Stream; + +/** + * DTO representing the signed payload content from the external Anvil marking server. + */ +@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class) +public class AnvilPayloadDTO { + private final UUID id; + private final Long userId; + private final String skillAssignmentId; + private final String skillId; + private final String subskillId; + private final JsonNode question; + private final JsonNode questionAttempt; + private final Number marks; + private final Date timestamp; + + /** + * Constructor. + * + * @param id - UUID identifying this attempt + * @param userId - the ID of the user who answered the question + * @param skillAssignmentId - the skill assignment ID (currently always null) + * @param skillId - the skill ID, must match the app ID from the URL + * @param subskillId - the subskill ID + * @param question - the question content + * @param questionAttempt - the student's answer + * @param marks - the marks awarded (0 or 1) + * @param timestamp - the datetime at which the response was signed + */ + @JsonCreator + public AnvilPayloadDTO( + @JsonSetter(nulls = Nulls.FAIL) @JsonProperty(value = "id", required = true) final UUID id, + @JsonSetter(nulls = Nulls.FAIL) @JsonProperty(value = "user_id", required = true) final Long userId, + @JsonProperty(value = "skill_assignment_id", required = true) final String skillAssignmentId, + @JsonSetter(nulls = Nulls.FAIL) @JsonProperty(value = "skill_id", required = true) final String skillId, + @JsonSetter(nulls = Nulls.FAIL) @JsonProperty(value = "subskill_id", required = true) + final String subskillId, + @JsonSetter(nulls = Nulls.FAIL) @JsonProperty(value = "question", required = true) final JsonNode question, + @JsonSetter(nulls = Nulls.FAIL) @JsonProperty(value = "question_attempt", required = true) + final JsonNode questionAttempt, + @JsonSetter(nulls = Nulls.FAIL) @JsonProperty(value = "marks", required = true) final Number marks, + @JsonSetter(nulls = Nulls.FAIL) @JsonProperty(value = "timestamp", required = true) final Date timestamp) { + Validate.isTrue(userId != 0); + Validate.isTrue(skillAssignmentId == null); + Validate.isTrue((marks instanceof Integer n) && (n == 0 || n == 1)); + Stream.of(question, questionAttempt).forEach(f -> Validate.isTrue(f.isObject())); + Stream.of(skillId, subskillId).forEach(Validate::notEmpty); + + this.id = id; + this.userId = userId; + this.skillAssignmentId = null; + this.skillId = skillId; + this.subskillId = subskillId; + this.question = question; + this.questionAttempt = questionAttempt; + this.marks = marks; + this.timestamp = timestamp; + } + + public UUID getId() { + return id; + } + + public Long getUserId() { + return userId; + } + + public String getSkillAssignmentId() { + return skillAssignmentId; + } + + public String getSkillId() { + return skillId; + } + + public String getSubskillId() { + return subskillId; + } + + public JsonNode getQuestion() { + return question; + } + + public JsonNode getQuestionAttempt() { + return questionAttempt; + } + + public Number getMarks() { + return marks; + } + + public Date getTimestamp() { + return timestamp; + } +} diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/ISkillsAttemptManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/ISkillsAttemptManager.java new file mode 100644 index 0000000000..1d703af4a9 --- /dev/null +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/ISkillsAttemptManager.java @@ -0,0 +1,9 @@ +package uk.ac.cam.cl.dtg.isaac.quiz; + +import uk.ac.cam.cl.dtg.isaac.dto.AnvilPayloadDTO; +import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; + +/** Persistence interface for recording Anvil skills question attempts. */ +public interface ISkillsAttemptManager { + void registerSkillsAttempt(final AnvilPayloadDTO attempt) throws SegueDatabaseException; +} diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/Constants.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/Constants.java index 5a6e27249b..08c28a58a2 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/Constants.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/Constants.java @@ -192,6 +192,7 @@ public enum AuthenticationCaveat { * Constant representing the key for the HMAC Salt - used in HMAC calculations. */ public static final String HMAC_SALT = "HMAC_SALT"; + public static final String SKILLS_HMAC_SECRET = "SKILLS_HMAC_SECRET"; public static final int TRUNCATED_TOKEN_LENGTH = 8; // Search stuff diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java b/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java index fabccbeda2..67fb5dd377 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java @@ -38,6 +38,7 @@ import org.reflections.Reflections; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import uk.ac.cam.cl.dtg.isaac.quiz.ISkillsAttemptManager; import uk.ac.cam.cl.dtg.isaac.api.managers.AssignmentManager; import uk.ac.cam.cl.dtg.isaac.api.managers.GameManager; import uk.ac.cam.cl.dtg.isaac.api.managers.QuizAssignmentManager; @@ -55,6 +56,7 @@ import uk.ac.cam.cl.dtg.isaac.dao.PgQuizAssignmentPersistenceManager; import uk.ac.cam.cl.dtg.isaac.dao.PgQuizAttemptPersistenceManager; import uk.ac.cam.cl.dtg.isaac.dao.PgQuizQuestionAttemptPersistenceManager; +import uk.ac.cam.cl.dtg.isaac.dao.PgSkillsAttemptManager; import uk.ac.cam.cl.dtg.isaac.dos.AbstractUserPreferenceManager; import uk.ac.cam.cl.dtg.isaac.dos.ILocationHistory; import uk.ac.cam.cl.dtg.isaac.dos.IUserAlerts; @@ -191,6 +193,7 @@ public class SegueGuiceConfigurationModule extends AbstractModule implements Ser private static UserAccountManager userManager = null; private static UserAuthenticationManager userAuthenticationManager = null; private static IQuestionAttemptManager questionPersistenceManager = null; + private static ISkillsAttemptManager skillsAttemptManager = null; private static SegueJobService segueJobService = null; private static ILogManager logManager; @@ -874,6 +877,25 @@ private IQuestionAttemptManager getQuestionManager(final PostgresSqlDb ds, final return questionPersistenceManager; } + /** + * SkillAttemptManager. + * + * @param ds - postgres data source + * @return a singleton for question persistence. + */ + @Inject + @Provides + @Singleton + private ISkillsAttemptManager getSkillAttemptManager(final PostgresSqlDb ds) { + // this needs to be a singleton as it provides a temporary cache for anonymous question attempts. + if (null == skillsAttemptManager) { + skillsAttemptManager = new PgSkillsAttemptManager(ds); + log.info("Creating singleton of ISkillsAttemptManager"); + } + + return skillsAttemptManager; + } + /** * This provides a singleton of the GroupManager. * diff --git a/src/main/resources/db_scripts/migrations/2026-06-skills_question_attempts.sql b/src/main/resources/db_scripts/migrations/2026-06-skills_question_attempts.sql new file mode 100644 index 0000000000..6ed5c2b71b --- /dev/null +++ b/src/main/resources/db_scripts/migrations/2026-06-skills_question_attempts.sql @@ -0,0 +1,15 @@ +CREATE TABLE public.skills_question_attempts ( + id UUID PRIMARY KEY, + user_id INTEGER NOT NULL, + skill_assignment_id TEXT, + skill_id TEXT NOT NULL, + subskill_id TEXT NOT NULL, + question JSONB NOT NULL, + question_attempt JSONB NOT NULL, + marks INTEGER NOT NULL, + timestamp TIMESTAMP WITH TIME ZONE NOT NULL, + CONSTRAINT user_id_skills_question_attempts_fk FOREIGN KEY (user_id) REFERENCES public.users (id) + ON UPDATE CASCADE ON DELETE CASCADE -- account deletion does not delete user so this won't clear attempt either +); + +ALTER TABLE public.skills_question_attempts OWNER to rutherford; diff --git a/src/main/resources/db_scripts/postgres-rutherford-create-script.sql b/src/main/resources/db_scripts/postgres-rutherford-create-script.sql index db99928ea9..ca2c14f0c1 100644 --- a/src/main/resources/db_scripts/postgres-rutherford-create-script.sql +++ b/src/main/resources/db_scripts/postgres-rutherford-create-script.sql @@ -1542,7 +1542,24 @@ ALTER TABLE ONLY public.user_streak_freezes ALTER TABLE ONLY public.user_streak_targets ADD CONSTRAINT user_streak_targets_user_id_fkey FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE CASCADE; +-- +-- Name: skill_question_attempts; Type: TABLE; Schema: public; Owner: rutherford +-- +CREATE TABLE public.skills_question_attempts ( + id UUID PRIMARY KEY, + user_id INTEGER NOT NULL, + skill_assignment_id TEXT, + skill_id TEXT NOT NULL, + subskill_id TEXT NOT NULL, + question JSONB NOT NULL, + question_attempt JSONB NOT NULL, + marks INTEGER NOT NULL, + timestamp TIMESTAMP WITH TIME ZONE NOT NULL, + CONSTRAINT user_id_skills_question_attempts_fk FOREIGN KEY (user_id) REFERENCES public.users (id) + ON UPDATE CASCADE ON DELETE CASCADE -- account deletion does not delete user so this won't clear attempt either +); +ALTER TABLE public.skills_question_attempts OWNER to rutherford; -- -- PostgreSQL database dump complete -- diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/ElasticSearchTestHelper.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/ElasticSearchTestHelper.java new file mode 100644 index 0000000000..964d8097e3 --- /dev/null +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/ElasticSearchTestHelper.java @@ -0,0 +1,54 @@ +package uk.ac.cam.cl.dtg.isaac.api; + +import org.json.JSONObject; +import uk.ac.cam.cl.dtg.isaac.dos.content.Content; +import uk.ac.cam.cl.dtg.segue.dao.content.ContentSubclassMapper; +import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; +import uk.ac.cam.cl.dtg.segue.etl.ElasticSearchIndexer; + +import java.util.List; + +import static org.testcontainers.shaded.com.google.common.collect.Maps.immutableEntry; + +/** + * Test utility for indexing content directly into Elasticsearch, bypassing the Git content pipeline. + * Use instead of text fixtures for more self-contained tests and to avoid needing to use the same + * database state across all tests. + * */ +public class ElasticSearchTestHelper { + private final ElasticSearchIndexer elasticSearchProvider; + private final GitContentManager contentManager; + private final ContentSubclassMapper contentMapper; + + /** Constructor.*/ + public ElasticSearchTestHelper(final ElasticSearchIndexer elasticSearchProvider, + final GitContentManager contentManager, + final ContentSubclassMapper contentMapper) { + this.elasticSearchProvider = elasticSearchProvider; + this.contentManager = contentManager; + this.contentMapper = contentMapper; + } + + /** Indexes a typed content object and returns it. */ + public T persist(final T content) throws Exception { + elasticSearchProvider.bulkIndexWithIDs( + contentManager.getCurrentContentSHA(), + "content", + List.of(immutableEntry( + content.getId(), contentMapper.getSharedContentObjectMapper().writeValueAsString(content)) + ) + ); + return content; + } + + /** Indexes a raw JSON content object with a fixed ID of {@code "i1"} and returns it. */ + public JSONObject persistJSON(final JSONObject contentJSON) throws Exception { + contentJSON.put("id", "i1"); + elasticSearchProvider.bulkIndexWithIDs( + contentManager.getCurrentContentSHA(), + "content", + List.of(immutableEntry("i1", contentJSON.toString())) + ); + return contentJSON; + } +} diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/IsaacIntegrationTestWithREST.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/IsaacIntegrationTestWithREST.java index f767c08fa6..8608e91da8 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/IsaacIntegrationTestWithREST.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/IsaacIntegrationTestWithREST.java @@ -31,7 +31,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; import static uk.ac.cam.cl.dtg.segue.api.Constants.*; /** @@ -40,6 +39,9 @@ * interacting with the server. As the server runs in-process, mocking and debugging still work. */ public class IsaacIntegrationTestWithREST extends AbstractIsaacIntegrationTest { + static final ElasticSearchTestHelper elasticHelper = + new ElasticSearchTestHelper(elasticSearchProvider, contentManager, contentMapper); + private final Set cleanups = new HashSet<>(); @SuppressWarnings({"checkstyle:EmptyCatchBlock", "checkstyle:MissingJavadocMethod"}) @@ -128,6 +130,15 @@ public static class TestApp extends Application { public Set getSingletons() { return TestApp.facades; } + + @Override + public Set> getClasses() { + return Set.of( + uk.ac.cam.cl.dtg.isaac.configuration.RestEasyJacksonConfiguration.class, + uk.ac.cam.cl.dtg.segue.configuration.exceptionMappers.JacksonExceptionMapper.class, + uk.ac.cam.cl.dtg.isaac.configuration.exceptionMappers.UnhandledExceptionMapper.class + ); + } } } @@ -154,13 +165,16 @@ public TestResponse get(final String url) { public TestResponse post(final String url, final Object body) { Invocation.Builder request = client.target(baseUrl + url).request(MediaType.APPLICATION_JSON); - Response response = builder.apply(request).post(Entity.json(body)); + Object serializable = body instanceof JSONObject j ? j.toString() : body; + Response response = builder.apply(request).post(Entity.json(serializable)); registerCleanup.accept(response::close); return new TestResponse(response); } public TestClient loginAs(final RegisteredUser user) { - Invocation.Builder request = client.target(baseUrl + "/auth/SEGUE/authenticate").request(MediaType.APPLICATION_JSON); + Invocation.Builder request = client + .target(baseUrl + "/auth/SEGUE/authenticate") + .request(MediaType.APPLICATION_JSON); LocalAuthDTO body = new LocalAuthDTO(); body.setEmail(user.getEmail()); body.setPassword("test1234"); @@ -178,7 +192,7 @@ static class TestResponse { void assertError(final String message, final Response.Status status) { assertEquals(status.getStatusCode(), response.getStatus()); - assertTrue(this.readEntityAsJsonUnchecked().getString("errorMessage").contains(message)); + assertThat(this.readEntityAsJsonUnchecked().getString("errorMessage")).contains(message); } void assertError(final String message, final String status) { diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/QuestionFacadeIT.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/QuestionFacadeIT.java index f1eaed5b9b..ceb070634c 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/QuestionFacadeIT.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/QuestionFacadeIT.java @@ -1,31 +1,21 @@ package uk.ac.cam.cl.dtg.isaac.api; -import com.google.inject.Injector; import org.json.JSONArray; import org.json.JSONObject; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; -import uk.ac.cam.cl.dtg.isaac.dos.IsaacDndQuestion; import uk.ac.cam.cl.dtg.isaac.dos.content.Content; import uk.ac.cam.cl.dtg.isaac.dos.content.DndChoice; -import uk.ac.cam.cl.dtg.isaac.quiz.IsaacDndValidator; -import uk.ac.cam.cl.dtg.isaac.quiz.IsaacDndValidatorTest; -import uk.ac.cam.cl.dtg.isaac.quiz.IsaacStringMatchValidator; import uk.ac.cam.cl.dtg.segue.api.QuestionFacade; -import uk.ac.cam.cl.dtg.segue.configuration.SegueGuiceConfigurationModule; import jakarta.ws.rs.core.Response; import java.util.List; import java.util.Map; -import static org.easymock.EasyMock.createNiceMock; -import static org.easymock.EasyMock.expect; -import static org.easymock.EasyMock.replay; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.testcontainers.shaded.com.google.common.collect.Maps.immutableEntry; import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacDndValidatorTest.*; @SuppressWarnings("checkstyle:MissingJavadocType") @@ -71,7 +61,7 @@ public void rightAnswer() throws Exception { class DndQuestion { @Test public void invalidQuestion() throws Exception { - var question = persistJSON(new JSONObject() + var question = elasticHelper.persistJSON(new JSONObject() .put("type", "isaacDndQuestion") .put("items", new JSONArray().put(new JSONObject().put("id", "item_id").put("type", "item"))) .put("children", new JSONArray().put( @@ -109,7 +99,7 @@ public void invalidQuestion() throws Exception { public void badRequest_ErrorReturned( final String answerStr, final String emsg, final String estate ) throws Exception { - var dndQuestion = persist(question(correctChoice(item(item_3cm, "leg_1")))); + var dndQuestion = elasticHelper.persist(question(correctChoice(item(item_3cm, "leg_1")))); var response = testServer().client().post(url(dndQuestion.getId()), answerStr); response.assertError(emsg, estate); } @@ -124,7 +114,7 @@ public void badRequest_ErrorReturned( public void badRequest_IncorrectReturnedWithExplanation( final String answerStr, final String emsg ) throws Exception { - var dndQuestion = persist(question( + var dndQuestion = elasticHelper.persist(question( correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse")) )); var response = testServer().client().post(url(dndQuestion.getId()), answerStr).readEntityAsJson(); @@ -138,7 +128,7 @@ public void badRequest_IncorrectReturnedWithExplanation( "{\"type\": \"dndChoice\", \"items\": []}" }, delimiter = '|') public void emptyAnswer_IncorrectReturned(final String answerStr) throws Exception { - var dndQuestion = persist(question(correctChoice(item(item_3cm, "leg_1")))); + var dndQuestion = elasticHelper.persist(question(correctChoice(item(item_3cm, "leg_1")))); var response = testServer().client().post(url(dndQuestion.getId()), answerStr).readEntityAsJson(); assertFalse(response.getBoolean("correct")); assertEquals( @@ -158,7 +148,7 @@ public void emptyAnswer_IncorrectReturned(final String answerStr) throws Excepti public void correctAnswer_CorrectReturned(final String answerStr) throws Exception { var dndQuestion = question(correctChoice(item(item_3cm, "leg_1"))); dndQuestion.setChildren(List.of(new Content("[drop-zone:leg_1]"))); - persist(dndQuestion); + elasticHelper.persist(dndQuestion); var response = testServer().client().post(url(dndQuestion.getId()), answerStr).readEntityAsJson(); assertTrue(response.getBoolean("correct")); @@ -168,7 +158,7 @@ public void correctAnswer_CorrectReturned(final String answerStr) throws Excepti @Test public void wrongAnswer_IncorrectReturned() throws Exception { - var dndQuestion = persist(question( + var dndQuestion = elasticHelper.persist(question( correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse")) )); var answer = choice(item(item_3cm, "leg_2"), item(item_4cm, "hypothenuse"), item(item_5cm, "leg_1")); @@ -182,7 +172,7 @@ public void wrongAnswer_IncorrectReturned() throws Exception { @Test public void answerWithMatchingExplanation_ExplanationReturned() throws Exception { var explanation = new Content("That's right!"); - var dndQuestion = persist(question(correctChoice( + var dndQuestion = elasticHelper.persist(question(correctChoice( explanation, item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse") ))); @@ -200,7 +190,7 @@ public void detailedItemFeedbackRequested_DropZonesCorrectReturned() throws Exce correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse")) ); dndQuestion.setDetailedItemFeedback(true); - persist(dndQuestion); + elasticHelper.persist(dndQuestion); var answer = choice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse")); var response = testServer().client().post(url(dndQuestion.getId()), answer).readEntityAsJson(); @@ -213,28 +203,6 @@ public void detailedItemFeedbackRequested_DropZonesCorrectReturned() throws Exce } } - private IsaacDndQuestion persist(final IsaacDndQuestion question) throws Exception { - elasticSearchProvider.bulkIndexWithIDs( - contentManager.getCurrentContentSHA(), - "content", - List.of(immutableEntry( - question.getId(), contentMapper.getSharedContentObjectMapper().writeValueAsString(question)) - ) - ); - return question; - } - - private JSONObject persistJSON(final JSONObject questionJSON) throws Exception { - questionJSON.put("id", "i1"); - elasticSearchProvider.bulkIndexWithIDs( - contentManager.getCurrentContentSHA(), - "content", - List.of(immutableEntry("i1", questionJSON.toString())) - ); - return questionJSON; - } - - private TestServer testServer() throws Exception { return startServer( new QuestionFacade(properties, contentMapper, contentManager, userAccountManager, diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/SkillsFacadeIT.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/SkillsFacadeIT.java new file mode 100644 index 0000000000..a4114d9168 --- /dev/null +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/SkillsFacadeIT.java @@ -0,0 +1,329 @@ +package uk.ac.cam.cl.dtg.isaac.api; + +import org.apache.commons.codec.binary.Base64; +import org.json.JSONArray; +import org.json.JSONObject; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import uk.ac.cam.cl.dtg.isaac.api.managers.SkillsManager; +import uk.ac.cam.cl.dtg.isaac.dao.PgSkillsAttemptManager; +import uk.ac.cam.cl.dtg.segue.api.AuthenticationFacade; +import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; +import uk.ac.cam.cl.dtg.segue.search.ElasticSearchProvider; + +import jakarta.ws.rs.core.Response; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.UUID; +import java.util.function.Consumer; +import java.util.stream.Stream; +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.within; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static uk.ac.cam.cl.dtg.isaac.api.ITConstants.REGRESSION_TEST_PAGE_ID; +import static uk.ac.cam.cl.dtg.isaac.api.ITConstants.TEST_STUDENT_ID; + +@SuppressWarnings("checkstyle:MissingJavadocType") +public class SkillsFacadeIT extends IsaacIntegrationTestWithREST { + private static final String HMAC_SECRET = "integration-test-anvil-hmac-secret"; + private static final String HMAC_SHA_256 = "HmacSHA256"; + private static final String VALID_URL = validUrl(); + private static final String VALID_APP_ID = VALID_URL.split("/")[2]; + private static final String VALID_PAYLOAD = validPayload(null); + private static final String VALID_HMAC = sign(HMAC_SECRET, VALID_PAYLOAD, HMAC_SHA_256); + private static final JSONObject VALID_BODY = new JSONObject().put("payload", VALID_PAYLOAD).put("hmac", VALID_HMAC); + + @Test + public void notLoggedIn_Returns401() throws Exception { + var response = testServer().client().post("/skills/unknown_app/answer", VALID_BODY); + response.assertError("You must be logged in to access this resource.", Response.Status.UNAUTHORIZED); + } + + @Nested + class AppIdCheck { + @Test + public void elasticsearchUnavailable_Returns404() throws Exception { + var client = testServer(brokenContentManager()).client().loginAs(integrationTestUsers.TEST_STUDENT); + var response = client.post("/skills/unknown_app/answer", VALID_BODY); + response.assertError("Error locating the version requested", Response.Status.NOT_FOUND); + } + + @Test + public void unknownApp_Returns404() throws Exception { + var client = testServer().client().loginAs(integrationTestUsers.TEST_STUDENT); + var response = client.post("/skills/unknown_app/answer", VALID_BODY); + response.assertError("No app found for given id: unknown_app", Response.Status.NOT_FOUND); + } + + @Test + public void idMatchesNonApp_Returns404() throws Exception { + var client = testServer().client().loginAs(integrationTestUsers.TEST_STUDENT); + var response = client.post("/skills/" + REGRESSION_TEST_PAGE_ID + "/answer", VALID_BODY); + response.assertError("No app found for given id: " + REGRESSION_TEST_PAGE_ID, Response.Status.NOT_FOUND); + } + } + + @Nested + class RequestPayloadCheck { + @Test + public void missingPayload_Returns400() throws Exception { + var client = testServer().client().loginAs(integrationTestUsers.TEST_STUDENT); + var response = client.post(VALID_URL, "{}"); + response.assertError("Invalid JSON provided!", Response.Status.BAD_REQUEST); + } + + @Test + public void numericPayload_Returns400() throws Exception { + var client = testServer().client().loginAs(integrationTestUsers.TEST_STUDENT); + var body = new JSONObject().put("payload", 123).put("hmac", sign(HMAC_SECRET, "123", HMAC_SHA_256)); + var response = client.post(VALID_URL, body); + response.assertError("Invalid payload", Response.Status.BAD_REQUEST); + } + + @Test + public void extraAttribute_Returns400() throws Exception { + var client = testServer().client().loginAs(integrationTestUsers.TEST_STUDENT); + var body = new JSONObject().put("payload", VALID_PAYLOAD).put("hmac", VALID_HMAC).put("extra", "value"); + var response = client.post(VALID_URL, body); + response.assertError("Invalid JSON provided!", Response.Status.BAD_REQUEST); + } + + @Test + public void oversizedPayload_Returns400() throws Exception { + var large = validPayload(p -> p.put("question_attempt", "x".repeat(10 * 1024 + 1))); + var body = new JSONObject().put("payload", large).put("hmac", sign(HMAC_SECRET, large, HMAC_SHA_256)); + var response = testServer().client().loginAs(integrationTestUsers.TEST_STUDENT).post(VALID_URL, body); + response.assertError("Payload too large", Response.Status.BAD_REQUEST); + } + } + + @Nested + class HmacVerification { + static Stream invalidBodies() { + return Stream.of( + Arguments.of("missing hmac", new JSONObject().put("payload", VALID_PAYLOAD), + "Invalid JSON provided!"), + Arguments.of("invalid hmac", new JSONObject().put("payload", VALID_PAYLOAD) + .put("hmac", "not-a-valid-signature"), "Invalid HMAC signature"), + Arguments.of("wrong secret", new JSONObject().put("payload", VALID_PAYLOAD) + .put("hmac", sign("wrong-secret", VALID_PAYLOAD, HMAC_SHA_256)), "Invalid HMAC signature"), + Arguments.of("tampered payload", new JSONObject().put("payload", "tampered_payload") + .put("hmac", VALID_HMAC), "Invalid HMAC signature"), + Arguments.of("wrong algorithm", new JSONObject().put("payload", VALID_PAYLOAD) + .put("hmac", sign(HMAC_SECRET, VALID_PAYLOAD, "HmacMD5")), "Invalid HMAC signature") + ); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("invalidBodies") + public void invalidBody_Returns400( + final String name, final JSONObject body, final String expectedMessage + ) throws Exception { + testServer().client() + .loginAs(integrationTestUsers.TEST_STUDENT) + .post(VALID_URL, body) + .assertError(expectedMessage, Response.Status.BAD_REQUEST); + } + } + + @Nested + class PayloadContentCheck { + static String MSG_IP = "Invalid payload"; + + static Stream invalidPayloads() { + var s = Stream.builder(); + + // id + addNonEmptyCasesFor("id", s); + s.add(Arguments.of("invalid id", validPayload(p -> p.put("id", "not-uuid")), MSG_IP)); + + // user_id + addNonEmptyCasesFor("user_id", s); + s.add(Arguments.of("non-numeric user_id", validPayload(p -> p.put("user_id", "ab")), MSG_IP)); + s.add(Arguments.of("wrong user_id", validPayload(p -> p.put("user_id", TEST_STUDENT_ID + 1)), + "Payload user_id does not match session")); + + // skill_assignment_id + s.add(Arguments.of("missing skill_assignment_id", validPayload(p -> p.remove("skill_assignment_id")), + MSG_IP)); + s.add(Arguments.of("non-null skill_assignment_id", + validPayload(p -> p.put("skill_assignment_id", "some_id")), MSG_IP)); + + // skill_id + addNonEmptyCasesFor("skill_id", s); + s.add(Arguments.of("wrong skill_id", validPayload(p -> p.put("skill_id", "wrong_skill_id")), + "Payload skill_id does not match app")); + + // subskill_id + addNonEmptyCasesFor("subskill_id", s); + + // question + addNonEmptyCasesFor("question", s); + s.add(Arguments.of("invalid question JSON, str", validPayload(p -> p.put("question", "ab")), MSG_IP)); + s.add(Arguments.of("invalid question JSON, number", validPayload(p -> p.put("question", 1)), MSG_IP)); + + // question_attempt + addNonEmptyCasesFor("question_attempt", s); + s.add(Arguments.of("invalid attempts JSON", validPayload(p -> p.put("question_attempt", "a")), MSG_IP)); + + // marks + addNonEmptyCasesFor("marks", s); + s.add(Arguments.of("invalid mark 2", validPayload(p -> p.put("marks", 2)), MSG_IP)); + s.add(Arguments.of("invalid mark -0.4", validPayload(p -> p.put("marks", -0.4)), MSG_IP)); + + // timestamp + addNonEmptyCasesFor("timestamp", s); + s.add(Arguments.of("invalid timestamp", validPayload(p -> p.put("timestamp", "n/d")), MSG_IP)); + s.add(Arguments.of("expired timestamp", validPayload(p -> p.put("timestamp", "2022-01-01T13:00:00Z")), + "Payload timestamp is outside the allowed window")); + + // other cases + s.add(Arguments.of("extra value", validPayload(p -> p.put("extra", "value")), MSG_IP)); + + return s.build(); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("invalidPayloads") + public void invalidPayload_Returns400( + final String name, final JSONObject payload, final String expectedMessage + ) throws Exception { + testServer().client() + .loginAs(integrationTestUsers.TEST_STUDENT) + .post(VALID_URL, new JSONObject() + .put("payload", payload.toString()) + .put("hmac", sign(HMAC_SECRET, payload.toString(), HMAC_SHA_256))) + .assertError(expectedMessage, Response.Status.BAD_REQUEST); + } + + private static void addNonEmptyCasesFor(final String fieldName, final Stream.Builder s) { + s.add(Arguments.of("missing " + fieldName, validPayload(p -> p.remove(fieldName)), MSG_IP)); + s.add(Arguments.of("null " + fieldName, validPayload(p -> p.put(fieldName, JSONObject.NULL)), MSG_IP)); + s.add(Arguments.of("empty " + fieldName, validPayload(p -> p.put(fieldName, "")), MSG_IP)); + } + } + + @Test + public void duplicateAttemptId_Returns409() throws Exception { + var testP = validPayload(null); + var testBody = new JSONObject().put("payload", testP).put("hmac", sign(HMAC_SECRET, testP, HMAC_SHA_256)); + var client = testServer().client().loginAs(integrationTestUsers.TEST_STUDENT); + client.post(VALID_URL, testBody).readEntity(String.class); + client.post(VALID_URL, testBody).assertError("Duplicate attempt ID", Response.Status.CONFLICT); + } + + @Test + public void happy_happy() throws Exception { + var expected = new JSONObject(VALID_PAYLOAD); + var actualReq = testServer().client() + .loginAs(integrationTestUsers.TEST_STUDENT) + .post(VALID_URL, VALID_BODY) + .readEntityAsJson(); + + // assert there is exactly 1 row in the database + try (var conn = postgresSqlDb.getDatabaseConnection(); + var pst = conn.prepareStatement("SELECT COUNT(*) FROM public.skills_question_attempts"); + var rs = pst.executeQuery()) { + rs.next(); + assertEquals(1, rs.getInt(1)); + } + + // assert everything has been recorded to the database + try (var conn = postgresSqlDb.getDatabaseConnection(); + var pst = conn.prepareStatement(""" + SELECT id, user_id, skill_assignment_id, skill_id, subskill_id, question->>'text' AS question_text, + question->>'answer' AS question_answer, question_attempt->>'result' AS result, marks, timestamp + FROM public.skills_question_attempts"""); + var actualDb = pst.executeQuery()) { + actualDb.next(); + assertEquals(expected.getString("id"), actualDb.getString("id")); + assertEquals(expected.getInt("user_id"), actualDb.getInt("user_id")); + assertNull(actualDb.getString("skill_assignment_id")); + assertEquals(expected.getString("skill_id"), actualDb.getString("skill_id")); + assertEquals(expected.get("subskill_id").toString(), actualDb.getString("subskill_id")); + assertEquals(expected.getJSONObject("question").getString("text"), actualDb.getString("question_text")); + assertEquals(expected.getJSONObject("question").getString("answer"), actualDb.getString("question_answer")); + assertEquals(expected.getJSONObject("question_attempt").getString("result"), actualDb.getString("result")); + assertEquals(expected.getInt("marks"), actualDb.getInt("marks")); + assertThat(actualDb.getTimestamp("timestamp").toInstant()) + .isCloseTo(expected.getString("timestamp"), within(5, ChronoUnit.SECONDS)); + } + + // assert the request echoes back the payload + assertEquals(expected.getString("id"), actualReq.getString("id")); + assertEquals(expected.getInt("user_id"), actualReq.getInt("user_id")); + assertEquals(expected.getString("skill_id"), actualReq.getString("skill_id")); + assertEquals(expected.get("subskill_id").toString(), actualReq.getString("subskill_id")); + assertEquals(expected.getJSONObject("question").toString(), actualReq.getJSONObject("question").toString()); + assertEquals(expected.getJSONObject("question_attempt").toString(), + actualReq.getJSONObject("question_attempt").toString()); + assertEquals(expected.getInt("marks"), actualReq.getInt("marks")); + assertFalse(actualReq.has("skill_assignment_id")); + } + + private static GitContentManager brokenContentManager() throws Exception { + var brokenClient = ElasticSearchProvider.getClient("localhost", 1, "elastic", "elastic"); + var brokenProvider = new ElasticSearchProvider(brokenClient); + return new GitContentManager(null, brokenProvider, mainMapper, contentMapper, properties); + } + + private static String validUrl() { + try { + var page = elasticHelper.persistJSON(new JSONObject() + .put("type", "page") + .put("children", new JSONArray().put(new JSONObject().put("type", "anvilApp"))) + ); + return "/skills/" + page.getString("id") + "/answer"; + } catch (final Exception e) { + throw new RuntimeException(e); + } + } + + private static String validPayload(final Consumer op) { + var defaultPayload = new JSONObject() + .put("id", UUID.randomUUID().toString()) + .put("user_id", TEST_STUDENT_ID) + .put("skill_assignment_id", JSONObject.NULL) + .put("skill_id", SkillsFacadeIT.VALID_APP_ID) + .put("subskill_id", 21) + .put("question", new JSONObject().put("text", "2+2").put("answer", "4")) + .put("question_attempt", new JSONObject().put("result", "4")) + .put("marks", 1) + .put("timestamp", Instant.now().toString()); + if (op != null) { + op.accept(defaultPayload); + } + return defaultPayload.toString(); + } + + private static String sign(final String key, final String payload, final String algo) { + try { + var signingKey = new SecretKeySpec(key.getBytes(), algo); + var mac = Mac.getInstance(algo); + mac.init(signingKey); + return new String(Base64.encodeBase64(mac.doFinal(payload.getBytes()))); + } catch (final Exception e) { + return "NOT_SIGNED"; + } + } + + private TestServer testServer() throws Exception { + return testServer(contentManager); + } + + private TestServer testServer(final GitContentManager cm) throws Exception { + var sm = new SkillsManager(properties, new PgSkillsAttemptManager(postgresSqlDb)); + return startServer( + new AuthenticationFacade(properties, userAccountManager, logManager, misuseMonitor), + new SkillsFacade(properties, userAccountManager, logManager, cm, sm) + ); + } +} diff --git a/src/test/resources/segue-integration-test-config.yaml b/src/test/resources/segue-integration-test-config.yaml index 45ae8836e9..e2b2d1fa66 100644 --- a/src/test/resources/segue-integration-test-config.yaml +++ b/src/test/resources/segue-integration-test-config.yaml @@ -58,6 +58,7 @@ MAX_CONCURRENT_WEB_SOCKETS_PER_USER: "10" FASTTRACK_GAMEBOARD_WHITELIST: ft_core_2017,ft_core_2018 # Security HMAC_SALT: 0982374959082374098572930SUPER-SECRET-KEY5873246586857342867 +SKILLS_HMAC_SECRET: integration-test-anvil-hmac-secret SESSION_EXPIRY_SECONDS_DEFAULT: "432000" SESSION_EXPIRY_SECONDS_REMEMBERED: "1209600" EMAIL_VERIFICATION_ENDPOINT_TOKEN: some-very-secret-token