Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions docs/cron-setup.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,18 @@ Configure this header in your scheduler. The secret is set in `build.properties`

## Timezone

All schedules below use **Asia/Singapore**. Configure your scheduler to use this timezone.
All schedules below use **UTC**. Configure your scheduler to use this timezone.

## Format 1: Human-Readable Schedule

| Endpoint | Description | Schedule (Asia/Singapore) |
| Endpoint | Description | Schedule (UTC) |
|----------|-------------|---------------------------|
| `/auto/feedbackSessionOpenedReminders` | Emails for sessions that just opened | Every hour at :02 |
| `/auto/feedbackSessionOpeningSoonReminders` | Emails for sessions opening in 24h | Every hour at :11 |
| `/auto/feedbackSessionClosingSoonReminders` | Reminders for sessions closing in 24h | Every hour at :06 |
| `/auto/feedbackSessionClosedReminders` | Emails for sessions that just closed | Every hour at :08 |
| `/auto/feedbackSessionPublishedReminders` | Emails for sessions just published | Every hour at :04 |
| `/auto/calculateUsageStatistics` | Gather usage statistics | Every hour at :01 |
| `/auto/datastoreBackup` | Monthly backup | 1st Sunday of month, 05:30 |
| `/auto/compileLogs` | Compile severe logs, send email | Every 5 minutes |
| `/auto/updateFeedbackSessionLogs` | Process feedback session logs | Every 15 minutes |

Expand All @@ -35,7 +34,7 @@ All schedules below use **Asia/Singapore**. Configure your scheduler to use this
Use these expressions with your scheduler of choice (e.g. Google Cloud Scheduler). Build the full URL as `https://your-teammates-app.com/auto/<path>`.

