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

docs: update README #616

Merged
merged 1 commit into from
Feb 7, 2024
Merged

docs: update README #616

merged 1 commit into from
Feb 7, 2024

Conversation

cflee
Copy link
Contributor

@cflee cflee commented Jan 2, 2024

Proposing some updates to the README, based on my experience integrating against Singpass and Myinfo portions of it, and supporting others who were integrating against Corppass and sgID.

  • Remove all references to some no-longer-in-use options that presumably date back to the SAML Singpass/Corppass era, i.e. SIGN_ASSERTION, SIGN_RESPONSE, ENCRYPT_ASSERTION and RESOLVE_ARTIFACT_REQUEST_SIGNED. This should fix users thinking these could be used to modify behaviour of the current Singpass/Corppass/Myinfo/sgID implementations.
  • Reorganise the README to be per use case, and document most options on this per use case basis.
    • I propose that this is clearer as it allows users to read just what they need instead of seeing a central list of options and having to read some detailed conditions on which options apply to which use case.
    • While the env vars or headers are generally reused across use cases, they have subtly different behaviour, e.g. sgID doesn't support per-request no-login-page default profile override via custom headers unlike Singpass/Corppass, and Myinfo v3 and sgID have the client certificates configured differently (Myinfo v3 requires two separate env vars for some legacy reason, I think some no-longer-in-use library required a separate dedicated public key PEM file, while sgID just requires one env var).
    • Deprioritise the Singpass/Corppass v1 to the end.
  • Add some hints, like sgID only working for the static Myinfo personas, being able to use any client_id, Myinfo is v3 only and not v4.
  • Update a note at the top to note that Mockpass also supports sgID. This should fix sgID users thinking that they have to do an additional integration against Singpass to use Mockpass for integration/e2e testing purposes.
  • Suggest an alternative of running directly with npx without npm install.

Copy link
Collaborator

@LoneRifle LoneRifle left a comment

Choose a reason for hiding this comment

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

lgtm

@LoneRifle LoneRifle merged commit 3794376 into opengovsg:main Feb 7, 2024
3 checks passed
@cflee cflee deleted the cflee/fix-readme branch April 5, 2024 04:42
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