Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

[fix] SAML assertion case insensitive matching to get display name#52992

Merged
kopancek merged 3 commits into
mainfrom
milan/saml_case_insensitive_assertions
Jun 6, 2023
Merged

[fix] SAML assertion case insensitive matching to get display name#52992
kopancek merged 3 commits into
mainfrom
milan/saml_case_insensitive_assertions

Conversation

@kopancek

@kopancek kopancek commented Jun 6, 2023

Copy link
Copy Markdown
Contributor

Also fixed a bug where we always returned an error from saml GetExternalAccountData function. We should only return an error there if the return value is nil.

Test plan

Added a unit test.

Also fixed a bug where we always returned an error from saml
GetExternalAccountData function. We should only return an error
there if the return value is nil.
@kopancek kopancek requested review from a team and 0xnmn June 6, 2023 09:29
@kopancek kopancek self-assigned this Jun 6, 2023
@cla-bot cla-bot Bot added the cla-signed label Jun 6, 2023
@sourcegraph-bot

sourcegraph-bot commented Jun 6, 2023

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff 096bd0b...7ecbbae.

Notify File(s)
@unknwon enterprise/cmd/frontend/internal/auth/saml/BUILD.bazel
enterprise/cmd/frontend/internal/auth/saml/user.go
enterprise/cmd/frontend/internal/auth/saml/user_test.go

@sashaostrikov sashaostrikov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!


