feat: add identity user parameter and use that for mtls#996
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| for database in databases { | ||
| if min_pool_size > 0 { | ||
| if min_pool_size > 0 | ||
| && user.server_password.is_none() |
There was a problem hiding this comment.
This and the line below feel related enough to justify encapsulating in a method
There was a problem hiding this comment.
Yes. I don't even know if it's right though yet, I just added this to make the warning go away. Need to 🤔 🤔 🤔 🤔 🤔
|
|
||
| /// Get the user TLS identity. | ||
| pub fn identity(&self, user: impl ToUser) -> Option<&str> { | ||
| if let Some(cluster) = self.databases.get(&user.to_user()) { |
There was a problem hiding this comment.
Can't figure out how to do a multi line selection on mobile but I'd write this as self.databases.get(&user.to_user()).and_then(|cluster| cluster.identity())
| stream.tls_cn() == Some(user) | ||
| let identity = databases::databases() | ||
| .identity((user, database)) | ||
| .map(|s| s.to_string()); |
There was a problem hiding this comment.
Why are we cloning this only to immediately go back to a reference on the next line?
There was a problem hiding this comment.
It's coming from a global ArcSwap and we don't want to keep a reference to that while we check because of its fast path/slow path optimization that scares me.
There was a problem hiding this comment.
Also because borrow checker said so
|
Found 1 test failure on Blacksmith runners: Failure
|
Co-authored-by: Sage Griffin <sage@sagetheprogrammer.com>
If I can figure out how to write one for this. TLS is ...like the worst. |
2 participants
Instead of matching on
user, match onidentity, allowing multiple users with the same identity.