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

Mockpass no longer defaults to first NRIC if login page disabled #120

Closed
chadlwilson opened this issue Oct 13, 2020 · 2 comments
Closed

Comments

@chadlwilson
Copy link

chadlwilson commented Oct 13, 2020

In earlier versions before 2.5.0, mockpass used to default to the first NRIC in the assertions.saml.singPass list (or the MOCKPASS_NRIC env var) in the returned SAML artifact, if the login page was disabled.

In the current implementation when the login page is disabled, the index in the SAML artifact ID is -1 which then cannot be looked up, and mockpass returns no NRIC in the UserName inside the SAML Artifact; which is a breaking change for most users I imagine.

This seemed to happen in 7f29add - previous code had a default here which no longer exists in the new implementation here

Was this change intentional?

@LoneRifle
Copy link
Collaborator

Hey @chadlwilson - thanks for the catch! The change was unintentional, so I'll try to put in a fix for this later today.

@chadlwilson
Copy link
Author

Thanks! 🙏

justin-tay pushed a commit to justin-tay/mockpass that referenced this issue Nov 18, 2020
If `SHOW_LOGIN_PAGE` is not true, the artifact sent to the
`/soap` and `/token` endpoints will translate to an index of -1.
If so, ensure that we default to what was specified in
`assertions.{singPassNric, corpPassNric}`

Fixes opengovsg#120
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

No branches or pull requests

2 participants