max / makenotwork
11 files changed,
+351 insertions,
-77 deletions
| @@ -54,6 +54,42 @@ This link expires in 24 hours. | |||
| 54 | 54 | self.transport.send_email(to_email, subject, &body).await | |
| 55 | 55 | } | |
| 56 | 56 | ||
| 57 | + | /// Sent when someone tries to sign up with an email that already has an | |
| 58 | + | /// account. The signup page deliberately does not reveal that the address is | |
| 59 | + | /// registered (enumeration defense); this email is the channel that reaches | |
| 60 | + | /// the real owner so they can recover, since only they receive it. | |
| 61 | + | pub async fn send_account_exists( | |
| 62 | + | &self, | |
| 63 | + | to_email: &str, | |
| 64 | + | login_url: &str, | |
| 65 | + | reset_url: &str, | |
| 66 | + | ) -> Result<()> { | |
| 67 | + | let subject = "You already have a Makenotwork account"; | |
| 68 | + | let body = format!( | |
| 69 | + | r#"Hi, | |
| 70 | + | ||
| 71 | + | Someone (probably you) just tried to sign up for Makenotwork with this email | |
| 72 | + | address, but you already have an account. | |
| 73 | + | ||
| 74 | + | To sign in: | |
| 75 | + | ||
| 76 | + | {login} | |
| 77 | + | ||
| 78 | + | If you've forgotten your password, you can reset it here: | |
| 79 | + | ||
| 80 | + | {reset} | |
| 81 | + | ||
| 82 | + | If this wasn't you, nothing was created or changed and you can safely ignore | |
| 83 | + | this email. | |
| 84 | + | ||
| 85 | + | - Makenotwork"#, | |
| 86 | + | login = login_url, | |
| 87 | + | reset = reset_url, | |
| 88 | + | ); | |
| 89 | + | ||
| 90 | + | self.transport.send_email(to_email, subject, &body).await | |
| 91 | + | } | |
| 92 | + | ||
| 57 | 93 | /// Send an account lockout notification | |
| 58 | 94 | pub async fn send_lockout_notification( | |
| 59 | 95 | &self, |
| @@ -769,22 +769,26 @@ async fn token_refresh(state: &AppState, secret: &str, req: TokenRequest) -> Res | |||
| 769 | 769 | return Ok(oauth_error("invalid_grant")); | |
| 770 | 770 | } | |
| 771 | 771 | ||
| 772 | - | // Revocation + liveness: the same gates the access-token extractor applies, | |
| 773 | - | // so password change / suspend / app-deactivate kill the refresh lineage. | |
| 774 | - | let user = db::users::get_user_by_id(&state.db, consumed.user_id).await?; | |
| 775 | - | let live = match user { | |
| 776 | - | Some(u) if !(u.is_suspended() || u.is_deactivated()) => { | |
| 777 | - | u.jwt_invalidated_at | |
| 778 | - | .map(|inv| consumed.issued_after.timestamp() > inv.timestamp()) | |
| 779 | - | .unwrap_or(true) | |
| 772 | + | // Revocation + liveness via the one shared gate the token extractors use, so | |
| 773 | + | // app-deactivate / suspend / password change AND sync-device removal all | |
| 774 | + | // kill the refresh lineage (the M-Sec1 parity fix: this path previously | |
| 775 | + | // skipped `sync_jwt_invalidated_at`). A liveness failure (Unauthorized) | |
| 776 | + | // revokes the chain and denies; a real infrastructure error propagates as | |
| 777 | + | // 5xx without touching the chain. | |
| 778 | + | match synckit_auth::assert_token_live( | |
| 779 | + | state, | |
| 780 | + | consumed.app_id, | |
| 781 | + | consumed.user_id, | |
| 782 | + | consumed.issued_after.timestamp(), | |
| 783 | + | ) | |
| 784 | + | .await | |
| 785 | + | { | |
| 786 | + | Ok(()) => {} | |
| 787 | + | Err(AppError::Unauthorized) => { | |
| 788 | + | db::oauth::revoke_refresh_chain(&state.db, consumed.chain_id).await?; | |
| 789 | + | return Ok(oauth_error("invalid_grant")); | |
| 780 | 790 | } | |
| 781 | - | _ => false, | |
| 782 | - | }; | |
| 783 | - | let app = db::synckit::get_sync_app_by_id(&state.db, consumed.app_id).await?; | |
| 784 | - | let app_live = app.map(|a| a.is_active).unwrap_or(false); | |
| 785 | - | if !live || !app_live { | |
| 786 | - | db::oauth::revoke_refresh_chain(&state.db, consumed.chain_id).await?; | |
| 787 | - | return Ok(oauth_error("invalid_grant")); | |
| 791 | + | Err(e) => return Err(e), | |
| 788 | 792 | } | |
| 789 | 793 | ||
| 790 | 794 | // Downgrade-only scope: a refresh may narrow but never widen. |
| @@ -126,12 +126,33 @@ pub async fn step_account_create( | |||
| 126 | 126 | let email_taken = db::users::get_user_by_email(&state.db, &email) | |
| 127 | 127 | .await? | |
| 128 | 128 | .is_some(); | |
| 129 | - | if username_taken && email_taken { | |
| 130 | - | return return_error(Some("username"), "This username and email are already registered"); | |
| 131 | - | } else if username_taken { | |
| 129 | + | // Username collisions are safe to reveal — usernames are public handles | |
| 130 | + | // (profile URLs expose them) and the user must pick a free one. An EMAIL | |
| 131 | + | // collision must NOT be revealed: "this email is already registered" is an | |
| 132 | + | // account-existence oracle for a private identifier (ultra-fuzz Run 4 m1). | |
| 133 | + | // So when only the email is taken, do not error — return the same step-2 | |
| 134 | + | // response a fresh signup returns and send an "account exists" email out of | |
| 135 | + | // band. Only the real owner receives that email, so the recovery path | |
| 136 | + | // reaches them without the page confirming the address. (Residual: a fresh | |
| 137 | + | // signup also sets a session cookie, which this path cannot; the explicit | |
| 138 | + | // textual reveal, the actual finding, is gone.) | |
| 139 | + | if username_taken { | |
| 132 | 140 | return return_error(Some("username"), "This username is already taken"); | |
| 133 | - | } else if email_taken { | |
| 134 | - | return return_error(Some("email"), "This email is already registered"); | |
| 141 | + | } | |
| 142 | + | if email_taken { | |
| 143 | + | let login_url = format!("{}/login", state.config.host_url); | |
| 144 | + | let reset_url = format!("{}/forgot-password", state.config.host_url); | |
| 145 | + | let email_client = state.email.clone(); | |
| 146 | + | let to_email = email.to_string(); | |
| 147 | + | state.bg.spawn("account-exists notice", async move { | |
| 148 | + | if let Err(e) = email_client | |
| 149 | + | .send_account_exists(&to_email, &login_url, &reset_url) | |
| 150 | + | .await | |
| 151 | + | { | |
| 152 | + | tracing::error!(error = ?e, "failed to send account-exists email"); | |
| 153 | + | } | |
| 154 | + | }); | |
| 155 | + | return Ok(render_step_profile().into_response()); | |
| 135 | 156 | } | |
| 136 | 157 | ||
| 137 | 158 | let password_len = form.password.chars().count(); |
| @@ -117,6 +117,25 @@ async fn handle_new_issue( | |||
| 117 | 117 | } | |
| 118 | 118 | }; | |
| 119 | 119 | ||
| 120 | + | // Authorization: a private repo's issue tracker is not open to the world. | |
| 121 | + | // The sender must be the owner or a collaborator (the same read-access model | |
| 122 | + | // `resolve_repo` applies over HTTP/SSH). Public/unlisted repos accept issues | |
| 123 | + | // from any verified user. Returning OK (not 403) keeps the inbound path from | |
| 124 | + | // being an existence oracle for private repos (ultra-fuzz Run 4 M-Sec3). | |
| 125 | + | if repo.visibility == db::Visibility::Private && sender.id != owner_user.id { | |
| 126 | + | match db::repo_collaborators::is_collaborator(&state.db, repo.id, sender.id).await { | |
| 127 | + | Ok(true) => {} | |
| 128 | + | Ok(false) => { | |
| 129 | + | tracing::info!(owner = %owner, repo = %repo_name, "inbound-issues: sender lacks access to private repo"); | |
| 130 | + | return StatusCode::OK; | |
| 131 | + | } | |
| 132 | + | Err(e) => { | |
| 133 | + | tracing::error!(error = ?e, "inbound-issues: collaborator check failed"); | |
| 134 | + | return StatusCode::OK; | |
| 135 | + | } | |
| 136 | + | } | |
| 137 | + | } | |
| 138 | + | ||
| 120 | 139 | // Create the issue | |
| 121 | 140 | let title = payload.subject.trim(); | |
| 122 | 141 | if title.is_empty() { |
| @@ -129,6 +129,29 @@ pub(super) async fn sync_auth( | |||
| 129 | 129 | // Successful auth — reset failed login counter | |
| 130 | 130 | db::auth::reset_failed_login(&state.db, user.id).await?; | |
| 131 | 131 | ||
| 132 | + | // Register the session's billing key under the per-key cap AT MINT TIME, so a | |
| 133 | + | // token can never be issued for a key beyond the developer's paid allowance. | |
| 134 | + | // Previously the key was claimed lazily on first write and the JWT `key` | |
| 135 | + | // claim was accepted on `!is_empty()` alone, letting a token minter spread | |
| 136 | + | // storage across unlimited synthetic keys and evade the per-key fairness cap | |
| 137 | + | // (ultra-fuzz Run 4 M-Sec2). Only `per_key` developer apps are capped; | |
| 138 | + | // internal and `bulk`/`app_wide` apps are uncapped and skip the claim. | |
| 139 | + | // `claim_key` is idempotent for an already-claimed key and enforces the cap | |
| 140 | + | // atomically under the usage-row lock. | |
| 141 | + | let billing = db::synckit_billing::get_app_with_billing(&state.db, app.id) | |
| 142 | + | .await? | |
| 143 | + | .ok_or(AppError::Unauthorized)?; | |
| 144 | + | if !billing.is_internal && billing.enforcement_mode == "per_key" { | |
| 145 | + | let key_cap = billing.key_cap.unwrap_or(0); | |
| 146 | + | let claim = db::synckit_billing::claim_key(&state.db, app.id, &req.key, Some(key_cap)).await?; | |
| 147 | + | if claim.cap_reached { | |
| 148 | + | return Err(AppError::PaymentRequired(format!( | |
| 149 | + | "key limit reached ({} of {key_cap} keys claimed); release an unused key or raise the cap", | |
| 150 | + | claim.total_claimed | |
| 151 | + | ))); | |
| 152 | + | } | |
| 153 | + | } | |
| 154 | + | ||
| 132 | 155 | let token = synckit_auth::create_sync_token(secret, user.id, app.id, &req.key)?; | |
| 133 | 156 | ||
| 134 | 157 | Ok(Json(SyncAuthResponse { |
| @@ -53,6 +53,24 @@ fn sniff_html_like(data: &[u8]) -> Option<&'static str> { | |||
| 53 | 53 | pub fn verify_content_type(data: &[u8], claimed_type: FileType) -> LayerResult { | |
| 54 | 54 | let detected = infer::get(data); | |
| 55 | 55 | ||
| 56 | + | // No media file (audio/image/video) legitimately begins with HTML / SVG / | |
| 57 | + | // XML / script markers. `infer` matches binary magic and won't classify | |
| 58 | + | // plain markup, so a markup payload reaches the per-type arms as | |
| 59 | + | // `detected == None` and is rejected only as long as that arm stays strict. | |
| 60 | + | // Catch it for every non-Download type up front, regardless of the per-type | |
| 61 | + | // logic, so an XSS polyglot can't sneak in if image/video acceptance is ever | |
| 62 | + | // loosened (ultra-fuzz Run 4 m2 — close the latent vector now). The Download | |
| 63 | + | // arm has its own allowance + sniff. | |
| 64 | + | if !matches!(claimed_type, FileType::Download) | |
| 65 | + | && let Some(marker) = sniff_html_like(data) | |
| 66 | + | { | |
| 67 | + | return LayerResult { | |
| 68 | + | layer: "content_type", | |
| 69 | + | verdict: LayerVerdict::Fail, | |
| 70 | + | detail: Some(format!("File text-sniffed as markup: {marker}")), | |
| 71 | + | }; | |
| 72 | + | } | |
| 73 | + | ||
| 56 | 74 | match claimed_type { | |
| 57 | 75 | FileType::Audio | FileType::Insertion => { | |
| 58 | 76 | // Audio files must be detected as audio/* or application/ogg. |
| @@ -435,7 +435,31 @@ async fn run_pipeline_and_decide( | |||
| 435 | 435 | // and this scan's clamav layer errored). See `resolve_pass_status`. | |
| 436 | 436 | let is_trusted = db::users::is_upload_trusted(&ctx.db, job.user_id).await?; | |
| 437 | 437 | let clamav_down = !ctx.clamav_healthy.load(Ordering::Relaxed); | |
| 438 | - | Ok(resolve_pass_status(result.status, is_trusted, clamav_down, &result.layers)) | |
| 438 | + | let status = resolve_pass_status(result.status, is_trusted, clamav_down, &result.layers); | |
| 439 | + | ||
| 440 | + | // Visibility for the narrow fail-open window: a trusted upload passes Clean | |
| 441 | + | // while its clamav layer errored but the health probe has NOT yet observed | |
| 442 | + | // clamd down. resolve_pass_status deliberately lets this through (a single | |
| 443 | + | // transient error must not hold every trusted upload), but it is the one | |
| 444 | + | // path where a file is accepted on reduced AV coverage without a hold — so | |
| 445 | + | // an operator must be able to see it rather than have it pass silently | |
| 446 | + | // (ultra-fuzz Run 4 M-Sec4). | |
| 447 | + | if status == FileScanStatus::Clean && is_trusted && !clamav_down { | |
| 448 | + | let clamav_errored = result | |
| 449 | + | .layers | |
| 450 | + | .iter() | |
| 451 | + | .any(|l| l.layer == "clamav" && l.verdict == LayerVerdict::Error); | |
| 452 | + | if clamav_errored { | |
| 453 | + | tracing::warn!( | |
| 454 | + | s3_key = %job.s3_key, | |
| 455 | + | user_id = %job.user_id, | |
| 456 | + | "clamav layer errored while the health probe still reports clamd up; \ | |
| 457 | + | trusted upload passed on reduced AV coverage (fail-open window)" | |
| 458 | + | ); | |
| 459 | + | } | |
| 460 | + | } | |
| 461 | + | ||
| 462 | + | Ok(status) | |
| 439 | 463 | } | |
| 440 | 464 | ||
| 441 | 465 | /// Update the per-entity `scan_status` column for the kinds that have one. |
| @@ -108,6 +108,61 @@ pub fn decode_sync_token(secret: &str, token: &str) -> Result<SyncClaims, AppErr | |||
| 108 | 108 | Ok(data.claims) | |
| 109 | 109 | } | |
| 110 | 110 | ||
| 111 | + | /// Liveness gate for every SyncKit-derived credential: the sync JWT, the OAuth | |
| 112 | + | /// userinfo access token, and the OAuth refresh lineage. This is the single | |
| 113 | + | /// place that knows the revocation columns, so no caller can check a subset of | |
| 114 | + | /// them (the bug class behind ultra-fuzz Run 4 M-Sec1, where `OAuthUser` and the | |
| 115 | + | /// refresh path checked `jwt_invalidated_at` but silently skipped the | |
| 116 | + | /// device-removal column `sync_jwt_invalidated_at`). It enforces, in order: | |
| 117 | + | /// - the app is still active, | |
| 118 | + | /// - the user is neither suspended nor deactivated, | |
| 119 | + | /// - the credential was issued AFTER a password change (`jwt_invalidated_at`), | |
| 120 | + | /// - the credential was issued AFTER a sync-device removal | |
| 121 | + | /// (`sync_jwt_invalidated_at`). | |
| 122 | + | /// | |
| 123 | + | /// `issued_at <= invalidated_at` rejects (both have second resolution; `<=` | |
| 124 | + | /// closes the same-wall-second collision window). Returns [`AppError::Unauthorized`] | |
| 125 | + | /// for a liveness failure and a propagated error for an infrastructure failure, | |
| 126 | + | /// so callers can tell "revoke / deny" apart from "5xx, try again". | |
| 127 | + | pub async fn assert_token_live( | |
| 128 | + | state: &AppState, | |
| 129 | + | app_id: SyncAppId, | |
| 130 | + | user_id: UserId, | |
| 131 | + | issued_at: i64, | |
| 132 | + | ) -> Result<(), AppError> { | |
| 133 | + | let app = crate::db::synckit::get_sync_app_by_id(&state.db, app_id) | |
| 134 | + | .await? | |
| 135 | + | .ok_or(AppError::Unauthorized)?; | |
| 136 | + | if !app.is_active { | |
| 137 | + | return Err(AppError::Unauthorized); | |
| 138 | + | } | |
| 139 | + | ||
| 140 | + | let user = crate::db::users::get_user_by_id(&state.db, user_id) | |
| 141 | + | .await? | |
| 142 | + | .ok_or(AppError::Unauthorized)?; | |
| 143 | + | if user.is_suspended() || user.is_deactivated() { | |
| 144 | + | return Err(AppError::Unauthorized); | |
| 145 | + | } | |
| 146 | + | ||
| 147 | + | // Password-change revocation (kills web sessions and all derived tokens). | |
| 148 | + | if let Some(invalidated_at) = user.jwt_invalidated_at | |
| 149 | + | && issued_at <= invalidated_at.timestamp() | |
| 150 | + | { | |
| 151 | + | return Err(AppError::Unauthorized); | |
| 152 | + | } | |
| 153 | + | ||
| 154 | + | // Sync-device-removal revocation (bumped on device removal; deliberately | |
| 155 | + | // separate from web sessions). Checked here for EVERY credential type so a | |
| 156 | + | // removed device's OAuth refresh lineage dies with its sync token. | |
| 157 | + | if let Some(invalidated_at) = user.sync_jwt_invalidated_at | |
| 158 | + | && issued_at <= invalidated_at.timestamp() | |
| 159 | + | { | |
| 160 | + | return Err(AppError::Unauthorized); | |
| 161 | + | } | |
| 162 | + | ||
| 163 | + | Ok(()) | |
| 164 | + | } | |
| 165 | + | ||
| 111 | 166 | /// Authenticated sync user extracted from JWT Bearer token. | |
| 112 | 167 | pub struct SyncUser { | |
| 113 | 168 | pub user_id: UserId, | |
| @@ -143,40 +198,9 @@ impl FromRequestParts<AppState> for SyncUser { | |||
| 143 | 198 | ||
| 144 | 199 | let claims = decode_sync_token(secret, token)?; | |
| 145 | 200 | ||
| 146 | - | // Verify the app is still active (JWT may outlive app deactivation) | |
| 147 | - | let app = crate::db::synckit::get_sync_app_by_id(&state.db, claims.app) | |
| 148 | - | .await? | |
| 149 | - | .ok_or(AppError::Unauthorized)?; | |
| 150 | - | if !app.is_active { | |
| 151 | - | return Err(AppError::Unauthorized); | |
| 152 | - | } | |
| 153 | - | ||
| 154 | - | // Verify user is not suspended or deactivated (JWT may outlive suspension) | |
| 155 | - | let user = crate::db::users::get_user_by_id(&state.db, claims.sub) | |
| 156 | - | .await? | |
| 157 | - | .ok_or(AppError::Unauthorized)?; | |
| 158 | - | if user.is_suspended() || user.is_deactivated() { | |
| 159 | - | return Err(AppError::Unauthorized); | |
| 160 | - | } | |
| 161 | - | ||
| 162 | - | // Reject tokens issued before a password change (JWT revocation). | |
| 163 | - | // `<=` closes the 1-second collision window where a token minted in | |
| 164 | - | // the same wall-second as the password change would otherwise survive | |
| 165 | - | // revocation (`iat` and `invalidated_at` both have second resolution). | |
| 166 | - | if let Some(invalidated_at) = user.jwt_invalidated_at | |
| 167 | - | && claims.iat <= invalidated_at.timestamp() | |
| 168 | - | { | |
| 169 | - | return Err(AppError::Unauthorized); | |
| 170 | - | } | |
| 171 | - | ||
| 172 | - | // Sync-specific revocation, bumped on device removal. Same `<=` window | |
| 173 | - | // semantics as above; separate column so removing a sync device does | |
| 174 | - | // not invalidate the user's website sessions. | |
| 175 | - | if let Some(invalidated_at) = user.sync_jwt_invalidated_at | |
| 176 | - | && claims.iat <= invalidated_at.timestamp() | |
| 177 | - | { | |
| 178 | - | return Err(AppError::Unauthorized); | |
| 179 | - | } | |
| 201 | + | // App-active + user-live + revocation (password change AND device | |
| 202 | + | // removal), all in one sealed gate so no check can drift out of sync. | |
| 203 | + | assert_token_live(state, claims.app, claims.sub, claims.iat).await?; | |
| 180 | 204 | ||
| 181 | 205 | if claims.key.is_empty() { | |
| 182 | 206 | return Err(AppError::Unauthorized); | |
| @@ -258,9 +282,9 @@ pub fn decode_oauth_access_token(secret: &str, token: &str) -> Result<OAuthAcces | |||
| 258 | 282 | } | |
| 259 | 283 | ||
| 260 | 284 | /// Authenticated user extracted from an OAuth userinfo access token. Carries the | |
| 261 | - | /// granted scopes so the userinfo handler can gate fields per scope. Applies the | |
| 262 | - | /// same liveness checks as [`SyncUser`] — crucially the `jwt_invalidated_at` | |
| 263 | - | /// gate, so a password change also kills userinfo tokens. | |
| 285 | + | /// granted scopes so the userinfo handler can gate fields per scope. Runs the | |
| 286 | + | /// shared [`assert_token_live`] gate, so a password change AND a sync-device | |
| 287 | + | /// removal both kill userinfo tokens (the same gate `SyncUser` applies). | |
| 264 | 288 | pub struct OAuthUser { | |
| 265 | 289 | pub user_id: UserId, | |
| 266 | 290 | pub scopes: GrantedScopes, | |
| @@ -288,25 +312,11 @@ impl FromRequestParts<AppState> for OAuthUser { | |||
| 288 | 312 | ||
| 289 | 313 | let claims = decode_oauth_access_token(secret, token)?; | |
| 290 | 314 | ||
| 291 | - | let app = crate::db::synckit::get_sync_app_by_id(&state.db, claims.app) | |
| 292 | - | .await? | |
| 293 | - | .ok_or(AppError::Unauthorized)?; | |
| 294 | - | if !app.is_active { | |
| 295 | - | return Err(AppError::Unauthorized); | |
| 296 | - | } | |
| 297 | - | ||
| 298 | - | let user = crate::db::users::get_user_by_id(&state.db, claims.sub) | |
| 299 | - | .await? | |
| 300 | - | .ok_or(AppError::Unauthorized)?; | |
| 301 | - | if user.is_suspended() || user.is_deactivated() { | |
| 302 | - | return Err(AppError::Unauthorized); | |
| 303 | - | } | |
| 304 | - | ||
| 305 | - | if let Some(invalidated_at) = user.jwt_invalidated_at | |
| 306 | - | && claims.iat <= invalidated_at.timestamp() | |
| 307 | - | { | |
| 308 | - | return Err(AppError::Unauthorized); | |
| 309 | - | } | |
| 315 | + | // Same sealed liveness gate as SyncUser. Critically this now also | |
| 316 | + | // enforces `sync_jwt_invalidated_at`, so removing a sync device kills the | |
| 317 | + | // userinfo token too — the gate previously skipped the device-removal | |
| 318 | + | // column here (and on the refresh path), the M-Sec1 parity gap. | |
| 319 | + | assert_token_live(state, claims.app, claims.sub, claims.iat).await?; | |
| 310 | 320 | ||
| 311 | 321 | Ok(OAuthUser { | |
| 312 | 322 | user_id: claims.sub, |
| @@ -327,6 +327,52 @@ async fn inbound_new_issue_requires_verified_sender() { | |||
| 327 | 327 | } | |
| 328 | 328 | ||
| 329 | 329 | #[tokio::test] | |
| 330 | + | async fn inbound_new_issue_blocked_on_private_repo_for_non_collaborator() { | |
| 331 | + | // M-Sec3: inbound email issue creation must honor repo visibility. A verified | |
| 332 | + | // user who is neither owner nor collaborator cannot open issues on a PRIVATE | |
| 333 | + | // repo (it would be spam plus an existence oracle). The endpoint still returns | |
| 334 | + | // 200 (no oracle) but creates nothing; the owner is unaffected. | |
| 335 | + | let tmp = tempfile::TempDir::new().unwrap(); | |
| 336 | + | make_test_repo(tmp.path()); | |
| 337 | + | let mut h = setup_with_inbound(&tmp).await; | |
| 338 | + | ||
| 339 | + | // Make the repo private. | |
| 340 | + | sqlx::query("UPDATE git_repos SET visibility = 'private' WHERE name = 'testrepo'") | |
| 341 | + | .execute(&h.db).await.unwrap(); | |
| 342 | + | ||
| 343 | + | // A verified outsider: not the owner, not a collaborator. | |
| 344 | + | h.signup("outsider", "outsider@example.com", "password123").await; | |
| 345 | + | sqlx::query("UPDATE users SET email_verified = true WHERE email = 'outsider@example.com'") | |
| 346 | + | .execute(&h.db).await.unwrap(); | |
| 347 | + | ||
| 348 | + | h.client.set_bearer_token("test-inbound-secret"); | |
| 349 | + | let payload = inbound_payload( | |
| 350 | + | "testowner+testrepo@issues.makenot.work", | |
| 351 | + | "outsider@example.com", | |
| 352 | + | "Sneaky issue", | |
| 353 | + | "Body", | |
| 354 | + | ); | |
| 355 | + | let resp = h.client.post_json("/postmark/inbound-issues", &payload).await; | |
| 356 | + | assert_eq!(resp.status, 200, "inbound returns 200 (no existence oracle): {}", resp.text); | |
| 357 | + | let count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM issues") | |
| 358 | + | .fetch_one(&h.db).await.unwrap(); | |
| 359 | + | assert_eq!(count, 0, "non-collaborator must not open an issue on a private repo"); | |
| 360 | + | ||
| 361 | + | // Control: the owner CAN still open an issue on their own private repo. | |
| 362 | + | let payload = inbound_payload( | |
| 363 | + | "testowner+testrepo@issues.makenot.work", | |
| 364 | + | "testowner@example.com", | |
| 365 | + | "Owner issue", | |
| 366 | + | "Body", | |
| 367 | + | ); | |
| 368 | + | let resp = h.client.post_json("/postmark/inbound-issues", &payload).await; | |
| 369 | + | assert_eq!(resp.status, 200, "owner inbound should succeed: {}", resp.text); | |
| 370 | + | let count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM issues") | |
| 371 | + | .fetch_one(&h.db).await.unwrap(); | |
| 372 | + | assert_eq!(count, 1, "owner can still open an issue on their own private repo"); | |
| 373 | + | } | |
| 374 | + | ||
| 375 | + | #[tokio::test] | |
| 330 | 376 | async fn inbound_reply_creates_comment() { | |
| 331 | 377 | let tmp = tempfile::TempDir::new().unwrap(); | |
| 332 | 378 | make_test_repo(tmp.path()); |
| @@ -457,3 +457,37 @@ async fn drift_job_reconciles_per_key_and_app_totals_from_sync_blobs() { | |||
| 457 | 457 | .unwrap(); | |
| 458 | 458 | assert_eq!(k2, 350); | |
| 459 | 459 | } | |
| 460 | + | ||
| 461 | + | #[tokio::test] | |
| 462 | + | async fn sync_auth_enforces_per_key_cap_at_token_mint() { | |
| 463 | + | // M-Sec2: a per_key app's key cap is now enforced when the session token is | |
| 464 | + | // MINTED, so a token can never be issued for a key beyond the paid cap. The | |
| 465 | + | // key was previously claimed lazily on first write and /sync/auth accepted | |
| 466 | + | // any non-empty key, letting a minter spread storage across synthetic keys. | |
| 467 | + | let (mut h, _blobs) = harness_with_billing_and_blobs().await; | |
| 468 | + | let user_id = h.signup("pk_auth", "pk_auth@example.com", "Password1!").await; | |
| 469 | + | let (app_id, api_key) = create_draft_app(&h.db, user_id).await; | |
| 470 | + | activate_per_key(&mut h, app_id, 1, 1).await; // cap = 1 key | |
| 471 | + | ||
| 472 | + | let body = |key: &str| { | |
| 473 | + | json!({ | |
| 474 | + | "api_key": api_key, | |
| 475 | + | "email": "pk_auth@example.com", | |
| 476 | + | "password": "Password1!", | |
| 477 | + | "key": key, | |
| 478 | + | }) | |
| 479 | + | .to_string() | |
| 480 | + | }; | |
| 481 | + | ||
| 482 | + | // First key claims the only slot. | |
| 483 | + | let r = h.client.post_json("/api/sync/auth", &body("key-a")).await; | |
| 484 | + | assert_eq!(r.status, 200, "first key within cap must auth: {}", r.text); | |
| 485 | + | ||
| 486 | + | // A second, distinct key is over the cap -> 402 PaymentRequired, no token. | |
| 487 | + | let r = h.client.post_json("/api/sync/auth", &body("key-b")).await; | |
| 488 | + | assert_eq!(r.status, 402, "key beyond the per-key cap must be refused at mint: {}", r.text); | |
| 489 | + | ||
| 490 | + | // The already-claimed key still authenticates (idempotent claim). | |
| 491 | + | let r = h.client.post_json("/api/sync/auth", &body("key-a")).await; | |
| 492 | + | assert_eq!(r.status, 200, "already-claimed key must still auth: {}", r.text); | |
| 493 | + | } |
| @@ -123,3 +123,42 @@ async fn failed_sync_auth_is_recorded_in_the_audit_log() { | |||
| 123 | 123 | .unwrap(); | |
| 124 | 124 | assert_eq!(events, 1, "a failed sync auth must be recorded in the audit log"); | |
| 125 | 125 | } | |
| 126 | + | ||
| 127 | + | #[tokio::test] | |
| 128 | + | async fn deleting_a_device_invalidates_oauth_userinfo_tokens_too() { | |
| 129 | + | // M-Sec1: the OAuth userinfo extractor (and the refresh path) previously | |
| 130 | + | // checked only `jwt_invalidated_at`, so a removed device's userinfo token | |
| 131 | + | // survived until expiry. The shared `assert_token_live` gate now enforces | |
| 132 | + | // `sync_jwt_invalidated_at` for every SyncKit-derived credential, closing the | |
| 133 | + | // parity gap that migration 150 left open. | |
| 134 | + | let mut h = TestHarness::new().await; | |
| 135 | + | let user_id = h.signup("sec_oauth", "sec_oauth@example.com", "Password1!").await; | |
| 136 | + | let (app_id, _api_key) = create_internal_app(&h.db, user_id).await; | |
| 137 | + | ||
| 138 | + | // Mint a userinfo-scoped OAuth access token directly and present it. | |
| 139 | + | let scopes = makenotwork::oauth_scope::GrantedScopes::parse("profile:read"); | |
| 140 | + | let token = makenotwork::synckit_auth::create_oauth_access_token( | |
| 141 | + | "test-synckit-jwt-secret", | |
| 142 | + | user_id, | |
| 143 | + | app_id, | |
| 144 | + | "user-key", | |
| 145 | + | &scopes, | |
| 146 | + | ) | |
| 147 | + | .expect("mint userinfo token"); | |
| 148 | + | h.client.set_bearer_token(&token); | |
| 149 | + | ||
| 150 | + | // Valid right now. | |
| 151 | + | let ok = h.client.get("/oauth/userinfo").await; | |
| 152 | + | assert_eq!(ok.status, 200, "userinfo token must work before device removal: {}", ok.text); | |
| 153 | + | ||
| 154 | + | // Simulate device removal: bump sync_jwt_invalidated_at past the token's iat. | |
| 155 | + | sqlx::query("UPDATE users SET sync_jwt_invalidated_at = NOW() + INTERVAL '5 seconds' WHERE id = $1") | |
| 156 | + | .bind(user_id) | |
| 157 | + | .execute(&h.db) | |
| 158 | + | .await | |
| 159 | + | .unwrap(); | |
| 160 | + | ||
| 161 | + | // The userinfo token is now rejected — the parity gap is closed. | |
| 162 | + | let denied = h.client.get("/oauth/userinfo").await; | |
| 163 | + | assert_eq!(denied.status, 401, "device removal must kill the userinfo token: {}", denied.text); | |
| 164 | + | } |