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

ET-577: Elation - deprecate password grant and add client credentials #533

Merged

Conversation

nckhell
Copy link
Contributor

@nckhell nckhell commented Dec 11, 2024

Tests

✅ Password grant

Screenshot 2024-12-11 at 08 54 00

✅ Client credentials

Screenshot 2024-12-11 at 08 54 53

PR Type

Enhancement, Bug fix


Description

  • Added support for client_credentials OAuth grant type and deprecated the password grant type while maintaining backward compatibility.
  • Updated SettingsValidationSchema to make username and password optional and marked them as deprecated in the settings.
  • Refactored findAppointments action to use the new validation approach with z.object.
  • Removed unused FindAppointmentSchema and settingsSchema to simplify the codebase.
  • Improved type definitions and validation logic for better maintainability.
  • Made minor formatting adjustments in the getPatient action.

Changes walkthrough 📝

Relevant files
Enhancement
findAppointments.ts
Refactor appointment finding logic with updated schema validation.

extensions/elation/actions/findAppointments.ts

  • Updated to use validate function with z.object for schema validation.
  • Replaced FindAppointmentSchema with FindAppointmentFieldSchema.
  • Integrated SettingsValidationSchema for settings validation.
  • +13/-3   
    client.ts
    Add client credentials support and deprecate password grant.

    extensions/elation/client.ts

  • Introduced support for client_credentials OAuth grant type.
  • Deprecated password grant type while maintaining backward
    compatibility.
  • Added helper functions to determine grant type and build request
    configurations.
  • Integrated SettingsValidationSchema for settings validation.
  • +75/-12 
    settings.ts
    Deprecate username and password settings in Elation configuration.

    extensions/elation/settings.ts

  • Marked username and password settings as deprecated and optional.
  • Updated descriptions to reflect deprecation of password grant type.
  • Adjusted SettingsValidationSchema to make username and password
    optional.
  • +13/-6   
    appointment.ts
    Remove unused types and align with updated schema.             

    extensions/elation/types/appointment.ts

  • Removed unused FindAppointmentSchema type.
  • Updated type definitions to align with new schema structure.
  • +0/-2     
    appointment.zod.ts
    Simplify appointment validation by removing unused schema.

    extensions/elation/validation/appointment.zod.ts

  • Removed FindAppointmentSchema as it is no longer used.
  • Retained FindAppointmentFieldSchema for field-level validation.
  • +0/-5     
    Formatting
    getPatient.ts
    Code formatting improvements for Get Patient action.         

    extensions/elation/actions/getPatient/getPatient.ts

  • Minor formatting adjustments in the description and code.
  • No functional changes introduced.
  • +4/-5     
    Cleanup
    settings.zod.ts
    Remove redundant settings schema.                                               

    extensions/elation/validation/settings.zod.ts

  • Deleted settingsSchema as it has been replaced by
    SettingsValidationSchema.
  • +0/-22   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Refactoring Validation
    The refactoring from FindAppointmentSchema to using z.object for validation should be thoroughly tested to ensure that the new validation logic correctly handles all cases previously covered.

    Authentication Logic
    The new authentication logic introduces support for both 'password' and 'client_credentials' grant types. This dual support mechanism needs careful review to ensure that it does not introduce security flaws or logic errors.

    Deprecation Notice
    The deprecation of 'username' and 'password' in settings must be clearly communicated to users, and the impact of these changes on existing implementations should be assessed.

    Copy link

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @nckhell nckhell changed the title ET-577: deprecate password grant and add client credentials ET-577: Elation - deprecate password grant and add client credentials Dec 11, 2024
    @nckhell nckhell merged commit 405af92 into main Dec 12, 2024
    1 check passed
    @nckhell nckhell deleted the et-577-elation-extension-make-username-and-password-optional-in branch December 12, 2024 08:59
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants