From 22db4f35c628040488e2f21a29a0f5987409b331 Mon Sep 17 00:00:00 2001 From: Tyler Cloutier Date: Wed, 23 Oct 2024 09:41:45 -0600 Subject: [PATCH 01/11] Small naming fixes. Fixes database identity creation --- crates/client-api/src/lib.rs | 2 +- crates/client-api/src/routes/database.rs | 43 +++++++++++++----------- crates/client-api/src/util.rs | 12 +++---- crates/lib/src/identity.rs | 9 ----- 4 files changed, 31 insertions(+), 35 deletions(-) diff --git a/crates/client-api/src/lib.rs b/crates/client-api/src/lib.rs index f0c5dadb1e4..23d783bc573 100644 --- a/crates/client-api/src/lib.rs +++ b/crates/client-api/src/lib.rs @@ -131,7 +131,7 @@ pub trait ControlStateWriteAccess: Send + Sync { async fn register_tld(&self, identity: &Identity, tld: Tld) -> anyhow::Result; async fn create_dns_record( &self, - identity: &Identity, + owner_identity: &Identity, domain: &DomainName, database_identity: &Identity, ) -> anyhow::Result; diff --git a/crates/client-api/src/routes/database.rs b/crates/client-api/src/routes/database.rs index a2a8342074d..54fb2bb290d 100644 --- a/crates/client-api/src/routes/database.rs +++ b/crates/client-api/src/routes/database.rs @@ -1,6 +1,6 @@ use crate::auth::{ anon_auth_middleware, SpacetimeAuth, SpacetimeAuthHeader, SpacetimeEnergyUsed, SpacetimeExecutionDurationMicros, - SpacetimeIdentity, SpacetimeIdentityToken, + SpacetimeIdentity, SpacetimeIdentityToken, LOCALHOST, }; use crate::routes::subscribe::generate_random_address; use crate::util::{ByteStringBody, NameOrIdentity}; @@ -384,17 +384,17 @@ pub async fn info( State(worker_ctx): State, Path(InfoParams { name_or_identity }): Path, ) -> axum::response::Result { - log::trace!("Trying to resolve address: {:?}", name_or_identity); - let address = name_or_identity.resolve(&worker_ctx).await?.into(); - log::trace!("Resolved address to: {address:?}"); - let database = worker_ctx_find_database(&worker_ctx, &address) + log::trace!("Trying to resolve database identity: {:?}", name_or_identity); + let database_identity = name_or_identity.resolve(&worker_ctx).await?.into(); + log::trace!("Resolved identity to: {database_identity:?}"); + let database = worker_ctx_find_database(&worker_ctx, &database_identity) .await? .ok_or((StatusCode::NOT_FOUND, "No such database."))?; - log::trace!("Fetched database from the worker db for address: {address:?}"); + log::trace!("Fetched database from the worker db for address: {database_identity:?}"); let host_type: &str = database.host_type.as_ref(); let response_json = json!({ - "address": database.database_identity, + "database_identity": database.database_identity, "owner_identity": database.owner_identity, "host_type": host_type, "initial_program": database.initial_program, @@ -679,13 +679,14 @@ pub async fn publish( // You should not be able to publish to a database that you do not own // so, unless you are the owner, this will fail. - let (db_addr, db_name) = match name_or_identity { + let (database_identity, db_name) = match name_or_identity { Some(noa) => match noa.try_resolve(&ctx).await? { Ok(resolved) => resolved.into(), Err(domain) => { // `name_or_address` was a `NameOrAddress::Name`, but no record // exists yet. Create it now with a fresh address. - let database_identity = Identity::placeholder(); + let auth = SpacetimeAuth::alloc(&ctx).await?; + let database_identity = auth.identity; ctx.create_dns_record(&auth.identity, &domain, &database_identity) .await .map_err(log_and_500)?; @@ -693,18 +694,22 @@ pub async fn publish( } }, None => { - let database_identity = Identity::placeholder(); + let auth = SpacetimeAuth::alloc(&ctx).await?; + let database_identity = auth.identity; (database_identity, None) } }; - log::trace!("Publishing to the address: {}", db_addr.to_hex()); + log::trace!("Publishing to the address: {}", database_identity.to_hex()); let op = { - let exists = ctx.get_database_by_identity(&db_addr).map_err(log_and_500)?.is_some(); + let exists = ctx + .get_database_by_identity(&database_identity) + .map_err(log_and_500)? + .is_some(); if clear && exists { - ctx.delete_database(&auth.identity, &db_addr) + ctx.delete_database(&auth.identity, &database_identity) .await .map_err(log_and_500)?; } @@ -720,7 +725,7 @@ pub async fn publish( .publish_database( &auth.identity, DatabaseDef { - database_identity: db_addr, + database_identity, program_bytes: body.into(), num_replicas: 1, host_type: HostType::Wasm, @@ -747,7 +752,7 @@ pub async fn publish( Ok(axum::Json(PublishResult::Success { domain: db_name.as_ref().map(ToString::to_string), - database_identity: db_addr, + database_identity, op, })) } @@ -783,14 +788,14 @@ pub async fn set_name( State(ctx): State, Query(SetNameQueryParams { domain, - database_identity: address, + database_identity, }): Query, Extension(auth): Extension, ) -> axum::response::Result { - let address = Identity::from(address); + let database_identity = Identity::from(database_identity); let database = ctx - .get_database_by_identity(&address) + .get_database_by_identity(&database_identity) .map_err(log_and_500)? .ok_or((StatusCode::NOT_FOUND, "No such database."))?; @@ -800,7 +805,7 @@ pub async fn set_name( let domain = domain.parse().map_err(|_| DomainParsingRejection)?; let response = ctx - .create_dns_record(&auth.identity, &domain, &address) + .create_dns_record(&auth.identity, &domain, &database_identity) .await // TODO: better error code handling .map_err(log_and_500)?; diff --git a/crates/client-api/src/util.rs b/crates/client-api/src/util.rs index 27c222fe73e..652906a2c74 100644 --- a/crates/client-api/src/util.rs +++ b/crates/client-api/src/util.rs @@ -91,16 +91,16 @@ impl NameOrIdentity { ctx: &(impl ControlStateReadAccess + ?Sized), ) -> axum::response::Result> { Ok(match self { - Self::Identity(addr) => Ok(ResolvedIdentity { - identity: Identity::from(*addr), + Self::Identity(identity) => Ok(ResolvedIdentity { + identity: Identity::from(*identity), domain: None, }), Self::Name(name) => { let domain = name.parse().map_err(|_| DomainParsingRejection)?; - let address = ctx.lookup_identity(&domain).map_err(log_and_500)?; - match address { - Some(address) => Ok(ResolvedIdentity { - identity: address, + let identity = ctx.lookup_identity(&domain).map_err(log_and_500)?; + match identity { + Some(identity) => Ok(ResolvedIdentity { + identity, domain: Some(domain), }), None => Err(domain), diff --git a/crates/lib/src/identity.rs b/crates/lib/src/identity.rs index 6a93027010e..eed44fa74ee 100644 --- a/crates/lib/src/identity.rs +++ b/crates/lib/src/identity.rs @@ -1,8 +1,6 @@ use crate::from_hex_pad; use blake3; use core::mem; -use rand; -use rand::Rng; use spacetimedb_bindings_macro::{Deserialize, Serialize}; use spacetimedb_sats::hex::HexString; use spacetimedb_sats::{hash, impl_st, u256, AlgebraicType, AlgebraicValue}; @@ -53,13 +51,6 @@ impl spacetimedb_metrics::typed_prometheus::AsPrometheusLabel for Identity { impl Identity { pub const ZERO: Self = Self::from_u256(u256::ZERO); - pub fn placeholder() -> Self { - // Generate a random identity. - let mut rng = rand::thread_rng(); - let mut random_bytes = [0u8; 32]; - rng.fill(&mut random_bytes); - Identity::from_byte_array(random_bytes) - } /// Returns an `Identity` defined as the given `bytes` byte array. pub const fn from_byte_array(bytes: [u8; 32]) -> Self { // SAFETY: The transmute is an implementation of `u256::from_ne_bytes`, From d4fd11f5675d7978bd8722efb80c9c6d6b80c58c Mon Sep 17 00:00:00 2001 From: Tyler Cloutier Date: Wed, 23 Oct 2024 10:16:04 -0600 Subject: [PATCH 02/11] Fix warnings and errors --- crates/client-api/src/routes/database.rs | 2 +- crates/testing/src/modules.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/client-api/src/routes/database.rs b/crates/client-api/src/routes/database.rs index 54fb2bb290d..2b651998e27 100644 --- a/crates/client-api/src/routes/database.rs +++ b/crates/client-api/src/routes/database.rs @@ -1,6 +1,6 @@ use crate::auth::{ anon_auth_middleware, SpacetimeAuth, SpacetimeAuthHeader, SpacetimeEnergyUsed, SpacetimeExecutionDurationMicros, - SpacetimeIdentity, SpacetimeIdentityToken, LOCALHOST, + SpacetimeIdentity, SpacetimeIdentityToken }; use crate::routes::subscribe::generate_random_address; use crate::util::{ByteStringBody, NameOrIdentity}; diff --git a/crates/testing/src/modules.rs b/crates/testing/src/modules.rs index c82dfd5a629..711298ee32a 100644 --- a/crates/testing/src/modules.rs +++ b/crates/testing/src/modules.rs @@ -6,6 +6,7 @@ use std::time::Instant; use spacetimedb::messages::control_db::HostType; use spacetimedb::Identity; +use spacetimedb_client_api::auth::SpacetimeAuth; use spacetimedb_client_api::routes::subscribe::generate_random_address; use spacetimedb_lib::ser::serde::SerializeWrapper; use tokio::runtime::{Builder, Runtime}; @@ -152,7 +153,7 @@ impl CompiledModule { let env = spacetimedb_standalone::StandaloneEnv::init(config).await.unwrap(); // TODO: Fix this when we update identity generation. let identity = Identity::ZERO; - let db_identity = Identity::placeholder(); + let db_identity = SpacetimeAuth::alloc(&env).await.unwrap().identity; let client_address = generate_random_address(); let program_bytes = self From 9b077a5d0fbfd4b2eb752f353aab400a9a1e8b51 Mon Sep 17 00:00:00 2001 From: Tyler Cloutier Date: Wed, 23 Oct 2024 10:24:00 -0600 Subject: [PATCH 03/11] cargo fmt --- crates/client-api/src/routes/database.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/client-api/src/routes/database.rs b/crates/client-api/src/routes/database.rs index 2b651998e27..ea01020646d 100644 --- a/crates/client-api/src/routes/database.rs +++ b/crates/client-api/src/routes/database.rs @@ -1,6 +1,6 @@ use crate::auth::{ anon_auth_middleware, SpacetimeAuth, SpacetimeAuthHeader, SpacetimeEnergyUsed, SpacetimeExecutionDurationMicros, - SpacetimeIdentity, SpacetimeIdentityToken + SpacetimeIdentity, SpacetimeIdentityToken, }; use crate::routes::subscribe::generate_random_address; use crate::util::{ByteStringBody, NameOrIdentity}; From 340d2b98abe8d74e5517f48710995e7def4fdd04 Mon Sep 17 00:00:00 2001 From: Tyler Cloutier Date: Wed, 23 Oct 2024 11:01:47 -0600 Subject: [PATCH 04/11] renamed self.address to self.database_identity --- smoketests/__init__.py | 18 +++++++++--------- smoketests/tests/auto_migration.py | 6 +++--- smoketests/tests/describe.py | 6 +++--- smoketests/tests/domains.py | 6 +++--- smoketests/tests/new_user_flow.py | 2 +- smoketests/tests/permissions.py | 14 +++++++------- smoketests/tests/sql.py | 2 +- smoketests/tests/zz_docker.py | 2 +- 8 files changed, 28 insertions(+), 28 deletions(-) diff --git a/smoketests/__init__.py b/smoketests/__init__.py index 83a8e32b1fa..5520cc1ebbf 100644 --- a/smoketests/__init__.py +++ b/smoketests/__init__.py @@ -159,20 +159,20 @@ def spacetime(cls, *args, **kwargs): return spacetime(*args, **kwargs) def _check_published(self): - if not hasattr(self, "address"): + if not hasattr(self, "database_identity"): raise Exception("Cannot use this function without publishing a module") def call(self, reducer, *args, anon=False): self._check_published() anon = ["--anonymous"] if anon else [] - self.spacetime("call", *anon, "--", self.address, reducer, *map(json.dumps, args)) + self.spacetime("call", *anon, "--", self.database_identity, reducer, *map(json.dumps, args)) def logs(self, n): return [log["message"] for log in self.log_records(n)] def log_records(self, n): self._check_published() - logs = self.spacetime("logs", "--format=json", "-n", str(n), "--", self.address) + logs = self.spacetime("logs", "--format=json", "-n", str(n), "--", self.database_identity) return list(map(json.loads, logs.splitlines())) def publish_module(self, domain=None, *, clear=True, capture_stderr=True): @@ -184,7 +184,7 @@ def publish_module(self, domain=None, *, clear=True, capture_stderr=True): capture_stderr=capture_stderr, ) self.resolved_identity = re.search(r"identity: ([0-9a-fA-F]+)", publish_output)[1] - self.address = domain if domain is not None else self.resolved_identity + self.database_identity = domain if domain is not None else self.resolved_identity @classmethod def reset_config(cls): @@ -215,7 +215,7 @@ def subscribe(self, *queries, n): env = os.environ.copy() env["SPACETIME_CONFIG_FILE"] = str(self.config_path) - args = [SPACETIME_BIN, "subscribe", self.address, "-t", "60", "-n", str(n), "--print-initial-update", "--", *queries] + args = [SPACETIME_BIN, "subscribe", self.database_identity, "-t", "60", "-n", str(n), "--print-initial-update", "--", *queries] fake_args = ["spacetime", *args[1:]] log_cmd(fake_args) @@ -273,19 +273,19 @@ def setUpClass(cls): def tearDown(self): # if this single test method published a database, clean it up now - if "address" in self.__dict__: + if "database_identity" in self.__dict__: try: # TODO: save the credentials in publish_module() - self.spacetime("delete", self.address) + self.spacetime("delete", self.database_identity) except Exception: pass @classmethod def tearDownClass(cls): - if hasattr(cls, "address"): + if hasattr(cls, "database_identity"): try: # TODO: save the credentials in publish_module() - cls.spacetime("delete", cls.address) + cls.spacetime("delete", cls.database_identity) except Exception: pass diff --git a/smoketests/tests/auto_migration.py b/smoketests/tests/auto_migration.py index 57900167797..bad7a3d1a8c 100644 --- a/smoketests/tests/auto_migration.py +++ b/smoketests/tests/auto_migration.py @@ -66,7 +66,7 @@ class AddTableAutoMigration(Smoketest): def assertSql(self, sql, expected): self.maxDiff = None - sql_out = self.spacetime("sql", self.address, sql) + sql_out = self.spacetime("sql", self.database_identity, sql) sql_out = "\n".join([line.rstrip() for line in sql_out.splitlines()]) expected = "\n".join([line.rstrip() for line in expected.splitlines()]) self.assertMultiLineEqual(sql_out, expected) @@ -98,7 +98,7 @@ def test_add_table_auto_migration(self): ) self.write_module_code(self.MODULE_CODE_UPDATED) - self.publish_module(self.address, clear=False) + self.publish_module(self.database_identity, clear=False) logging.info("Updated") @@ -176,6 +176,6 @@ def test_reject_schema_changes(self): with self.assertRaises(Exception): self.write_module_code(self.MODULE_CODE_UPDATED) - self.publish_module(self.address, clear=False) + self.publish_module(self.database_identity, clear=False) logging.info("Rejected as expected.") diff --git a/smoketests/tests/describe.py b/smoketests/tests/describe.py index 69be3cb9601..30a5a07317c 100644 --- a/smoketests/tests/describe.py +++ b/smoketests/tests/describe.py @@ -26,6 +26,6 @@ class ModuleDescription(Smoketest): def test_describe(self): """Check describing a module""" - self.spacetime("describe", self.address) - self.spacetime("describe", self.address, "reducer", "say_hello") - self.spacetime("describe", self.address, "table", "person") + self.spacetime("describe", self.database_identity) + self.spacetime("describe", self.database_identity, "reducer", "say_hello") + self.spacetime("describe", self.database_identity, "table", "person") diff --git a/smoketests/tests/domains.py b/smoketests/tests/domains.py index bd72eaabbf1..4b4d39997c0 100644 --- a/smoketests/tests/domains.py +++ b/smoketests/tests/domains.py @@ -42,9 +42,9 @@ def test_set_name(self): rand_name = random_string() self.spacetime("dns", "register-tld", rand_name) - self.spacetime("dns", "set-name", rand_name, self.address) + self.spacetime("dns", "set-name", rand_name, self.database_identity) lookup_result = self.spacetime("dns", "lookup", rand_name).strip() - self.assertEqual(lookup_result, self.address) + self.assertEqual(lookup_result, self.database_identity) - names = self.spacetime("dns", "reverse-lookup", self.address).splitlines() + names = self.spacetime("dns", "reverse-lookup", self.database_identity).splitlines() self.assertIn(rand_name, names) \ No newline at end of file diff --git a/smoketests/tests/new_user_flow.py b/smoketests/tests/new_user_flow.py index 567a77f8e9d..dcfd7f5bb8f 100644 --- a/smoketests/tests/new_user_flow.py +++ b/smoketests/tests/new_user_flow.py @@ -43,7 +43,7 @@ def test_new_user_flow(self): self.assertEqual(self.logs(5).count("Hello, World!"), 2) self.assertEqual(self.logs(5).count("Hello, Tyler!"), 1) - out = self.spacetime("sql", self.address, "SELECT * FROM person") + out = self.spacetime("sql", self.database_identity, "SELECT * FROM person") # The spaces after the name are important self.assertMultiLineEqual(out, """\ name diff --git a/smoketests/tests/permissions.py b/smoketests/tests/permissions.py index df842bb7155..3d145b76716 100644 --- a/smoketests/tests/permissions.py +++ b/smoketests/tests/permissions.py @@ -32,7 +32,7 @@ def test_delete(self): self.reset_config() with self.assertRaises(Exception): - self.spacetime("delete", self.address) + self.spacetime("delete", self.database_identity) def test_describe(self): """Ensure that anyone can describe any database""" @@ -42,7 +42,7 @@ def test_describe(self): self.reset_config() self.new_identity() - self.spacetime("describe", self.address) + self.spacetime("describe", self.database_identity) def test_logs(self): """Ensure that we are not able to view the logs of a module that we don't have permission to view""" @@ -57,7 +57,7 @@ def test_logs(self): self.reset_config() identity = self.new_identity(default=True) with self.assertRaises(Exception): - self.spacetime("logs", self.address, "-n", "10000") + self.spacetime("logs", self.database_identity, "-n", "10000") def test_publish(self): """This test checks to make sure that you cannot publish to an address that you do not own.""" @@ -68,11 +68,11 @@ def test_publish(self): self.reset_config() with self.assertRaises(Exception): - self.spacetime("publish", self.address, "--project-path", self.project_path, "--clear-database", "--yes") + self.spacetime("publish", self.database_identity, "--project-path", self.project_path, "--clear-database", "--yes") # Check that this holds without `--clear-database`, too. with self.assertRaises(Exception): - self.spacetime("publish", self.address, "--project-path", self.project_path) + self.spacetime("publish", self.database_identity, "--project-path", self.project_path) class PrivateTablePermissions(Smoketest): @@ -104,7 +104,7 @@ class PrivateTablePermissions(Smoketest): def test_private_table(self): """Ensure that a private table can only be queried by the database owner""" - out = self.spacetime("sql", self.address, "select * from secret") + out = self.spacetime("sql", self.database_identity, "select * from secret") self.assertMultiLineEqual(out, """\ answer -------- @@ -115,7 +115,7 @@ def test_private_table(self): self.new_identity() with self.assertRaises(Exception): - self.spacetime("sql", self.address, "select * from secret") + self.spacetime("sql", self.database_identity, "select * from secret") with self.assertRaises(Exception): self.subscribe("SELECT * FROM secret", n=0) diff --git a/smoketests/tests/sql.py b/smoketests/tests/sql.py index 6ad8bfa1165..9ba491d9d22 100644 --- a/smoketests/tests/sql.py +++ b/smoketests/tests/sql.py @@ -90,7 +90,7 @@ class SqlFormat(Smoketest): def assertSql(self, sql, expected): self.maxDiff = None - sql_out = self.spacetime("sql", self.address, sql) + sql_out = self.spacetime("sql", self.database_identity, sql) sql_out = "\n".join([line.rstrip() for line in sql_out.splitlines()]) expected = "\n".join([line.rstrip() for line in expected.splitlines()]) self.assertMultiLineEqual(sql_out, expected) diff --git a/smoketests/tests/zz_docker.py b/smoketests/tests/zz_docker.py index 0bb9a452113..a5d236cda67 100644 --- a/smoketests/tests/zz_docker.py +++ b/smoketests/tests/zz_docker.py @@ -130,7 +130,7 @@ def test_restart_module(self): restart_docker() - sql_out = self.spacetime("sql", self.address, "SELECT name FROM person WHERE id = 3") + sql_out = self.spacetime("sql", self.database_identity, "SELECT name FROM person WHERE id = 3") self.assertMultiLineEqual(sql_out, """ name \n------------\n "Samantha" \n""") @requires_docker From a56d210b9ec42d34286140f472b021ff2d85b09b Mon Sep 17 00:00:00 2001 From: Tyler Cloutier Date: Wed, 23 Oct 2024 11:11:32 -0600 Subject: [PATCH 05/11] Aliasing bug --- crates/client-api/src/routes/database.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/client-api/src/routes/database.rs b/crates/client-api/src/routes/database.rs index ea01020646d..9de0ac068d5 100644 --- a/crates/client-api/src/routes/database.rs +++ b/crates/client-api/src/routes/database.rs @@ -685,8 +685,8 @@ pub async fn publish( Err(domain) => { // `name_or_address` was a `NameOrAddress::Name`, but no record // exists yet. Create it now with a fresh address. - let auth = SpacetimeAuth::alloc(&ctx).await?; - let database_identity = auth.identity; + let database_auth = SpacetimeAuth::alloc(&ctx).await?; + let database_identity = database_auth.identity; ctx.create_dns_record(&auth.identity, &domain, &database_identity) .await .map_err(log_and_500)?; From c07b4f677419b0cb5b2f64139c69d5cfef227fa7 Mon Sep 17 00:00:00 2001 From: Tyler Cloutier Date: Wed, 23 Oct 2024 11:17:41 -0600 Subject: [PATCH 06/11] variable rename --- crates/client-api/src/routes/database.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/client-api/src/routes/database.rs b/crates/client-api/src/routes/database.rs index 9de0ac068d5..e51c3d7aee3 100644 --- a/crates/client-api/src/routes/database.rs +++ b/crates/client-api/src/routes/database.rs @@ -694,8 +694,8 @@ pub async fn publish( } }, None => { - let auth = SpacetimeAuth::alloc(&ctx).await?; - let database_identity = auth.identity; + let database_auth = SpacetimeAuth::alloc(&ctx).await?; + let database_identity = database_auth.identity; (database_identity, None) } }; From 07d166ddbee8884ea0aa00f54fe142e2892e25fe Mon Sep 17 00:00:00 2001 From: Jeffrey Dallatezza Date: Thu, 24 Oct 2024 11:35:28 -0700 Subject: [PATCH 07/11] Add support for short-lived token --- crates/client-api/src/auth.rs | 41 ++++++-- crates/client-api/src/routes/identity.rs | 6 +- crates/core/src/auth/token_validation.rs | 113 ++++++++++++++--------- 3 files changed, 106 insertions(+), 54 deletions(-) diff --git a/crates/client-api/src/auth.rs b/crates/client-api/src/auth.rs index e4218dd042a..8cfd3ad68bc 100644 --- a/crates/client-api/src/auth.rs +++ b/crates/client-api/src/auth.rs @@ -98,43 +98,63 @@ impl SpacetimeCreds { pub struct SpacetimeAuth { pub creds: SpacetimeCreds, pub identity: Identity, + pub subject: String, + pub issuer: String, } use jsonwebtoken; -struct TokenClaims { +pub struct TokenClaims { pub issuer: String, pub subject: String, pub audience: Vec, } +impl From for TokenClaims { + fn from(claims: SpacetimeAuth) -> Self { + Self { + issuer: claims.issuer, + subject: claims.subject, + // This will need to be changed when we care about audiencies. + audience: Vec::new(), + } + } +} + impl TokenClaims { // Compute the id from the issuer and subject. - fn id(&self) -> Identity { + pub fn id(&self) -> Identity { Identity::from_claims(&self.issuer, &self.subject) } - fn encode_and_sign(&self, private_key: &EncodingKey) -> Result { + pub fn encode_and_sign_with_expiry(&self, private_key: &EncodingKey, expiry: Option) -> Result { + let iat = SystemTime::now(); + let exp = expiry.map(|dur| iat + dur); let claims = SpacetimeIdentityClaims2 { identity: self.id(), subject: self.subject.clone(), issuer: self.issuer.clone(), audience: self.audience.clone(), - iat: SystemTime::now(), - exp: None, + iat, + exp, }; let header = jsonwebtoken::Header::new(jsonwebtoken::Algorithm::ES256); jsonwebtoken::encode(&header, &claims, private_key) } + + pub fn encode_and_sign(&self, private_key: &EncodingKey) -> Result { + self.encode_and_sign_with_expiry(private_key, None) + } } impl SpacetimeAuth { /// Allocate a new identity, and mint a new token for it. pub async fn alloc(ctx: &(impl NodeDelegate + ControlStateDelegate + ?Sized)) -> axum::response::Result { // Generate claims with a random subject. + let subject = Uuid::new_v4().to_string(); let claims = TokenClaims { issuer: ctx.local_issuer(), - subject: Uuid::new_v4().to_string(), + subject: subject.clone(), // Placeholder audience. audience: vec!["spacetimedb".to_string()], }; @@ -145,15 +165,14 @@ impl SpacetimeAuth { SpacetimeCreds::from_signed_token(token) }; - Ok(Self { creds, identity }) + Ok(Self { creds, identity, subject, issuer: ctx.local_issuer() }) } /// Get the auth credentials as headers to be returned from an endpoint. pub fn into_headers(self) -> (TypedHeader, TypedHeader) { - let Self { creds, identity } = self; ( - TypedHeader(SpacetimeIdentity(identity)), - TypedHeader(SpacetimeIdentityToken(creds)), + TypedHeader(SpacetimeIdentity(self.identity)), + TypedHeader(SpacetimeIdentityToken(self.creds)), ) } } @@ -222,6 +241,8 @@ impl axum::extract::FromRequestParts for Space let auth = SpacetimeAuth { creds, identity: claims.identity, + subject: claims.subject, + issuer: claims.issuer, }; Ok(Self { auth: Some(auth) }) } diff --git a/crates/client-api/src/routes/identity.rs b/crates/client-api/src/routes/identity.rs index b30dd058449..717ebefaeec 100644 --- a/crates/client-api/src/routes/identity.rs +++ b/crates/client-api/src/routes/identity.rs @@ -10,7 +10,7 @@ use spacetimedb::auth::identity::encode_token_with_expiry; use spacetimedb_lib::de::serde::DeserializeWrapper; use spacetimedb_lib::Identity; -use crate::auth::{SpacetimeAuth, SpacetimeAuthRequired}; +use crate::auth::{SpacetimeAuth, SpacetimeAuthRequired, TokenClaims}; use crate::{log_and_500, ControlStateDelegate, NodeDelegate}; #[derive(Debug, Clone, Serialize, Deserialize)] @@ -101,7 +101,9 @@ pub async fn create_websocket_token( SpacetimeAuthRequired(auth): SpacetimeAuthRequired, ) -> axum::response::Result { let expiry = Duration::from_secs(60); - let token = encode_token_with_expiry(ctx.private_key(), auth.identity, Some(expiry)).map_err(log_and_500)?; + let claims: TokenClaims = TokenClaims::from(auth); + let token = claims.encode_and_sign_with_expiry(ctx.private_key(), Some(expiry)).map_err(log_and_500)?; + // let token = encode_token_with_expiry(ctx.private_key(), auth.identity, Some(expiry)).map_err(log_and_500)?; Ok(axum::Json(WebsocketTokenResponse { token })) } diff --git a/crates/core/src/auth/token_validation.rs b/crates/core/src/auth/token_validation.rs index c760f4f75d4..710f6a6611a 100644 --- a/crates/core/src/auth/token_validation.rs +++ b/crates/core/src/auth/token_validation.rs @@ -60,6 +60,7 @@ impl TokenValidator for UnimplementedTokenValidator { } } +/* pub struct FullTokenValidator { pub public_key: DecodingKey, pub caching_validator: CachingOidcTokenValidator, @@ -70,7 +71,7 @@ impl TokenValidator for FullTokenValidator { async fn validate_token(&self, token: &str) -> Result { let issuer = get_raw_issuer(token)?; if issuer == "localhost" { - let claims = LocalTokenValidator { + let claims = BasicTokenValidator { public_key: self.public_key.clone(), issuer, } @@ -81,23 +82,33 @@ impl TokenValidator for FullTokenValidator { self.caching_validator.validate_token(token).await } } + */ pub async fn validate_token( local_key: DecodingKey, local_issuer: &str, token: &str, ) -> Result { - let issuer = get_raw_issuer(token)?; - if issuer == local_issuer { - let claims = LocalTokenValidator { - public_key: local_key, - issuer, + let local_key_error = { + + let first_validator = BasicTokenValidator { + public_key: local_key.clone(), + issuer: None, + }; + match first_validator.validate_token(token).await { + Ok(claims) => return Ok(claims), + Err(e) => e, + } + }; + + // If that fails, we try the OIDC validator. + let issuer = get_raw_issuer(token)?; + // If we are the issuer, then we should have already validated the token. + // TODO: "localhost" should not be hard-coded. + if issuer == local_issuer { + return Err(local_key_error); } - .validate_token(token) - .await?; - return Ok(claims); - } - GLOBAL_OIDC_VALIDATOR.clone().validate_token(token).await + GLOBAL_OIDC_VALIDATOR.clone().validate_token(token).await } pub struct InitialTestingTokenValidator { @@ -107,15 +118,25 @@ pub struct InitialTestingTokenValidator { #[async_trait] impl TokenValidator for InitialTestingTokenValidator { async fn validate_token(&self, token: &str) -> Result { - let issuer = get_raw_issuer(token)?; - if issuer == "localhost" { - let claims = LocalTokenValidator { + // Initially, we check if we signed the key. + let local_key_error = { + + let first_validator = BasicTokenValidator { public_key: self.public_key.clone(), - issuer, + issuer: None, + }; + match first_validator.validate_token(token).await { + Ok(claims) => return Ok(claims), + Err(e) => e, } - .validate_token(token) - .await?; - return Ok(claims); + }; + + // If that fails, we try the OIDC validator. + let issuer = get_raw_issuer(token)?; + // If we are the issuer, then we should have already validated the token. + // TODO: "localhost" should not be hard-coded. + if issuer == "localhost" { + return Err(local_key_error); } let validator = OidcTokenValidator; validator.validate_token(token).await @@ -123,9 +144,11 @@ impl TokenValidator for InitialTestingTokenValidator { } // This verifies against a given public key and expected issuer. -struct LocalTokenValidator { +// The issuer should only be None if we are checking with a local key. +// We do that because we signed short-lived keys with different issuers. +struct BasicTokenValidator { pub public_key: DecodingKey, - pub issuer: String, + pub issuer: Option, } lazy_static! { @@ -139,7 +162,7 @@ lazy_static! { } #[async_trait] -impl TokenValidator for LocalTokenValidator { +impl TokenValidator for BasicTokenValidator { async fn validate_token(&self, token: &str) -> Result { // TODO: Make this stored in the struct so we don't need to keep creating it. let mut validation = Validation::new(jsonwebtoken::Algorithm::ES256); @@ -149,20 +172,26 @@ impl TokenValidator for LocalTokenValidator { jsonwebtoken::Algorithm::HS256, ]; validation.set_required_spec_claims(&REQUIRED_CLAIMS); - validation.set_issuer(&[self.issuer.clone()]); + + if let Some(expected_issuer) = &self.issuer { + validation.set_issuer(&[expected_issuer.clone()]); + } // TODO: We should require a specific audience at some point. validation.validate_aud = false; let data = decode::(token, &self.public_key, &validation)?; let claims = data.claims; - if claims.issuer != self.issuer { - return Err(TokenValidationError::Other(anyhow::anyhow!( - "Issuer mismatch: got {:?}, expected {:?}", - claims.issuer, - self.issuer - ))); - } + if let Some(expected_issuer) = &self.issuer { + if claims.issuer != *expected_issuer { + return Err(TokenValidationError::Other(anyhow::anyhow!( + "Issuer mismatch: got {:?}, expected {:?}", + claims.issuer, + expected_issuer + ))); + } + + } claims.try_into() } } @@ -268,9 +297,9 @@ impl TokenValidator for JwksValidator { .keys .get(&kid) .ok_or_else(|| TokenValidationError::KeyIDNotFound)?; - let validator = LocalTokenValidator { + let validator = BasicTokenValidator { public_key: key.decoding_key.clone(), - issuer: self.issuer.clone(), + issuer: Some(self.issuer.clone()), }; return validator.validate_token(token).await; } @@ -280,9 +309,9 @@ impl TokenValidator for JwksValidator { let mut last_error = TokenValidationError::Other(anyhow::anyhow!("No kid found")); for (kid, key) in &self.keyset.keys { log::debug!("Trying key {}", kid); - let validator = LocalTokenValidator { + let validator = BasicTokenValidator { public_key: key.decoding_key.clone(), - issuer: self.issuer.clone(), + issuer: Some(self.issuer.clone()), }; match validator.validate_token(token).await { Ok(claims) => return Ok(claims), @@ -305,7 +334,7 @@ mod tests { use crate::auth::identity::{IncomingClaims, SpacetimeIdentityClaims2}; use crate::auth::token_validation::{ - CachingOidcTokenValidator, LocalTokenValidator, OidcTokenValidator, TokenValidator, + CachingOidcTokenValidator, BasicTokenValidator, OidcTokenValidator, TokenValidator, }; use jsonwebkey as jwk; use jsonwebtoken::{DecodingKey, EncodingKey}; @@ -379,9 +408,9 @@ mod tests { { // Test that we can validate it. - let validator = LocalTokenValidator { + let validator = BasicTokenValidator { public_key: kp.public_key.clone(), - issuer: issuer.to_string(), + issuer: Some(issuer.to_string()), }; let parsed_claims: SpacetimeIdentityClaims2 = validator.validate_token(&token).await?; @@ -391,9 +420,9 @@ mod tests { } { // Now try with the wrong expected issuer. - let validator = LocalTokenValidator { + let validator = BasicTokenValidator { public_key: kp.public_key.clone(), - issuer: "otherissuer".to_string(), + issuer: Some("otherissuer".to_string()), }; assert!(validator.validate_token(&token).await.is_err()); @@ -422,9 +451,9 @@ mod tests { { // Test that we can validate it. - let validator = LocalTokenValidator { + let validator = BasicTokenValidator { public_key: kp.public_key.clone(), - issuer: issuer.to_string(), + issuer: Some(issuer.to_string()), }; let parsed_claims: SpacetimeIdentityClaims2 = validator.validate_token(&token).await?; @@ -436,9 +465,9 @@ mod tests { // We generate a new keypair and try to decode with that key. let other_kp = KeyPair::generate_p256()?; // Now try with the wrong expected issuer. - let validator = LocalTokenValidator { + let validator = BasicTokenValidator { public_key: other_kp.public_key.clone(), - issuer: "otherissuer".to_string(), + issuer: Some("otherissuer".to_string()), }; assert!(validator.validate_token(&token).await.is_err()); From eed796f3c5b41e4e166323d3685a493a10488e03 Mon Sep 17 00:00:00 2001 From: Jeffrey Dallatezza Date: Thu, 24 Oct 2024 12:30:26 -0700 Subject: [PATCH 08/11] Add testing of resigned token and FullTokenValidator --- crates/client-api/src/auth.rs | 21 ++- crates/client-api/src/routes/identity.rs | 8 +- crates/core/src/auth/token_validation.rs | 155 ++++++++++++++--------- 3 files changed, 114 insertions(+), 70 deletions(-) diff --git a/crates/client-api/src/auth.rs b/crates/client-api/src/auth.rs index 8cfd3ad68bc..952aea20b3f 100644 --- a/crates/client-api/src/auth.rs +++ b/crates/client-api/src/auth.rs @@ -104,7 +104,7 @@ pub struct SpacetimeAuth { use jsonwebtoken; -pub struct TokenClaims { +struct TokenClaims { pub issuer: String, pub subject: String, pub audience: Vec, @@ -123,11 +123,15 @@ impl From for TokenClaims { impl TokenClaims { // Compute the id from the issuer and subject. - pub fn id(&self) -> Identity { + fn id(&self) -> Identity { Identity::from_claims(&self.issuer, &self.subject) } - pub fn encode_and_sign_with_expiry(&self, private_key: &EncodingKey, expiry: Option) -> Result { + fn encode_and_sign_with_expiry( + &self, + private_key: &EncodingKey, + expiry: Option, + ) -> Result { let iat = SystemTime::now(); let exp = expiry.map(|dur| iat + dur); let claims = SpacetimeIdentityClaims2 { @@ -165,7 +169,12 @@ impl SpacetimeAuth { SpacetimeCreds::from_signed_token(token) }; - Ok(Self { creds, identity, subject, issuer: ctx.local_issuer() }) + Ok(Self { + creds, + identity, + subject, + issuer: ctx.local_issuer(), + }) } /// Get the auth credentials as headers to be returned from an endpoint. @@ -175,6 +184,10 @@ impl SpacetimeAuth { TypedHeader(SpacetimeIdentityToken(self.creds)), ) } + + pub fn resign_with_expiry(&self, private_key: &EncodingKey, expiry: Duration) -> Result { + TokenClaims::from(self.clone()).encode_and_sign_with_expiry(private_key, Some(expiry)) + } } #[cfg(test)] diff --git a/crates/client-api/src/routes/identity.rs b/crates/client-api/src/routes/identity.rs index 717ebefaeec..48c375fe632 100644 --- a/crates/client-api/src/routes/identity.rs +++ b/crates/client-api/src/routes/identity.rs @@ -6,11 +6,10 @@ use http::header::CONTENT_TYPE; use http::StatusCode; use serde::{Deserialize, Serialize}; -use spacetimedb::auth::identity::encode_token_with_expiry; use spacetimedb_lib::de::serde::DeserializeWrapper; use spacetimedb_lib::Identity; -use crate::auth::{SpacetimeAuth, SpacetimeAuthRequired, TokenClaims}; +use crate::auth::{SpacetimeAuth, SpacetimeAuthRequired}; use crate::{log_and_500, ControlStateDelegate, NodeDelegate}; #[derive(Debug, Clone, Serialize, Deserialize)] @@ -101,8 +100,9 @@ pub async fn create_websocket_token( SpacetimeAuthRequired(auth): SpacetimeAuthRequired, ) -> axum::response::Result { let expiry = Duration::from_secs(60); - let claims: TokenClaims = TokenClaims::from(auth); - let token = claims.encode_and_sign_with_expiry(ctx.private_key(), Some(expiry)).map_err(log_and_500)?; + let token = auth + .resign_with_expiry(ctx.private_key(), expiry) + .map_err(log_and_500)?; // let token = encode_token_with_expiry(ctx.private_key(), auth.identity, Some(expiry)).map_err(log_and_500)?; Ok(axum::Json(WebsocketTokenResponse { token })) } diff --git a/crates/core/src/auth/token_validation.rs b/crates/core/src/auth/token_validation.rs index 710f6a6611a..aee937c4e36 100644 --- a/crates/core/src/auth/token_validation.rs +++ b/crates/core/src/auth/token_validation.rs @@ -60,39 +60,22 @@ impl TokenValidator for UnimplementedTokenValidator { } } -/* -pub struct FullTokenValidator { - pub public_key: DecodingKey, - pub caching_validator: CachingOidcTokenValidator, +pub struct FullTokenValidator { + pub local_key: DecodingKey, + pub local_issuer: String, + pub oidc_validator: T, + // pub caching_validator: CachingOidcTokenValidator, } #[async_trait] -impl TokenValidator for FullTokenValidator { +impl TokenValidator for FullTokenValidator +where + T: TokenValidator + Send + Sync, +{ async fn validate_token(&self, token: &str) -> Result { - let issuer = get_raw_issuer(token)?; - if issuer == "localhost" { - let claims = BasicTokenValidator { - public_key: self.public_key.clone(), - issuer, - } - .validate_token(token) - .await?; - return Ok(claims); - } - self.caching_validator.validate_token(token).await - } -} - */ - -pub async fn validate_token( - local_key: DecodingKey, - local_issuer: &str, - token: &str, -) -> Result { let local_key_error = { - let first_validator = BasicTokenValidator { - public_key: local_key.clone(), + public_key: self.local_key.clone(), issuer: None, }; match first_validator.validate_token(token).await { @@ -105,42 +88,25 @@ pub async fn validate_token( let issuer = get_raw_issuer(token)?; // If we are the issuer, then we should have already validated the token. // TODO: "localhost" should not be hard-coded. - if issuer == local_issuer { + if issuer == self.local_issuer { return Err(local_key_error); } - GLOBAL_OIDC_VALIDATOR.clone().validate_token(token).await -} - -pub struct InitialTestingTokenValidator { - pub public_key: DecodingKey, + self.oidc_validator.validate_token(token).await + } } -#[async_trait] -impl TokenValidator for InitialTestingTokenValidator { - async fn validate_token(&self, token: &str) -> Result { - // Initially, we check if we signed the key. - let local_key_error = { - - let first_validator = BasicTokenValidator { - public_key: self.public_key.clone(), - issuer: None, - }; - match first_validator.validate_token(token).await { - Ok(claims) => return Ok(claims), - Err(e) => e, - } - }; - - // If that fails, we try the OIDC validator. - let issuer = get_raw_issuer(token)?; - // If we are the issuer, then we should have already validated the token. - // TODO: "localhost" should not be hard-coded. - if issuer == "localhost" { - return Err(local_key_error); - } - let validator = OidcTokenValidator; - validator.validate_token(token).await - } +// This is a helper function that uses a global JWK cache. We should remove this eventually, and make the server hold on to its own. +pub async fn validate_token( + local_key: DecodingKey, + local_issuer: &str, + token: &str, +) -> Result { + let validator = FullTokenValidator { + local_key, + local_issuer: local_issuer.to_string(), + oidc_validator: GLOBAL_OIDC_VALIDATOR.clone(), + }; + validator.validate_token(token).await } // This verifies against a given public key and expected issuer. @@ -190,8 +156,7 @@ impl TokenValidator for BasicTokenValidator { expected_issuer ))); } - - } + } claims.try_into() } } @@ -334,7 +299,7 @@ mod tests { use crate::auth::identity::{IncomingClaims, SpacetimeIdentityClaims2}; use crate::auth::token_validation::{ - CachingOidcTokenValidator, BasicTokenValidator, OidcTokenValidator, TokenValidator, + BasicTokenValidator, CachingOidcTokenValidator, FullTokenValidator, OidcTokenValidator, TokenValidator, }; use jsonwebkey as jwk; use jsonwebtoken::{DecodingKey, EncodingKey}; @@ -476,6 +441,61 @@ mod tests { Ok(()) } + async fn assert_validation_fails(validator: &T, token: &str) -> anyhow::Result<()> { + let result = validator.validate_token(token).await; + if result.is_ok() { + let claims = result.unwrap(); + anyhow::bail!("Validation succeeded when it should have failed: {:?}", claims); + } + Ok(()) + } + + #[tokio::test] + async fn resigned_token_ignores_issuer() -> anyhow::Result<()> { + // Test that the decoding key must work for LocalTokenValidator. + let kp = KeyPair::generate_p256()?; + let local_issuer = "test1"; + let external_issuer = "other_issuer"; + let subject = "test_subject"; + + let orig_claims = IncomingClaims { + identity: None, + subject: subject.to_string(), + issuer: external_issuer.to_string(), + audience: vec![], + iat: std::time::SystemTime::now(), + exp: None, + }; + let header = jsonwebtoken::Header::new(jsonwebtoken::Algorithm::ES256); + let token = jsonwebtoken::encode(&header, &orig_claims, &kp.private_key)?; + + // First, try the successful case with the FullTokenValidator. + { + let validator = FullTokenValidator { + local_key: kp.public_key.clone(), + local_issuer: local_issuer.to_string(), + oidc_validator: OidcTokenValidator, + }; + + let parsed_claims: SpacetimeIdentityClaims2 = validator.validate_token(&token).await?; + assert_eq!(parsed_claims.issuer, external_issuer); + assert_eq!(parsed_claims.subject, subject); + assert_eq!(parsed_claims.identity, Identity::from_claims(external_issuer, subject)); + } + // Double check that this token would fail with an OidcTokenValidator. + assert_validation_fails(&OidcTokenValidator, &token).await?; + // Double check that validation fails if we check the issuer. + assert_validation_fails( + &BasicTokenValidator { + public_key: kp.public_key.clone(), + issuer: Some(local_issuer.to_string()), + }, + &token, + ) + .await?; + Ok(()) + } + use axum::routing::get; use axum::Json; use axum::Router; @@ -601,4 +621,15 @@ mod tests { let v = CachingOidcTokenValidator::get_default(); run_oidc_test(v).await } + + #[tokio::test] + async fn test_full_validator_fallback() -> anyhow::Result<()> { + let kp = KeyPair::generate_p256()?; + let v = FullTokenValidator { + local_key: kp.public_key.clone(), + local_issuer: "local_issuer".to_string(), + oidc_validator: OidcTokenValidator, + }; + run_oidc_test(v).await + } } From 8d15a06f6b0792a6297872c3fc87ff7d9d96650e Mon Sep 17 00:00:00 2001 From: Jeffrey Dallatezza Date: Thu, 24 Oct 2024 12:34:02 -0700 Subject: [PATCH 09/11] change resign to re_sign --- crates/client-api/src/auth.rs | 5 ++++- crates/client-api/src/routes/identity.rs | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/client-api/src/auth.rs b/crates/client-api/src/auth.rs index 952aea20b3f..ac80fc6515f 100644 --- a/crates/client-api/src/auth.rs +++ b/crates/client-api/src/auth.rs @@ -185,7 +185,10 @@ impl SpacetimeAuth { ) } - pub fn resign_with_expiry(&self, private_key: &EncodingKey, expiry: Duration) -> Result { + // Sign a new token with the same claims and a new expiry. + // Note that this will not change the issuer, so the private_key might not match. + // We do this to create short-lived tokens that we will be able to verify. + pub fn re_sign_with_expiry(&self, private_key: &EncodingKey, expiry: Duration) -> Result { TokenClaims::from(self.clone()).encode_and_sign_with_expiry(private_key, Some(expiry)) } } diff --git a/crates/client-api/src/routes/identity.rs b/crates/client-api/src/routes/identity.rs index 48c375fe632..e8c1bf03f23 100644 --- a/crates/client-api/src/routes/identity.rs +++ b/crates/client-api/src/routes/identity.rs @@ -101,7 +101,7 @@ pub async fn create_websocket_token( ) -> axum::response::Result { let expiry = Duration::from_secs(60); let token = auth - .resign_with_expiry(ctx.private_key(), expiry) + .re_sign_with_expiry(ctx.private_key(), expiry) .map_err(log_and_500)?; // let token = encode_token_with_expiry(ctx.private_key(), auth.identity, Some(expiry)).map_err(log_and_500)?; Ok(axum::Json(WebsocketTokenResponse { token })) From 26112f7622b06fb45f35a50e51cab88bc632e0d1 Mon Sep 17 00:00:00 2001 From: Jeffrey Dallatezza Date: Thu, 24 Oct 2024 14:31:34 -0700 Subject: [PATCH 10/11] Make some more things public for use in private --- crates/client-api/src/auth.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/crates/client-api/src/auth.rs b/crates/client-api/src/auth.rs index ac80fc6515f..5b03075341b 100644 --- a/crates/client-api/src/auth.rs +++ b/crates/client-api/src/auth.rs @@ -61,7 +61,7 @@ impl SpacetimeCreds { pub fn decode_token(&self, public_key: &DecodingKey) -> Result { decode_token(public_key, self.token()).map(|x| x.claims) } - fn from_signed_token(token: String) -> Self { + pub fn from_signed_token(token: String) -> Self { Self { token } } /// Mint a new credentials JWT for an identity. @@ -104,7 +104,7 @@ pub struct SpacetimeAuth { use jsonwebtoken; -struct TokenClaims { +pub struct TokenClaims { pub issuer: String, pub subject: String, pub audience: Vec, @@ -122,12 +122,20 @@ impl From for TokenClaims { } impl TokenClaims { + pub fn new(issuer: String, subject: String) -> Self { + Self { + issuer, + subject, + audience: Vec::new(), + } + } + // Compute the id from the issuer and subject. - fn id(&self) -> Identity { + pub fn id(&self) -> Identity { Identity::from_claims(&self.issuer, &self.subject) } - fn encode_and_sign_with_expiry( + pub fn encode_and_sign_with_expiry( &self, private_key: &EncodingKey, expiry: Option, From 9c58218deb4b3fd72ab1e135a0d21b44826d829e Mon Sep 17 00:00:00 2001 From: Jeffrey Dallatezza Date: Fri, 25 Oct 2024 12:41:18 -0700 Subject: [PATCH 11/11] Add some comments --- crates/client-api/src/routes/identity.rs | 3 +++ crates/core/src/auth/token_validation.rs | 13 ++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/crates/client-api/src/routes/identity.rs b/crates/client-api/src/routes/identity.rs index e8c1bf03f23..8ab1e8c4a71 100644 --- a/crates/client-api/src/routes/identity.rs +++ b/crates/client-api/src/routes/identity.rs @@ -95,6 +95,9 @@ pub struct WebsocketTokenResponse { pub token: String, } +// This endpoint takes a token from a client and sends a newly signed token with a 60s expiry. +// Note that even if the token has a different issuer, we will sign it with our key. +// This is ok because `FullTokenValidator` checks if we signed the token before worrying about the issuer. pub async fn create_websocket_token( State(ctx): State, SpacetimeAuthRequired(auth): SpacetimeAuthRequired, diff --git a/crates/core/src/auth/token_validation.rs b/crates/core/src/auth/token_validation.rs index aee937c4e36..f788e80cabf 100644 --- a/crates/core/src/auth/token_validation.rs +++ b/crates/core/src/auth/token_validation.rs @@ -60,11 +60,13 @@ impl TokenValidator for UnimplementedTokenValidator { } } +// This validator accepts any tokens signed with the local key (regardless of issuer). +// If it is not signed with the local key, we will try to validate it with the OIDC validator. +// We do this because we sign short lived tokens with different issuers. pub struct FullTokenValidator { pub local_key: DecodingKey, pub local_issuer: String, pub oidc_validator: T, - // pub caching_validator: CachingOidcTokenValidator, } #[async_trait] @@ -191,8 +193,13 @@ impl async_cache::Fetcher> for KeyFetcher { log::info!("Fetching key for issuer {}", raw_issuer.clone()); // TODO: Consider checking for trailing slashes or requiring a scheme. let oidc_url = format!("{}/.well-known/openid-configuration", raw_issuer); - // TODO: log errors here. - let keys = Jwks::from_oidc_url(oidc_url).await?; + let key_or_error = Jwks::from_oidc_url(oidc_url).await; + // TODO: We should probably add debouncing to avoid spamming the logs. + // Alternatively we could add a backoff before retrying. + if let Err(e) = &key_or_error { + log::warn!("Error fetching public key for issuer {}: {:?}", raw_issuer, e); + } + let keys = key_or_error?; let validator = JwksValidator { issuer: raw_issuer.clone(), keyset: keys,