Polishing security dsl

[pull-vert]

1. replace current 'security' DSL nested in 'webFlux' and 'webMvc' by 2 distinct new root DSLs : 'webFluxSecurity' and 'webMvcSecurity'. This allows to declare security in a security dedicated configuration file, without being forced to embed security inside web DSL as today (this was a poor decision of mine, sorry for that ;) ).

This is an example with the new version that shows that we can have all security stuff in a dedicated place (just replace with webMvcSecurity for MVC) :

val securityConfig = configuration {
    beans {
        bean<Argon2PasswordEncoder>()
        /* declare other useful beans for security here if needed */
    }

    webFluxSecurity {
        authenticationManager = repoAuthenticationManager

        http {
            authorizeExchange {
                authorize("/public-view", permitAll)
                authorize("/view", hasRole("USER"))
            }
            httpBasic {}
        }
    }
}

Then import this security configuration, together with others, in your main App DSL :

val app = reactiveWebApplication {
    enable(dataConfig)
    enable(securityConfig) // <- here it is
    enable(webConfig)
}

Two other changes are provided in this branch :

2. allow using ref<MyClass>() inside SecurityDsl, in real world application I needed it
3. add a new configuration property securityContextRepository in WebMVC and WebFlux security DSLs

[sdeleuze]

I am wondering if we could instead evolve Spring Fu to allow merging 2 webMvc or webFlux blocks. That would be a more natural API and would work with your use case. Any thoughts?

[pull-vert]

@sdeleuze Thanks for your reply

Indeed security is conceptually strongly linked to web, but I now tend to think their configuration is better separated.

This kind of security configuration below feels less clear for me, as it would allow (and users will be tempted to) add some web only config inside security config.

val securityConfig = configuration {
    webFlux {
        /* here user will be tempted to add some pure web stuff that has nothing to do with security */
        security {
            authenticationManager = repoAuthenticationManager

            http {
                authorizeExchange {
                    authorize("/public-view", permitAll)
                    authorize("/view", hasRole("USER"))
                }
                httpBasic {}
            }
        }
    }
}

[sdeleuze]

@pull-vert Ok I see what you mean, and indeed security is managed as a kind of distinct concerned than web, with http { } to do the link at mapping level. But what puzzles me is webFluxSecurity and webMvcSecurity names that look a smell. If we have a top level element, it should probably named security { }. Do you see a way to achieve that?

[pull-vert]

@sdeleuze With current implementation I don't see a solution, as both webFluxSecurity and webMvcSecurity are extension functions of ConfigurationDsl.

But a change can make this possible : make reactiveWebApplication expose a new ReactiveApplicationDsl scope, instead of the ApplicationDsl that is currently used by both reactive and blocking apps.

fun reactiveWebApplication(dsl: ReactiveApplicationDsl.() -> Unit) //...

ReactiveApplicationDsl would extend a new ReactiveConfigurationDsl. This change would also require to add new reactiveApplication and reactiveConfiguration DSLs for non web apps and sub-configurations :

fun reactiveApplication(dsl: ReactiveApplicationDsl.() -> Unit) //...

fun reactiveConfiguration(dsl: ReactiveConfigurationDsl.() -> Unit)

With these changes the ReactiveConfigurationDsl.security { } would be the Reactive version, and the ConfigurationDsl.security { } would be the MVC one.

Going further, it would also allow to have just web instead of both webFlux and webMVC, expose r2dbc only for reactive apps, and maybe more but these would be breaking changes for the current API.

edit : Current ConfigurationDsl would have to be renamed to AbstractConfigurationDsl. All current DSLs can continue to be extensions on it to ensure backward compatibility if needed, maybe with a deprecation notice. AbstractConfigurationDsl would have 2 distinct implementations : ConfigurationDsl and ReactiveConfigurationDsl

[sdeleuze]

Ok thanks I will have a deeper look based on your PR and feedback.

[sdeleuze]

@pull-vert What about using the type of the ApplicationContext (usually defined via https://github.com/spring-projects-experimental/spring-fu/blob/master/kofu/src/main/kotlin/org/springframework/fu/kofu/Kofu.kt) in order to decide what variant of the security initializer should be instantiated via a single security { } DSL?

[pull-vert]

@sdeleuze To expose 2 distinct security DSLs to user (with distinct types for every properties) at compile time I require 2 distinct types to extend.

Your solution (check the type of the ApplicationContext) will only be possible at runtime, or maybe I am missing a Kotlin possiblility I am not aware of.

[sdeleuze]

Yeah indeed the 2 DSLs are too differents to do that, so I am think I am back to preferring webFlux { security { ... } } and webMvc { security { ... } } + adding the possibility to merge multiple webFlux and webMvc blocks.

This kind of security configuration below feels less clear for me, as it would allow (and users will be tempted to) add some web only config inside security config.

I am not too concerned about that, its really up to users to decide and I am not convinced we would promote something error prone by doing that. And that's something that would have other use cases like:

val securityConfig = configuration {
    webFlux {
        security {
            authenticationManager = repoAuthenticationManager
            http {
                authorizeExchange {
                    authorize("/public-view", permitAll)
                    authorize("/view", hasRole("USER"))
                }
                httpBasic {}
            }
        }
    }
}

val routerConfig = configuration {
    webFlux {
        router {
            GET("/") { ... }
        }
    }
}

val convertersConfig = configuration {
    webFlux {
        converters {
            // ...
        }
    }
}

I think it deserves some time to converge toward a common solution so I move it to 0.5.0 since we have to release 0.4.3 tomorrow.

[pull-vert]

Yeah indeed the 2 DSLs are too differents to do that, so I am think I am back to preferring webFlux { security { ... } } and webMvc { security { ... } }

Thanks a lot for your answer, I pushed a new commit so that this branch could be integrated in a next release without the behavior change I first introduced.