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

SAML Re-review #1804

Merged
merged 21 commits into from
Jan 10, 2025
Merged

SAML Re-review #1804

merged 21 commits into from
Jan 10, 2025

Conversation

RobAndrewHurst
Copy link
Contributor

@RobAndrewHurst RobAndrewHurst commented Jan 9, 2025

SAML Authentication Re-review

This PR revisits the SAML-based Single Sign-On (SSO) authentication in xyz.

Key Changes

  • Migrated from saml2-js to @node-saml/node-saml package
  • Re-reviewed SAML authentication module
  • Implemented SSO login/logout flows
  • rewrote certificate handling
  • rewrote authorization endpoints
  • Added SAML properties into the cookie token.

Package Migration

The core change is moving from the deprecated saml2-js to the maintained @node-saml/node-saml package.
This brings:

  • Better maintainability
  • Improved security
  • Modern promise-based API
  • Active community support

Setup Instructions

SAML Authentication Setup

This module handles SAML-based Single Sign-On (SSO) authentication. Here's how to set it up:

  1. Certificate Generation
    Generate your Service Provider (SP) certificate pair:

    # Generate private key
    openssl genrsa -out ${SAML_SP_CRT}.pem 2048
    
    # Generate public certificate
    openssl req -new -x509 -key ${SAML_SP_CRT}.pem -out ${SAML_SP_CRT}.crt -days 36500
  2. File Structure Setup
    Place certificates in your project root:

    /xyz
    ├── ${SAML_SP_CRT}.pem     # Your SP private key
    ├── ${SAML_SP_CRT}.crt     # Your SP public certificate
    └── ${SAML_IDP_CRT}.crt    # Identity Provider's certificate
  3. Identity Provider Setup
    Configure your IdP (e.g., Auth0, Okta) with:

    • Your SP's Entity ID (issuer)
    • Your SP's public certificate (${SAML_SP_CRT}.crt)
    • Your ACS URL (callback URL)
  4. Environment Variables
    Required variables for SAML strategy initialization:

    # Required Core Settings
    SAML_ACS=http://your-domain/saml/acs
    SAML_SSO=https://your-idp/saml/login
    SAML_ENTITY_ID=your-service-identifier
    
    # Certificate Paths (without file extensions)
    SAML_SP_CRT=sp_certificate
    SAML_IDP_CRT=idp_certificate
    
    # Additional Settings
    SAML_SLO=https://your-idp/saml/logout
    SAML_SIGNATURE_ALGORITHM=sha256

Implementation Details

Endpoints

  • /saml/metadata - Returns SP metadata
  • /saml/login - Initiates SSO login
  • /saml/acs - Handles SAML assertions
  • /saml/logout - Handles SSO logout (optional)
  • /saml/logout/callback - Processes logout responses (optional)

Authentication Flow

  1. User requests protected resource
  2. Redirected to /saml/login
  3. IdP authenticates user
  4. SAML response sent to ACS
  5. User profile created from SAML attributes
  6. JWT token generated and set as cookie

Logout Flow

  1. User hits the logout endpoint /?logout=true
  2. User is routed to /saml/logout endpoint
  3. SLO Endpoint is generated.
  4. redirected to SLO.
  5. /saml/logout/callback endpoint is hit for clean up.

Security Considerations

  • Private keys must be kept secure
  • Using SHA-256 for signatures
  • Implementing proper session management
  • Certificate rotation process needed

Testing

  • Generated test certificates
  • Tested with Auth0 as IdP
    • Please test other providers
  • Verified login/logout flows
  • Checked signature validation

Notes

  • ACL integration is optional but recommended

express.js Dismissed Show dismissed Hide dismissed
express.js Dismissed Show dismissed Hide dismissed
express.js Dismissed Show dismissed Hide dismissed
express.js Dismissed Show dismissed Hide dismissed
express.js Dismissed Show dismissed Hide dismissed
express.js Dismissed Show dismissed Hide dismissed
express.js Dismissed Show dismissed Hide dismissed
express.js Dismissed Show dismissed Hide dismissed
express.js Dismissed Show dismissed Hide dismissed
express.js Dismissed Show dismissed Hide dismissed
@RobAndrewHurst RobAndrewHurst linked an issue Jan 10, 2025 that may be closed by this pull request
@RobAndrewHurst RobAndrewHurst changed the title saml rewrite SAML Re-review Jan 10, 2025
@RobAndrewHurst RobAndrewHurst marked this pull request as ready for review January 10, 2025 11:07
@GEOLYTIX GEOLYTIX deleted a comment from gitguardian bot Jan 10, 2025
@GEOLYTIX GEOLYTIX deleted a comment from gitguardian bot Jan 10, 2025
Copy link

gitguardian bot commented Jan 10, 2025

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

Since your pull request originates from a forked repository, GitGuardian is not able to associate the secrets uncovered with secret incidents on your GitGuardian dashboard.
Skipping this check run and merging your pull request will create secret incidents on your GitGuardian dashboard.

🔎 Detected hardcoded secret in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
1293524 Triggered Generic Password 86d147f public/views/login/_login.js View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@RobAndrewHurst RobAndrewHurst merged commit c5a0c27 into GEOLYTIX:minor Jan 10, 2025
7 checks passed
@RobAndrewHurst RobAndrewHurst deleted the saml branch January 10, 2025 16:21
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.

Include SAML module from fork
2 participants