Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove SAML metadata monkeypatches #3396

Closed
bmesuere opened this issue Feb 15, 2022 · 2 comments · Fixed by #5169
Closed

Remove SAML metadata monkeypatches #3396

bmesuere opened this issue Feb 15, 2022 · 2 comments · Fixed by #5169
Labels
chore Repository/build/dependency maintenance low priority Thing we want to see implemented at some point student Things summer interns can do

Comments

@bmesuere
Copy link
Member

Belnet requires that specific fields are present in the SAML metadata. The gem we used didn't support this so we monkeypatched the code that generated the metadata to add these fields:

<md:Organization>
<md:OrganizationName xml:lang="en">UGent - Dodona</md:OrganizationName>
<md:OrganizationName xml:lang="nl">UGent - Dodona</md:OrganizationName>
<md:OrganizationName xml:lang="fr">UGent - Dodona</md:OrganizationName>
<md:OrganizationDisplayName xml:lang="en">UGent - Dodona</md:OrganizationDisplayName>
<md:OrganizationDisplayName xml:lang="nl">UGent - Dodona</md:OrganizationDisplayName>
<md:OrganizationDisplayName xml:lang="fr">UGent - Dodona</md:OrganizationDisplayName>
<md:OrganizationURL xml:lang="en">https://dodona.ugent.be</md:OrganizationURL>
<md:OrganizationURL xml:lang="nl">https://dodona.ugent.be</md:OrganizationURL>
<md:OrganizationURL xml:lang="fr">https://dodona.ugent.be</md:OrganizationURL>
</md:Organization>
<md:ContactPerson contactType="technical">
<md:GivenName>Dodona</md:GivenName>
<md:SurName>Helpdesk</md:SurName>
<md:EmailAddress>[email protected]</md:EmailAddress>
</md:ContactPerson>

In a recent version, ruby saml added support for custom metadata fields. This means that the monkeypatch can probably be removed.

More specifically, I think lib/SAML/metadata.rb can be removed (after inspecting the edit history to check if other changes weren't made). The fields that are added in line 128 till 166 should be moved to separate class as is shown in SAML-Toolkits/ruby-saml#602

The change should be validated by comparing the generated metadata at /users/saml/metadata before and after the fix, and by signing in into ugent from naos.

@bmesuere bmesuere added chore Repository/build/dependency maintenance low priority Thing we want to see implemented at some point labels Feb 15, 2022
@bmesuere bmesuere added the student Things summer interns can do label Nov 17, 2023
@chvp
Copy link
Member

chvp commented Nov 20, 2023

Reopening, since this was reverted in #5171.

@jorg-vr
Copy link
Contributor

jorg-vr commented Nov 20, 2023

See #5175

@jorg-vr jorg-vr closed this as not planned Won't fix, can't repro, duplicate, stale Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Repository/build/dependency maintenance low priority Thing we want to see implemented at some point student Things summer interns can do
Projects
Status: Done
3 participants