const testAuthnResponse = `<samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" Destination="http://localhost:3080/.auth/saml/acs" ID="ID_4f2db416-8815-4d9c-84b5-871eb79ce4f4" InResponseTo="_b5744ff7-7066-4db6-a19f-baa925227c8a" IssueInstant="2018-05-20T17:12:06.795Z" Version="2.0"><saml:Issuer>http://localhost:3220/auth/realms/master</saml:Issuer><dsig:Signature xmlns:dsig="http://www.w3.org/2000/09/xmldsig#"><dsig:SignedInfo><dsig:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/><dsig:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/><dsig:Reference URI="#ID_4f2db416-8815-4d9c-84b5-871eb79ce4f4"><dsig:Transforms><dsig:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/><dsig:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/></dsig:Transforms><dsig:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/><dsig:DigestValue>LAWFTwzvNHyOLqwimF3QR5dJgEfCxs2RXUdp+raMdRk=</dsig:DigestValue></dsig:Reference></dsig:SignedInfo><dsig:SignatureValue>xg13DUl1G80iCxATDN0R2QGUBHq+n6N9J389zTBM36ploAbWtvnI29IuW+aRaO69cUKsHBGH3YIV7njUNcDOOHMX1b9K+hooqaRyGfKISnvnaLZ+/R3yXZf+pAFshvtgWkaS+29zmNP9+5j3X/j9Gj9buoIlL5f51MO8fXlYJtdxqIhFoYZWcrttstxQhENjskFezYPyepl5F49m+FY5nYKh75WcG51NI+/VSYqWQd7MeUompPTONbt8Kwtj7YGizNbJseEOt1EI5wn+7eFvq/DkpJAuKDB4jnjbjadQmEbUIfKew5u/EEn6WDVnidL9vQQh/ZVOmFqL77iqBbQPLw==</dsig:SignatureValue><dsig:KeyInfo><dsig:KeyName>jR3UQTQOE9k8iqTK77NrOBahhyFNT2p3B2lF1I3ov1g</dsig:KeyName><dsig:X509Data><dsig:X509Certificate>MIICmzCCAYMCBgFjcZU/LjANBgkqhkiG9w0BAQsFADARMQ8wDQYDVQQDDAZtYXN0ZXIwHhcNMTgwNTE4MDQ0ODE2WhcNMjgwNTE4MDQ0OTU2WjARMQ8wDQYDVQQDDAZtYXN0ZXIwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDXZpJeHraEt9FPk478+RoMtP9RV83Ew/XRZhNKI4BPoY5MjRVuvaabvMOE5X1AK9Z0cEU++m/Y0LuHg3A4kQdPw3BGPBfGm0WSD6DEN42TcF3dc8XBA/osDNW5i6rZM071che8XtKNHcW9ZAv9ETfJeUb4NHFRkRg3K1lZ5kCwt0JNo+0akQ2EdQXXu/uEeQV49rOADr+Lp6GLhmGeCckC8xzBiNxZwR4pJsz9XWgB6fSdpIGvWhAnBfFZyyZIHnVuRnm2wJ53Exg6h2RB3SFYu3PXXuIHeuH71pel5WwnecTVTwV/RMwkAGLdCNC9jp9tdDtThhWLn4E9D0wZkpU9AgMBAAEwDQYJKoZIhvcNAQELBQADggEBAKT/zyjvSM09Fk2ON4rMSExnyrw6LXuJJOZlB0eD22KruQ53AikfKz5nJLCFLc0PT4PmK06s9OF0HG95k4jiiuvAdNMXZSLUGNcbaODeJ/ZzCJJp0cB2rWEmAqbKruXzBpTFttlgsW4mgpkvGxORztfhksiyAX0bLcNWtsQecl3fpvoVrJiIHXStD3c/v4exE2QPkuvhLCzwI2oXrrhrovyTKjCbyn2//lqOfFziA8X/ini3R/L4UzTVB5SWAz/LtkpgipPOwNpVqwErnZamexm6S38QX+OZ+uhZY/1JfTugs9vpXwRvj/xamGr8r+MqornuQiEBBNiCbCJ6B4iUWh4=</dsig:X509Certificate></dsig:X509Data><dsig:KeyValue><dsig:RSAKeyValue><dsig:Modulus>12aSXh62hLfRT5OO/PkaDLT/UVfNxMP10WYTSiOAT6GOTI0Vbr2mm7zDhOV9QCvWdHBFPvpv2NC7h4NwOJEHT8NwRjwXxptFkg+gxDeNk3Bd3XPFwQP6LAzVuYuq2TNO9XIXvF7SjR3FvWQL/RE3yXlG+DRxUZEYNytZWeZAsLdCTaPtGpENhHUF17v7hHkFePazgA6/i6ehi4ZhngnJAvMcwYjcWcEeKSbM/V1oAen0naSBr1oQJwXxWcsmSB51bkZ5tsCedxMYOodkQd0hWLtz117iB3rh+9aXpeVsJ3nE1U8Ff0TMJABi3QjQvY6fbXQ7U4YVi5+BPQ9MGZKVPQ==</dsig:Modulus><dsig:Exponent>AQAB</dsig:Exponent></dsig:RSAKeyValue></dsig:KeyValue></dsig:KeyInfo></dsig:Signature><samlp:Status><samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/></samlp:Status><saml:Assertion xmlns="urn:oasis:names:tc:SAML:2.0:assertion" ID="ID_68f3f1bf-05b3-4c13-a2c6-63a4885ed70b" IssueInstant="2018-05-20T17:12:06.795Z" Version="2.0"><saml:Issuer>http://localhost:3220/auth/realms/master</saml:Issuer><saml:Subject><saml:NameID Format="urn:oasis:names:tc:SAML:2.0:nameid-format:persistent">G-58956f28-7bf5-448d-923a-bd39438c2a9e</saml:NameID><saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer"><saml:SubjectConfirmationData InResponseTo="_b5744ff7-7066-4db6-a19f-baa925227c8a" NotOnOrAfter="2018-05-20T17:13:04.795Z" Recipient="http://localhost:3080/.auth/saml/acs"/></saml:SubjectConfirmation></saml:Subject><saml:Conditions NotBefore="2018-05-20T17:12:04.795Z" NotOnOrAfter="2018-05-20T17:13:04.795Z"><saml:AudienceRestriction><saml:Audience>http://localhost:3080/.auth/saml/metadata</saml:Audience></saml:AudienceRestriction></saml:Conditions><saml:AuthnStatement AuthnInstant="2018-05-20T17:12:06.795Z" SessionIndex="0c9f6960-c426-4d45-9b0c-b11870cd8338::bc174b26-a300-4e78-9d76-49f2521e7b65"><saml:AuthnContext><saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified</saml:AuthnContextClassRef></saml:AuthnContext></saml:AuthnStatement><saml:AttributeStatement><saml:Attribute FriendlyName="surname" Name="urn:oid:2.5.4.4" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">Yang</saml:AttributeValue></saml:Attribute><saml:Attribute FriendlyName="givenName" Name="urn:oid:2.5.4.42" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">Bob</saml:AttributeValue></saml:Attribute><saml:Attribute FriendlyName="email" Name="urn:oid:1.2.840.113549.1.9.1" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">bob@example.com</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">view-profile</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">uma_authorization</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">manage-account</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">manage-account-links</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">admin</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">view-realm</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">manage-identity-providers</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">query-realms</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">manage-clients</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">query-clients</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">manage-authorization</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">query-users</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">view-users</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">manage-users</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">view-events</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">create-realm</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">create-client</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">view-clients</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">view-identity-providers</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">query-groups</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">manage-events</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">impersonation</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">view-authorization</saml:AttributeValue></saml:Attribute><saml:Attribute Name="Role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">manage-realm</saml:AttributeValue></saml:Attribute></saml:AttributeStatement></saml:Assertion></samlp:Response>`

func stringPointer(s string) *string {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my favourite thing ever, so much pleasure when I add it every single time 😁

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would love for this to be in some global utils or even builtin the standard Go library...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I once started such conversation in one of our channels, but was told that util stuff is not the go-way 😁

@kopancek kopancek merged commit c22b75a into main Jun 6, 2023
@kopancek kopancek deleted the milan/saml_case_insensitive_assertions branch June 6, 2023 10:25
github-actions Bot pushed a commit that referenced this pull request Jun 6, 2023
…52992)

Also fixed a bug where we always returned an error from saml
GetExternalAccountData function. We should only return an error there if
the return value is nil.

## Test plan

Added a unit test.

(cherry picked from commit c22b75a)
ErikaRS pushed a commit that referenced this pull request Jun 22, 2023
…52992)

Also fixed a bug where we always returned an error from saml
GetExternalAccountData function. We should only return an error there if
the return value is nil.

## Test plan

Added a unit test.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants