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

ManagementWebSecurityConfigurerAdapter is not overridable #1901

Closed
paskos opened this issue Nov 11, 2014 · 27 comments
Closed

ManagementWebSecurityConfigurerAdapter is not overridable #1901

paskos opened this issue Nov 11, 2014 · 27 comments
Assignees
Labels
status: duplicate A duplicate of another issue type: enhancement A general enhancement

Comments

@paskos
Copy link

paskos commented Nov 11, 2014

Despite having

@ConditionalOnMissingBean({ ManagementWebSecurityConfigurerAdapter.class })
protected static class ManagementWebSecurityConfigurerAdapter

user cannot override this bean because it's impossible to create a bean of type ManagementWebSecurityConfigurerAdapter or extending it.

In my project we have multiple instance of AuthenticationManager and the default one injected in ManagementWebSecurityConfigurerAdapter is not the one we want to inject in it.

We don't want to completely recreate a WebSecurityConfigurerAdapterand copy/paste all its content just to inject a particular AuthenticationManager.

Thanks

@philwebb
Copy link
Member

@dsyer any suggestions? Should that condition change to WebSecurityConfigurerAdapter?

@dsyer
Copy link
Member

dsyer commented Nov 18, 2014

Not the latter (since there might be many). I think copy-paste of all the code in there is actually the best solution for now (and set management.security.enabled=false to let Boot know you want to do it yourself).

@dsyer dsyer added the question label Nov 18, 2014
@paskos
Copy link
Author

paskos commented Nov 18, 2014

an alternative is to make ManagementWebSecurityConfigurerAdapter public then @ConditionalOnMissingBean({ ManagementWebSecurityConfigurerAdapter.class }) would prevent the default configuration to be created because a bean of type ManagementWebSecurityConfigurerAdapter (ours) would replace it.

@dsyer
Copy link
Member

dsyer commented Nov 18, 2014

Yes, that is an alternative, but it makes something public that we might not want or need to be. A more useful solution would be to add callbacks/customizers for some of the more common management security concerns. In the meantime you can take control and roll it yourself.

@rakpan
Copy link

rakpan commented Apr 29, 2015

@dsyer is there any plan to allow custom behavior for Management security? In our case, we have a custom WebSecurityConfigurerAdapter which integrates with Spring Security SAML project. I have all the relevant roles/authorities in control through that custom config. Having another class for Management security seems to be an overhead.

@dsyer
Copy link
Member

dsyer commented Apr 29, 2015

I'm not sure I follow. If you want the management end points to have their own security (separate from the rest of the app), it kind of needs to be in a separate class. On, the other hand, if you want the same rules for management and other endpoints, then you already have something.

@rakpan
Copy link

rakpan commented Apr 30, 2015

@dsyer it is actual about the second scenario.. The only way it looks like to achieve thus is to have a dummy ManagementSecurityAutoConfiguration class with the endpoint security turned off.. Please let me know if this is wrong

@kakawait
Copy link
Contributor

