Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add db table for login tokens which allows for invalidation #3818

Merged
merged 21 commits into from
Oct 9, 2023
Merged
Show file tree
Hide file tree
Changes from 15 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
17 changes: 9 additions & 8 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ actix-web = { version = "4.3.1", default-features = false, features = [
"compress-brotli",
"compress-gzip",
"compress-zstd",
"cookies",
] }
tracing = "0.1.37"
tracing-actix-web = { version = "0.7.5", default-features = false }
Expand Down Expand Up @@ -168,5 +169,4 @@ prometheus = { version = "0.13.3", features = ["process"], optional = true }
actix-web-prom = { version = "0.6.0", optional = true }
serial_test = { workspace = true }
clap = { version = "4.3.19", features = ["derive"] }
actix-web-httpauth = "0.8.1"
lemmy_federate = { version = "0.19.0-rc.1", path = "crates/federate" }
15 changes: 11 additions & 4 deletions api_tests/src/post.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,11 @@ import {
waitUntil,
waitForPost,
alphaUrl,
loginUser,
} from "./shared";
import { PostView } from "lemmy-js-client/dist/types/PostView";
import { CreatePost } from "lemmy-js-client/dist/types/CreatePost";
import { LemmyHttp } from "lemmy-js-client";
import { LemmyHttp, Login } from "lemmy-js-client";

let betaCommunity: CommunityView | undefined;

