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

[EVAKA-HOTFIX] Support Single Logout w/o 3rd party cookies #738

Merged
merged 20 commits into from
May 12, 2021

Commits on May 11, 2021

  1. APIGW: don't use deprecated body-parser directly

    - Express still actually uses the same body-parser package/methods behind the scenes but as the library user we should use the official wrapper methods `express.json()` and `express.urlencoded()`
    - Options stay as-is as the implementations stay as-is
    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    8b782ad View commit details
    Browse the repository at this point in the history
  2. APIGW: split SAML config creation from Strategies

    - Allows separate instation of the `SAML` class (from `passport-saml`) used for parsing/verifying SAML messages
    - Will later be used to parse logout tokens from SAML LogoutRequest messages where cookies aren't available
    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    4907f8e View commit details
    Browse the repository at this point in the history
  3. APIGW: clarify logout start error logging

    - No SAML error will ever be present when starting the logout flow from the SP (eVaka itself) -> don't waste time attempting to parse SAML errors and clarify the error description
    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    a49d0e8 View commit details
    Browse the repository at this point in the history
  4. APIGW: support SLO without cookies, drop logout cookie

    - Many browsers are starting to disable 3rd party cookies by default (even SameSite None) which breaks Single Logout with passport-saml that currently only supports logging out with a cookie
        - Browser details: https://webkit.org/blog/10218/full-third-party-cookie-blocking-and-more/ and https://blog.google/products/chrome/more-intuitive-privacy-and-security-controls-chrome/
        - passport-saml issues: node-saml/passport-saml#419 and node-saml/passport-saml#221
        - This is actually a real security issue as passport-saml will give a successful looking response from `passport.authenticate()` in the logout callback which results in IdPs showing "successfully logged out" messages to the user -- even though the user is still fully logged in!
        - This also causes an usability issue when the user tries to initate SLO from another application, fails to end the eVaka session, attempts a logout from eVaka and gets an ugly JSON error message as a response when APIGW attempts to make a LogoutRequest to the IdP that already ended the session -> will be fixed separately
    - Other systems like Shibboleth get around the 3rd party cookie issue with Single Logout by not utilizing cookies at all for this but instead use the SAML nameID (and sessionIndex) properties presented in all SAML LogoutRequest messages
        - Source: https://wiki.shibboleth.net/confluence/display/DEV/IdP+SameSite+Testing#IdPSameSiteTesting-SameSiteandSingleLogout
    - When logouts are always only done through SAML there's no need for the logout cookie itself but the idea is actually useful as an effective "secondary index" for Redis:
        - By storing a nameID + sessionIndex keyed item pointing to the session ID, we effectively create a second index that can be used with just the SAML LogoutRequest's nameID and sessionIndex properties
    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    3203430 View commit details
    Browse the repository at this point in the history
  5. APIGW: Make SAML parser optional for Express logout

    - Not relevant for e.g. mobile logouts where the request will never have a SAMLRequest query parameter or body -> make it optional
    - Add debug logging for easier understanding of login/logout flow events
    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    3f9ec9f View commit details
    Browse the repository at this point in the history
  6. Use a more recognizable separator for SLO token key

    - Just for easier debugging and makes key collisions slightly less likely
    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    11b8190 View commit details
    Browse the repository at this point in the history
  7. APIGW: fall back to logoutToken in logout

    - Instead of re-generating the token from session details (if no SAMLRequest Profile was available), use the logoutToken's value directly
    - Trying to avoid having too much SAML-specific stuff in the session implementation
    - Also unify code style for generating session and logout token Redis keys
    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    d8bfa3a View commit details
    Browse the repository at this point in the history
  8. APIGW: Refactor SAML-specifics out of session module

    - Instead of having anything specifically SAML-related in the session module, take the logout token in as just a string and move the parsing and nameID+sessionIndex logic into more SAML-specific modules
    - This also allows dropping unnecessary arguments from all other routes than logout callback
        - But requires the login callback route to have logic for generating the token initially before saving it
    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    541544d View commit details
    Browse the repository at this point in the history
  9. APIGW: Tighten ESLint config for Promises

    - Disallow floating Promises to prevent unexpected order of operations in async functions where an async function is called but not awaited
    - Already enabled in frontend projects
    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    2d5c062 View commit details
    Browse the repository at this point in the history
  10. Allow manually setting cookies in GatewayTester

    - Enables temporarily removing and re-adding the same cookies
        - Useful for simulating missing cookies e.g. in 3rd party SAML request cases
    - Augment "tough-cookie" types to add missing `sameSiteContext` property for `setCookie` options object
        - Missing from `@types/tough-cookie@4.0.0` but implemented in library
        - Extremely relevant for 3rd party cookie testing
    - Also return the actual Cookie instance from getCookie (all current usage just checks for truthyness/not-undefined which is still valid) -> enables new setCookie usage
    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    c77891d View commit details
    Browse the repository at this point in the history
  11. APIGW: add .editorconfig

    - Match frontends config
    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    a79a2d8 View commit details
    Browse the repository at this point in the history
  12. APIGW: Add test cert and key for SLO testing

    - For test use only, named accordingly
    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    37281c5 View commit details
    Browse the repository at this point in the history
  13. APIGW: Allow overriding mock config for Suomi.fi Strategy for testing

    - To allow testing cases where using the actual full Strategy instead of a dummy strategy is needed, allow overriding the `sfiMock` config with an argument that defaults to the config to not break current usage
    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    8b6d034 View commit details
    Browse the repository at this point in the history
  14. GatewayTester: allow overriding login POST data

    - Allows using the tester with non-dummy passport strategies
    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    bd9e408 View commit details
    Browse the repository at this point in the history
  15. AsyncRedisClient: support array of keys for del, workaround for redis…

    …-mock
    
    - Unfortunately redis-mock currently has an open issue (yeahoffline/redis-mock#135) and an un-merged fix for it (yeahoffline/redis-mock#178) that means it doesn't support multiple arguments (i.e. spread) for the `.del()` method -> to allow using the mock library in tests add support to AsyncRedisClient for using an array of keys instead of argument-per-key and always provide the keys to the underlying redis client as an array instead of using the spread operator
    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    9a174ea View commit details
    Browse the repository at this point in the history
  16. APIGW: Regression tests for SAML Single Logout

    - As passport-saml and SAML authentication as a whole is very deeply integrated into the application, tests for it need to unfortunately set up its own instances of almost everything for the app in order to override important configs
        - Session store must use Redis as the Single Logout token system relies on it being available (instead of the default MemoryStore for local development) -> need to override client setup to use a in-memory mock-Redis
        - SAML configuration must use "real" certificates but obviously nothing from a real environment
        - SAML configuration needs additional adjustments to simplify testing
    - As simulating real 3rd party cookie situations would be really complex, deleting and re-adding cookies is used to replicate the situation
        - Also implement a reference test case where cookies are available
    - Add necessary XML + crypto dependencies for creating and parsing SAML messages
        - Although passport-saml has the SAML class (with missing official types...), it doesn't really provide useful methods/abstractions -- they go too far into request handling instead of just parsing and generating messages -- so decided to impelement these manually
    - Based on the wonderful examples from node-saml/passport-saml#419
    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    dc56a6c View commit details
    Browse the repository at this point in the history
  17. Fix typo

    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    b9d8e0b View commit details
    Browse the repository at this point in the history
  18. APIGW: SAML parsing cleanup

    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    784c1f3 View commit details
    Browse the repository at this point in the history
  19. APIGW: Improve test failure stack traces for requests

    - Axios eats the stack traces for failed requests -> explicitly expect specific response status codes outside of axios by always validating status to `true`
    mikkopiu committed May 11, 2021
    Configuration menu
    Copy the full SHA
    68206de View commit details
    Browse the repository at this point in the history

Commits on May 12, 2021

  1. APIGW: Use real app instance for SAML tests, mock Redis in tests

    - Globally mock redis with redis-mock in all Jest tests for APIGW
        - Allows removal of test-specific logic from apps regarding Redis clients
    - Merge Suomi.fi SAML configs into one dynamic config that is once again controlled with `config.sfiMock`
        - No reason to duplicate when fallbacks and "if test then" logic can be integrated into the configs/constants
        - Still return an empty config object when Suomi.fi mocking is enabled (i.e. local dev, other tests than SAML tests) to prevent fallout from missing required props
    - Utilize dynamic imports and brute force to temporarily disable Suomi.fi mocking for SAML tests ONLY
        - Not pretty but allows use of the actual enduser app instance (instead of faking it) and configs
        - Slightly less brittle against changes in the implementation
    mikkopiu committed May 12, 2021
    Configuration menu
    Copy the full SHA
    6a62fcd View commit details
    Browse the repository at this point in the history