+1 I have same requirement about overriding Actuator security (see related stackoverflow question http://stackoverflow.com/questions/33525037/overriding-spring-security-actuator-without-loosing-configurable-endpoints).

Today I'm:

I think copy-paste of all the code in there is actually the best solution for now

But it will be very appreciable to have some sort of configurer or public access to avoid copying source code.

@philwebb philwebb added the for: team-attention An issue we'd like other members of the team to review label Nov 24, 2015
@wilkinsona wilkinsona added team-meeting and removed for: team-attention An issue we'd like other members of the team to review labels Nov 25, 2015
@philwebb philwebb added for: team-attention An issue we'd like other members of the team to review and removed team-meeting labels Jan 6, 2016
@dsyer
Copy link
Member

dsyer commented Jan 27, 2016

We don't think opening up ManagementWebSecurityConfigurerAdapter to be directly extendable by users is a good idea, but we do think that the @ConditionalOnMissingBean might be useful. The proposal is to add a marker interface ManagementWebSecurityConfigurer and put that in the condition, so anyone that wants to can replace the existing configurer just by adding a bean definition.

@dsyer dsyer added type: enhancement A general enhancement and removed for: team-attention An issue we'd like other members of the team to review question labels Jan 27, 2016
@dsyer dsyer added this to the 1.4.0.M2 milestone Jan 27, 2016
@dsyer dsyer self-assigned this Jan 27, 2016
@ShijunK
Copy link

ShijunK commented Feb 18, 2016

@dsyer , it is fine that if Spring team does not want to open up management or the app web security configurer adapters. However, please do prioritize the @ConditionalOnMissingBean change. It will be clearer, customized security configurer replace default auto config completely. Instead of using @order trick. Thanks.

@philwebb philwebb modified the milestones: 1.4.0.M3, 1.4.0.M2 Apr 12, 2016
@philwebb philwebb modified the milestones: 1.4.0.RC1, 1.4.0.M3 May 17, 2016
@dsyer
Copy link
Member

dsyer commented Jun 13, 2016

There's already a @ConditionalOnProperty there (management.security.enabled) so I'm not sure I want to add the marker interface as well. The only benefit might be that if you want to switch it off you have to change the config properties. I don't see that as a huge burden, and it simplifies the security configuration options down to "standard" features (like an enabled flag).

@dsyer dsyer added the status: on-hold We can't start working on this issue yet label Jun 13, 2016
@philwebb philwebb removed this from the 1.4.0.RC1 milestone Jun 14, 2016
@ShijunK
Copy link

ShijunK commented Jun 21, 2016

what a pity, I don't think management.security.enabled is a good substitute to marker interface. It confuse people, when management.security.enabled=false means "hi, we have customized security config".

@dsyer
Copy link
Member

dsyer commented Jun 21, 2016

I don't think I follow that argument. My recommendation (which is reflected in docs I think) is not to disable the defaults anyway. You certainly shouldn't fee like you need to disable defaults just to add new rules. Maybe we can make it easier?

@ShijunK
Copy link

ShijunK commented Jul 1, 2016

@dsyer , could you please point to the docs section?

my original request is more specific, I want to use other authentication for management endpoints
#5172

yes, I can add customized security rule via WebSecurityConfigurerAdapter, but what about override for management end point? I don't think there is an option, because the ManagementWebSecurityConfigurerAdapter only check one property. On or off. Is there a way I can say "keep management.security.enabled, but use a different authentication besides HTTP basic"?

@dsyer
Copy link
Member

dsyer commented Jul 1, 2016

Is there a way I can say "keep management.security.enabled, but use a different authentication besides HTTP basic"?

Yes. You can add a WebSecurityConfigurerAdapter with a lower order than the default one. That's what it says you should do (http://docs.spring.io/spring-boot/docs/current-SNAPSHOT/reference/htmlsingle/#boot-features-security-actuator).

@ezraroi
Copy link

ezraroi commented Aug 6, 2016

@dsyer ,
I am trying to configure my own access rules over the management endpoints. I need to expose some of them (info and health) without any authentication and also get the full endpoint information. Looks that for this i need ti swutch the security off in the management.security.enabled, otherwise i am getting a summery of the health state.

Once I do this, looks like my own configuration is no longer valid and all management endpoints are accessible without any authentication
Adding my code:

@EnableWebSecurity
@EnableGlobalMethodSecurity(prePostEnabled = true)
@ConditionalOnProperty(prefix = "swiss.security.kerb", name = "enabled", matchIfMissing = true)
@Order(ManagementServerProperties.ACCESS_OVERRIDE_ORDER)
public class KerberosSecurityConfig extends WebSecurityConfigurerAdapter
{
    public static final int ACCESS_OVERRIDE_ORDER = ManagementServerProperties.ACCESS_OVERRIDE_ORDER - 1;

    @Inject
    protected void configureGlobal(AuthenticationManagerBuilder auth, KerberosAuthenticationProvider kerbProvider,
            KerberosServiceAuthenticationProvider kerbServiceProvider)
    {
        auth
                .authenticationProvider(kerbProvider)
                .authenticationProvider(kerbServiceProvider);
    }

    @Override
    protected void configure(HttpSecurity http) throws Exception
    {
        http.authorizeRequests()
                .antMatchers("/manage/health","/manage/info").permitAll()
                .antMatchers("/manage/*", "/debug/*").hasRole("ADMIN")
                .and()
//              .anonymous().disable()
                .csrf().disable().logout().disable()
                .authorizeRequests()
                .anyRequest().authenticated()
                .and()
                .exceptionHandling()
                .authenticationEntryPoint(new SpnegoEntryPoint())
                .and()
                .addFilter(getBasicAuthenticationFilter(authenticationManagerBean()))
                .addFilterBefore(
                        getSpnegoAuthenticationProcessingFilter(authenticationManagerBean()),
                        BasicAuthenticationFilter.class);
    }

    private BasicAuthenticationFilter getBasicAuthenticationFilter(AuthenticationManager authManager)
    {
        return new BasicAuthenticationFilter(authManager);
    }

    private SpnegoAuthenticationProcessingFilter getSpnegoAuthenticationProcessingFilter(AuthenticationManager authManager)
    {
        SpnegoAuthenticationProcessingFilter filter = new SpnegoAuthenticationProcessingFilter();
        filter.setFailureHandler((request, response, exception) ->
        {
            response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
            response.flushBuffer();
        });
        filter.setAuthenticationManager(authManager);
        return filter;
    }

}

@dsyer
Copy link
Member

dsyer commented Aug 6, 2016

There are a couple of odd things about that code, but it's quite hard to put it in context. How about putting a minimal sample on github that illustrates your question/confusion? (I suspect you are making a mistake somewhere but it's hard to say for sure.)

