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

[CL-3645] Update Vienna SAML IDP metadata #4968

Merged
merged 2 commits into from
May 29, 2023

Conversation

alexander-cit
Copy link
Contributor

@alexander-cit alexander-cit commented May 25, 2023

What's done?

I just ran these commands

wget https://mein.wien.gv.at/stdportal-idp/extern.wien.gv.at -O back/engines/commercial/id_vienna_saml/config/saml/citizen/idp_metadata_production.xml
wget --no-check-certificate https://idp-test.wien.gv.at/stdportal-idp/test.wien.gv.at -O back/engines/commercial/id_vienna_saml/config/saml/citizen/idp_metadata_test.xml

This ticket mentions that XML should be fixed manually, but I'm not sure how.

Pay attention when implementing their metadata. They had a funky XML that required manual changes in their document for the employee IdP already (double entry for some kind of value)

Testing

I'm not sure how to test it properly. On production, accessing this URL works https://mitgestalten.wien.gv.at/auth/vienna_citizen/ (the same as clicking the login button, after registering at https://mein.wien.gv.at/), but locally it fails with 500, the same as the login button.

I hoped it would work on this epic URL https://viennasaml.epic.citizenlab.co/en/ (it was tested here before), but it also fails with Beim Verarbeiten des Requests ist ein Fehler aufgetreten with both production and test environment configured in the vienna_citizen_login feature.

I ran our tests in engines/commercial/id_vienna_saml/spec/ with production config and they all passed.

So, the only option I see is to test it in production.

Can we make such changes easier and more reliable?

Probably, yes.

  1. We could use the parse_remote_to_hash method by getting metadata right here https://mein.wien.gv.at/stdportal-idp/extern.wien.gv.at.
  2. We could use federation. Vienna's federation URL is not accessible, but maybe it should be reached via some proxy. It seems the federation is supported by ruby-saml (first, it wasn't supported, then it was implemented), but not by omniauth-saml.

But taking into account very limited testing capabilities, it would be really hard to implement any of it.

Changelog

Changed

  • CL-3645 Update Vienna SAML IDP metadata

I just ran this command
wget https://mein.wien.gv.at/stdportal-idp/extern.wien.gv.at -O back/engines/commercial/id_vienna_saml/config/saml/citizen/idp_metadata_production.xml
`wget --no-check-certificate https://idp-test.wien.gv.at/stdportal-idp/test.wien.gv.at -O back/engines/commercial/id_vienna_saml/config/saml/citizen/idp_metadata_test.xml`
@cl-dev-bot
Copy link
Collaborator

Messages
📖 Changelog provided 🎉
📖 Jira issue: CL-3645
📖

Run the e2e tests

📖 Check translation progress

Generated by 🚫 dangerJS against 7a93992

@alexander-cit
Copy link
Contributor Author

@adessy @kogre Could you please take a look at it today? Because the deadline for it is on Tuesday, and Monday is a public holiday in Belgium.

@alexander-cit
Copy link
Contributor Author

Agreed with Koen to release it, test on production, and roll it back if it doesn't work.

@alexander-cit alexander-cit merged commit b6ac973 into master May 29, 2023
@alexander-cit alexander-cit deleted the CL-3645-saml-change-vienna branch May 29, 2023 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants