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

nixos/keycloak: add realmImportsDirectory config #206729

Closed
wants to merge 1 commit into from

Conversation

costrouc
Copy link
Member

@costrouc costrouc commented Dec 18, 2022

Description of changes

Adding an option to the keycloak nixos module that allows for importing/updating realms via nixos configuration. This uses https://www.keycloak.org/server/importExport in keycloak.

Things done

Added option services.keycloak.realmImportsDirectory

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Dec 18, 2022
@costrouc costrouc force-pushed the keycloak-realm-imports branch 3 times, most recently from a2f2459 to 076d83a Compare December 18, 2022 17:53
@costrouc
Copy link
Member Author

costrouc commented Dec 18, 2022

Ready for review. I have tested this out locally.

When the realm configuration files are large I'm seeing an error but this seems to be this related to this issue keycloak/keycloak#14733 but is not an issue with this specific PR.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Dec 18, 2022
@costrouc costrouc requested a review from talyz December 18, 2022 18:08
@costrouc costrouc force-pushed the keycloak-realm-imports branch from 076d83a to 9eead0e Compare December 19, 2022 02:26
@talyz
Copy link
Contributor

talyz commented Dec 19, 2022

Hi! Thanks for working on this.

One issue I can see arising from this is file/directory permissions - the service runs with DynamicUser set, so there's no globally available user to assign file ownership. We don't want to force the user to make the files world-readable, since they may contain sensitive data.

A solution could be to treat them the same way we already do ssl certificates / keys and create LoadCredential entries for all realms. This would change the option type to a list of paths to single-file json realm specifications that we could then copy to data/import and start Keycloak with --import-realm. This would also mean that the realms wouldn't be overridden on every start, which is probably more useful, since this feature mostly seems like a "restore from a backup" thing you'd want to do once.

Additionally, this should be added to the tests.

@costrouc
Copy link
Member Author

costrouc commented Dec 19, 2022

@talyz thanks for getting back!

One issue I can see arising from this is file/directory permissions - the service runs with DynamicUser set, so there's no globally available user to assign file ownership. We don't want to force the user to make the files world-readable, since they may contain sensitive data.

This isn't in the PR but keycloak supports environment variables within the json files see https://www.keycloak.org/server/importExport in the Using Environment Variables within the Realm Configuration Files section. If we had a way to pass environment variables to the systemd start. Would this be worth including the in the PR?

This would also mean that the realms wouldn't be overridden on every start, which is probably more useful, since this feature mostly seems like a "restore from a backup" thing you'd want to do once.

For me personally I like the idea of it overriding every time since this would allow me to declaratively manage users/groups/clients etc but I definitely see how only importing if the realm doesn't exist is valuable (and probably preferred by most). Maybe I add an option for override true/false?

Additionally, this should be added to the tests.

Will work on this once we have the scope down.

@talyz
Copy link
Contributor

talyz commented Dec 19, 2022

This isn't in the PR but keycloak supports environment variables within the json files see https://www.keycloak.org/server/importExport in the Using Environment Variables within the Realm Configuration Files section. If we had a way to pass environment variables to the systemd start. Would this be worth including the in the PR?

I suppose this could be done with systemd.services.keycloak.environment, but I'm not sure how useful it is in practice.

For me personally I like the idea of it overriding every time since this would allow me to declaratively manage users/groups/clients etc but I definitely see how only importing if the realm doesn't exist is valuable (and probably preferred by most). Maybe I add an option for override true/false?

I would love to support declarative realms as well, but in my mind, that would entail having a settings-like interface where you can write the realm spec in nix. The json format keycloak uses doesn't feel like it's made to be written manually (it's not documented, afaik, and I'm not sure how stable it is). I think you'd need to use the API to get the level of simplicity required and to be able to get a system where a part of a realm is declarative and another isn't - if a user changes their password, we don't want to reset it when the server restarts, for example.

@costrouc
Copy link
Member Author

costrouc commented Dec 19, 2022

I would love to support declarative realms as well, but in my mind, that would entail having a settings-like interface where you can write the realm spec in nix.

If I reworked this PR in this way how would you feel? Last time I added a bunch of nixos options there was some pushback due to the complexity. I would be happy to add this similar to how it was done for grafana https://github.com/nixos/nixpkgs/blob/nixpkgs-unstable/nixos/modules/services/monitoring/grafana.nix.

The specification is reasonably easy to find with an export. Here is an example with some of the options.

let keycloak-realms = pkgs.runCommand "keycloak-realms" {} ''
      mkdir -p $out
      echo '${builtins.toJSON example-realm}' > $out/example-realm.json
   '';

    # https://github.com/keycloak/keycloak-demo/blob/master/demo-realm.json
    example-realm = {
      realm = "demo1";
      enabled = true;

      users = [{
        username = "costrouc";
        enabled = true;
        firstName = "Chris";
        lastName = "Ostrouchov";
        credentials = [{
          type = "password";
          value = "test";
        }];
        realmRoles = [
          "user"
        ];
      }];

      groups = [{
        name = "admin-group";
        path = "/admin-group";
        attributes = {
          key = ["value1" "value2"];
        };
        realmRoles = [];
      }];

      roles = {
        realm = [
          {
            name = "user";
            description = "User privileges";
            attributes = {};
          }
          {
            name = "admin";
            description = "Administrator privileges";
            attributes = {};
          }
        ];
      };

      defaultRoles = [
        "user"
      ];

      clients = [
        {
          clientId = "conda-store";
          enabled = true;
          publicClient = false;
          serviceAccountsEnabled = true;
          redirectUris = [
            "https://example.com/callback"
          ];
          webOrigins = [
            "https://example.com"
          ];
        }
        {
          clientId = "demo-new-one";
          enabled = true;
          publicClient = false;
          clientAuthenticatorType = "client-secret";
          secret = "mysupersecret";
          redirectUris = [
            "https://demo-new-one.com/callback"
          ];
        }
      ];
    };
in {
services.keycloak.realmImportsDirectory = "${keycloak-realm}";
```}

@costrouc costrouc closed this Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants