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

Vanguard integration #2254

Merged
merged 9 commits into from
Jun 24, 2019
Merged

Vanguard integration #2254

merged 9 commits into from
Jun 24, 2019

Conversation

nahidakbar
Copy link
Contributor

@nahidakbar nahidakbar commented Jun 12, 2019

What this PR does

Fixes #2008

This PR contains functionality necessary for connecting magda instances to vanguard through ws-fed protocol.

How to test.

  1. Make sure the metadata set is correct (has the right return url etc): https://magda-files.s3-ap-southeast-2.amazonaws.com/WSFED-ENV.xml
  2. Goto https://cloud.thirdparty.authentication.business.gov.au/sts/claims/ and set user data
  3. Goto login page, click on login via vanguard and then set third party integration test as login provider

For whoever picking this up, remaining stuff including private keys etc is here: https://drive.google.com/drive/folders/1vniUnRqOIfvyUy8nekDueITwEUUKzs96

Checklist

  • Unit tests aren't applicable
  • I've updated CHANGES.md with what I changed.
  • I've linked this PR to an issue in ZenHub (core dev team only)

@nahidakbar nahidakbar marked this pull request as ready for review June 13, 2019 02:35
magda-gateway/src/createOrGetUserToken.ts Outdated Show resolved Hide resolved
magda-gateway/src/oauth2/vanguard.ts Outdated Show resolved Hide resolved
@@ -43,6 +43,7 @@
"passport-facebook": "^2.0.0",
"passport-google-oauth20": "^1.0.0",
"passport-local": "^1.0.0",
"passport-wsfed-saml2": "^3.0.17",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we might need to fork our version of this with the x509 and cryptiles problems patched out :(. No need for you to get to this before you go though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will native x509 dependency cause problems? It just needs build tools for npm install.

Copy link
Contributor

Choose a reason for hiding this comment

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

Apparently it breaks on node > 8?

Copy link
Contributor

Choose a reason for hiding this comment

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

There's already a PR to get rid of it, but it's been sitting since October :(

@AlexGilleran AlexGilleran merged commit b7647f8 into master Jun 24, 2019
@AlexGilleran AlexGilleran deleted the vanguard branch June 24, 2019 06:11
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.

SPIKE - Integration with ADFS Login
2 participants