Skip to content

Commit 6531868

Browse files
feat: enforce research participation consent (#524)
1 parent 28a9019 commit 6531868

38 files changed

Lines changed: 906 additions & 185 deletions

.github/workflows/ci-quality-gates.yml

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -185,10 +185,7 @@ jobs:
185185
run: |
186186
echo "🔍 Running intelligence-service quality checks..."
187187
cd server/intelligence-service
188-
189-
if [ ! -d ".venv" ]; then
190-
poetry install --no-interaction --no-root
191-
fi
188+
poetry install --no-interaction --no-root --with dev
192189
193190
ISSUES_FOUND=()
194191
@@ -225,10 +222,7 @@ jobs:
225222
run: |
226223
echo "🔍 Running webhook-ingest quality checks..."
227224
cd server/webhook-ingest
228-
229-
if [ ! -d ".venv" ]; then
230-
poetry install --no-interaction --no-root
231-
fi
225+
poetry install --no-interaction --no-root --with dev
232226
233227
ISSUES_FOUND=()
234228
@@ -270,9 +264,7 @@ jobs:
270264
271265
# Setup Python environments
272266
cd server/intelligence-service
273-
if [ ! -d ".venv" ]; then
274-
poetry install --no-interaction --no-root
275-
fi
267+
poetry install --no-interaction --no-root --with dev
276268
cd ../..
277269
278270
# Setup application-server environment

AGENTS.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ Run the relevant commands locally before opening a PR:
3232
| Intelligence service lint/type check | `poetry run black --check .`, `poetry run flake8 .`, `poetry run mypy .` inside `server/intelligence-service`. |
3333
| Webhook ingest lint | `poetry run black --check .` and `poetry run flake8 .` inside `server/webhook-ingest`. |
3434

35-
Document any skipped gate in the PR description with a rationale.
35+
Document any skipped gate in the PR description with a rationale. Always finish a change set by running `npm run format` followed by `npm run lint` so both styling and type checks reflect the final state.
3636

3737
## 4. Code generation & forbidden edits
3838
We rely heavily on generated artifacts. Never hand-edit these directories—regenerate instead:
@@ -54,6 +54,7 @@ Regeneration is destructive; stash local edits before running these commands. Ch
5454
1. Spin up PostgreSQL through Docker (ensure Docker is running or set `CI=true` with a ready Postgres).
5555
2. Snapshot the schema, run Liquibase diff, and create a timestamped changelog file.
5656
3. Tear down the temporary container.
57+
- Trim the generated changelog to only the real schema deltas (e.g., new columns). Never commit the raw diff wholesale—prune back to the minimal change set before renaming it into `db/changelog/`.
5758
- After drafting a changelog, run `npm run db:generate-erd-docs` and `npm run db:generate-models:intelligence-service` to keep ERD docs and SQLAlchemy models in sync.
5859
- Never manually edit generated Liquibase diff sections unless you fully understand the implications. Prefer creating a follow-up changelog to fix mistakes.
5960

@@ -76,6 +77,7 @@ Regeneration is destructive; stash local edits before running these commands. Ch
7677
- Reuse existing DTO converters/mappers instead of duplicating mapping logic. Look at `gitprovider.team` for established patterns.
7778
- Security: new endpoints must enforce permissions using the existing security utilities (`EnsureAdminUser`, etc.).
7879
- Keep Liquibase changelog IDs monotonic and descriptive. Align entity annotations with the generated change sets.
80+
- Annotate record components in DTOs with `@NonNull` whenever the API should require them so the generated OpenAPI schema matches the backend contract.
7981
- When integrating with the intelligence-service client, always regenerate (`npm run generate:api:intelligence-service:client`) after touching the spec and commit the updated Java files.
8082

8183
## 8. Python services expectations

docker/compose.app.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ services:
1212
LEGAL_PRIVACY_HTML: ${LEGAL_PRIVACY_HTML}
1313
SENTRY_ENVIRONMENT: ${SENTRY_ENVIRONMENT}
1414
SENTRY_DSN: ${SENTRY_DSN}
15+
POSTHOG_ENABLED: ${POSTHOG_ENABLED:-false}
1516
POSTHOG_PROJECT_API_KEY: ${POSTHOG_PROJECT_API_KEY}
1617
POSTHOG_API_HOST: ${POSTHOG_API_HOST}
1718
ports:
@@ -90,6 +91,10 @@ services:
9091
LANGFUSE_PUBLIC_KEY: ${LANGFUSE_PUBLIC_KEY}
9192
LANGFUSE_SECRET_KEY: ${LANGFUSE_SECRET_KEY}
9293
LANGFUSE_HOST: ${LANGFUSE_HOST}
94+
POSTHOG_ENABLED: ${POSTHOG_ENABLED:-false}
95+
POSTHOG_API_HOST: ${POSTHOG_API_HOST:-https://app.posthog.com}
96+
POSTHOG_PROJECT_ID: ${POSTHOG_PROJECT_ID:-}
97+
POSTHOG_PERSONAL_API_KEY: ${POSTHOG_PERSONAL_API_KEY:-}
9398
depends_on:
9499
- postgres
95100
restart: unless-stopped

docs/contributor/erd/schema.mmd

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ erDiagram
314314
VARCHAR(255) type
315315
INTEGER league_points "NOT NULL"
316316
BOOLEAN notifications_enabled "NOT NULL"
317+
BOOLEAN participate_in_research "NOT NULL"
317318
}
318319

319320
Workspace {

server/application-server/openapi.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2749,8 +2749,13 @@ components:
27492749
UserSettings:
27502750
type: object
27512751
properties:
2752+
participateInResearch:
2753+
type: boolean
27522754
receiveNotifications:
27532755
type: boolean
2756+
required:
2757+
- participateInResearch
2758+
- receiveNotifications
27542759
UserTeams:
27552760
type: object
27562761
properties:

server/application-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/pullrequestreviewcomment/github/GitHubPullRequestReviewCommentSyncService.java

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -123,11 +123,13 @@ public PullRequestReviewComment processPullRequestReviewComment(
123123

124124
pullRequestReviewRepository
125125
.findById(ghPullRequestReviewComment.getPullRequestReviewId())
126-
.ifPresentOrElse(result::setReview, () -> logger.error(
127-
"Failed to link review for pull request review comment {}: {}",
128-
ghPullRequestReviewComment.getId(),
129-
"Review not found"
130-
));
126+
.ifPresentOrElse(result::setReview, () ->
127+
logger.error(
128+
"Failed to link review for pull request review comment {}: {}",
129+
ghPullRequestReviewComment.getId(),
130+
"Review not found"
131+
)
132+
);
131133

132134
attachAuthor(ghPullRequestReviewComment, result);
133135

@@ -143,10 +145,7 @@ public PullRequestReviewComment processPullRequestReviewComment(
143145
return persisted;
144146
}
145147

146-
private PullRequestReviewComment updateIfNewer(
147-
GHPullRequestReviewComment source,
148-
PullRequestReviewComment target
149-
) {
148+
private PullRequestReviewComment updateIfNewer(GHPullRequestReviewComment source, PullRequestReviewComment target) {
150149
try {
151150
if (target.getUpdatedAt() == null || target.getUpdatedAt().isBefore(source.getUpdatedAt())) {
152151
return pullRequestReviewCommentConverter.update(source, target);
@@ -158,10 +157,7 @@ private PullRequestReviewComment updateIfNewer(
158157
}
159158
}
160159

161-
private void attachAuthor(
162-
GHPullRequestReviewComment ghPullRequestReviewComment,
163-
PullRequestReviewComment comment
164-
) {
160+
private void attachAuthor(GHPullRequestReviewComment ghPullRequestReviewComment, PullRequestReviewComment comment) {
165161
try {
166162
GHUser user = ghPullRequestReviewComment.getUser();
167163
if (user == null) {
@@ -258,7 +254,10 @@ public void deletePullRequestReviewComment(long commentId) {
258254
.findById(commentId)
259255
.ifPresent(comment -> {
260256
var thread = comment.getThread();
261-
boolean isRootComment = thread != null && thread.getRootComment() != null && thread.getRootComment().getId().equals(commentId);
257+
boolean isRootComment =
258+
thread != null &&
259+
thread.getRootComment() != null &&
260+
thread.getRootComment().getId().equals(commentId);
262261

263262
if (isRootComment) {
264263
pullRequestReviewThreadRepository.delete(thread);

server/application-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/pullrequestreviewthread/github/GitHubPullRequestReviewThreadSyncService.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,10 @@ public PullRequestReviewThread processThreadEvent(GHEventPayloadPullRequestRevie
6969
.toList();
7070

7171
for (GHPullRequestReviewComment comment : sortedComments) {
72-
PullRequestReviewComment persisted = commentSyncService.processPullRequestReviewComment(comment, ghPullRequest);
72+
PullRequestReviewComment persisted = commentSyncService.processPullRequestReviewComment(
73+
comment,
74+
ghPullRequest
75+
);
7376
if (persisted != null) {
7477
thread = persisted.getThread();
7578
}

server/application-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/user/User.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,9 @@ public class User extends BaseGitServiceEntity {
9898
@NonNull
9999
private boolean notificationsEnabled = true;
100100

101+
@NonNull
102+
private boolean participateInResearch = true;
103+
101104
// Current ranking points for the leaderboard leagues
102105
private int leaguePoints;
103106

server/application-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/user/UserController.java

Lines changed: 55 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,17 @@
11
package de.tum.in.www1.hephaestus.gitprovider.user;
22

3+
import de.tum.in.www1.hephaestus.integrations.posthog.PosthogClientException;
34
import java.util.Optional;
45
import org.keycloak.admin.client.Keycloak;
56
import org.slf4j.Logger;
67
import org.slf4j.LoggerFactory;
78
import org.springframework.beans.factory.annotation.Autowired;
89
import org.springframework.beans.factory.annotation.Value;
10+
import org.springframework.http.HttpStatus;
911
import org.springframework.http.ResponseEntity;
12+
import org.springframework.security.core.Authentication;
13+
import org.springframework.security.core.annotation.AuthenticationPrincipal;
14+
import org.springframework.security.core.context.SecurityContextHolder;
1015
import org.springframework.security.oauth2.core.oidc.StandardClaimNames;
1116
import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationToken;
1217
import org.springframework.web.bind.annotation.DeleteMapping;
@@ -46,20 +51,33 @@ public ResponseEntity<UserProfileDTO> getUserProfile(@PathVariable String login)
4651
}
4752

4853
@DeleteMapping
49-
public ResponseEntity<Void> deleteUser(JwtAuthenticationToken auth) {
50-
if (auth == null) {
51-
logger.error("No authentication token found.");
52-
return ResponseEntity.badRequest().body(null);
54+
public ResponseEntity<Void> deleteUser(@AuthenticationPrincipal JwtAuthenticationToken auth) {
55+
JwtAuthenticationToken token = resolveAuthentication(auth);
56+
if (token == null) {
57+
logger.error("No authentication token found");
58+
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
5359
}
5460

55-
String userId = auth.getToken().getClaimAsString(StandardClaimNames.SUB);
56-
logger.info("Deleting user {}", userId);
57-
var response = keycloak.realm(realm).users().delete(userId);
61+
String keycloakUserId = token.getToken().getClaimAsString(StandardClaimNames.SUB);
62+
var gitUser = userRepository.getCurrentUser();
63+
if (gitUser.isEmpty()) {
64+
logger.warn("Could not resolve Git provider user for Keycloak subject {}", keycloakUserId);
65+
}
66+
67+
try {
68+
userService.deleteUserTrackingData(gitUser, keycloakUserId);
69+
} catch (PosthogClientException exception) {
70+
logger.error("Failed to remove analytics data before deleting user {}", keycloakUserId, exception);
71+
return ResponseEntity.status(HttpStatus.BAD_GATEWAY).build();
72+
}
73+
74+
logger.info("Deleting user {}", keycloakUserId);
75+
var response = keycloak.realm(realm).users().delete(keycloakUserId);
5876
if (response.getStatus() != 204) {
5977
logger.error("Failed to delete user account: {}", response.getStatusInfo().getReasonPhrase());
60-
return ResponseEntity.badRequest().body(null);
78+
return ResponseEntity.status(HttpStatus.BAD_GATEWAY).build();
6179
}
62-
return ResponseEntity.ok().build();
80+
return ResponseEntity.noContent().build();
6381
}
6482

6583
@GetMapping("/settings")
@@ -74,13 +92,39 @@ public ResponseEntity<UserSettingsDTO> getUserSettings() {
7492
}
7593

7694
@PostMapping("/settings")
77-
public ResponseEntity<UserSettingsDTO> updateUserSettings(@RequestBody UserSettingsDTO userSettings) {
95+
public ResponseEntity<UserSettingsDTO> updateUserSettings(
96+
@AuthenticationPrincipal JwtAuthenticationToken auth,
97+
@RequestBody UserSettingsDTO userSettings
98+
) {
7899
var user = userRepository.getCurrentUser();
79100
if (user.isEmpty()) {
80101
return ResponseEntity.notFound().build();
81102
}
82103

83-
UserSettingsDTO updatedUserSettings = userService.updateUserSettings(user.get(), userSettings);
104+
JwtAuthenticationToken token = resolveAuthentication(auth);
105+
String keycloakUserId = null;
106+
if (token != null) {
107+
keycloakUserId = token.getToken().getClaimAsString(StandardClaimNames.SUB);
108+
} else {
109+
logger.warn("Updating user settings without an authenticated principal");
110+
boolean switchingOffResearch =
111+
Boolean.FALSE.equals(userSettings.participateInResearch()) && user.get().isParticipateInResearch();
112+
if (switchingOffResearch) {
113+
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
114+
}
115+
}
116+
UserSettingsDTO updatedUserSettings = userService.updateUserSettings(user.get(), userSettings, keycloakUserId);
84117
return ResponseEntity.ok(updatedUserSettings);
85118
}
119+
120+
private JwtAuthenticationToken resolveAuthentication(JwtAuthenticationToken injectedToken) {
121+
if (injectedToken != null) {
122+
return injectedToken;
123+
}
124+
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
125+
if (authentication instanceof JwtAuthenticationToken jwtAuthenticationToken) {
126+
return jwtAuthenticationToken;
127+
}
128+
return null;
129+
}
86130
}

server/application-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/user/UserService.java

Lines changed: 72 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,25 @@
99
import de.tum.in.www1.hephaestus.gitprovider.pullrequestreview.PullRequestReviewRepository;
1010
import de.tum.in.www1.hephaestus.gitprovider.repository.RepositoryInfoDTO;
1111
import de.tum.in.www1.hephaestus.gitprovider.repository.RepositoryRepository;
12+
import de.tum.in.www1.hephaestus.integrations.posthog.PosthogClient;
13+
import de.tum.in.www1.hephaestus.integrations.posthog.PosthogClientException;
1214
import jakarta.transaction.Transactional;
1315
import java.time.Instant;
1416
import java.util.ArrayList;
1517
import java.util.Comparator;
18+
import java.util.LinkedHashSet;
1619
import java.util.List;
20+
import java.util.Objects;
1721
import java.util.Optional;
1822
import java.util.Set;
1923
import java.util.stream.Collectors;
2024
import org.slf4j.Logger;
2125
import org.slf4j.LoggerFactory;
2226
import org.springframework.beans.factory.annotation.Autowired;
27+
import org.springframework.http.HttpStatus;
2328
import org.springframework.stereotype.Service;
29+
import org.springframework.util.StringUtils;
30+
import org.springframework.web.server.ResponseStatusException;
2431

2532
@Service
2633
public class UserService {
@@ -45,6 +52,9 @@ public class UserService {
4552
@Autowired
4653
private PullRequestReviewInfoDTOConverter pullRequestReviewInfoDTOConverter;
4754

55+
@Autowired
56+
private PosthogClient posthogClient;
57+
4858
@Transactional
4959
public Optional<UserProfileDTO> getUserProfile(String login) {
5060
logger.info("Getting user profile with login: " + login);
@@ -90,13 +100,71 @@ public Optional<UserProfileDTO> getUserProfile(String login) {
90100

91101
public UserSettingsDTO getUserSettings(User user) {
92102
logger.info("Getting user settings with userId: " + user);
93-
return new UserSettingsDTO(user.isNotificationsEnabled());
103+
return new UserSettingsDTO(user.isNotificationsEnabled(), user.isParticipateInResearch());
94104
}
95105

96-
public UserSettingsDTO updateUserSettings(User user, UserSettingsDTO userSettings) {
106+
public UserSettingsDTO updateUserSettings(User user, UserSettingsDTO userSettings, String keycloakUserId) {
97107
logger.info("Updating user settings with userId: " + user);
98-
user.setNotificationsEnabled(userSettings.receiveNotifications());
108+
user.setNotificationsEnabled(
109+
Objects.requireNonNull(userSettings.receiveNotifications(), "receiveNotifications must not be null")
110+
);
111+
boolean previousParticipation = user.isParticipateInResearch();
112+
boolean participatesInResearch = Objects.requireNonNull(
113+
userSettings.participateInResearch(),
114+
"participateInResearch must not be null"
115+
);
116+
user.setParticipateInResearch(participatesInResearch);
99117
userRepository.save(user);
100-
return new UserSettingsDTO(user.isNotificationsEnabled());
118+
if (previousParticipation && !participatesInResearch) {
119+
if (!StringUtils.hasText(keycloakUserId)) {
120+
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Missing authentication subject");
121+
}
122+
try {
123+
boolean anyDeleted = deletePosthogIdentities(user, keycloakUserId);
124+
if (!anyDeleted) {
125+
logger.warn("No PostHog person matched the provided identifiers for user {}", user.getLogin());
126+
}
127+
} catch (PosthogClientException exception) {
128+
throw new ResponseStatusException(
129+
HttpStatus.BAD_GATEWAY,
130+
"Failed to revoke analytics consent",
131+
exception
132+
);
133+
}
134+
}
135+
return new UserSettingsDTO(user.isNotificationsEnabled(), user.isParticipateInResearch());
136+
}
137+
138+
public void deleteUserTrackingData(Optional<User> user, String keycloakUserId) {
139+
try {
140+
boolean anyDeleted = deletePosthogIdentities(user.orElse(null), keycloakUserId);
141+
if (!anyDeleted) {
142+
logger.warn(
143+
"No PostHog person matched the provided identifiers for user {} during account deletion",
144+
user.map(User::getLogin).orElse("unknown")
145+
);
146+
}
147+
} catch (PosthogClientException exception) {
148+
throw exception;
149+
}
150+
}
151+
152+
private boolean deletePosthogIdentities(User user, String primaryDistinctId) {
153+
Set<String> distinctIds = new LinkedHashSet<>();
154+
if (StringUtils.hasText(primaryDistinctId)) {
155+
distinctIds.add(primaryDistinctId);
156+
}
157+
if (user != null) {
158+
distinctIds.add(String.valueOf(user.getId()));
159+
}
160+
161+
boolean anyDeleted = false;
162+
for (String distinctId : distinctIds) {
163+
if (!StringUtils.hasText(distinctId)) {
164+
continue;
165+
}
166+
anyDeleted = posthogClient.deletePersonData(distinctId) || anyDeleted;
167+
}
168+
return anyDeleted;
101169
}
102170
}

0 commit comments

Comments
 (0)