Skip to content

Added support for Anonymous Authentication #6198

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 12, 2018
Merged

Added support for Anonymous Authentication #6198

merged 1 commit into from
Dec 12, 2018

Conversation

ankurpathak
Copy link
Contributor

  1. Created new WebFilter AnonymousAuthenticationWebFilter to
    for anonymous authentication
  2. Created class AnonymousSpec, method anonymous to configure
    anonymous authentication in ServerHttpSecurity
  3. Added ANONYMOUS_AUTHENTICATION order after AUTHENTICATION for
    anonymous authentication in SecurityWebFiltersOrder

Pull request for github issue:
#5934

@ankurpathak ankurpathak mentioned this pull request Nov 30, 2018
Closed
@ankurpathak ankurpathak force-pushed the gh-5934-fix branch 2 times, most recently from c983bc4 to f9e8b81 Compare December 1, 2018 07:12
rwinch
rwinch requested changes Dec 3, 2018
View reviewed changes
Copy link
Member

@rwinch rwinch left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @ankurpathak! I have provided feedback inline. Can you als please add tests for this?

config/src/main/java/org/springframework/security/config/web/server/ServerHttpSecurity.java Outdated
* public SecurityWebFilterChain springSecurityFilterChain(ServerHttpSecurity http) {
* http
* // ...
* .csrf().key(key)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix this Javadoc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fixed like this:

/**
	 * @since 5.2.0
	 * @author Ankur Pathak
	 * Configures annonymous authentication
	 *
	 * <pre class="code">
	 *  &#064;Bean
	 *  public SecurityWebFilterChain springSecurityFilterChain(ServerHttpSecurity http) {
	 *      http
	 *          // ...
	 *          .anonymous.key("key")
	 *          .authorities("ROLE_ANONYMOUS");
	 *      return http.build();
	 *  }
	 * </pre>
	 */

...org/springframework/security/web/server/authentication/AnonymousAuthenticationWebFilter.java Outdated
@Override
public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
Context context = ReactiveSecurityContextHolder.getContext()
.filter(Objects::isNull)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you will ever get a null value here since it would just be empty.

...org/springframework/security/web/server/authentication/AnonymousAuthenticationWebFilter.java Outdated
return securityContext;
}))
.as(ReactiveSecurityContextHolder::withSecurityContext);
return Mono.subscriberContext()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should only set the subscriberContext if it changes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it this way. But lots of tests are failing lile RequestCacheTests etc are failing. I have tried almost every thing:

@Override
	public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {

		return ReactiveSecurityContextHolder.getContext()
				.switchIfEmpty(Mono.defer(() -> {
						SecurityContext securityContext = new SecurityContextImpl();
						securityContext.setAuthentication(createAuthentication(exchange));
						return chain.filter(exchange)
								.subscriberContext(ReactiveSecurityContextHolder.withSecurityContext(Mono.just(securityContext)))
								.then(Mono.empty());
				})).flatMap(securityContext -> chain.filter(exchange));

	}

I have written Test for it like this and this test is wrking fine:

@Test
	public void filterWhenDefaultsAndNoAuthenticationThenContinues() {

		WebTestClient client = WebTestClient.bindToController(HttpMeController.class)
				.webFilter(new AnonymousAuthenticationWebFilter(UUID.randomUUID().toString()))
				.argumentResolvers(config -> {
				})
				.build();

		 client.get()
				.uri("/me")
				.exchange()
				.expectStatus().isOk()
				.expectBody(String.class).isEqualTo("anonymousUser");
	}

	@RestController
	@RequestMapping("/me")
	public static class HttpMeController {
		@GetMapping
		public Mono<String> me(ServerWebExchange exchange){
			return ReactiveSecurityContextHolder
					.getContext()
					.map(SecurityContext::getAuthentication)
					.map(Authentication::getPrincipal)
					.ofType(String.class);
		}
	}

This test is working fine. I don't know where i am going wrong. Can you please help??

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please push the code and point me to the tests that are failing?

Copy link
Contributor Author

@ankurpathak ankurpathak Dec 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I have pushed my changes.
  2. Here is a list of failing tests:
org.springframework.security.config.web.server.RequestCacheTests > requestCacheNoOp FAILED
    org.junit.ComparisonFailure at RequestCacheTests.java:96

org.springframework.security.config.web.server.RequestCacheTests > defaultFormLoginRequestCache FAILED
    org.junit.ComparisonFailure at RequestCacheTests.java:64

org.springframework.security.config.web.server.OAuth2LoginTests > defaultLoginPageWithMultipleClientRegistrationsThenLinks FAILED
    org.junit.ComparisonFailure at OAuth2LoginTests.java:89

org.springframework.security.config.web.server.OAuth2LoginTests > defaultLoginPageWithSingleClientRegistrationThenRedirect FAILED
    java.lang.AssertionError at OAuth2LoginTests.java:123

org.springframework.security.config.web.server.FormLoginTests > defaultLoginPage FAILED
    org.junit.ComparisonFailure at FormLoginTests.java:69

org.springframework.security.config.web.server.FormLoginTests > customLoginPage FAILED
    org.junit.ComparisonFailure at FormLoginTests.java:115

org.springframework.security.config.web.server.LogoutSpecTests > customLogout FAILED
    org.junit.ComparisonFailure at LogoutSpecTests.java:99

org.springframework.security.config.web.server.LogoutSpecTests > defaultLogout FAILED
    org.junit.ComparisonFailure at LogoutSpecTests.java:54

org.springframework.security.config.web.server.AuthorizeExchangeSpecTests > antMatchersWhenMethodAndPatternsThenDiscriminatesByMethod FAILED
    java.lang.AssertionError at AuthorizeExchangeSpecTests.java:55
        Caused by: java.lang.AssertionError at AuthorizeExchangeSpecTests.java:55

org.springframework.security.config.web.server.AuthorizeExchangeSpecTests > antMatchersWhenPatternsThenAnyMethod FAILED
    java.lang.AssertionError at AuthorizeExchangeSpecTests.java:77
        Caused by: java.lang.AssertionError at AuthorizeExchangeSpecTests.java:77

org.springframework.security.config.web.server.ExceptionHandlingSpecTests > defaultAuthenticationEntryPoint FAILED
    java.lang.AssertionError at ExceptionHandlingSpecTests.java:55
        Caused by: java.lang.AssertionError at ExceptionHandlingSpecTests.java:55

org.springframework.security.config.web.server.ExceptionHandlingSpecTests > customAuthenticationEntryPoint FAILED
    java.lang.AssertionError at ExceptionHandlingSpecTests.java:79
        Caused by: java.lang.AssertionError at ExceptionHandlingSpecTests.java:79

org.springframework.security.config.web.server.ServerHttpSecurityTests > basicWhenNoCredentialsThenUnauthorized FAILED
    java.lang.AssertionError at ServerHttpSecurityTests.java:138
        Caused by: java.lang.AssertionError at ServerHttpSecurityTests.java:138

org.springframework.security.config.web.server.ServerHttpSecurityTests > defaults FAILED
    java.lang.IllegalStateException at ServerHttpSecurityTests.java:96

org.springframework.security.config.web.server.OAuth2ResourceServerSpecTests > getWhenCustomBearerTokenEntryPointThenResponds FAILED
    java.lang.AssertionError at OAuth2ResourceServerSpecTests.java:260
        Caused by: java.lang.AssertionError at OAuth2ResourceServerSpecTests.java:260

org.springframework.security.config.annotation.web.reactive.EnableWebFluxSecurityTests > defaultMediaAllThenUnAuthorized FAILED
    java.lang.AssertionError at EnableWebFluxSecurityTests.java:113
        Caused by: java.lang.AssertionError at EnableWebFluxSecurityTests.java:113

org.springframework.security.config.annotation.web.reactive.EnableWebFluxSecurityTests > defaultRequiresAuthentication FAILED
    java.lang.AssertionError at EnableWebFluxSecurityTests.java:96
        Caused by: java.lang.AssertionError at EnableWebFluxSecurityTests.java:96

org.springframework.security.config.annotation.web.reactive.EnableWebFluxSecurityTests > multiWorks FAILED
    java.lang.AssertionError at EnableWebFluxSecurityTests.java:298
        Caused by: java.lang.AssertionError at EnableWebFluxSecurityTests.java:298

@rwinch rwinch added the status: waiting-for-feedback We need additional information before we can continue label Dec 3, 2018
@rwinch
Copy link
Member

rwinch commented Dec 5, 2018 •
edited
Loading

At the moment the biggest problem seems to be that the default behavior has changed. We should require users to explicitly opt into anonymous authentication as making it a default is a breaking change.

I also created #6235 for AuthenticatedReactiveAuthorizationManager to consider AnonymousAuthenticationToken using an AuthenticationTrustResolver

@ankurpathak
Copy link
Contributor Author

ankurpathak commented Dec 6, 2018 •
edited
Loading

At the moment the biggest problem seems to be that the default behavior has changed. We should require users to explicitly opt into anonymous authentication as making it a default is a breaking change.

I also created #6235 for AuthenticatedReactiveAuthorizationManager to consider AnonymousAuthenticationToken using an AuthenticationTrustResolver

Thanks for helpling. I have made changes as per your comment making it non default. Now
all tests are passing. Also added few more tests for it in ServerHttpSecurityTests.

rwinch
rwinch requested changes Dec 6, 2018
View reviewed changes
Copy link
Member

@rwinch rwinch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes. I replied inline. I think we will also need to wait until #6235 has been merged for this to be merged.

config/src/main/java/org/springframework/security/config/web/server/ServerHttpSecurity.java Outdated
if (this.anonymous == null) {
this.anonymous = new AnonymousSpec();
}
return this.anonymous.authorities("ROLE_USER");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be ROLE_ANONYMOUS. However given the defaults in AnonymousSpec you should just delete it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed it.

@ankurpathak ankurpathak force-pushed the gh-5934-fix branch 2 times, most recently from 342f546 to 559952f Compare December 7, 2018 12:55
@rwinch
Copy link
Member

rwinch commented Dec 7, 2018

Thanks we are now just waiting on #6235 being merged

@ankurpathak
Added support for Anonymous Authentication
48edf49 
1. Created new WebFilter AnonymousAuthenticationWebFilter to
for anonymous authentication
2. Created class AnonymousSpec, method anonymous to configure
anonymous authentication in ServerHttpSecurity
3. Added ANONYMOUS_AUTHENTICATION order after AUTHENTICATION for
anonymous authentication in SecurityWebFiltersOrder
4. Added tests for anonymous authentication in
AnonymousAuthenticationWebFilterTests and ServerHttpSecurityTests
5. Added support for Controller in WebTestClientBuilder

Fixes: spring-projectsgh-5934
@mibo mibo mentioned this pull request Dec 10, 2018
Closed
@rwinch rwinch removed the status: waiting-for-feedback We need additional information before we can continue label Dec 12, 2018
@rwinch rwinch self-assigned this Dec 12, 2018
@rwinch rwinch added status: duplicate A duplicate of another issue in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement Reactive labels Dec 12, 2018
@rwinch rwinch added this to the 5.2.0.M1 milestone Dec 12, 2018
@rwinch rwinch merged commit 2b369cf into spring-projects:master Dec 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rwinch rwinch rwinch approved these changes

Assignees

@rwinch rwinch

Labels
in: web An issue in web modules (web, webmvc) status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Milestone
5.2.0.M1
Development

Successfully merging this pull request may close these issues.
2 participants
@ankurpathak @rwinch