From e790392644bb85f9f3bc73ec5457273353345a76 Mon Sep 17 00:00:00 2001 From: stadust <43299462+stadust@users.noreply.github.com> Date: Sat, 5 Oct 2024 16:15:18 +0100 Subject: [PATCH 1/5] fix: enable `legacy_accounts` feature right in pointercrate-example It needs to be set for both the API and the pages crates. Otherwise, the frontend will just receive a 404 when registering. Signed-off-by: stadust <43299462+stadust@users.noreply.github.com> --- pointercrate-example/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pointercrate-example/Cargo.toml b/pointercrate-example/Cargo.toml index 290b31823..37196a5a5 100644 --- a/pointercrate-example/Cargo.toml +++ b/pointercrate-example/Cargo.toml @@ -16,6 +16,6 @@ pointercrate-demonlist = { version = "0.1.0", path = "../pointercrate-demonlist" pointercrate-demonlist-api = { version = "0.1.0", path = "../pointercrate-demonlist-api" } pointercrate-demonlist-pages = { version = "0.1.0", path = "../pointercrate-demonlist-pages" } pointercrate-user = { version = "0.1.0", path = "../pointercrate-user" } -pointercrate-user-api = { version = "0.1.0", path = "../pointercrate-user-api" } +pointercrate-user-api = { version = "0.1.0", path = "../pointercrate-user-api", features = ["legacy_accounts"] } pointercrate-user-pages = { version = "0.1.0", path = "../pointercrate-user-pages", features = ["legacy_accounts"] } rocket = "0.5.1" From 3743ffa9227df417620579393684668f49a91422 Mon Sep 17 00:00:00 2001 From: stadust <43299462+stadust@users.noreply.github.com> Date: Sat, 5 Oct 2024 13:48:43 +0100 Subject: [PATCH 2/5] refactor: Provide generate functions to create/validate JWTs `generate_jwt` and `validate_jwt` create and validate JWTs signed with the given `AuthenticatedUsers` signing key. They are generic over the claims, giing a generic API for managing JWTs without needing to make `jwt_secret` a public function. While we're at it, use the JWT standard `sub` field to store the user ID we are expecting, and use the jsonwebtoken library's validation to ensure we always get the expected `sub` value. Also switch CSRF token's to this generalized procedure. This has the consequence that now they are signed using the user's secret key, instead of the global one. No functional change should follow from this, however to ensure that CSRF tokens do not work as access tokens, annotate all claims structs with `serde(deny_unknown_fields)`. Also move the "read ID from jwt by unsafely decoding it" stuff into `auth/mod.rs`, for better encapsulation of the claims structs. Signed-off-by: stadust <43299462+stadust@users.noreply.github.com> --- pointercrate-user/src/auth/get.rs | 24 +----- pointercrate-user/src/auth/mod.rs | 138 ++++++++++++++++-------------- 2 files changed, 78 insertions(+), 84 deletions(-) diff --git a/pointercrate-user/src/auth/get.rs b/pointercrate-user/src/auth/get.rs index cd8160028..f7483bb46 100644 --- a/pointercrate-user/src/auth/get.rs +++ b/pointercrate-user/src/auth/get.rs @@ -1,11 +1,4 @@ -use std::collections::HashSet; - -use crate::{ - auth::{AccessClaims, AuthenticatedUser}, - error::Result, - User, -}; -use jsonwebtoken::{DecodingKey, Validation}; +use crate::{auth::AuthenticatedUser, error::Result, User}; use log::{debug, info}; use pointercrate_core::error::CoreError; use sqlx::{Error, PgConnection}; @@ -14,23 +7,14 @@ impl AuthenticatedUser { pub async fn token_auth(access_token: &str, csrf_token: Option<&str>, connection: &mut PgConnection) -> Result { info!("We are expected to perform token authentication"); - // Well this is reassuring. Also we directly deconstruct it and only save the ID - // so we don't accidentally use unsafe values later on - let mut no_validation = Validation::default(); - no_validation.insecure_disable_signature_validation(); - no_validation.validate_exp = false; - no_validation.required_spec_claims = HashSet::new(); - - let AccessClaims { id, .. } = jsonwebtoken::decode(access_token, &DecodingKey::from_secret(b""), &no_validation) - .map_err(|_| CoreError::Unauthorized)? - .claims; + let sub = AuthenticatedUser::peek_jwt_sub(access_token)?; - debug!("The token identified the user with id {}, validating...", id); + debug!("The token identified the user with id {}, validating...", sub); // Note that at this point we haven't validated the access token OR the csrf token yet. // However, the key they are signed with encompasses the password salt for the user they supposedly // identify, so we need to retrieve that. - let user = Self::by_id(id, connection).await?.validate_access_token(access_token)?; + let user = Self::by_id(sub, connection).await?.validate_access_token(access_token)?; if let Some(csrf_token) = csrf_token { user.validate_csrf_token(csrf_token)? diff --git a/pointercrate-user/src/auth/mod.rs b/pointercrate-user/src/auth/mod.rs index cf91aa361..51961382e 100644 --- a/pointercrate-user/src/auth/mod.rs +++ b/pointercrate-user/src/auth/mod.rs @@ -7,11 +7,10 @@ pub use self::patch::PatchMe; use crate::{error::Result, User}; -use jsonwebtoken::{DecodingKey, EncodingKey}; +use jsonwebtoken::{errors::ErrorKind, DecodingKey, EncodingKey, Validation}; use legacy::LegacyAuthenticatedUser; -use log::warn; use pointercrate_core::error::CoreError; -use serde::{Deserialize, Serialize}; +use serde::{de::DeserializeOwned, Deserialize, Serialize}; use std::{ collections::HashSet, time::{Duration, SystemTime, UNIX_EPOCH}, @@ -26,16 +25,18 @@ pub enum AuthenticatedUser { Legacy(LegacyAuthenticatedUser), } -#[derive(Debug, Deserialize, Serialize, Copy, Clone)] -pub struct AccessClaims { - pub id: i32, +#[derive(Debug, Serialize, Deserialize, Clone)] +#[serde(deny_unknown_fields)] // to prevent a CSRF token to work as an access token +struct AccessClaims { + sub: String, } -#[derive(Debug, Deserialize, Serialize, Copy, Clone)] -pub struct CSRFClaims { - pub id: i32, - pub exp: u64, - pub iat: u64, +#[derive(Debug, Deserialize, Serialize, Clone)] +#[serde(deny_unknown_fields)] +struct CSRFClaims { + sub: String, // we're using the jsonwebtoken library's validation to check this field, which expect it to be a string + exp: u64, + iat: u64, } impl AuthenticatedUser { @@ -57,41 +58,69 @@ impl AuthenticatedUser { key } - pub fn generate_access_token(&self) -> String { + pub fn generate_jwt(&self, claims: &C) -> String { jsonwebtoken::encode( &jsonwebtoken::Header::default(), - &AccessClaims { id: self.user().id }, + &claims, &EncodingKey::from_secret(&self.jwt_secret()), ) .unwrap() } - pub fn validate_access_token(self, token: &str) -> Result { - // TODO: maybe one day do something with this - let mut validation = jsonwebtoken::Validation::default(); - validation.validate_exp = false; - validation.required_spec_claims = HashSet::default(); + pub fn validate_jwt(&self, jwt: &str, mut validation: Validation) -> Result { + validation.sub = Some(self.user().id.to_string()); + validation.required_spec_claims.insert("sub".to_string()); - jsonwebtoken::decode::(token, &DecodingKey::from_secret(&self.jwt_secret()), &validation) + jsonwebtoken::decode::(jwt, &DecodingKey::from_secret(&self.jwt_secret()), &validation) .map_err(|err| { - warn!("Token validation FAILED for account {}: {}", self.user(), err); - - CoreError::Unauthorized.into() - }) - .and_then(move |token_data| { - // sanity check, should never fail - if token_data.claims.id != self.user().id { - log::error!( - "Access token for user {} decoded successfully even though user {} is logged in", - token_data.claims.id, - self.user() - ); - - Err(CoreError::Unauthorized.into()) + if err.into_kind() == ErrorKind::InvalidSubject { + CoreError::internal_server_error(format!( + "Token for user with id {:?} decoded successfully using key for user with id {}", + Self::peek_jwt_sub(jwt), + self.user().id + )) + .into() } else { - Ok(self) + CoreError::Unauthorized.into() } }) + .map(|token_data| token_data.claims) + } + + pub fn peek_jwt_sub(jwt: &str) -> Result { + // Well this is reassuring. However, we only extract the id, and ensure the remaining + // values of the token are not even stored by using `struct _Unsafe` (serde will ignore + // superfluous fields during deserialization since its not tagged `deny_unknown_fields`) + let mut no_validation = Validation::default(); + no_validation.insecure_disable_signature_validation(); + no_validation.validate_exp = false; + no_validation.set_required_spec_claims(&["sub"]); + + #[derive(Deserialize)] + struct _Unsafe { + sub: String + } + + jsonwebtoken::decode::<_Unsafe>(jwt, &DecodingKey::from_secret(b""), &no_validation) + .map_err(|_| CoreError::Unauthorized)? + .claims + .sub + .parse() + .map_err(|_| CoreError::Unauthorized.into()) + } + + pub fn generate_access_token(&self) -> String { + self.generate_jwt(&AccessClaims { + sub: self.user().id.to_string() + }) + } + + pub fn validate_access_token(self, token: &str) -> Result { + let mut validation = Validation::default(); + validation.validate_exp = false; + validation.required_spec_claims = HashSet::new(); + + self.validate_jwt::(token, validation).map(|_| self) } pub fn generate_csrf_token(&self) -> String { @@ -101,43 +130,16 @@ impl AuthenticatedUser { .expect("time went backwards (and this is probably gonna bite me in the ass when it comes to daytimesaving crap)"); let claim = CSRFClaims { - id: self.user().id, + sub: self.user().id.to_string(), iat: since_epoch.as_secs(), exp: (since_epoch + Duration::from_secs(3600)).as_secs(), }; - jsonwebtoken::encode( - &jsonwebtoken::Header::default(), - &claim, - &EncodingKey::from_secret(&pointercrate_core::config::secret()), - ) - .unwrap() + self.generate_jwt(&claim) } pub fn validate_csrf_token(&self, token: &str) -> Result<()> { - let mut validation = jsonwebtoken::Validation::default(); - validation.validate_exp = false; - validation.required_spec_claims = HashSet::new(); - - jsonwebtoken::decode::(token, &DecodingKey::from_secret(&pointercrate_core::config::secret()), &validation) - .map_err(|err| { - warn!("Access token validation FAILED for account {}: {}", self.user(), err); - - CoreError::Unauthorized.into() - }) - .and_then(|token_data| { - if token_data.claims.id != self.user().id { - warn!( - "User {} attempt to authenticate using CSRF token generated for user {}", - self.user(), - token_data.claims.id - ); - - Err(CoreError::Unauthorized.into()) - } else { - Ok(()) - } - }) + self.validate_jwt::(&token, Validation::default()).map(|_| ()) } fn salt(&self) -> Vec { @@ -216,4 +218,12 @@ mod tests { assert!(patrick.validate_access_token(&patricks_access_token).is_ok()); assert!(jacob.validate_access_token(&patricks_access_token).is_err()); } + + #[test] + fn test_peek_jwt_sub() { + let patrick = patrick(); + + let patricks_csrf_token = patrick.generate_csrf_token(); + assert_eq!(AuthenticatedUser::peek_jwt_sub(&patricks_csrf_token).unwrap(), patrick.user().id) + } } From f23ddee0ca070a18627d7408d509c12c0c9ddaa0 Mon Sep 17 00:00:00 2001 From: stadust <43299462+stadust@users.noreply.github.com> Date: Sat, 5 Oct 2024 21:27:12 +0100 Subject: [PATCH 3/5] Drop `iat` field from CSRF tokens We were not doing anything with it anyway, and the check of the `exp` field is completely independent of it. Signed-off-by: stadust <43299462+stadust@users.noreply.github.com> --- pointercrate-user/src/auth/mod.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pointercrate-user/src/auth/mod.rs b/pointercrate-user/src/auth/mod.rs index 51961382e..5395ebba7 100644 --- a/pointercrate-user/src/auth/mod.rs +++ b/pointercrate-user/src/auth/mod.rs @@ -36,7 +36,6 @@ struct AccessClaims { struct CSRFClaims { sub: String, // we're using the jsonwebtoken library's validation to check this field, which expect it to be a string exp: u64, - iat: u64, } impl AuthenticatedUser { @@ -125,14 +124,14 @@ impl AuthenticatedUser { pub fn generate_csrf_token(&self) -> String { let start = SystemTime::now(); - let since_epoch = start + let exp = (start + Duration::from_secs(3600)) .duration_since(UNIX_EPOCH) - .expect("time went backwards (and this is probably gonna bite me in the ass when it comes to daytimesaving crap)"); + .expect("one hour in the future is earlier than the Unix Epoch. Wtf?") + .as_secs(); let claim = CSRFClaims { sub: self.user().id.to_string(), - iat: since_epoch.as_secs(), - exp: (since_epoch + Duration::from_secs(3600)).as_secs(), + exp, }; self.generate_jwt(&claim) From 934ca504fa2a83ef95a378a38a6c6962148ae2f6 Mon Sep 17 00:00:00 2001 From: stadust <43299462+stadust@users.noreply.github.com> Date: Sat, 5 Oct 2024 16:17:48 +0100 Subject: [PATCH 4/5] refactor: create pointercrate-user config.rs Move the `secret()` function from pointercrate-core to pointercrate-user, since the secret key is only used in pointercrate-user. Also opens us up for having more pointercrate-user specific configuration options (such as oauth client id). Signed-off-by: stadust <43299462+stadust@users.noreply.github.com> --- pointercrate-core/src/config.rs | 22 ---------------------- pointercrate-user/src/auth/mod.rs | 2 +- pointercrate-user/src/config.rs | 21 +++++++++++++++++++++ pointercrate-user/src/lib.rs | 1 + 4 files changed, 23 insertions(+), 23 deletions(-) create mode 100644 pointercrate-user/src/config.rs diff --git a/pointercrate-core/src/config.rs b/pointercrate-core/src/config.rs index 532bd3483..4240db5f9 100644 --- a/pointercrate-core/src/config.rs +++ b/pointercrate-core/src/config.rs @@ -1,25 +1,3 @@ -use crate::util::from_env_or_default; -use log::error; -use std::{fs::File, io::Read}; - pub fn database_url() -> String { std::env::var("DATABASE_URL").expect("DATABASE_URL is not set") } - -pub fn secret() -> Vec { - let path: String = from_env_or_default("SECRET_FILE", ".secret".into()); - - match File::open(path) { - Ok(file) => file.bytes().collect::, _>>().unwrap(), - Err(err) if cfg!(debug_assertions) => { - // needed for integration tests/CI - error!( - "Failed to read secret, using an unsecure default since this is a debug build- {:?}", - err - ); - - vec![0x0; 64] - }, - Err(err) => panic!("Unable to open secret file: {:?}", err), - } -} diff --git a/pointercrate-user/src/auth/mod.rs b/pointercrate-user/src/auth/mod.rs index 5395ebba7..1ba6116a8 100644 --- a/pointercrate-user/src/auth/mod.rs +++ b/pointercrate-user/src/auth/mod.rs @@ -52,7 +52,7 @@ impl AuthenticatedUser { } fn jwt_secret(&self) -> Vec { - let mut key: Vec = pointercrate_core::config::secret(); + let mut key: Vec = crate::config::secret(); key.extend(self.salt()); key } diff --git a/pointercrate-user/src/config.rs b/pointercrate-user/src/config.rs new file mode 100644 index 000000000..d4e9815a7 --- /dev/null +++ b/pointercrate-user/src/config.rs @@ -0,0 +1,21 @@ +use std::{fs::File, io::Read}; + +use pointercrate_core::util::from_env_or_default; + +pub fn secret() -> Vec { + let path: String = from_env_or_default("SECRET_FILE", ".secret".into()); + + match File::open(path) { + Ok(file) => file.bytes().collect::, _>>().unwrap(), + Err(err) if cfg!(debug_assertions) => { + // needed for integration tests/CI + log::error!( + "Failed to read secret, using an unsecure default since this is a debug build - {:?}", + err + ); + + vec![0x0; 64] + }, + Err(err) => panic!("Unable to open secret file: {:?}", err), + } +} \ No newline at end of file diff --git a/pointercrate-user/src/lib.rs b/pointercrate-user/src/lib.rs index e2d60e7f1..f2599266c 100644 --- a/pointercrate-user/src/lib.rs +++ b/pointercrate-user/src/lib.rs @@ -25,6 +25,7 @@ pub mod error; mod paginate; mod patch; mod video; +pub mod config; pub const ADMINISTRATOR: Permission = Permission::new("Administrator", 0x4000); pub const MODERATOR: Permission = Permission::new("Moderator", 0x2000); From e3a689eb56b6a7a4fd0bacca0832942c03a65e73 Mon Sep 17 00:00:00 2001 From: stadust <43299462+stadust@users.noreply.github.com> Date: Sat, 5 Oct 2024 10:21:48 +0100 Subject: [PATCH 5/5] Squashed commit of the following: + Moved google account id migration after all migrations added to master in the meantime. + Dropped change of password_hash to being optional + Break AuthenticatedUser::oauth2_callback into smaller functions + Drop `email_address` column from database. Was originally added when I only wanted to add email-based password reset, but with oauth there's no need. + Repurpose the `password_hash` field into a generic `salt` field for oauth accounts. This allows implementation the token reset. + Remove UserError::NotApplicable. It was the same as UserError::NonLegacy + Move the oauth endpoints to `pages.rs`, because they're not really part of the API (they cannot be used programmatically) commit 5bd3860920aad144dc90665569c9dd06e33ae07e Author: Natalia <54771118+peonii@users.noreply.github.com> Date: Sat Sep 14 21:24:35 2024 +0200 fix merge conflicts, (mostly) move over to feature flags commit 8730c8910576e679d2130e3371f839984161d21b Merge: 2734b866 5b6b7b3a Author: Natalia <54771118+peonii@users.noreply.github.com> Date: Sat Sep 14 21:24:31 2024 +0200 fix merge conflicts, (mostly) move over to feature flags commit 2734b86620b3a68e0ac2d71d20fe1c3b2b596b30 Author: peony Date: Tue Jul 9 11:27:55 2024 +0200 make lax cookies strict again commit e585a31317402c69915a42bf5d77603f5432d48a Author: peony Date: Tue Jul 9 11:25:07 2024 +0200 minor fixes commit 0abfc86781e40ed5e672c3cda24404e718b33fb5 Author: peony Date: Sun Jul 7 23:01:06 2024 +0200 remove unnecessary endpoints commit e334894fa7fd00d69ea244af895d89d4615640ed Merge: 8d44dd9b 00a6d387 Author: peony <54771118+peonii@users.noreply.github.com> Date: Sun Jul 7 22:55:28 2024 +0200 Merge branch 'master' into master commit 8d44dd9b6747a9009d94e3d5d2474a82f03116b9 Author: peony Date: Sun Jul 7 22:53:39 2024 +0200 account linking commit d91dfe1768a61b1ea283ae669d79cbf2217abef8 Author: peony Date: Mon May 20 23:35:00 2024 +0200 code cleanup, add explanation for possibly questionable code commit da39d62d84b9fe80f222e6e73675990320365a79 Author: peony Date: Mon May 20 23:20:23 2024 +0200 google oauth2 stub, nullable password migration Signed-off-by: stadust <43299462+stadust@users.noreply.github.com> --- .gitignore | 4 +- Cargo.lock | 4 + ...41005093210_add_google_account_id.down.sql | 6 + ...0241005093210_add_google_account_id.up.sql | 6 + pointercrate-example/Cargo.toml | 4 +- pointercrate-user-api/Cargo.toml | 19 ++- pointercrate-user-api/src/endpoints/auth.rs | 2 +- pointercrate-user-api/src/lib.rs | 2 + pointercrate-user-api/src/pages.rs | 141 +++++++++++++++++- pointercrate-user-pages/Cargo.toml | 3 +- .../src/account/profile.rs | 13 +- pointercrate-user-pages/src/login.rs | 21 ++- pointercrate-user-pages/static/js/login.js | 67 +++------ pointercrate-user/Cargo.toml | 14 +- pointercrate-user/src/auth/get.rs | 28 +++- pointercrate-user/src/auth/mod.rs | 19 ++- pointercrate-user/src/auth/oauth2/get.rs | 87 +++++++++++ pointercrate-user/src/auth/oauth2/mod.rs | 59 ++++++++ pointercrate-user/src/auth/oauth2/patch.rs | 29 ++++ pointercrate-user/src/auth/oauth2/post.rs | 64 ++++++++ pointercrate-user/src/config.rs | 17 ++- pointercrate-user/src/error.rs | 8 + pointercrate-user/src/lib.rs | 2 +- 23 files changed, 537 insertions(+), 82 deletions(-) create mode 100644 migrations/20241005093210_add_google_account_id.down.sql create mode 100644 migrations/20241005093210_add_google_account_id.up.sql create mode 100644 pointercrate-user/src/auth/oauth2/get.rs create mode 100644 pointercrate-user/src/auth/oauth2/mod.rs create mode 100644 pointercrate-user/src/auth/oauth2/patch.rs create mode 100644 pointercrate-user/src/auth/oauth2/post.rs diff --git a/.gitignore b/.gitignore index e2993f728..7afbdbe09 100644 --- a/.gitignore +++ b/.gitignore @@ -3,9 +3,11 @@ .secret .idea .vscode +**/.DS_Store +.DS_Store # random stuff I have because of enabling eslint in vscode node_modules eslint.config.mjs package-lock.json -package.json \ No newline at end of file +package.json diff --git a/Cargo.lock b/Cargo.lock index 9b6e5306a..3b048ac29 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1870,10 +1870,12 @@ dependencies = [ "bcrypt", "derive_more", "futures", + "getrandom", "jsonwebtoken", "lazy_static", "log", "pointercrate-core", + "reqwest", "serde", "serde_json", "sqlx", @@ -1885,6 +1887,7 @@ name = "pointercrate-user-api" version = "0.1.0" dependencies = [ "base64 0.22.1", + "getrandom", "governor", "log", "nonzero_ext", @@ -1894,6 +1897,7 @@ dependencies = [ "pointercrate-user", "pointercrate-user-pages", "rocket", + "serde", "serde_urlencoded", "sqlx", ] diff --git a/migrations/20241005093210_add_google_account_id.down.sql b/migrations/20241005093210_add_google_account_id.down.sql new file mode 100644 index 000000000..0343943af --- /dev/null +++ b/migrations/20241005093210_add_google_account_id.down.sql @@ -0,0 +1,6 @@ +-- Add down migration script here +-- Add down migration script here + +ALTER TABLE members + DROP COLUMN google_account_id, + ADD COLUMN email_address EMAIL; diff --git a/migrations/20241005093210_add_google_account_id.up.sql b/migrations/20241005093210_add_google_account_id.up.sql new file mode 100644 index 000000000..906b4be00 --- /dev/null +++ b/migrations/20241005093210_add_google_account_id.up.sql @@ -0,0 +1,6 @@ +-- Add up migration script here +-- Add up migration script here + +ALTER TABLE members + ADD COLUMN google_account_id VARCHAR(255) UNIQUE NULL, + DROP COLUMN email_address; diff --git a/pointercrate-example/Cargo.toml b/pointercrate-example/Cargo.toml index 37196a5a5..c4d187c5f 100644 --- a/pointercrate-example/Cargo.toml +++ b/pointercrate-example/Cargo.toml @@ -16,6 +16,6 @@ pointercrate-demonlist = { version = "0.1.0", path = "../pointercrate-demonlist" pointercrate-demonlist-api = { version = "0.1.0", path = "../pointercrate-demonlist-api" } pointercrate-demonlist-pages = { version = "0.1.0", path = "../pointercrate-demonlist-pages" } pointercrate-user = { version = "0.1.0", path = "../pointercrate-user" } -pointercrate-user-api = { version = "0.1.0", path = "../pointercrate-user-api", features = ["legacy_accounts"] } -pointercrate-user-pages = { version = "0.1.0", path = "../pointercrate-user-pages", features = ["legacy_accounts"] } +pointercrate-user-api = { version = "0.1.0", path = "../pointercrate-user-api", features = ["legacy_accounts", "oauth2"] } +pointercrate-user-pages = { version = "0.1.0", path = "../pointercrate-user-pages", features = ["legacy_accounts", "oauth2"] } rocket = "0.5.1" diff --git a/pointercrate-user-api/Cargo.toml b/pointercrate-user-api/Cargo.toml index f6d2eaaf6..8c20cbac5 100644 --- a/pointercrate-user-api/Cargo.toml +++ b/pointercrate-user-api/Cargo.toml @@ -7,18 +7,21 @@ edition.workspace = true # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -rocket = {version = "0.5.1", features = ["json"]} -sqlx = { version = "0.8", default-features = false, features = [ "runtime-tokio-native-tls", "macros", "postgres", "chrono", "migrate" ] } -pointercrate-user = {path = "../pointercrate-user"} -pointercrate-user-pages = {path = "../pointercrate-user-pages"} -pointercrate-core = {path = "../pointercrate-core"} -pointercrate-core-api = {path = "../pointercrate-core-api"} -pointercrate-core-pages = {path = "../pointercrate-core-pages"} +rocket = { version = "0.5.1", features = ["json"] } +sqlx = { version = "0.8", default-features = false, features = ["runtime-tokio-native-tls", "macros", "postgres", "chrono", "migrate"] } +pointercrate-user = { path = "../pointercrate-user" } +pointercrate-user-pages = { path = "../pointercrate-user-pages" } +pointercrate-core = { path = "../pointercrate-core" } +pointercrate-core-api = { path = "../pointercrate-core-api" } +pointercrate-core-pages = { path = "../pointercrate-core-pages" } log = "0.4.22" base64 = "0.22.1" nonzero_ext = "0.3.0" serde_urlencoded = "0.7.0" governor = "0.6.0" +serde = { version = "1.0.210", features = ["derive"], optional = true } +getrandom = { version = "0.2.15", optional = true } [features] -legacy_accounts = ["pointercrate-user/legacy_accounts"] \ No newline at end of file +legacy_accounts = ["pointercrate-user/legacy_accounts"] +oauth2 = ["pointercrate-user/oauth2", "serde", "getrandom"] diff --git a/pointercrate-user-api/src/endpoints/auth.rs b/pointercrate-user-api/src/endpoints/auth.rs index 89d7edae7..7b72ff290 100644 --- a/pointercrate-user-api/src/endpoints/auth.rs +++ b/pointercrate-user-api/src/endpoints/auth.rs @@ -15,7 +15,6 @@ use rocket::{ State, }; use std::net::IpAddr; - #[cfg(feature = "legacy_accounts")] use { pointercrate_core::pool::PointercratePool, @@ -65,6 +64,7 @@ pub async fn login( pub async fn invalidate(mut auth: BasicAuth) -> Result { match auth.user { AuthenticatedUser::Legacy(legacy) => legacy.invalidate_all_tokens(auth.secret, &mut auth.connection).await?, + AuthenticatedUser::OAuth2(oauth) => oauth.invalidate_all_tokens(&mut auth.connection).await?, // I have no clue what we'll do here } auth.connection.commit().await.map_err(UserError::from)?; diff --git a/pointercrate-user-api/src/lib.rs b/pointercrate-user-api/src/lib.rs index 64525655c..4d6d55d61 100644 --- a/pointercrate-user-api/src/lib.rs +++ b/pointercrate-user-api/src/lib.rs @@ -23,6 +23,8 @@ pub fn setup(rocket: Rocket) -> Rocket { auth_routes.extend(rocket::routes![endpoints::auth::register]); #[cfg(feature = "legacy_accounts")] page_routes.extend(rocket::routes![pages::register]); + #[cfg(feature = "oauth2")] + page_routes.extend(rocket::routes![pages::authorize, pages::callback]); rocket .manage(ratelimits) diff --git a/pointercrate-user-api/src/pages.rs b/pointercrate-user-api/src/pages.rs index d976d3471..3cf85c003 100644 --- a/pointercrate-user-api/src/pages.rs +++ b/pointercrate-user-api/src/pages.rs @@ -1,23 +1,30 @@ +//! Endpoints that are only intended to be used from the browser +//! +//! These are not part of the pointercrate API, either because they simply serve +//! HTML content, or because they are related to browser based authentication flows +//! (they set the access_token cookie, or are oauth related). + use crate::{ auth::{BasicAuth, TokenAuth}, ratelimits::UserRatelimits, }; use pointercrate_core::permission::PermissionsManager; +#[cfg(any(feature = "legacy_accounts", feature = "oauth2"))] +use pointercrate_core::pool::PointercratePool; +#[cfg(feature = "oauth2")] +use pointercrate_core_api::error::ErrorResponder; use pointercrate_core_api::response::Page; use pointercrate_core_pages::head::HeadLike; use pointercrate_user::error::UserError; use pointercrate_user_pages::account::AccountPageConfig; - use rocket::{ http::{Cookie, CookieJar, SameSite, Status}, response::Redirect, State, }; use std::net::IpAddr; - #[cfg(feature = "legacy_accounts")] use { - pointercrate_core::pool::PointercratePool, pointercrate_user::{ auth::legacy::{LegacyAuthenticatedUser, Registration}, auth::AuthenticatedUser, @@ -26,6 +33,134 @@ use { rocket::serde::json::Json, }; +#[cfg(feature = "oauth2")] +#[derive(serde::Serialize, serde::Deserialize)] +struct OAuthClaims { + sub: String, + nonce: u64, + exp: u64, +} + +#[cfg(feature = "oauth2")] +#[rocket::get("/authorize")] +pub fn authorize( + ip: IpAddr, ratelimits: &State, cookies: &CookieJar<'_>, auth: Option, +) -> Result { + use std::time::{Duration, SystemTime, UNIX_EPOCH}; + + use pointercrate_core::error::CoreError; + use pointercrate_user::config; + use rocket::time::OffsetDateTime; + + ratelimits.login_attempts(ip)?; + + let mut redirect_uri = "https://accounts.google.com/o/oauth2/v2/auth".to_string() + + format!("?client_id={}", config::google_client_id()).as_str() + + "&response_type=code" + + "&prompt=consent" + + "&scope=profile" + + "&redirect_uri=http%3A%2F%2Flocalhost%3A1971%2Fcallback"; + + if let Some(auth) = auth { + let mut nonce = [0u8; 8]; + getrandom::getrandom(&mut nonce).map_err(|err| CoreError::internal_server_error(err.to_string()))?; + let nonce = u64::from_le_bytes(nonce); + + redirect_uri = redirect_uri + + "&state=" + + &auth.user.generate_jwt(&OAuthClaims { + nonce, + sub: auth.user.user().id.to_string(), + exp: (SystemTime::now() + Duration::from_secs(5 * 60)) + .duration_since(UNIX_EPOCH) + .expect("time went backwards") + .as_secs(), + }); + + cookies.add( + Cookie::build(("oauth_nonce", nonce.to_string())) + .http_only(true) + .secure(!cfg!(debug_assertions)) + .expires(OffsetDateTime::now_utc() + Duration::from_secs(5 * 60)) + .same_site(SameSite::Strict) + .path("/api/v1/auth/callback"), + ) + } + + Ok(Redirect::to(redirect_uri)) +} + +#[cfg(feature = "oauth2")] +#[rocket::get("/callback?&")] +pub async fn callback( + auth: Option, pool: &State, ip: IpAddr, ratelimits: &State, code: &str, + state: Option<&str>, cookies: &CookieJar<'_>, +) -> Result, ErrorResponder> { + use pointercrate_core::error::CoreError; + use pointercrate_user::auth::AuthenticatedUser; + use rocket::response::content::RawHtml; + + ratelimits.login_attempts(ip)?; + + let user = match (state, auth) { + (Some(jwt), Some(mut auth)) => { + let claims = auth.user.validate_jwt::(jwt, Default::default())?; + + let nonce = cookies.get("oauth_nonce").ok_or(UserError::Core(CoreError::Unauthorized))?; + + if nonce.value() != claims.nonce.to_string() { + return Err(CoreError::Unauthorized.into()); + } + + cookies.remove(nonce.clone()); + + let user = auth.user.upgrade_legacy_account(code, &mut auth.connection).await?; + + auth.connection.commit().await.map_err(UserError::from)?; + + user + }, + + (None, None) => { + let mut connection = pool.transaction().await.map_err(UserError::from)?; + let user = AuthenticatedUser::by_oauth_code(code, &mut connection).await?; + connection.commit().await.map_err(UserError::from)?; + user + }, + + // If we do not have the state parameter, it means that we were not logged in during the request to /authorize, e.g. + // we wanted to create a new account. However, if now we are logged in, we could be in the scenario where some attacker + // started the oauth flow, and then tricked someone else into clicking the callback link they got from their "registration attempt". + // In other words: We cannot verify that the person logged in now is the same person that originally called /authorize. + // Thus return 401 UNAUTHORIZED. + _ => return Err(CoreError::Unauthorized.into()), + }; + + let cookie = Cookie::build(("access_token", user.generate_access_token())) + .http_only(true) + .secure(!cfg!(debug_assertions)) + .same_site(SameSite::Strict) + .path("/"); + + cookies.add(cookie); + + // We cannot use a HTTP redirect here, because HTTP redirect preserve "Referer" informatoin. Since we arrive + // at /callback after a redirect from google, this data will point to some google domain, and thus if we redirect + // here, we will open /account in the context of this referal from google. However, our access_token cookie is set + // with "Same-Site: strict", meaning it is not sent along for requests that are the result of a cross-domain referal, + // so even if we successfully login, we would just be dropped off at the login screen, until the user manually + // navigates somewhere else. + // + // However, "redirects" initiated by javascript loose the referer context, and thus if we instead do the below, + // the browser will send the access_token cookie along with the next request, and we end up on the profile + // page, as wanted. + Ok(RawHtml( + r#" + Redirecting...Redirecting... + "#, + )) +} + #[rocket::get("/login")] pub async fn login_page(auth: Option) -> Result { auth.map(|_| Redirect::to(rocket::uri!(account_page))) diff --git a/pointercrate-user-pages/Cargo.toml b/pointercrate-user-pages/Cargo.toml index a2b953688..b162b0863 100644 --- a/pointercrate-user-pages/Cargo.toml +++ b/pointercrate-user-pages/Cargo.toml @@ -15,4 +15,5 @@ async-trait = "0.1.82" sqlx = { version = "0.8", default-features = false, features = [ "runtime-tokio-native-tls", "macros", "postgres", "chrono", "migrate" ] } [features] -legacy_accounts = ["pointercrate-user/legacy_accounts"] \ No newline at end of file +legacy_accounts = ["pointercrate-user/legacy_accounts"] +oauth2 = ["pointercrate-user/oauth2"] \ No newline at end of file diff --git a/pointercrate-user-pages/src/account/profile.rs b/pointercrate-user-pages/src/account/profile.rs index 201e018af..6232c865c 100644 --- a/pointercrate-user-pages/src/account/profile.rs +++ b/pointercrate-user-pages/src/account/profile.rs @@ -96,7 +96,12 @@ impl AccountPageTab for ProfileTab { } div.flex.no-stretch { input.button.red.hover #delete-account type = "button" style = "margin: 15px auto 0px;" value="Delete My Account"; - input.button.blue.hover #change-password type = "button" style = "margin: 15px auto 0px;" value="Change Password"; + @if authenticated_user.is_legacy() { + input.button.blue.hover #change-password type = "button" style = "margin: 15px auto 0px;" value="Change Password"; + a.button.blue.hover #link-google href="/api/v1/auth/authorize?legacy=true" type = "button" style = "margin: 15px auto 0px;" { + "Link Google" + }; + } } } } @@ -155,7 +160,7 @@ impl AccountPageTab for ProfileTab { (edit_display_name_dialog()) (edit_youtube_link_dialog()) (change_password_dialog()) - (delete_account_dialog()) + (delete_account_dialog(!authenticated_user.is_legacy())) } } } @@ -258,7 +263,9 @@ fn change_password_dialog() -> Markup { } } -fn delete_account_dialog() -> Markup { +fn delete_account_dialog(is_google: bool) -> Markup { + // TODO: Add an alternative flow for Google authenticated users + html! { div.overlay.closable { div.dialog #delete-acc-dialog { diff --git a/pointercrate-user-pages/src/login.rs b/pointercrate-user-pages/src/login.rs index c0611106d..0b8e4cee7 100644 --- a/pointercrate-user-pages/src/login.rs +++ b/pointercrate-user-pages/src/login.rs @@ -45,6 +45,22 @@ fn login_page_body() -> Markup { } else { html!() }; + let oauth2_login = if cfg!(feature = "oauth2") { + html! { + div.flex.col { + h2 {"Login with Google"} + p { + "Log in or create a new pointercrate account with your Google account." + } + form.flex.col.grow #oauth2-form novalidate = "" { + p.info-red.output {} + input.button.blue.hover type = "submit" style = "margin: 15px auto 0px;" value = "Log in with Google"; + } + } + } + } else { + html!() + }; html! { div.m-center.flex.panel.fade.col.wrap style = "margin: 100px 0px;"{ h1.underlined.pad { @@ -55,9 +71,9 @@ fn login_page_body() -> Markup { } div.flex #login { div.flex.col { - h2 {"Login"} + h2 {"Login with Password"} p { - "Log in to an existing pointercrate account. You have 3 login attempts by 30 minutes. If you do not have an account yet, register on the right or below. " + "Log in to an existing pointercrate account. You have 3 login attempts by 30 minutes. If you do not have an account yet, log in with Google instead." } form.flex.col.grow #login-form novalidate = "" { p.info-red.output {} @@ -75,6 +91,7 @@ fn login_page_body() -> Markup { input.button.blue.hover type = "submit" style = "margin: 15px auto 0px;" value="Log in"; } } + (oauth2_login) (legacy_register) } } diff --git a/pointercrate-user-pages/static/js/login.js b/pointercrate-user-pages/static/js/login.js index 066f1cb9d..5caff7c13 100644 --- a/pointercrate-user-pages/static/js/login.js +++ b/pointercrate-user-pages/static/js/login.js @@ -1,4 +1,9 @@ -import { Form, valueMissing, tooShort, post } from "/static/core/js/modules/form.js?v=4"; +import { + Form, + valueMissing, + tooShort, + post, +} from "/static/core/js/modules/form.js"; function initializeLoginForm() { var loginForm = new Form(document.getElementById("login-form")); @@ -9,25 +14,25 @@ function initializeLoginForm() { loginUsername.addValidator(valueMissing, "Username required"); loginUsername.addValidator( tooShort, - "Username too short. It needs to be at least 3 characters long." + "Username too short. It needs to be at least 3 characters long.", ); loginPassword.clearOnInvalid = true; loginPassword.addValidator(valueMissing, "Password required"); loginPassword.addValidator( tooShort, - "Password too short. It needs to be at least 10 characters long." + "Password too short. It needs to be at least 10 characters long.", ); - loginForm.onSubmit(function(event) { + loginForm.onSubmit(function (event) { post("/login/", { Authorization: - "Basic " + btoa(loginUsername.value + ":" + loginPassword.value) + "Basic " + btoa(loginUsername.value + ":" + loginPassword.value), }) - .then(response => { + .then((response) => { window.location = "/account/"; }) - .catch(response => { + .catch((response) => { console.log(response); if (response.status === 401) { loginPassword.errorText = "Invalid credentials"; @@ -38,51 +43,15 @@ function initializeLoginForm() { }); } -function intializeRegisterForm() { - var registerForm = new Form(document.getElementById("register-form")); +function intializeGoogleForm() { + var registerForm = new Form(document.getElementById("google-form")); - var registerUsername = registerForm.input("register-username"); - var registerPassword = registerForm.input("register-password"); - var registerPasswordRepeat = registerForm.input("register-password-repeat"); - - registerUsername.addValidator(valueMissing, "Username required"); - registerUsername.addValidator( - tooShort, - "Username too short. It needs to be at least 3 characters long." - ); - - registerPassword.addValidator(valueMissing, "Password required"); - registerPassword.addValidator( - tooShort, - "Password too short. It needs to be at least 10 characters long." - ); - - registerPasswordRepeat.addValidator(valueMissing, "Password required"); - registerPasswordRepeat.addValidator( - tooShort, - "Password too short. It needs to be at least 10 characters long." - ); - registerPasswordRepeat.addValidator( - rpp => rpp.value == registerPassword.value, - "Passwords don't match" - ); - - registerForm.onSubmit(function(event) { - post("/register/", {}, registerForm.serialize()) - .then(response => { - window.location = "/account/"; - }) - .catch(response => { - if (response.status === 409) { - registerUsername.errorText = "This username is already taken. Please choose another one"; - } else { - registerForm.setError(response.data.message); - } - }); + registerForm.onSubmit(function (event) { + window.location = "/api/v1/auth/authorize"; }); } -$(document).ready(function() { +$(document).ready(function () { initializeLoginForm(); - intializeRegisterForm(); + intializeGoogleForm(); }); diff --git a/pointercrate-user/Cargo.toml b/pointercrate-user/Cargo.toml index 9fc40394e..12a927fc0 100644 --- a/pointercrate-user/Cargo.toml +++ b/pointercrate-user/Cargo.toml @@ -7,10 +7,15 @@ edition.workspace = true # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -pointercrate-core = {path = "../pointercrate-core"} +pointercrate-core = { path = "../pointercrate-core" } serde = "1.0.210" derive_more = "0.99.18" -sqlx = { version = "0.8", default-features = false, features = [ "runtime-tokio-native-tls", "macros", "postgres", "chrono" ] } +sqlx = { version = "0.8", default-features = false, features = [ + "runtime-tokio-native-tls", + "macros", + "postgres", + "chrono", +] } jsonwebtoken = "9.3.0" log = "0.4.22" futures = "0.3.8" @@ -19,6 +24,9 @@ lazy_static = "1.5.0" bcrypt = "0.15.1" url = "2.5.2" serde_json = "1.0.128" +reqwest = { version = "0.12.4", features = ["json"] } +getrandom = "0.2.15" [features] -legacy_accounts = [] \ No newline at end of file +legacy_accounts = [] +oauth2 = [] diff --git a/pointercrate-user/src/auth/get.rs b/pointercrate-user/src/auth/get.rs index f7483bb46..67b68d224 100644 --- a/pointercrate-user/src/auth/get.rs +++ b/pointercrate-user/src/auth/get.rs @@ -25,7 +25,7 @@ impl AuthenticatedUser { pub(in crate::auth) async fn by_id(id: i32, connection: &mut PgConnection) -> Result { let row = sqlx::query!( - r#"SELECT member_id, members.name, permissions::integer, display_name, youtube_channel::text, password_hash FROM members WHERE member_id = $1"#, + r#"SELECT member_id, members.name, permissions::integer, display_name, youtube_channel::text, password_hash, google_account_id FROM members WHERE member_id = $1"#, id ) .fetch_one(connection) @@ -34,13 +34,23 @@ impl AuthenticatedUser { match row { Err(Error::RowNotFound) => Err(CoreError::Unauthorized.into()), Err(err) => Err(err.into()), - Ok(row) => Ok(AuthenticatedUser::legacy(construct_from_row!(row), row.password_hash)), + Ok(row) => { + if row.google_account_id.is_some() { + Ok(AuthenticatedUser::oauth2( + construct_from_row!(row), + row.google_account_id.unwrap(), + row.password_hash, + )) + } else { + Ok(AuthenticatedUser::legacy(construct_from_row!(row), row.password_hash)) + } + }, } } pub(in crate::auth) async fn by_name(name: &str, connection: &mut PgConnection) -> Result { let row = sqlx::query!( - r#"SELECT member_id, members.name, permissions::integer, display_name, youtube_channel::text, password_hash FROM members WHERE members.name = $1"#, + r#"SELECT member_id, members.name, permissions::integer, display_name, youtube_channel::text, password_hash, google_account_id FROM members WHERE members.name = $1"#, name.to_string() ) .fetch_one(connection) @@ -49,7 +59,17 @@ impl AuthenticatedUser { match row { Err(Error::RowNotFound) => Err(CoreError::Unauthorized.into()), Err(err) => Err(err.into()), - Ok(row) => Ok(AuthenticatedUser::legacy(construct_from_row!(row), row.password_hash)), + Ok(row) => { + if row.google_account_id.is_some() { + Ok(AuthenticatedUser::oauth2( + construct_from_row!(row), + row.google_account_id.unwrap(), + row.password_hash, + )) + } else { + Ok(AuthenticatedUser::legacy(construct_from_row!(row), row.password_hash)) + } + }, } } } diff --git a/pointercrate-user/src/auth/mod.rs b/pointercrate-user/src/auth/mod.rs index 1ba6116a8..8eaa9397a 100644 --- a/pointercrate-user/src/auth/mod.rs +++ b/pointercrate-user/src/auth/mod.rs @@ -9,6 +9,7 @@ pub use self::patch::PatchMe; use crate::{error::Result, User}; use jsonwebtoken::{errors::ErrorKind, DecodingKey, EncodingKey, Validation}; use legacy::LegacyAuthenticatedUser; +use oauth2::OAuth2AuthenticatedUser; use pointercrate_core::error::CoreError; use serde::{de::DeserializeOwned, Deserialize, Serialize}; use std::{ @@ -19,10 +20,12 @@ use std::{ mod delete; mod get; pub mod legacy; +pub mod oauth2; mod patch; pub enum AuthenticatedUser { Legacy(LegacyAuthenticatedUser), + OAuth2(OAuth2AuthenticatedUser), } #[derive(Debug, Serialize, Deserialize, Clone)] @@ -34,20 +37,29 @@ struct AccessClaims { #[derive(Debug, Deserialize, Serialize, Clone)] #[serde(deny_unknown_fields)] struct CSRFClaims { - sub: String, // we're using the jsonwebtoken library's validation to check this field, which expect it to be a string + sub: String, // we're using the jsonwebtoken library's validation to check this field, which expect it to be a string exp: u64, } impl AuthenticatedUser { + pub fn is_legacy(&self) -> bool { + match self { + AuthenticatedUser::Legacy(_) => true, + AuthenticatedUser::OAuth2(_) => false, + } + } + pub fn into_user(self) -> User { match self { AuthenticatedUser::Legacy(legacy) => legacy.into_user(), + AuthenticatedUser::OAuth2(oauth2) => oauth2.into_user(), } } pub fn user(&self) -> &User { match self { AuthenticatedUser::Legacy(legacy) => legacy.user(), + AuthenticatedUser::OAuth2(oauth2) => oauth2.user(), } } @@ -97,7 +109,7 @@ impl AuthenticatedUser { #[derive(Deserialize)] struct _Unsafe { - sub: String + sub: String, } jsonwebtoken::decode::<_Unsafe>(jwt, &DecodingKey::from_secret(b""), &no_validation) @@ -110,7 +122,7 @@ impl AuthenticatedUser { pub fn generate_access_token(&self) -> String { self.generate_jwt(&AccessClaims { - sub: self.user().id.to_string() + sub: self.user().id.to_string(), }) } @@ -144,6 +156,7 @@ impl AuthenticatedUser { fn salt(&self) -> Vec { match self { AuthenticatedUser::Legacy(legacy) => legacy.salt(), + AuthenticatedUser::OAuth2(oauth2) => oauth2.salt(), } } diff --git a/pointercrate-user/src/auth/oauth2/get.rs b/pointercrate-user/src/auth/oauth2/get.rs new file mode 100644 index 000000000..31fe3ca02 --- /dev/null +++ b/pointercrate-user/src/auth/oauth2/get.rs @@ -0,0 +1,87 @@ +use super::OAuth2AuthenticatedUser; +use crate::auth::oauth2::GoogleUserInfo; +use crate::auth::AuthenticatedUser; +use crate::config; +use crate::error::UserError; +use crate::{Result, User}; +use jsonwebtoken::{Algorithm, DecodingKey, Validation}; +use pointercrate_core::error::CoreError; +use serde::Deserialize; +use sqlx::{Error, PgConnection}; + +impl AuthenticatedUser { + /// Resolves the given google oauth code for a google access token. Then tries + /// to find a pointercrate account linked to the google account for which we now have + /// an access token. If it finds one, return it. Otherwise, create a new pointercrate + /// account and link it to the google account. + pub async fn by_oauth_code(code: &str, mut connection: &mut PgConnection) -> Result { + let guser_info = get_oauth2_id(code).await?; + + match OAuth2AuthenticatedUser::by_google_account(&guser_info.id, &mut connection).await { + Ok(oauth_user) => Ok(AuthenticatedUser::OAuth2(oauth_user)), + Err(UserError::UserNotFoundGoogleAccount { .. }) => AuthenticatedUser::register_oauth(guser_info, &mut connection).await, + Err(err) => Err(err.into()), + } + } +} + +impl OAuth2AuthenticatedUser { + async fn by_google_account(id: &str, connection: &mut PgConnection) -> Result { + let row = sqlx::query!( + r#"SELECT member_id, members.name, CAST(permissions AS integer), display_name, youtube_channel::text, password_hash FROM members WHERE google_account_id = $1"#, + id + ) + .fetch_one(connection) + .await; + + match row { + Err(Error::RowNotFound) => Err(UserError::UserNotFoundGoogleAccount { + google_account_id: id.to_string(), + }), + Err(err) => Err(err.into()), + Ok(row) => Ok(OAuth2AuthenticatedUser { + user: construct_from_row!(row), + b64_salt: row.password_hash, + google_account_id: id.to_string(), + }), + } + } +} + +#[derive(Deserialize)] +struct GoogleTokenResponse { + id_token: String, +} + +pub(in crate::auth::oauth2) async fn get_oauth2_id(code: &str) -> Result { + let client = reqwest::Client::new(); + + let response = client + .post("https://oauth2.googleapis.com/token") + .form(&[ + ("code", code), + ("client_id", &config::google_client_id()), + ("client_secret", &config::google_client_secret()), + ("redirect_uri", &config::google_redirect_uri()), + ("grant_type", "authorization_code"), + ]) + .send() + .await + .map_err(|_| CoreError::Unauthorized)?; + + let response: GoogleTokenResponse = response.json().await.map_err(|_| CoreError::Unauthorized)?; + + // We can safely disable all validation here, as Google recommends to not + // validate a fresh token, as it is guaranteed to be valid. + // + // https://developers.google.com/identity/openid-connect/openid-connect#obtainuserinfo + let key = DecodingKey::from_secret(&[]); + let mut validation = Validation::new(Algorithm::HS256); + validation.insecure_disable_signature_validation(); + validation.validate_exp = false; + validation.validate_aud = false; + + jsonwebtoken::decode::(&response.id_token, &key, &validation) + .map_err(|_| CoreError::Unauthorized.into()) + .map(|token_data| token_data.claims) +} diff --git a/pointercrate-user/src/auth/oauth2/mod.rs b/pointercrate-user/src/auth/oauth2/mod.rs new file mode 100644 index 000000000..0817d83bf --- /dev/null +++ b/pointercrate-user/src/auth/oauth2/mod.rs @@ -0,0 +1,59 @@ +use base64::Engine; +use getrandom::getrandom; +use pointercrate_core::error::CoreError; + +use crate::Result; +use crate::User; + +use super::AuthenticatedUser; + +#[cfg(feature = "oauth2")] +pub mod get; +#[cfg(feature = "oauth2")] +mod patch; +mod post; + +pub struct OAuth2AuthenticatedUser { + user: User, + google_account_id: String, + b64_salt: String, +} + +#[cfg(feature = "oauth2")] +#[derive(serde::Deserialize)] +pub struct GoogleUserInfo { + #[serde(rename = "sub")] + pub id: String, + pub name: String, +} + +impl OAuth2AuthenticatedUser { + pub fn into_user(self) -> User { + self.user + } + + pub fn user(&self) -> &User { + &self.user + } + + pub fn salt(&self) -> Vec { + // unwrap okay: we trust our own database to not contain nonsense here + base64::prelude::BASE64_STANDARD.decode(&self.b64_salt).unwrap() + } +} + +impl AuthenticatedUser { + pub fn oauth2(user: User, google_account_id: String, b64_salt: String) -> Self { + AuthenticatedUser::OAuth2(OAuth2AuthenticatedUser { + user, + google_account_id, + b64_salt, + }) + } +} + +fn generate_salt() -> Result { + let mut salt = [0u8; 16]; + getrandom(&mut salt).map_err(|err| CoreError::internal_server_error(err.to_string()))?; + Ok(base64::prelude::BASE64_STANDARD.encode(salt)) +} diff --git a/pointercrate-user/src/auth/oauth2/patch.rs b/pointercrate-user/src/auth/oauth2/patch.rs new file mode 100644 index 000000000..f911b3852 --- /dev/null +++ b/pointercrate-user/src/auth/oauth2/patch.rs @@ -0,0 +1,29 @@ +use pointercrate_core::error::CoreError; +use sqlx::PgConnection; + +use crate::auth::AuthenticatedUser; +use crate::Result; + +use super::{generate_salt, get::get_oauth2_id}; + +impl AuthenticatedUser { + pub async fn upgrade_legacy_account(self, oauth_code: &str, connection: &mut PgConnection) -> Result { + if !self.is_legacy() { + return Err(CoreError::Unauthorized.into()); + } + + let guser_info = get_oauth2_id(oauth_code).await?; + let b64_salt = generate_salt()?; + + sqlx::query!( + "UPDATE members SET google_account_id = $1, password_hash = $2 WHERE member_id = $3", + guser_info.id, + b64_salt, + self.user().id + ) + .execute(connection) + .await?; + + Ok(AuthenticatedUser::oauth2(self.into_user(), guser_info.id, b64_salt)) + } +} diff --git a/pointercrate-user/src/auth/oauth2/post.rs b/pointercrate-user/src/auth/oauth2/post.rs new file mode 100644 index 000000000..b7cf64e39 --- /dev/null +++ b/pointercrate-user/src/auth/oauth2/post.rs @@ -0,0 +1,64 @@ +use sqlx::PgConnection; + +use super::{generate_salt, OAuth2AuthenticatedUser}; +#[cfg(feature = "oauth2")] +use crate::auth::oauth2::GoogleUserInfo; +use crate::auth::AuthenticatedUser; +use crate::Result; + +impl OAuth2AuthenticatedUser { + pub async fn invalidate_all_tokens(self, connection: &mut PgConnection) -> Result<()> { + log::warn!("Invalidating all tokens for user {}", self.user); + let b64_salt = generate_salt()?; + sqlx::query!( + "UPDATE members SET password_hash = $1 WHERE member_id = $2", + b64_salt, + self.user().id + ) + .execute(connection) + .await?; + + Ok(()) + } +} + +impl AuthenticatedUser { + #[cfg(feature = "oauth2")] + pub(in crate::auth::oauth2) async fn register_oauth( + guser_info: GoogleUserInfo, connection: &mut PgConnection, + ) -> Result { + use crate::User; + // This will never conflict with an existing user + // According to Google, the account ID is always unique + // https://developers.google.com/identity/openid-connect/openid-connect#an-id-tokens-payload + let name = format!("{}#{}", guser_info.name, guser_info.id); + let b64_salt = generate_salt()?; + + let id = sqlx::query!( + "INSERT INTO + members (name, display_name, google_account_id, password_hash) + VALUES + ($1, $2, $3, $4) RETURNING member_id + ", + name, + guser_info.name, + guser_info.id, + b64_salt + ) + .fetch_one(connection) + .await? + .member_id; + + Ok(Self::oauth2( + User { + id, + name, + permissions: 0, + display_name: Some(guser_info.name), + youtube_channel: None, + }, + guser_info.id, + b64_salt, + )) + } +} diff --git a/pointercrate-user/src/config.rs b/pointercrate-user/src/config.rs index d4e9815a7..2b7effcce 100644 --- a/pointercrate-user/src/config.rs +++ b/pointercrate-user/src/config.rs @@ -18,4 +18,19 @@ pub fn secret() -> Vec { }, Err(err) => panic!("Unable to open secret file: {:?}", err), } -} \ No newline at end of file +} + +#[cfg(feature = "oauth2")] +pub fn google_client_id() -> String { + std::env::var("GOOGLE_CLIENT_ID").expect("GOOGLE_CLIENT_ID is not set") +} + +#[cfg(feature = "oauth2")] +pub fn google_client_secret() -> String { + std::env::var("GOOGLE_CLIENT_SECRET").expect("GOOGLE_CLIENT_SECRET is not set") +} + +#[cfg(feature = "oauth2")] +pub fn google_redirect_uri() -> String { + std::env::var("GOOGLE_REDIRECT_URI").expect("GOOGLE_REDIRECT_URI is not set") +} diff --git a/pointercrate-user/src/error.rs b/pointercrate-user/src/error.rs index b77be6291..6af37844e 100644 --- a/pointercrate-user/src/error.rs +++ b/pointercrate-user/src/error.rs @@ -40,6 +40,12 @@ pub enum UserError { #[display(fmt = "No user with name {} found", user_name)] UserNotFoundName { user_name: String }, + #[display(fmt = "No user with google account {} found", google_account_id)] + UserNotFoundGoogleAccount { google_account_id: String }, + + #[display(fmt = "The user is already linked to a Google account")] + AlreadyLinked, + /// `409 CONFLICT` error returned if a user tries to register with a name that's already taken /// /// Error Code `40902` @@ -98,7 +104,9 @@ impl PointercrateError for UserError { PermissionNotAssignable { .. } => 40305, UserNotFound { .. } => 40401, UserNotFoundName { .. } => 40401, + UserNotFoundGoogleAccount { .. } => 40401, NameTaken => 40902, + AlreadyLinked => 40903, InvalidUsername => 42202, InvalidPassword => 42204, NotYouTube => 42226, diff --git a/pointercrate-user/src/lib.rs b/pointercrate-user/src/lib.rs index f2599266c..366b0e743 100644 --- a/pointercrate-user/src/lib.rs +++ b/pointercrate-user/src/lib.rs @@ -20,12 +20,12 @@ use std::{ #[macro_use] mod get; pub mod auth; +pub mod config; mod delete; pub mod error; mod paginate; mod patch; mod video; -pub mod config; pub const ADMINISTRATOR: Permission = Permission::new("Administrator", 0x4000); pub const MODERATOR: Permission = Permission::new("Moderator", 0x2000);