Jwt auth#90
Conversation
6441e5d to
ccad7ff
Compare
| #[derive(Debug)] | ||
| pub enum JwtAuthToken { | ||
| /// Fully-formatted `"Bearer <value>"` ready to insert into gRPC metadata. | ||
| Static(tonic::metadata::MetadataValue<tonic::metadata::Ascii>), |
There was a problem hiding this comment.
Should we use SensitiveString?
| /// Site ID sent in the `x-site-id` metadata header on every request. | ||
| /// | ||
| /// Supports Vector's `${ENV_VAR}` interpolation, e.g. `site_id = "${SITE_ID}"`. | ||
| pub site_id: String, |
There was a problem hiding this comment.
This doesn't seem necessary (because JWT would already have the site-id). May be we should remove this?
There was a problem hiding this comment.
The disadvantage of having it is that we'll have this failure-mode where one sets incorrect site-id (one that doesn't agree with token).
| #[configurable_component] | ||
| #[derive(Clone, Debug)] | ||
| #[serde(rename_all = "snake_case", tag = "type")] | ||
| pub enum JwtTokenConfig { |
There was a problem hiding this comment.
s/JwtTokenConfig/AuthToken/
| /// | ||
| /// When configured, each request carries a bearer token and site ID in its gRPC metadata. | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub auth: Option<VectorSinkAuthConfig>, |
There was a problem hiding this comment.
If we drop site_id above, we can then simplify this to Option<AuthToken> (proposed name)?
| NoHost, | ||
|
|
||
| #[snafu(display("JWT token unavailable: {}", message))] | ||
| JwtTokenUnavailable { message: String }, |
| if let Some(auth) = &self.auth { | ||
| let metadata = request.metadata(); | ||
| let authorization = metadata.get("authorization").and_then(|v| v.to_str().ok()); | ||
| let site_id = metadata.get("x-site-id").and_then(|v| v.to_str().ok()); |
There was a problem hiding this comment.
This is unnecessary, let us kill this.
| let metadata = request.metadata(); | ||
| let authorization = metadata.get("authorization").and_then(|v| v.to_str().ok()); | ||
| let site_id = metadata.get("x-site-id").and_then(|v| v.to_str().ok()); | ||
| auth.validate(authorization, site_id).map_err(|e| match e { |
There was a problem hiding this comment.
let us not check here, let us build Validator (suggested in a comment above) and hold it in Service and check immediately after the map on line 75 below.
| /// When omitted, all incoming requests are accepted without authentication. | ||
| /// See [`JwtAuthConfig`] for full documentation. | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub auth: Option<JwtAuthConfig>, |
There was a problem hiding this comment.
Create a wrapper type that also includes member_key suggested above.
| const TEST_PRIVATE_KEY: &str = "-----BEGIN PRIVATE KEY----- | ||
| MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDJ5D7lpMrGJpl7 | ||
| zCcZ73XqbzBaagaPa9QDoGmypTbOoiysnnmcTHfy+wcP2aBlDTC8aB+7iPdZr0tA | ||
| ENdzIQ0/kZFBWCdwqAtQYDyfGuZx9y+3E9I8RFleDqDSwA6aUrSoesC9OBHztebX | ||
| 0m4T9dAWzn8Vr3CYKVpp4XcYwfX6iWszCm43zv4fCJu/qYX67IvOP8h66OMBZ8s7 | ||
| A4K15z1n8ScI3R6v6amc94iB7z2B9hdvuoTKk89dF5XGxE1ZVnIzSPr/8/oQQJgG | ||
| RaYqQAViy4kPmctW4uaI9ajQPIQe58LpNh1lDw+aLRHO/e0SCqbUNARTLSdSIwNV | ||
| 3dltWgS9AgMBAAECggEAHPo4NuDYw+kdZYHvaM8QdyYfZBLMv0AkTaL0GNKS08S+ | ||
| McaLQO5O1x7FrDY5yddDU/+D8nhdvE8nN1pTejBXxPSBS0Y6XvaXrSErAlErm1b1 | ||
| z8q2BbVvuErUNXugfPD7AiWgTWhjVz4YFIkdCJtjEyrvXa7xM73XvtPAMtsAEcXv | ||
| MgeRaZVdIledQUozu72RfPuG0yYWG5j+1W1IjNDcuLvld+RrZZ6JqyedhHMwlsFU | ||
| bi1DDGaBvp7jkDr6hDp81dqUVposvq+yw3THoyDnQCNxrSCfDpRkYk7DWJKVD8XS | ||
| 6GvFHuHfaktzm+KkUHBQAebGn6qM+3QBIOWXZkHBdwKBgQDwhVtLUNnz7LLOlAxH | ||
| /IF5WM96DoPilOG548yMt/81Zez9QzgJXhxefhCpl2ZQDUCWr9CFvn+98XFai8jt | ||
| voVQMV23AGi6nJJ+jGw9koQUt/uYAxZ4U8tG0KqxVGhmrab1MfTpLp2mQWkJN7y1 | ||
| Hk5moPHwpQhxW73qlzwR8Ug8FwKBgQDW4nX8ZvFfmyJcrckquh0KMpILe5i+klmd | ||
| ENU7TmlQ8Sq1QX2j+w4gOWpUR6/bnij1XeEsI21z10Sv3yEgu2E8V7Cqf9mJX0in | ||
| +H5+WpEbTHqgfWhA8wXoZIizRfHDKOsOnhNmTFMBBrcp0zd4V1N1xH+APkw1q3jF | ||
| YxnmMAMmSwKBgBH5xYLxffiO/iYWRnyy0HJjQs5ae1zZx6z+63Cw56/z+CxNc8iv | ||
| cetV/KTQHeNpuiQI68qzHBT0EIa138R08r21ks10iF86CHDQyd4oLxrlTTZlNK61 | ||
| hIG8YqVyK4NRAyNcInOy+jFMvi7kLYRTyYQ+DxbvHpxqQN1hhCnLIJztAoGAakX9 | ||
| zCKtZXc3+1YHk5YQHqb8C6nI1RdUMpXMn1QcSee8E4CcPqk/RzieGaiKlLcX0qHn | ||
| ZwjubMgeNEzJ+YIyiMFloi0wzPvO1yPSi3MHKNUeIJllIhoO5ewyn1cMRlTKS6Rq | ||
| O8Grm2pS0+CeImot4KSZ2jb1QeXYCOcGPA2qwRkCgYEAnCI12DQuInN8nLEo4qtq | ||
| XEgyvUZ0fGaezcmeT4hhY94l0/HXS0D0qXs/f/rvfFFnvRYlEyiycA4pClkNRNkM | ||
| TM9RBaFTEKw9NQP895KUx6hHIAM/LB1Qyf7cDixtwf8ly7Gqhx4vU9tCiiDGSr9Z | ||
| T+QEb2Rxj5SJ8cGbNr+NAEI= | ||
| -----END PRIVATE KEY-----"; | ||
|
|
||
| const TEST_PUBLIC_KEY: &str = "-----BEGIN PUBLIC KEY----- | ||
| MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAyeQ+5aTKxiaZe8wnGe91 | ||
| 6m8wWmoGj2vUA6BpsqU2zqIsrJ55nEx38vsHD9mgZQ0wvGgfu4j3Wa9LQBDXcyEN | ||
| P5GRQVgncKgLUGA8nxrmcfcvtxPSPERZXg6g0sAOmlK0qHrAvTgR87Xm19JuE/XQ | ||
| Fs5/Fa9wmClaaeF3GMH1+olrMwpuN87+Hwibv6mF+uyLzj/IeujjAWfLOwOCtec9 | ||
| Z/EnCN0er+mpnPeIge89gfYXb7qEypPPXReVxsRNWVZyM0j6//P6EECYBkWmKkAF | ||
| YsuJD5nLVuLmiPWo0DyEHufC6TYdZQ8Pmi0Rzv3tEgqm1DQEUy0nUiMDVd3ZbVoE | ||
| vQIDAQAB | ||
| -----END PUBLIC KEY-----"; | ||
|
|
||
| fn now_secs() -> u64 { | ||
| SystemTime::now() | ||
| .duration_since(UNIX_EPOCH) | ||
| .unwrap() | ||
| .as_secs() | ||
| } | ||
|
|
||
| fn make_token(extra: HashMap<&str, serde_json::Value>) -> String { | ||
| let mut claims = serde_json::Map::new(); | ||
| claims.insert("sub".into(), serde_json::json!("test-agent")); | ||
| claims.insert("exp".into(), serde_json::json!(now_secs() + 3600)); | ||
| claims.insert("site_ids".into(), serde_json::json!(["site-123"])); | ||
| for (k, v) in extra { | ||
| claims.insert(k.into(), v); | ||
| } | ||
| let key = EncodingKey::from_rsa_pem(TEST_PRIVATE_KEY.as_bytes()).unwrap(); | ||
| encode(&Header::new(Algorithm::RS256), &claims, &key).unwrap() | ||
| } |
There was a problem hiding this comment.
move this to src/test_util/auth.rs or some such?
There was a problem hiding this comment.
Also, there is another test-caller above that needs it, let us dedup these
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn valid_token_and_site_id_delivers() { |
There was a problem hiding this comment.
let us convert these tests to use a mix of keys (eg. site_id, site, foo etc) and also add new test-cases for precedence (eg. when an event has both site_id and foo, the former is used) etc
No description provided.