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

Add Support for Customizable OpenID Profile Fields via Environment Variable #4362

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

geodanchev
Copy link

@geodanchev geodanchev commented Oct 9, 2024

This feature introduces a new environment variable, OPENID_CUSTOM_DATA, that allows developers to define which fields from the OpenID profile should be dynamically fetched and stored in the user model. The fields specified in this variable will be fetched from the OpenID provider (e.g., Microsoft Graph API) and saved in a new, flexible customOpenIdData field in the user schema.

Documentation Updates Notice: here

Summary

Better explanation could be found in this Feature Request

Change Type

Please delete any irrelevant options.

  • New feature (non-breaking change which adds functionality)

Checklist

Please delete any irrelevant options.

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • I have commented in any complex areas of my code
  • I have made pertinent documentation changes
  • My changes do not introduce new warnings
  • A pull request for updating the documentation has been submitted.

@danny-avila
Copy link
Owner

Thanks, I will include your changes with the next release cycle as a lot of people depend on OpenID, and I am very careful with updating things here. This should be done in the next few days.

@@ -73,6 +73,33 @@ function convertToUsername(input, defaultValue = '') {
return defaultValue;
}

async function mapCustomOpenIdData(accessToken, customOpenIdFields) {
const customData = {};
const fieldsQueryURL = `https://graph.microsoft.com/v1.0/me?$select=${customOpenIdFields.join(',')}`;
Copy link
Owner

@danny-avila danny-avila Oct 28, 2024

Choose a reason for hiding this comment

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

This seems unique to Microsoft OpenID services. The PR notes and documentation write-up make it seem like it could be applicable to many different OIDC providers but I don't think this is usually the case.

Different OIDC providers may have different ways of accessing additional user information:

  • Google has its People API
  • Auth0 has its Management API
  • Okta has its User API
  • etc.

1 or more of these approaches would be fine for me to merge:

  1. Make it explicitly Microsoft-specific and name it accordingly (e.g., mapMicrosoftCustomOpenIdData)

  2. Make it provider-agnostic by:

   async function mapCustomOpenIdData(accessToken, customOpenIdFields, providerConfig) {
     const { userInfoEndpoint, fieldMapping } = providerConfig;
     // Use configurable endpoint and field mapping logic
     // ...
   }
  1. Create an abstraction layer with provider-specific implementations:
const PROVIDER_MAPPERS = {
  microsoft: MicrosoftDataMapper,
  google: GoogleDataMapper,
  // Add more providers as needed
};

class OpenIdDataMapper {
  static getMapper(provider) {
    const MapperClass = PROVIDER_MAPPERS[provider];
    if (!MapperClass) {
      throw new Error(`No mapper found for provider: ${provider}`);
    }
    return new MapperClass();
  }
}

// Example implementations
class MicrosoftDataMapper {
  async mapCustomData(accessToken, customOpenIdFields) {
    const fieldsQueryURL = `https://graph.microsoft.com/v1.0/me?$select=${customOpenIdFields.join(',')}`;
    // Microsoft-specific implementation...
  }
}

class GoogleDataMapper {
  async mapCustomData(accessToken, customOpenIdFields) {
    const fieldsQueryURL = `https://people.googleapis.com/v1/people/me...`;
    // Google-specific implementation...
  }
}

Copy link
Owner

Choose a reason for hiding this comment

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

I would prefer the 3rd approach, since that seems like it would be the cleanest and easily allow implementations for Okta, Microsoft, etc.

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