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

Implement realm otp, webauthn, webauthn passwordless and bruteforce properties #312

Merged
merged 9 commits into from
Jun 19, 2024
Merged

Implement realm otp, webauthn, webauthn passwordless and bruteforce properties #312

merged 9 commits into from
Jun 19, 2024

Conversation

TuningYourCode
Copy link
Contributor

@TuningYourCode TuningYourCode commented Jun 11, 2024

This PR implements all webauthn and bruteforce properties on a realm supported by at least Keycloak 22.0.0.

WebAuthn Extra Origins and passwordless WebAuthn properties are not yet supported as these were introduced in later Keycloak versions.

@TuningYourCode
Copy link
Contributor Author

@treydock do you know what i did wrong or if there is some kind of debug possibility?

Properties seem to work in my own builds so i don't get why acceptance tests fail.

@treydock
Copy link
Owner

The Keycloak 24.0.3 failures are the following:

       	Error: Parameter web_authn_policy_authenticator_attachment failed on Keycloak_realm[test]: Invalid value "cross-platform". Valid values are plattform, cross-plattform, not specified. (file: /tmp/apply_manifest_151227864.pp.V1dPa1, line: 2)
     Failure/Error: expect(data['rememberMe']).to eq(true)
       
       expected: true
            got: false
     Failure/Error: expect(names).to eq(['profile'])
       
       expected: ["profile"]
            got: ["role_list", "profile", "acr", "web-origins", "roles", "email"]
     Failure/Error: expect(data['eventsEnabled']).to eq(true)
       
       expected: true
            got: false
     Failure/Error: expect(expected_roles - realm_roles).to eq([])
       
       expected: []
            got: ["other_new_role"]

@treydock
Copy link
Owner

For Keycloak 22.0.0 failures, it looks like something is configured in such a way to cause Keycloak to crash.

@TuningYourCode
Copy link
Contributor Author

Thanks, despite the typo i found the issue. It's related to the old version the tests are running against.

e.g. with KeyCloak 23.0.0 introduced the WebAuthn Extra Origins property (https://www.keycloak.org/docs/23.0.0/release_notes/index.html#webauthn-improvements) which was present in my initial PR. It was working in our setup as we already run KeyCloak version 24.0.5.

So i just dropped the properties which are not supported by the tested KeyCloak versions (i anyway don't need them for now)

@treydock
Copy link
Owner

You can only add these new properties that work with 24.x when that's the version being tested:

let(:new_properties) do
  if RSpec.configuration.keycloak_version.split('.').first.to_i >= 24
    [
       "key => 'value'",
       "anotherkey => 'anothervalue'",
    ]
  else
    []
  end
end

Then in the Puppet code for acceptance tests can do like:

#{new_properties.join("\n")}

I'd be fine merging with the acceptance test lines for 24.x properties commented out and work later to integrate them or just drop 22.x tests and uncomment them later.

Also need to include new properties in unit tests: https://github.com/treydock/puppet-module-keycloak/blob/master/spec/unit/puppet/type/keycloak_realm_spec.rb

@TuningYourCode TuningYourCode changed the title Implement realm webauthn and bruteforce properties Implement realm otp, webauthn, webauthn passwordless and bruteforce properties Jun 17, 2024
@TuningYourCode
Copy link
Contributor Author

From my point of view this PR should be ready for merging now. This PR only includes settings present in at least KeyCloak 22.0.0.

There might be "breaking changes" if somebody is using custom_properties for properties now implemented in the resource. Not sure if you consider them to be "breaking changes" that this PR has to wait for a major release.

I also created a seperate PR (#313) for raising minimal KeyCloak to version 23.0.7 (which is out of support if not the RedHat build is used - see: https://www.keycloak.org/security.html) which might definitly be a "breaking change" for a major puppet module version.

@treydock treydock merged commit a627fa4 into treydock:master Jun 19, 2024
28 checks passed
@treydock
Copy link
Owner

treydock commented Jun 19, 2024

This will be released as 11.2.0 once Github Actions complete

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