Provide fluent API to set up SecurityIdentityAugmentors and IdentityProviders such as Security JPA programmatically

[michalvavrik]

• part of the Provide a fluent API to set up Quarkus Security #16728 effort
• in relation to the https://quarkus.io/guides/security-jpa there are only following use cases:
  • you have multiple mechanisms, like form and basic and you want them to use different persistence unit or even one of them not using JPA-based identity providers
  • you are creating your own mechanism like new CustomHttpAuthenticationMechanism() and you want it to only use JPA Providers while there are other UsernamepasswordRequest identity providers
  • you just insist on doing everything programmatically

[github-actions]

🎊 PR Preview 3f428b9 has been successfully built and deployed to https://quarkus-pr-main-51279-preview.surge.sh/version/main/guides/

• Images of blog posts older than 3 months are not available.
• Newsletters older than 3 months are not available.

[quarkus-bot]

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 86e5b6a.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

[sberyozkin]

What is not clear to me is why these identity providers and augmentors must be used instead of CDI registered identity providers and augmentors, while the mechanism is not meant to be used instead of other mechanisms.

[michalvavrik]

Before I answer, I would like to summarize so that we speak about the same thing:

• this method adds a new mechanism (like new CustomAuthMechanism or any other mechanism at all) and allows you to use your own identity providers and augmentors
• if you want to use global identity providers or augmentors, then don't use it
• if you want to use global identity providers/augmentors and some other providers/augmentors, specific for only this mechanism, you can create collection with global identity providers/augmentors and your own, specific for only this mechanism; nothing stops you

What is not clear to me is why these identity providers and augmentors must be used instead of CDI registered identity providers and augmentors, while the mechanism is not meant to be used instead of other mechanisms.

• how many times do you want to use both identity providers registered as CDI beans and dedicated ones for this mechanism? I'd say you usually need one or two
• this methods facilitates programmatic setup for Quarkus Security JPA, if with httpSecurity.basic() we already use Quarkus Security JPA from CDI, then why would we need the programmatic setup at all? Only answer I could come with is flexibility - unlike CDI providers, which are there for every mechanism, here you can explicitly select which provider and augmentors you want to apply

[sberyozkin]

Well, CDI registered augmentor may be used with any mechanism.

I think it would make sense not to mix augmentors and identity providers in such methods.

I'd also consider providing a generic option for adding identity providers without focusing on JPA first, as part of the fluent API, where an option to add an identity provider becomes available only after a method registering a custom mechanism is chosen...

IMHO it should be a draft PR

[michalvavrik]

Well, CDI registered augmentor may be used with any mechanism.

I think it would make sense not to mix augmentors and identity providers in such methods.
I'd also consider providing a generic option for adding identity providers without focusing on JPA first, as part of the fluent API, where an option to add an identity provider becomes available only after a method registering a custom mechanism is chosen...

I wrote it based on this comment quarkusio/quarkus-security#76 (comment) which suggests this:

Mechanism -> Identity Provider(s) -> Augmentors(s) or Mechanism -> Augmentors(s)

and that is the hierarchy introduced here (except I chose one of tem - I don't order augmentors under identity providers, it would only bring work and no pros I know about).

whether we write it like:

httpSecurity.mechanism(form, jpa)

as my tests do, or we do something like:

httpSecurity.buildMechanism(form).identityProvider(jpa).augmentor(customAugmentor)

is just matter of finding the right name for that HttpSecurity method. I put it into one httpSecurity.mechanism method because I am not aware of a good name (we cannot do httpSecurity.mechanism(form).augmentor... because the return type is HttpSecurity). I don't mind rewriting it if you have a suggestion.

IMHO it should be a draft PR

I'll make it draft right now. Drafts make only sense when there is something to work on. Let's establish what is it.

[michalvavrik]

Well, CDI registered augmentor may be used with any mechanism.

Sure. What do you want to say? This PR gives you 3 options:

• use CDI augmentors if you don't configure augmentors via HttpSecurity
• use your own augmentors, whether you create instance or inject them
• use your own augmentors and CDI augmentors. Nothing stops you to do new ArrayList<>() then addAll(cdiAugmentors) and addAll(customAugmentors)

[sberyozkin]

basic(jpa().persistence("abc")) looks ok too, so the basic override will take IdentityProvider ? That should be fine and perhaps we can fail early if the identity provider's request type is not UsernamePasswordAuthenticationRequest

[michalvavrik]

basic(jpa().persistence("abc")) looks ok too, so the basic override will take IdentityProvider ? That should be fine and perhaps we can fail early if the identity provider's request type is not UsernamePasswordAuthenticationRequest

I think it will take some other trick, it doesn't make sense to have different jpa() static method for form and different for basic auth mechanism. I am starting working on this right now.

[sberyozkin]

@michalvavrik

Sure, perhaps IdentityProvider<UserNamePasswordAuthenticationRequest> = jpa().persistence("abc") must be created independently, and it should be basic(jpaIdentityProvider), and same for form, and both basic and form accepting only IdentityProvider<UserNamePasswordAuthenticationRequest>...

[michalvavrik]

@sberyozkin I think I know what you mean, but:

1. form-based auth mechanism also needs IdentityProvider<TrustedAuthenticationRequest>, so I planned to hide that from users since you are right it doesn't make sense to create basic(jpa()) with the IdentityProvider<TrustedAuthenticationRequest>, but we can't have 2 jpa() methods returning 2 different types; ---> I have some proposal for that, I'll show you when I push it. Probably tomorrow.
2. jpa().persistence("abc") is behind why I keep rewriting it again and again, and I am starting to think that this PR was better without persistence("abc"). The thing is, the persistence("abc") is determined by the package where user definition Hibernate entity is defined in. So, basically, we should be able to infer the persistence unit during the build (Hibernate knows which entity belongs to which PU), because currently, you cannot have 2 entities holding the identity information. I would rather just drop quarkus.security-jpa.persistence-unit-name and do the inference for user OOTB (I am almost sure it is possible, because whenever I tried to set different unit, validation failed on the Hibernate side). But that must be handled separately. I'd start with basic(jpa()) only.

Anyway, thanks for the input, I'll produce something soon (tomorrow?) and then we can discuss it based on the actual code.

[michalvavrik]

I have opened #51480.

[michalvavrik]

I run the OIDC wiremock IT Module tests and they are passing for me locally.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 86e5b6a.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.