```
# Timezone: Asia/Singapore (set in your scheduler)
# Timezone: UTC (set in your scheduler)
# Format: cron_expression | url_path

2 * * * * | /auto/feedbackSessionOpenedReminders
Expand All @@ -44,7 +43,6 @@ Use these expressions with your scheduler of choice (e.g. Google Cloud Scheduler
8 * * * * | /auto/feedbackSessionClosedReminders
4 * * * * | /auto/feedbackSessionPublishedReminders
1 * * * * | /auto/calculateUsageStatistics
30 5 1-7 * 0 | /auto/datastoreBackup
*/5 * * * * | /auto/compileLogs
1,16,31,46 * * * * | /auto/updateFeedbackSessionLogs
```
Expand All @@ -55,4 +53,4 @@ Use these expressions with your scheduler of choice (e.g. Google Cloud Scheduler
2. URL: `https://your-teammates-app.com/auto/feedbackSessionOpenedReminders`
3. Add header: `Authorization: Bearer <your-secret>`
4. Schedule: `2 * * * *` (Unix cron format)
5. Timezone: Asia/Singapore.
5. Timezone: UTC.
13 changes: 13 additions & 0 deletions src/main/java/teammates/common/datatransfer/UserInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,23 @@ public class UserInfo {
*/
public boolean isMaintainer;

/**
* True only for verified cron/worker internal callers (bearer + path), not for human app admins.
* See {@link teammates.common.util.InternalRequestAuth#isTrustedCronOrWorkerRequest}.
*/
public boolean isInternalService;

public UserInfo(String googleId) {
this.id = googleId;
}

/**
* Whether the caller may use endpoints shared by human app administrators and verified internal jobs.
*/
public boolean canAccessAsAdminOrInternalService() {
return isAdmin || isInternalService;
}
Comment on lines +48 to +50
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we move this directly into InternalServiceAction? This is not really the responsibility of the UserInfo class.


public String getId() {
return id;
}
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/teammates/common/util/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -69,6 +70,13 @@ public final class Config {
*/
public static final String CRON_AND_WORKER_SECRET;

/**
* UTF-8 bytes of {@link #CRON_AND_WORKER_SECRET} when that value is well-formed per
* {@link InternalRequestAuth#isCronAndWorkerSecretWellFormed(String)}; otherwise an empty array.
* Pre-computed for constant-time comparison without re-encoding on each request.
*/
public static final byte[] CRON_AND_WORKER_SECRET_BYTES;

/** Value of {@code app.encryption.key}. */
public static final String ENCRYPTION_KEY;

Expand Down Expand Up @@ -182,6 +190,9 @@ public final class Config {
CSRF_KEY = getProperty(properties, devProperties, "app.csrf.key");
BACKDOOR_KEY = getProperty(properties, devProperties, "app.backdoor.key");
CRON_AND_WORKER_SECRET = getProperty(properties, devProperties, "app.cron.and.worker.secret");
CRON_AND_WORKER_SECRET_BYTES = InternalRequestAuth.isCronAndWorkerSecretWellFormed(CRON_AND_WORKER_SECRET)
? CRON_AND_WORKER_SECRET.getBytes(StandardCharsets.UTF_8)
: new byte[0];
PRODUCTION_GCS_BUCKETNAME = getProperty(properties, devProperties, "app.production.gcs.bucketname");
POSTGRES_HOST = getProperty(properties, devProperties, "app.postgres.host");
POSTGRES_PORT = getProperty(properties, devProperties, "app.postgres.port");
Expand Down
18 changes: 17 additions & 1 deletion src/main/java/teammates/common/util/Const.java
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,19 @@ public static class ParamsNames {
* Represents custom header names used by the system.
*/
public static class HeaderNames {
public static final String COOKIE_KEY = "Cookie";
public static final String BACKDOOR_KEY = "Backdoor-Key";
public static final String CSRF_KEY = "CSRF-Key";
public static final String WEB_VERSION = "X-WEB-VERSION";
public static final String CSRF_TOKEN = "X-CSRF-TOKEN";
public static final String AUTHORIZATION_KEY = "Authorization";
}

/**
* HTTP authentication scheme names (RFC 6750; scheme matching is case-insensitive).
*/
public static class HttpAuthScheme {
public static final String BEARER_SCHEME = "Bearer";
}

/**
Expand Down Expand Up @@ -311,7 +320,6 @@ public static class ResourceURIs {
public static final String DATABUNDLE = URI_PREFIX + "/databundle";
public static final String SQL_DATABUNDLE = URI_PREFIX + "/databundle/sql";
public static final String DEADLINE_EXTENSION = URI_PREFIX + "/deadlineextension";
public static final String EXCEPTION = URI_PREFIX + "/exception";
public static final String ERROR_REPORT = URI_PREFIX + "/errorreport";
public static final String AUTH = URI_PREFIX + "/auth";
public static final String AUTH_REGKEY = URI_PREFIX + "/auth/regkey";
Expand Down Expand Up @@ -370,6 +378,14 @@ public static class ResourceURIs {
public static final String USER_COOKIE = URI_PREFIX + "/cookie";
}

/**
* User principal identifiers for verified internal cron/worker API callers.
*/
public static class InternalService {
public static final String CRON_SERVICE_USER_ID = "Cron-Service";
public static final String WORKER_SERVICE_USER_ID = "Worker-Service";
}

/**
* Represents URIs of endpoints used by cron jobs.
*/
Expand Down
38 changes: 36 additions & 2 deletions src/main/java/teammates/common/util/HttpRequestHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,39 @@ private HttpRequestHelper() {
// utility class
}

/**
* Returns the bearer credential from an {@code Authorization} header field-value, or {@code null} if the value
* is not a Bearer access token per OAuth 2.0 Bearer Token Usage (RFC 6750) and HTTP Authentication (RFC 7235):
* {@code Bearer} scheme name (case-insensitive), then {@code 1*SP} (ASCII space only, not HTAB), then the token.
*/
public static String parseBearerTokenFromAuthorizationHeader(String authorizationHeaderValue) {
if (authorizationHeaderValue == null) {
return null;
}
int schemeLen = Const.HttpAuthScheme.BEARER_SCHEME.length();
if (authorizationHeaderValue.length() <= schemeLen) {
return null;
}
if (!authorizationHeaderValue.regionMatches(true, 0, Const.HttpAuthScheme.BEARER_SCHEME, 0, schemeLen)) {
return null;
}
if (authorizationHeaderValue.charAt(schemeLen) != ' ') {
return null;
}
int i = schemeLen;
while (i < authorizationHeaderValue.length() && authorizationHeaderValue.charAt(i) == ' ') {
i++;
}
if (i >= authorizationHeaderValue.length()) {
return null;
}
char c = authorizationHeaderValue.charAt(i);
if (c != ' ' && Character.isWhitespace(c)) {
return null;
}
return authorizationHeaderValue.substring(i).trim();
}

/**
* Gets the parameters of the given HTTP request as key-value (possibly multi-values) mapping.
*/
Expand All @@ -43,10 +76,11 @@ static Map<String, Object> getRequestHeaders(HttpServletRequest req) {
Map<String, Object> headers = new HashMap<>();
Collections.list(req.getHeaderNames()).stream()
// Do not include cookie header/secret keys in production for privacy reasons
.filter(headerName -> Config.IS_DEV_SERVER || !"cookie".equalsIgnoreCase(headerName))
.filter(headerName -> Config.IS_DEV_SERVER || !Const.HeaderNames.COOKIE_KEY.equalsIgnoreCase(headerName))
.filter(headerName -> Config.IS_DEV_SERVER || !Const.HeaderNames.BACKDOOR_KEY.equalsIgnoreCase(headerName))
.filter(headerName -> Config.IS_DEV_SERVER || !Const.HeaderNames.CSRF_KEY.equalsIgnoreCase(headerName))
.filter(headerName -> Config.IS_DEV_SERVER || !"Authorization".equalsIgnoreCase(headerName))
.filter(headerName -> Config.IS_DEV_SERVER
|| !Const.HeaderNames.AUTHORIZATION_KEY.equalsIgnoreCase(headerName))
.forEach(headerName -> {
List<String> headerValues = Collections.list(req.getHeaders(headerName));
if (headerValues.size() == 1) {
Expand Down
29 changes: 17 additions & 12 deletions src/main/java/teammates/common/util/InternalRequestAuth.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
*/
public final class InternalRequestAuth {

private static final String BEARER_PREFIX = "Bearer ";

private InternalRequestAuth() {
// utility class
}
Expand All @@ -30,30 +28,37 @@ public static boolean isCronAndWorkerSecretWellFormed(String secret) {
* (including the application context path prefix when present).
*/
public static boolean isTrustedCronOrWorkerRequest(HttpServletRequest req) {
return isTrustedCronOrWorkerRequest(req, Config.CRON_AND_WORKER_SECRET);
return isTrustedCronOrWorkerRequest(req, Config.CRON_AND_WORKER_SECRET_BYTES);
}

/**
* Same as {@link #isTrustedCronOrWorkerRequest(HttpServletRequest)} but with an explicit expected secret
* (production uses {@link Config#CRON_AND_WORKER_SECRET}). Package-private for deterministic unit tests.
* (production uses {@link Config#CRON_AND_WORKER_SECRET_BYTES}). Package-private for deterministic unit tests.
*/
static boolean isTrustedCronOrWorkerRequest(HttpServletRequest req, String secret) {
if (req.getContextPath() == null) {
if (!isCronAndWorkerSecretWellFormed(secret)) {
return false;
}
if (!isCronAndWorkerSecretWellFormed(secret)) {
return isTrustedCronOrWorkerRequest(req, secret.getBytes(StandardCharsets.UTF_8));
}

/**
* Package-private for deterministic unit tests and for sharing logic with {@link #isTrustedCronOrWorkerRequest(
* HttpServletRequest, String)}.
*/
static boolean isTrustedCronOrWorkerRequest(HttpServletRequest req, byte[] secretBytes) {
if (req.getContextPath() == null) {
return false;
}
String auth = req.getHeader("Authorization");
if (auth == null || !auth.startsWith(BEARER_PREFIX)) {
if (secretBytes == null || secretBytes.length == 0) {
return false;
}
String token = auth.substring(BEARER_PREFIX.length()).trim();
if (token.isEmpty()) {
String token = HttpRequestHelper.parseBearerTokenFromAuthorizationHeader(
req.getHeader(Const.HeaderNames.AUTHORIZATION_KEY));
if (token == null || token.isEmpty()) {
return false;
}
if (!MessageDigest.isEqual(token.getBytes(StandardCharsets.UTF_8),
secret.getBytes(StandardCharsets.UTF_8))) {
if (!MessageDigest.isEqual(token.getBytes(StandardCharsets.UTF_8), secretBytes)) {
return false;
}
return isCronOrWorkerPath(req);
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/teammates/sqllogic/api/UserProvision.java
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,13 @@ public UserInfo getAdminOnlyUser(String userId) {
return userInfo;
}

/**
* User principal for verified cron/worker requests: not a human app admin; {@link UserInfo#isInternalService} only.
*/
public UserInfo getInternalServiceUser(String serviceId) {
UserInfo userInfo = new UserInfo(serviceId);
userInfo.isInternalService = true;
return userInfo;
}

}
7 changes: 3 additions & 4 deletions src/main/java/teammates/ui/servlets/RequestTraceFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,10 @@ public void doFilter(ServletRequest req, ServletResponse resp, FilterChain chain
}
}

// Worker requests (from Cloud Tasks with bearer token) may run longer.
// For worker requests, we set the limit to 10 minutes minus a small grace period.
// Worker / Cron requests (from Cloud Tasks with bearer token) may run longer.
// For these requests, we set the limit to 10 minutes minus a small grace period.
// For other requests, we keep the time limit at 1 minute.
boolean isWorkerRequest = InternalRequestAuth.isTrustedCronOrWorkerRequest(request)
&& InternalRequestAuth.isWorkerRequestPath(request);
boolean isWorkerRequest = InternalRequestAuth.isTrustedCronOrWorkerRequest(request);
int timeoutInSeconds = isWorkerRequest ? 10 * 60 - 5 : 60;

RequestTracer.init(traceId, spanId, timeoutInSeconds);
Expand Down
65 changes: 1 addition & 64 deletions src/main/java/teammates/ui/servlets/WebApiServlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,14 @@
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;

import org.apache.http.HttpStatus;
import org.hibernate.HibernateException;

import teammates.common.datatransfer.logs.RequestLogUser;
import teammates.common.exception.DeadlineExceededException;
import teammates.common.util.HibernateUtil;
import teammates.common.util.InternalRequestAuth;
import teammates.common.util.Logger;
import teammates.ui.request.InvalidHttpRequestBodyException;
import teammates.ui.webapi.Action;
import teammates.ui.webapi.ActionFactory;
import teammates.ui.webapi.ActionMappingException;
import teammates.ui.webapi.ActionResult;
import teammates.ui.webapi.EntityNotFoundException;
import teammates.ui.webapi.InvalidHttpParameterException;
import teammates.ui.webapi.InvalidOperationException;
import teammates.ui.webapi.JsonResult;
import teammates.ui.webapi.UnauthorizedAccessException;

/**
Expand Down Expand Up @@ -62,38 +53,8 @@ private void invokeServlet(HttpServletRequest req, HttpServletResponse resp) thr

statusCode = result.getStatusCode();
result.send(resp);
} catch (ActionMappingException e) {
statusCode = e.getStatusCode();
throwErrorBasedOnRequester(req, resp, e, statusCode);
} catch (InvalidHttpRequestBodyException | InvalidHttpParameterException e) {
statusCode = HttpStatus.SC_BAD_REQUEST;
throwErrorBasedOnRequester(req, resp, e, statusCode);
} catch (UnauthorizedAccessException uae) {
statusCode = HttpStatus.SC_FORBIDDEN;
log.warning(uae.getClass().getSimpleName() + " caught by WebApiServlet: " + uae.getMessage(), uae);
throwError(resp, statusCode,
uae.isShowErrorMessage() ? uae.getMessage() : "You are not authorized to access this resource.");
} catch (EntityNotFoundException enfe) {
statusCode = HttpStatus.SC_NOT_FOUND;
log.warning(enfe.getClass().getSimpleName() + " caught by WebApiServlet: " + enfe.getMessage(), enfe);
throwError(resp, statusCode, enfe.getMessage());
} catch (InvalidOperationException ioe) {
statusCode = HttpStatus.SC_CONFLICT;
log.warning(ioe.getClass().getSimpleName() + " caught by WebApiServlet: " + ioe.getMessage(), ioe);
throwError(resp, statusCode, ioe.getMessage());
} catch (DeadlineExceededException dee) {
statusCode = HttpStatus.SC_GATEWAY_TIMEOUT;
log.severe(dee.getClass().getSimpleName() + " caught by WebApiServlet", dee);
throwError(resp, statusCode, "The request exceeded the server timeout limit. Please try again later.");
} catch (HibernateException e) {
statusCode = HttpStatus.SC_INTERNAL_SERVER_ERROR;
log.severe(e.getClass().getSimpleName() + " caught by WebApiServlet: " + e.getMessage(), e);
throwError(resp, statusCode, e.getMessage());
} catch (Throwable t) {
statusCode = HttpStatus.SC_INTERNAL_SERVER_ERROR;
log.severe(t.getClass().getSimpleName() + " caught by WebApiServlet: " + t.getMessage(), t);
throwError(resp, statusCode,
"The server encountered an error when processing your request.");
statusCode = WebApiServletExceptionHandler.handleException(resp, t);
} finally {
RequestLogUser userInfo = new RequestLogUser();
String requestBody = null;
Expand Down Expand Up @@ -126,28 +87,4 @@ private ActionResult executeWithTransaction(Action action, HttpServletRequest re
}
}

private void throwErrorBasedOnRequester(HttpServletRequest req, HttpServletResponse resp, Exception e, int statusCode)
throws IOException {
boolean isTrustedWorkerRequest = InternalRequestAuth.isTrustedCronOrWorkerRequest(req)
&& InternalRequestAuth.isWorkerRequestPath(req);

if (isTrustedWorkerRequest) {
log.severe(e.getClass().getSimpleName() + " caught by WebApiServlet: " + e.getMessage(), e);

// For Cloud Tasks worker requests only: response status is not set to 4XX/5XX so the task is not
// retried when the failure is due to an improper request URL (retries would not help).
// Cron (/auto/*) requests are not included so schedulers still see real HTTP error status on failure.
// The action may be inaccurately marked as "success"; the severe log traces the problem.
throwError(resp, HttpStatus.SC_ACCEPTED, e.getMessage());
} else {
log.warning(e.getClass().getSimpleName() + " caught by WebApiServlet: " + e.getMessage(), e);
throwError(resp, statusCode, e.getMessage());
}
}

private void throwError(HttpServletResponse resp, int statusCode, String message) throws IOException {
JsonResult result = new JsonResult(message, statusCode);
result.send(resp);
}

}
Loading
Loading