Expand Down Expand Up @@ -391,8 +392,9 @@ test("Enforce site ban for federated user", async () => {
let alpha_user = new LemmyHttp(alphaUrl, {
headers: { Authorization: `Bearer ${alphaUserJwt.jwt ?? ""}` },
});
let alphaUserActorId = (await getSite(alpha_user)).my_user?.local_user_view
.person.actor_id;
let alphaUserPerson = (await getSite(alpha_user)).my_user?.local_user_view
.person;
let alphaUserActorId = alphaUserPerson?.actor_id;
if (!alphaUserActorId) {
throw "Missing alpha user actor id";
}
Expand Down Expand Up @@ -438,8 +440,13 @@ test("Enforce site ban for federated user", async () => {
);
expect(unBanAlpha.banned).toBe(false);

// Login gets invalidated by ban, need to login again
let newAlphaUserJwt = await loginUser(alpha, alphaUserPerson?.name!);
alpha_user.setHeaders({
Authorization: "Bearer " + newAlphaUserJwt.jwt ?? "",
});
// alpha makes new post in beta community, it federates
let postRes2 = await createPost(alpha_user, betaCommunity.community.id);
let postRes2 = await createPost(alpha_user, betaCommunity!.community.id);
let searchBeta3 = await waitForPost(beta, postRes2.post_view.post);

let alphaUserOnBeta2 = await resolvePerson(beta, alphaUserActorId!);
Expand Down
11 changes: 11 additions & 0 deletions api_tests/src/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,17 @@ export async function registerUser(
return api.register(form);
}

export async function loginUser(
api: LemmyHttp,
username: string,
): Promise<LoginResponse> {
let form: Login = {
username_or_email: username,
password: password,
};
return api.login(form);
}

export async function saveUserSettingsBio(
api: LemmyHttp,
): Promise<LoginResponse> {
Expand Down
1 change: 1 addition & 0 deletions crates/api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ url = { workspace = true }
wav = "1.0.0"
sitemap-rs = "0.2.0"
totp-rs = { version = "5.0.2", features = ["gen_secret", "otpauth"] }
actix-web-httpauth = "0.8.1"

[dev-dependencies]
serial_test = { workspace = true }
Expand Down
28 changes: 27 additions & 1 deletion crates/api/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use actix_web::{cookie::SameSite, http::header::Header, HttpRequest};
use actix_web_httpauth::headers::authorization::{Authorization, Bearer};
use base64::{engine::general_purpose::STANDARD_NO_PAD as base64, Engine};
use captcha::Captcha;
use lemmy_api_common::utils::local_site_to_slur_regex;
use lemmy_api_common::utils::{local_site_to_slur_regex, AUTH_COOKIE_NAME};
use lemmy_db_schema::source::local_site::LocalSite;
use lemmy_db_views::structs::LocalUserView;
use lemmy_utils::{
Expand Down Expand Up @@ -69,6 +71,30 @@ pub(crate) fn check_report_reason(reason: &str, local_site: &LocalSite) -> Resul
}
}

pub fn read_auth_token(req: &HttpRequest) -> Result<Option<String>, LemmyError> {
// Try reading jwt from auth header
let auth_header = Authorization::<Bearer>::parse(req).ok();
let jwt = if let Some(a) = auth_header {
Some(a.as_ref().token().to_string())
}
// If that fails, try auth cookie. Dont use the `jwt` cookie from lemmy-ui because
// its not http-only.
phiresky marked this conversation as resolved.
Show resolved Hide resolved
else {
let auth_cookie = req.cookie(AUTH_COOKIE_NAME);
if let Some(a) = &auth_cookie {
// ensure that its marked as httponly and secure
let secure = a.secure().unwrap_or_default();
let http_only = a.http_only().unwrap_or_default();
let same_site = a.same_site();
if !secure || !http_only || same_site != Some(SameSite::Strict) {
return Err(LemmyError::from(LemmyErrorType::AuthCookieInsecure));
}
}
auth_cookie.map(|c| c.value().to_string())
phiresky marked this conversation as resolved.
Show resolved Hide resolved
};
Ok(jwt)
}

pub(crate) fn check_totp_2fa_valid(
local_user_view: &LocalUserView,
totp_token: &Option<String>,
Expand Down
7 changes: 7 additions & 0 deletions crates/api/src/local_user/ban_person.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use lemmy_api_common::{
};
use lemmy_db_schema::{
source::{
login_token::LoginToken,
moderator::{ModBan, ModBanForm},
person::{Person, PersonUpdateForm},
},
Expand Down Expand Up @@ -44,6 +45,12 @@ pub async fn ban_from_site(
.await
.with_lemmy_type(LemmyErrorType::CouldntUpdateUser)?;

let local_user_id = LocalUserView::read_person(&mut context.pool(), data.person_id)
.await?
.local_user
.id;
LoginToken::invalidate_all(&mut context.pool(), local_user_id).await?;

// Remove their data if that's desired
let remove_data = data.remove_data.unwrap_or(false);
if remove_data {
Expand Down
25 changes: 11 additions & 14 deletions crates/api/src/local_user/change_password.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
use actix_web::web::{Data, Json};
use actix_web::{
web::{Data, Json},
HttpRequest,
};
use bcrypt::verify;
use lemmy_api_common::{
claims::Claims,
context::LemmyContext,
person::{ChangePassword, LoginResponse},
utils::password_length_check,
};
use lemmy_db_schema::source::local_user::LocalUser;
use lemmy_db_schema::source::{local_user::LocalUser, login_token::LoginToken};
use lemmy_db_views::structs::LocalUserView;
use lemmy_utils::{
claims::Claims,
error::{LemmyError, LemmyErrorType},
};
use lemmy_utils::error::{LemmyError, LemmyErrorType};

#[tracing::instrument(skip(context))]
pub async fn change_password(
data: Json<ChangePassword>,
req: HttpRequest,
context: Data<LemmyContext>,
local_user_view: LocalUserView,
) -> Result<Json<LoginResponse>, LemmyError> {
Expand All @@ -40,16 +42,11 @@ pub async fn change_password(
let updated_local_user =
LocalUser::update_password(&mut context.pool(), local_user_id, &new_password).await?;

LoginToken::invalidate_all(&mut context.pool(), local_user_view.local_user.id).await?;

// Return the jwt
Ok(Json(LoginResponse {
jwt: Some(
Claims::jwt(
updated_local_user.id.0,
&context.secret().jwt_secret,
&context.settings().hostname,
)?
.into(),
),
jwt: Some(Claims::generate(updated_local_user.id, req, &context).await?),
verify_email_sent: false,
registration_created: false,
}))
Expand Down
3 changes: 3 additions & 0 deletions crates/api/src/local_user/change_password_after_reset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use lemmy_api_common::{
};
use lemmy_db_schema::source::{
local_user::LocalUser,
login_token::LoginToken,
password_reset_request::PasswordResetRequest,
};
use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType};
Expand Down Expand Up @@ -34,6 +35,8 @@ pub async fn change_password_after_reset(
.await
.with_lemmy_type(LemmyErrorType::CouldntUpdateUser)?;

LoginToken::invalidate_all(&mut context.pool(), local_user_id).await?;

Ok(Json(LoginResponse {
jwt: None,
verify_email_sent: false,
Expand Down
14 changes: 14 additions & 0 deletions crates/api/src/local_user/list_logins.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use actix_web::web::{Data, Json};
use lemmy_api_common::context::LemmyContext;
use lemmy_db_schema::source::login_token::LoginToken;
use lemmy_db_views::structs::LocalUserView;
use lemmy_utils::error::LemmyError;

pub async fn list_logins(
context: Data<LemmyContext>,
local_user_view: LocalUserView,
) -> Result<Json<Vec<LoginToken>>, LemmyError> {
let logins = LoginToken::list(&mut context.pool(), local_user_view.local_user.id).await?;

Ok(Json(logins))
}
38 changes: 20 additions & 18 deletions crates/api/src/local_user/login.rs
Original file line number Diff line number Diff line change
@@ -1,28 +1,32 @@
use crate::check_totp_2fa_valid;
use actix_web::web::{Data, Json};
use actix_web::{
http::StatusCode,
web::{Data, Json},
HttpRequest,
HttpResponse,
};
use bcrypt::verify;
use lemmy_api_common::{
claims::Claims,
context::LemmyContext,
person::{Login, LoginResponse},
utils,
utils::check_user_valid,
utils::{check_user_valid, create_login_cookie},
};
use lemmy_db_schema::{
source::{local_site::LocalSite, registration_application::RegistrationApplication},
utils::DbPool,
RegistrationMode,
};
use lemmy_db_views::structs::{LocalUserView, SiteView};
use lemmy_utils::{
claims::Claims,
error::{LemmyError, LemmyErrorExt, LemmyErrorType},
};
use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType};

#[tracing::instrument(skip(context))]
pub async fn login(
data: Json<Login>,
req: HttpRequest,
context: Data<LemmyContext>,
) -> Result<Json<LoginResponse>, LemmyError> {
) -> Result<HttpResponse, LemmyError> {
let site_view = SiteView::read_local(&mut context.pool()).await?;

// Fetch that username / email
Expand Down Expand Up @@ -64,19 +68,17 @@ pub async fn login(
check_totp_2fa_valid(&local_user_view, &data.totp_2fa_token, &site_view.site.name)?;
}

// Return the jwt
Ok(Json(LoginResponse {
jwt: Some(
Claims::jwt(
local_user_view.local_user.id.0,
&context.secret().jwt_secret,
&context.settings().hostname,
)?
.into(),
),
let jwt = Claims::generate(local_user_view.local_user.id, req, &context).await?;

let json = LoginResponse {
jwt: Some(jwt.clone()),
verify_email_sent: false,
registration_created: false,
}))
};

let mut res = HttpResponse::build(StatusCode::OK).json(json);
res.add_cookie(&create_login_cookie(jwt))?;
Ok(res)
}

async fn check_registration_application(
Expand Down
23 changes: 23 additions & 0 deletions crates/api/src/local_user/logout.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
use crate::read_auth_token;
use activitypub_federation::config::Data;
use actix_web::{cookie::Cookie, HttpRequest, HttpResponse};
use lemmy_api_common::{context::LemmyContext, utils::AUTH_COOKIE_NAME};
use lemmy_db_schema::source::login_token::LoginToken;
use lemmy_db_views::structs::LocalUserView;
use lemmy_utils::error::LemmyResult;

#[tracing::instrument(skip(context))]
pub async fn logout(
req: HttpRequest,
// require login
_local_user_view: LocalUserView,
context: Data<LemmyContext>,
) -> LemmyResult<HttpResponse> {
let jwt = read_auth_token(&req)?.expect("user is logged in");
Copy link
Member

Choose a reason for hiding this comment

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

We should avoid expects in these API, because they panic and don't propagate any errors. I'll turn this into a NotLoggedIn error.

LoginToken::invalidate(&mut context.pool(), &jwt).await?;

let mut res = HttpResponse::Ok().finish();
let cookie = Cookie::new(AUTH_COOKIE_NAME, "");
res.add_removal_cookie(&cookie)?;
Ok(res)
}
Loading