Add support for using Key module to store API Key more securely

Created on 5 June 2024, about 1 year ago

Problem/Motivation

We use the Key module to store authentication outside of the config, this feature adds support for using it in place of the key in config.

Proposed resolution

If the Key module is installed the api_key textfield in the admin is replace by the Key modules key_select element.

When the api_key is then used in calls to Mailgun\Mailgun:create it is first (if Key module installed) retrieved from the key entity.

Remaining tasks

There are two different methods in the code that are making calls to Mailgun\Mailgun:create, \Drupal\mailgun\MailgunFactory::create and \Drupal\mailgun\MailgunHandler::validateMailgunApiKey. Ideally there should be a single method that is making the create call - I guess logically it should be the MailgunFactory class?

User interface changes

If the Key module is installed the admin form shows Key's select field in place of the textfield.

The same config name is used 'api_key' and the code will fallback to just passing the api_key through if a Key entity cannot be loaded - this will allow the module to be updated for someone with the Key module but not suddenly stop the Mailgun API from connecting.

Feature request
Status

Active

Version

2.0

Component

Code

Created by

🇬🇧United Kingdom altcom_neil

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @altcom_neil
  • Status changed to Needs work 5 days ago
  • 🇺🇦Ukraine bohart Lutsk, Ukraine

    Changed the version of this issue. Mailgun 2.1.0 is now released, which means new developments and disruptive changes should be targeted against the 2.1.x-dev branch.

    Merge requests from the community will be incredibly welcome.
    Note: Only GitLab merge requests (not *.patch files) are reviewed by the currently active maintainer.

    Thanks!

  • 🇮🇳India divyansh.gupta Jaipur

    Working on it!

  • 🇮🇳India divyansh.gupta Jaipur

    Converted patch into MR, also done some changes manually as patch was not applying directly.
    Please review!!

  • Pipeline finished with Success
    2 days ago
    Total: 315s
    #561519
  • 🇺🇦Ukraine bohart Lutsk, Ukraine

    @divyansh.gupta, please stop just move the patch files to merge requests on all issues without testing them, and mark issues as Needs review with failed pipelines. Thanks.

  • Pipeline finished with Failed
    2 days ago
    Total: 156s
    #561563
  • Pipeline finished with Failed
    1 day ago
    Total: 187s
    #562113
  • Pipeline finished with Failed
    1 day ago
    Total: 203s
    #562120
  • Pipeline finished with Success
    1 day ago
    Total: 131s
    #562154
  • 🇮🇳India divyansh.gupta Jaipur

    Sorry @bohart, for not testing and directly pushing the patch as MR, now i have worked and made all tests pass, also here is an screenshot proving that i have successfully made the key selection using key module.

    Please review!!

Production build 0.71.5 2024