feat(auth): Add version info for analytics (#758)

* add version to challenge response - to support analytics
getsentry · Sep 14, 2020 · 4cdbf34 · 4cdbf34
1 parent edfc6bd
commit 4cdbf34
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -28,6 +28,7 @@
 - Emit `processing.event.produced` for user report and session Kafka messages. ([#757](https://github.com/getsentry/relay/pull/757))
 - Improve performance of event processing by avoiding regex clone. ([#767](https://github.com/getsentry/relay/pull/767))
 - Assign a default name for unnamed attachments, which prevented attachments from being stored in Sentry. ([#769](https://github.com/getsentry/relay/pull/769))
+- Add Relay version version to challenge response. ([#758](https://github.com/getsentry/relay/pull/758))
 
 ## 20.8.0
 

diff --git a/py/CHANGELOG.md b/py/CHANGELOG.md
@@ -5,6 +5,7 @@
 - In PII configs, all options on hash and mask redactions (replacement characters, ignored characters, hash algorithm/key) are removed. If they still exist in the configuration, they are ignored. ([#760](https://github.com/getsentry/relay/pull/760))
 - Rename to the library target to `relay_cabi` and add documentation. ([#763](https://github.com/getsentry/relay/pull/763))
 - Update FFI bindings with a new implementation for error handling. ([#766](https://github.com/getsentry/relay/pull/766))
+- Add Relay version version to challenge response. ([#758](https://github.com/getsentry/relay/pull/758))
 
 ## 0.6.1
 

diff --git a/py/sentry_relay/auth.py b/py/sentry_relay/auth.py
@@ -120,6 +120,7 @@ def validate_register_response(data, signature, secret, max_age=60):
         "relay_id": uuid.UUID(response["relay_id"]),
         "token": response["token"],
         "public_key": response["public_key"],
+        "version": response["version"],
     }
 
 

diff --git a/py/tests/test_auth.py b/py/tests/test_auth.py
@@ -5,14 +5,16 @@
 
 UPSTREAM_SECRET = "secret"
 
-RELAY_ID = b"6b7d15b8-cee2-4354-9fee-dae7ef43e434"
-RELAY_KEY = b"kMpGbydHZSvohzeMlghcWwHd8MkreKGzl_ncdkZSOMg"
-REQUEST = b'{"relay_id":"6b7d15b8-cee2-4354-9fee-dae7ef43e434","public_key":"kMpGbydHZSvohzeMlghcWwHd8MkreKGzl_ncdkZSOMg","version":"20.8.0"}'
-REQUEST_SIG = "JIwzIb3kuOaVwgq_DRuPpquGVIIu0plfpOSvz_ixzfw_RmdyHr35cJrna7Jg_uXqNHQbSP1Yj0-4X5Omk9jcBA.eyJ0IjoiMjAyMC0wOS0wMVQxMzozNzoxNC40Nzk0NjVaIn0"
-TOKEN = "eyJ0aW1lc3RhbXAiOjE1OTg5Njc0MzQsInJlbGF5X2lkIjoiNmI3ZDE1YjgtY2VlMi00MzU0LTlmZWUtZGFlN2VmNDNlNDM0IiwicHVibGljX2tleSI6ImtNcEdieWRIWlN2b2h6ZU1sZ2hjV3dIZDhNa3JlS0d6bF9uY2RrWlNPTWciLCJyYW5kIjoiLUViNG9Hal80dUZYOUNRRzFBVmdqTjRmdGxaNU9DSFlNOFl2d1podmlyVXhUY0tFSWYtQzhHaldsZmgwQTNlMzYxWE01dVh0RHhvN00tbWhZeXpWUWcifQ:KJUDXlwvibKNQmex-_Cu1U0FArlmoDkyqP7bYIDGrLXudfjGfCjH-UjNsUHWVDnbM28YdQ-R2MBSyF51aRLQcw"
-RESPONSE = b'{"relay_id":"6b7d15b8-cee2-4354-9fee-dae7ef43e434","token":"eyJ0aW1lc3RhbXAiOjE1OTg5Njc0MzQsInJlbGF5X2lkIjoiNmI3ZDE1YjgtY2VlMi00MzU0LTlmZWUtZGFlN2VmNDNlNDM0IiwicHVibGljX2tleSI6ImtNcEdieWRIWlN2b2h6ZU1sZ2hjV3dIZDhNa3JlS0d6bF9uY2RrWlNPTWciLCJyYW5kIjoiLUViNG9Hal80dUZYOUNRRzFBVmdqTjRmdGxaNU9DSFlNOFl2d1podmlyVXhUY0tFSWYtQzhHaldsZmgwQTNlMzYxWE01dVh0RHhvN00tbWhZeXpWUWcifQ:KJUDXlwvibKNQmex-_Cu1U0FArlmoDkyqP7bYIDGrLXudfjGfCjH-UjNsUHWVDnbM28YdQ-R2MBSyF51aRLQcw"}'
-RESPONSE_SIG = "HUp3eybT_5AmRJ_QzutfvStKTeE-cgD_reLPjIf4OpoOJT_Hln8ThrFqGyT_C6P8qF1LHbFLcrYFvQy4iNaqAQ.eyJ0IjoiMjAyMC0wOS0wMVQxMzozNzoxNC40ODEwNTNaIn0"
-
+# NOTE in order to regenerate the test data (in case of changes) run the rust test:
+# test_generate_strings_for_test_auth_py and copy its output below
+RELAY_ID = b"29308cac-9783-40e9-98ac-b5503dffe3a4"
+RELAY_KEY = b"dXq9IiKDLgma0J8dLVITOdkpaU8mPZPJj18t4HCKTfs"
+REQUEST = b'{"relay_id":"29308cac-9783-40e9-98ac-b5503dffe3a4","public_key":"dXq9IiKDLgma0J8dLVITOdkpaU8mPZPJj18t4HCKTfs","version":"20.8.0"}'
+REQUEST_SIG = "VgFn-7B5JmbSPiM5bikxkn7DjImV8LkfW3UVQcXnK8nLumvLaS7ML0KTY7a7LlU_3grGtSNZlEUbBudOp__RDA.eyJ0IjoiMjAyMC0wOS0wOFQxMzozMzozNS45OTM5MDRaIn0"
+TOKEN = "eyJ0aW1lc3RhbXAiOjE1OTk1NzIwMTUsInJlbGF5X2lkIjoiMjkzMDhjYWMtOTc4My00MGU5LTk4YWMtYjU1MDNkZmZlM2E0IiwicHVibGljX2tleSI6ImRYcTlJaUtETGdtYTBKOGRMVklUT2RrcGFVOG1QWlBKajE4dDRIQ0tUZnMiLCJyYW5kIjoiQURNNG9yLVZNZ0Y1eTRLQUo2cHkyQnB5T3lmUmV1NGRjZTJCdmd5UHlSdnczRXFaUmc4SkE0NHdxVWdBVlBQMGhIeHR0am81YTdBYW93UEFVaUR2NUEifQ:XXYCNWscmAiBWzI84ToZWAGgmIrupWQufYcSBhIEcxiDxyBp_BRO0d_LN9wnc0tjtFcT9JViLoGCgfOt6vDS7A"
+RESPONSE = b'{"relay_id":"29308cac-9783-40e9-98ac-b5503dffe3a4","token":"eyJ0aW1lc3RhbXAiOjE1OTk1NzIwMTUsInJlbGF5X2lkIjoiMjkzMDhjYWMtOTc4My00MGU5LTk4YWMtYjU1MDNkZmZlM2E0IiwicHVibGljX2tleSI6ImRYcTlJaUtETGdtYTBKOGRMVklUT2RrcGFVOG1QWlBKajE4dDRIQ0tUZnMiLCJyYW5kIjoiQURNNG9yLVZNZ0Y1eTRLQUo2cHkyQnB5T3lmUmV1NGRjZTJCdmd5UHlSdnczRXFaUmc4SkE0NHdxVWdBVlBQMGhIeHR0am81YTdBYW93UEFVaUR2NUEifQ:XXYCNWscmAiBWzI84ToZWAGgmIrupWQufYcSBhIEcxiDxyBp_BRO0d_LN9wnc0tjtFcT9JViLoGCgfOt6vDS7A","version":"20.8.0"}'
+RESPONSE_SIG = "iPFV5KcSXDrhjY_99X8r_pMB1NQdw-YWF7hjvdrYpXmsaSier-mp1-3viWsEPIcTNbA76B4t51sjbSYFZPzXBg.eyJ0IjoiMjAyMC0wOS0wOFQxMzozMzozNS45OTU2ODJaIn0"
+RELAY_VERSION = "20.8.0"
 
 def test_basic_key_functions():
     sk, pk = sentry_relay.generate_key_pair()
@@ -56,6 +58,7 @@ def test_register_response():
     )
     assert resp["token"] == TOKEN
     assert resp["relay_id"] == uuid.UUID(RELAY_ID.decode("utf8"))
+    assert resp["version"] == RELAY_VERSION
 
 
 def test_is_version_supported():

diff --git a/relay-auth/src/lib.rs b/relay-auth/src/lib.rs
@@ -192,7 +192,7 @@ impl Clone for SecretKey {
     }
 }
 
-/// Reprensents the final registration.
+/// Represents the final registration.
 #[derive(Serialize, Deserialize, Debug)]
 pub struct Registration {
     relay_id: RelayId,
@@ -642,6 +642,7 @@ impl RegisterChallenge {
         RegisterResponse {
             relay_id: self.relay_id,
             token: self.token,
+            version: RelayVersion::current(),
         }
     }
 }
@@ -654,6 +655,8 @@ impl RegisterChallenge {
 pub struct RegisterResponse {
     relay_id: RelayId,
     token: SignedRegisterState,
+    #[serde(default)]
+    version: RelayVersion,
 }
 
 impl RegisterResponse {
@@ -687,6 +690,10 @@ impl RegisterResponse {
     pub fn token(&self) -> &str {
         self.token.as_str()
     }
+
+    pub fn version(&self) -> RelayVersion {
+        self.version
+    }
 }
 
 #[test]
@@ -786,6 +793,12 @@ fn test_registration() {
     assert_eq!(challenge.relay_id(), &relay_id);
     assert!(challenge.token().len() > 40);
 
+    // check the challenge contains the expected info
+    let state = SignedRegisterState(challenge_token.clone());
+    let register_state = state.unpack(upstream_secret, None).unwrap();
+    assert_eq!(register_state.public_key, pk);
+    assert_eq!(register_state.relay_id, relay_id);
+
     // create a response from the challenge
     let response = challenge.into_response();
 
@@ -801,6 +814,62 @@ fn test_registration() {
 
     assert_eq!(response.relay_id(), relay_id);
     assert_eq!(response.token(), challenge_token);
+    assert_eq!(response.version, LATEST_VERSION);
+}
+/// This is a pseudo-test to easily generate the strings used by test_auth.py
+/// You can copy the output to the top of the test_auth.py when there are changes in the
+/// exchanged authentication structures.
+/// It follows test_registration but instead of asserting it prints the strings  
+#[test]
+fn test_generate_strings_for_test_auth_py() {
+    let max_age = Duration::minutes(15);
+    println!("Generating test data for test_auth.py...");
+
+    // initial setup
+    let relay_id = generate_relay_id();
+    println!("RELAY_ID = b\"{}\"", relay_id);
+    let (sk, pk) = generate_key_pair();
+    println!("RELAY_KEY = b\"{}\"", pk);
+
+    // create a register request
+    let request = RegisterRequest::new(&relay_id, &pk);
+    println!("REQUEST = b'{}'", serde_json::to_string(&request).unwrap());
+
+    // sign it
+    let (request_bytes, request_sig) = sk.pack(&request);
+    println!("REQUEST_SIG = \"{}\"", request_sig);
+
+    // attempt to get the data through bootstrap unpacking.
+    let request =
+        RegisterRequest::bootstrap_unpack(&request_bytes, &request_sig, Some(max_age)).unwrap();
+
+    let upstream_secret = b"secret";
+
+    // create a challenge
+    let challenge = request.into_challenge(upstream_secret);
+    let challenge_token = challenge.token().to_owned();
+    println!("TOKEN = \"{}\"", challenge_token);
+
+    // create a response from the challenge
+    let response = challenge.into_response();
+    let serialized_response = serde_json::to_string(&response).unwrap();
+    let (_, response_sig) = sk.pack(&response);
+
+    println!("RESPONSE = b'{}'", serialized_response);
+    println!("RESPONSE_SIG = \"{}\"", response_sig);
+
+    println!("RELAY_VERSION = \"{}\"", &LATEST_VERSION);
+}
+
+/// Test we can still deserialize an old response that does not contain the version
+#[test]
+fn test_deserialize_old_response() {
+    let serialized_challenge = "{\"relay_id\":\"6b7d15b8-cee2-4354-9fee-dae7ef43e434\",\"token\":\"eyJ0aW1lc3RhbXAiOjE1OTg5Njc0MzQsInJlbGF5X2lkIjoiNmI3ZDE1YjgtY2VlMi00MzU0LTlmZWUtZGFlN2VmNDNlNDM0IiwicHVibGljX2tleSI6ImtNcEdieWRIWlN2b2h6ZU1sZ2hjV3dIZDhNa3JlS0d6bF9uY2RrWlNPTWciLCJyYW5kIjoiLUViNG9Hal80dUZYOUNRRzFBVmdqTjRmdGxaNU9DSFlNOFl2d1podmlyVXhUY0tFSWYtQzhHaldsZmgwQTNlMzYxWE01dVh0RHhvN00tbWhZeXpWUWcifQ:KJUDXlwvibKNQmex-_Cu1U0FArlmoDkyqP7bYIDGrLXudfjGfCjH-UjNsUHWVDnbM28YdQ-R2MBSyF51aRLQcw\"}";
+    let result: RegisterResponse = serde_json::from_str(serialized_challenge).unwrap();
+    assert_eq!(
+        result.relay_id,
+        Uuid::parse_str("6b7d15b8-cee2-4354-9fee-dae7ef43e434").unwrap()
+    )
 }
 
 #[test]

diff --git a/relay-cabi/src/auth.rs b/relay-cabi/src/auth.rs
@@ -160,6 +160,7 @@ struct RelayRegisterResponse<'a> {
     pub relay_id: RelayId,
     pub token: &'a str,
     pub public_key: &'a PublicKey,
+    pub version: RelayVersion,
 }
 
 /// Validates a register response.
@@ -187,6 +188,7 @@ pub unsafe extern "C" fn relay_validate_register_response(
         relay_id: response.relay_id(),
         token: response.token(),
         public_key: state.public_key(),
+        version: response.version(),
     };
 
     let json = serde_json::to_string(&relay_response)?;