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

Make RequiredActions configurable #28400

Closed
thomasdarimont opened this issue Apr 3, 2024 · 7 comments · Fixed by #28419
Closed

Make RequiredActions configurable #28400

thomasdarimont opened this issue Apr 3, 2024 · 7 comments · Fixed by #28419
Labels

Comments

@thomasdarimont
Copy link
Contributor

thomasdarimont commented Apr 3, 2024

Description

This feature request asks to add support for configurable properties to RequiredActions.
Currently support for configuration of required actions via the admin ui are quite limited.
The only allowed configuration options are:

  • enabled (is the required action enabled at all?)
  • default (is the required action set for new users by default?)
  • order / priority (controls the order in which required actions is presented to the user)

It would be helpful to allow developers to specify additional configurable properties org.keycloak.provider.ProviderConfigProperty for required actions, similar to how authenticator executions can be configured.

Discussion

No response

Motivation

Behaviour for required actions should be configurable. Currently one has to pull configuration from realm attributes, env variables or other means.
Having official support for configuration would allow us to have a common way to configure actions.

Details

Implementation ideas

Configuration options

There are multiple ways to implement support for configuration options for required actions, here are 2 options:

Option 1

Leverage org.keycloak.provider.ProviderFactory#getConfigMetadata()
Advantages:

  • easier, as it is already available API

Disadvantages:

  • no default config
  • no help text

Option 2

Make required actions infrastructure aware of org.keycloak.provider.ConfiguredProvider
Advantages:

  • more options (help text, default config)
  • Developers could opt-in to make their required actions configurable by implementing the ConfiguredProvider interface.

Disadvantages:

  • a bit more work
  • 2 different ways to define config properties (ProviderFactory#getConfigMetadata() vs. ConfiguredProvider#getConfigProperties())

I currently lean toward Option 1.

Required Action Configuration storage

The required actions infrastructure already supports the persistence of configuration metadata via
org.keycloak.models.jpa.entities.RequiredActionProviderEntity#config in the REQUIRED_ACTION_CONFIG table.

REST API extensions in AuthenticationManagementResource

The AuthenticationManagementResource already supports the management of required actions. It seems as the natural place to add support for required action config management.

I propose to add the following new endpoints to manage required action configuration within a realm bound to the required action "alias" / provider id.

  @Path("required-actions/{alias}/config-description")
  @GET
  @Produces(MediaType.APPLICATION_JSON)
  RequiredActionConfigInfoRepresentation getRequiredActionConfigDescription(@PathParam("alias") String alias);

  @Path("required-actions/{alias}/config")
  @GET
  @Produces(MediaType.APPLICATION_JSON)
  RequiredActionConfigRepresentation getRequiredActionConfig(@PathParam("alias") String alias);

  @Path("required-actions/{alias}/config")
  @DELETE
  void removeRequiredActionConfig(@PathParam("alias") String alias);

  @Path("required-actions/{alias}/config")
  @PUT
  @Consumes(MediaType.APPLICATION_JSON)
  void updateRequiredActionConfig(@PathParam("alias") String alias, RequiredActionConfigRepresentation rep);

Additional work

  • Introduce RequiredActionConfigModel
  • Extend RequiredActionContext to expose RequiredActionConfigModel
  • Extend admin-client libraries (jakarta, javaee, js/ts)
  • Extend admin ui (required action list with config button and required action config form)
  • Update documentation
  • Update tests (REST API, UI tests)
@thomasdarimont thomasdarimont added kind/feature Categorizes a PR related to a new feature status/triage labels Apr 3, 2024
thomasdarimont added a commit to thomasdarimont/keycloak that referenced this issue Apr 8, 2024
- Add tests for crud operations on configurable required actions
- Add support exposing the required action configuration via RequiredActionContext

Fixes keycloak#28400

Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>
@danielFesenmeyer
Copy link
Contributor

@thomasdarimont One question regarding order/priority - there is this open issue #16873 about how the defined order/priority affects should affect the order/priority in the user flow. Is this also part of this task?
Just asking because I'd like to implement a PR for mentioned issue.

@thomasdarimont
Copy link
Contributor Author

The ordering is currently not covered by this effort, but could be a next step. The order/priority could be a default configurable property for required actions, once they are configurable.

I'll take a look at the issue and see if it makes sense to add it now.

From what I've seen adjusting the ordering of required actions is not trivial, as there are multiple ways how those actions can be ordered (global order / priority, required actions on user level and required actions on authentication session level).

I think we need a clear picture of how the required action ordering should work in all those cases.

@danielFesenmeyer
Copy link
Contributor

@thomasdarimont Ok, let me know when I can assist.

@thomasdarimont
Copy link
Contributor Author

You could take a look the current PR for the configurable required actions and give it a try :)

Also if you have ideas for how the ordering should work in all cases I'm all ears :)

@danielFesenmeyer
Copy link
Contributor

Cool, I'll take a look at it.

@danielFesenmeyer
Copy link
Contributor

@thomasdarimont I've build a draft PR, based on your PR. Found that it could simply be rebased on keycloak main, so I've shared it here: #28956

@thomasdarimont
Copy link
Contributor Author

thomasdarimont commented Apr 22, 2024

@edewit I wanted to add support for help text to required actions and took some inspiration from the help icon for passwordpolicies. Could you tell me what would be the proper components to use?

Currently it looks like that:
image

However, I only want the normal text and the clickable help icon. When a user clicks on the icon the help text should appear like it does for password policies.

This is the current column definition for name:

{
            name: "name",
            displayKey: "action",
            width: 50,
            cellRenderer: (row: Row) => (
              <Text id={`label-${toKey(row.name)}`}>
                {t(`${row.data.providerId}`)}{" "}
                <HelpItem
                  helpText={t(`${row.data.helpText}`)}
                  fieldLabelId={`label-${toKey(row.name)}`}
                />
              </Text>
            ),
          },

thomasdarimont added a commit to thomasdarimont/keycloak that referenced this issue May 22, 2024
- Add tests for crud operations on configurable required actions
- Add support exposing the required action configuration via RequiredActionContext
- Make configSaveError message reusable in other contexts
- Introduced admin-ui specific endpoint for retrieving required actions with config metadata

Fixes keycloak#28400

Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>
Co-authored-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
mposolda pushed a commit that referenced this issue May 23, 2024
- Add tests for crud operations on configurable required actions
- Add support exposing the required action configuration via RequiredActionContext
- Make configSaveError message reusable in other contexts
- Introduced admin-ui specific endpoint for retrieving required actions with config metadata

Fixes #28400

Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>
Co-authored-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants