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

feat(app): dynamic authentication provider support #2167

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gashcrumb
Copy link
Member

@gashcrumb gashcrumb commented Jan 10, 2025

Description

This change adds support for loading authentication providers or modules
from dynamic plugins. An environment variable
ENABLE_AUTH_PROVIDER_MODULE_OVERRIDE controls whether or not the backend
installs the default authentication provider module. When this override
is enabled dynamic plugins can be used to supply custom authentication
providers. This change also adds a "components" configuration for
frontend dynamic plugins, which can be used to supply overrides for the
AppComponents option. This is required for dynamic plugins to be able to
provide a custom SignInPage component, for example:

dynamicPlugins:
  frontend:
    my-plugin-package:
      components:
        - name: SignInPage
          module: PluginRoot
          importName: SignInPage

Where the named export SignInPage will be mapped to
components.SignInPage when the frontend is initialized. Finally, to
ensure authentication providers can be managed by the user a new
providerSettings configuration field is available for frontend dynamic
plugins, which can be used to inform the user settings page of the new
provider, for example:

dynamicPlugins:
  frontend:
    my-plugin-package:
      providerSettings:
        - title: Github Two
          description: Sign in with GitHub Org Two
          provider: github-two

Each providerSettings item will be turned into a new row under the
"Authentication Providers" tab in the user settings. The provider
field is used to look up the API ref for the external auth provider by
expanding the simple provider name to the auth ID convention
core.auth.<provider name>.

PR acceptance criteria

Please make sure that the following steps are complete:

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (requirement for new features)
  • Add a screenshot if the change is UX/UI related

How to test changes / Special notes to the reviewer

It's mostly enough to ensure that this change does not break the existing e2e tests. However to actually try this out locally I've prepared this example here. And there's also a second less good example that shows another common use-case for this functionality here

Copy link

openshift-ci bot commented Jan 10, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from gashcrumb. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Contributor

Copy link
Contributor

Copy link
Contributor

This PR is stale because it has been open 7 days with no activity. Remove stale label or comment or this will be closed in 21 days.

1 similar comment
Copy link
Contributor

This PR is stale because it has been open 7 days with no activity. Remove stale label or comment or this will be closed in 21 days.

Copy link
Contributor

github-actions bot commented Feb 5, 2025

@gashcrumb gashcrumb force-pushed the RHIDP-5484 branch 4 times, most recently from 0411330 to e2d623d Compare February 10, 2025 12:28
This change adds support for loading authentication providers or modules
from dynamic plugins.  An environment variable
ENABLE_AUTH_PROVIDER_MODULE_OVERRIDE controls whether or not the backend
installs the default authentication provider module.  When this override
is enabled dynamic plugins can be used to supply custom authentication
providers.  This change also adds a "components" configuration for
frontend dynamic plugins, which can be used to supply overrides for the
AppComponents option.  This is required for dynamic plugins to be able to
provide a custom SignInPage component, for example:

```
dynamicPlugins:
  frontend:
    my-plugin-package:
      components:
        - name: SignInPage
          module: PluginRoot
          importName: SignInPage
```

Where the named export SignInPage will be mapped to
`components.SignInPage` when the frontend is initialized.  Finally, to
ensure authentication providers can be managed by the user a new
`providerSettings` configuration field is available for frontend dynamic
plugins, which can be used to inform the user settings page of the new
provider, for example:

```yaml
dynamicPlugins:
  frontend:
    my-plugin-package:
      providerSettings:
	- title: Github Two
	  description: Sign in with GitHub Org Two
	  provider: github-two
```

Each `providerSettings` item will be turned into a new row under the
"Authentication Providers" tab in the user settings.  The `provider`
field is used to look up the API ref for the external auth provider by
expanding the simple provider name to the auth ID convention
`core.auth.<provider name>`.

Signed-off-by: Stan Lewis <[email protected]>
Copy link
Contributor

@gashcrumb
Copy link
Member Author

gashcrumb commented Feb 10, 2025

Some feedback from @davidfestal that I will implement in the next day or so to help keep this future-proof against the new frontend system:

  • Rename "components" to "SignInPage" and remove "name" field, restricting this to the intended purpose
  • Look at not specifying apiFactories in the configuration for the examples, apiFactories should be discoverable from the plugin.
  • On that last point, revisit how API factories for external auth providers are discovered in the code
  • Use the complete API factory ID string in the config vs the simple provider name

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.

1 participant