refactor: OpenID strategy and Enhance role extraction and validation logic

[rubentalstra]

Summary

• Closes: [Enhancement]: Allow checking userinfo for OPENID_REQUIRED_ROLE_PARAMETER_PATH #6702

This pull request includes significant updates to the OpenID strategy and its related tests, focusing on enhancing functionality and improving code quality. The most important changes include adding new environment variables, refactoring functions for clarity and correctness, and updating test cases to reflect the new logic.

Enhancements to OpenID Strategy:

• .env.example: Added OPENID_USE_PKCE and OPENID_REQUIRED_ROLE_SOURCE environment variables to support PKCE and specify the source for required roles.

• api/strategies/openidStrategy.js: Refactored the downloadImage, getFullName, convertToUsername, and setupOpenId functions for better readability and added new functions extractRolesFrom and getUserRoles to handle role extraction logic.

Improvements to OpenID Strategy Tests:

• api/strategies/openidStrategy.spec.js: Updated mock configurations and added missing mock functions. Enhanced test cases to validate new logic, including role extraction and PKCE support. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13]

Change Type

• New feature (non-breaking change which adds functionality)
• This change requires a documentation update

Testing

Please describe your test process and include instructions so that we can reproduce your test. If there are any important variables for your testing configuration, list them here.

Test Configuration:

Checklist

• 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
• I have written tests demonstrating that my changes are effective or that my feature works
• Local unit tests pass with my changes
• Any changes dependent on mine have been merged and published in downstream modules.
• A pull request for updating the documentation has been submitted.

[Copilot]

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

• .env.example: Language not supported

Comments suppressed due to low confidence (2)

api/strategies/openidStrategy.js:231

• Fallbacking to an empty string for client_secret may lead to silent misconfiguration if a client secret is required. Consider explicitly validating its presence.

client_secret: process.env.OPENID_CLIENT_SECRET || '',

api/strategies/openidStrategy.js:46

• [nitpick] On download failure, returning an empty string may lead to type inconsistencies with the expected Buffer type. Consider returning null or a consistent type to clearly indicate a failure.

return await response.buffer();

[marty-sullivan]

@danny-avila @rubentalstra This functionality is important for Cornell, wondering if there is a timeline? Thanks