Skip to content

Skip actuator security autoconfig if resource server autoconfig is active #15472

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

Closed
wants to merge 3 commits into from

Conversation

sdoxsee
Copy link
Contributor

@sdoxsee sdoxsee commented Dec 14, 2018

I had a working Spring Boot 2.1.0 OAuth2 resource server application (with autoconfig based on https://docs.spring.io/spring-boot/docs/current/reference/htmlsingle/#boot-features-security-oauth2-server) but wanted to add the actuator. What happened was that the actuator security configuration autoconfiguration occurred before the OAuth2ResourceServerAutoconfiguration resulting in my endpoints becoming Basic-protected rather than OAuth2-protected.

I noticed that actuator security configuration was skipped if OAuth2 client autoconfiguration exists and figured that since resource server was new in 2.1.x that adding detection for its security configuration was missed (didn't see any tests for that level of detail...nor the right way to add tests for these "composed starter" cases).

I realize that this OAuth2-protects ALL the actuator endpoints (including the normally open health and info endpoints) but, without autoconfiguration detecting and creating a hybrid security config, this seemed to be this simplest logical solution and was more in line with the docs:

If Spring Security is on the classpath and no other WebSecurityConfigurerAdapter is present, all actuators other than /health and /info are secured by Spring Boot auto-configuration. If you define a custom WebSecurityConfigurerAdapter, Spring Boot auto-configuration will back off and you will be in full control of actuator access rules.

https://docs.spring.io/spring-boot/docs/current/reference/htmlsingle/#boot-features-security-actuator

@pivotal-issuemaster
Copy link

@sdoxsee Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 14, 2018
@pivotal-issuemaster
Copy link

@sdoxsee Thank you for signing the Contributor License Agreement!

@mbhave mbhave added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 14, 2018
@mbhave mbhave added this to the 2.1.x milestone Dec 14, 2018
@snicoll
Copy link
Member

snicoll commented Dec 20, 2018

@mbhave since you've triaged this one I was wondering if there wasn't a test missing there. Thoughts?

@mbhave
Copy link
Contributor

mbhave commented Dec 21, 2018

I had the same thought but it didn't look like we write explicit tests for@AutoConfigureBefore. That being said, it would be good to test this.

@sdoxsee Would you mind updating the PR with a test that adds ReactiveOAuth2ResourceServerAutoConfiguration to the contextRunner in ManagementWebSecurityAutoConfigurationTests and checks that the endpoints are then protected by OAuth?

@sdoxsee
Copy link
Contributor Author

sdoxsee commented Dec 21, 2018

@mbhave I'll give it a try when I have a chance over the next couple of weeks. Thanks

@mbhave
Copy link
Contributor

mbhave commented Dec 21, 2018

@sdoxsee Thanks, no worries if you can't get to it. I can add the tests as part of the merge.

…OAuth2ResourceServerAutoConfiguration are protected with Bearer
@sdoxsee
Copy link
Contributor Author

sdoxsee commented Dec 22, 2018

@mbhave Please let me know if you think this needs better/more tests. Thanks!

@mbhave
Copy link
Contributor

mbhave commented Dec 31, 2018

Thanks @sdoxsee. I left a comment on the commit. Let me know what you think.

… ReactiveManagementWebSecurityAutoConfiguration and ManagementWebSecurityConfigurerAdapter when ReactiveOAuth2ResourceServerWebSecurityConfiguration and OAuth2ResourceServerWebSecurityConfiguration respectively should take priority
@sdoxsee
Copy link
Contributor Author

sdoxsee commented Jan 1, 2019

@mbhave Thanks for your suggestion. Pushed a commit to address it. Happy New Year!

mbhave added a commit that referenced this pull request Jan 2, 2019
* gh-15472:
  Polish "Order actuator security config after resource-server config"
  Order actuator security config after resource-server config
@mbhave mbhave closed this in 5b99060 Jan 2, 2019
@mbhave
Copy link
Contributor

mbhave commented Jan 2, 2019

Thanks @sdoxsee. This is now merged on master along with this polish commit.

@mbhave mbhave modified the milestones: 2.1.x, 2.1.2 Jan 2, 2019
@sdoxsee
Copy link
Contributor Author

sdoxsee commented Jan 2, 2019

Awesome. Thanks @mbhave

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants