CSHARP-1000 Fix support for astra custom domain #600
Conversation
There was a problem hiding this comment.
Certainly seems to address the problem as described. A few nits based on my understanding of the code but I don't think either of them are problematic enough to hold things up if you need to get a release out.
It might be nice to have a test to confirm that SANs were being used for validation but, again, I don't think it's essential.
| var result = new List<string>(); | ||
|
|
||
| var subjectAlternativeName = cert.Extensions.Cast<X509Extension>() | ||
| .Where(n => n.Oid.Value == "2.5.29.17") //n.Oid.FriendlyName=="Subject Alternative Name") |
There was a problem hiding this comment.
Perhaps use the SubjectAlternateNameOid constant. you defined above here instead?
There was a problem hiding this comment.
Ahh I had to try multiple solutions and forgot about those constants... fixed
|
|
||
| foreach (var alternativeName in alternativeNames) | ||
| { | ||
| var groups = Regex.Match(alternativeName, @"^(.*)=(.*)").Groups; // @"^DNS Name=(.*)").Groups; |
There was a problem hiding this comment.
Is this intended to... replace the DnsNameRegex referenced above? Seems like it's trying to do the same thing with a different regex.
There was a problem hiding this comment.
Ahh I had to try multiple solutions and forgot about those constants... fixed
|
👍 to the most recent round of changes. Nice tests @joao-r-reis ! |
No description provided.