@ezraroi
Copy link

ezraroi commented Aug 6, 2016

@dsyer sure, will do so tomorrow, thanks

@ezraroi
Copy link

ezraroi commented Aug 7, 2016

Hi @dsyer ,
Check the following repo:
https://github.com/ezraroi/spring-security-issue

When you un comment the management.security.enbaled=false you get the following output on health endpoint:

{
"status":"UP","diskSpace":{"status":"UP","total":815336800256,"free":599872622592,"threshold":10485760}
}

The problem is that this also ignoring our security configuration for the manage endpoint.

But with the security enabled you will get only the overall status:

{"status":"UP"}

What we try to implement is to have the fully detailed response but still have our security configuration with the management endpoints.

Thanks

@ezraroi
Copy link

ezraroi commented Aug 21, 2016

Hi @dsyer. any chance you had time to look on this?

@dsyer
Copy link
Member

dsyer commented Aug 21, 2016

I haven't sorry, but the problem is probably easy to diagnose: you have to be fully authenticated to get the full endpoint response from /health (once spring security is present). We can't easily change that, and it certainly won't change in a bug fix release. Your only option is to authenticate those requests (it's not hard, just a bug of a weird thing to need to do).

@dsyer
Copy link
Member

dsyer commented Aug 21, 2016

I suppose the other option is to disable the health endpoint and write your own. Also easy.

@ezraroi
Copy link

ezraroi commented Aug 22, 2016

Thanks @dsyer

@ezraroi
Copy link

ezraroi commented Aug 22, 2016

Can we consider to enable to configure this in future releases?

@dsyer
Copy link
Member

dsyer commented Aug 22, 2016

We could, but the HealthMvcEndpoint explicitly needs the authenticated Principal to do the default logic, so we'd have to make that logic more complicated, and I'm not sure it's worth the extra complexity, given how easy it is to workaround.

@rainerfrey-inxmail
Copy link

rainerfrey-inxmail commented Dec 15, 2016

It would be great to have a callback to customize the authentication manager builder just for the management endpoints, without overriding the global authentication manager, and without overriding / replacing the (quite elaborate) HttpSecurity configuration for the management endpoints. Before I found this issue here, I asked this, needing exactly that: http://stackoverflow.com/questions/41140468/custom-actuator-authentication-and-different-custom-web-authentication-in-one

@wilkinsona
Copy link
Member

wilkinsona commented Sep 18, 2017

This has been superseded by the security simplifications in M4.

@wilkinsona wilkinsona removed this from the 2.0.0.M5 milestone Sep 18, 2017
@wilkinsona wilkinsona added status: duplicate A duplicate of another issue and removed status: on-hold We can't start working on this issue yet labels Sep 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

9 participants