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

HttpSecurityDsl should support apply method #11754

Closed
hanrw opened this issue Aug 26, 2022 · 4 comments
Closed

HttpSecurityDsl should support apply method #11754

hanrw opened this issue Aug 26, 2022 · 4 comments
Assignees
Labels
in: config An issue in spring-security-config type: enhancement A general enhancement
Milestone

Comments

@hanrw
Copy link
Contributor

hanrw commented Aug 26, 2022

Expected Behavior

@Bean
 fun authSecurityFilterChain(http: HttpSecurity): SecurityFilterChain {
     http {
         csrf {
             disable()
         }
         cors {
             disable()
         }
         apply(LoginFilterSecurityConfigurer()) {
             jwtLogin {
                 
             }
         }
     }
     return http.build()
 }

Current Behavior

@Bean
   fun authSecurityFilterChain(http: HttpSecurity): SecurityFilterChain {
       http {
           csrf {
               disable()
           }
           cors {
               disable()
           }
           http.apply(LoginFilterSecurityConfigurer()).jwtLogin {
           }
       }
       return http.build()
   }

The apply keyword will conflict with Kotlin fun

public inline fun <T> T.apply(block: T.() -> Unit): T {
    contract {
        callsInPlace(block, InvocationKind.EXACTLY_ONCE)
    }
    block()
    return this
}
@hanrw hanrw added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Aug 26, 2022
@sjohnr sjohnr self-assigned this Aug 29, 2022
@sjohnr sjohnr added in: config An issue in spring-security-config and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 29, 2022
@sjohnr
Copy link
Member

sjohnr commented Aug 29, 2022

@hanrw thanks for the suggestion!

The apply keyword will conflict with Kotlin fun

I'm not 100% sure but it seems like it will take different method parameters so it shouldn't conflict, right?

I think this would be a nice addition, so we don't need to mix a Java and Kotlin approach when using the DSL. Would you like to submit a PR?

@rishiraj88
Copy link
Contributor

Is 'apply' onsidered a keyword here?

@hanrw
Copy link
Contributor Author

hanrw commented Aug 30, 2022

@hanrw thanks for the suggestion!

The apply keyword will conflict with Kotlin fun

I'm not 100% sure but it seems like it will take different method parameters so it shouldn't conflict, right?

I think this would be a nice addition, so we don't need to mix a Java and Kotlin approach when using the DSL. Would you like to submit a PR?
The solution could be add apply function in HttpSecurityDsl

    fun <C : SecurityConfigurerAdapter<DefaultSecurityFilterChain, HttpSecurity>> apply(configurer: C): C {
        return this.http.apply(configurer)
    }

Any suggestion?

@sjohnr
Copy link
Member

sjohnr commented Aug 30, 2022

Hi @hanrw!

Any suggestion?

I think that looks like a good start. Just a heads up that if you do submit a PR, we may take a pause on merging it as most new enhancements that will make it into the next release are already scheduled.

Is 'apply' onsidered a keyword here?

I don't believe so, @rishiraj88. I tried something like what @hanrw provided above and it compiles fine as far as I can tell. It would just be an overloaded version of the apply method since it takes different parameters.

hanrw added a commit to hanrw/spring-security that referenced this issue Aug 31, 2022
hanrw added a commit to hanrw/spring-security that referenced this issue Sep 7, 2022
hanrw added a commit to hanrw/spring-security that referenced this issue Sep 14, 2022
@sjohnr sjohnr closed this as completed in 45bbd86 Sep 14, 2022
@sjohnr sjohnr added this to the 5.8.0-M3 milestone Sep 14, 2022
@sjohnr sjohnr assigned hanrw and unassigned sjohnr Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: config An issue in spring-security-config type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants