#11510: Add isFullyAuthenticated to AuthenticatedTrustResolver

[karthikeyan-r]

Spring Security 11510 - Implemented isFullyAuthenticated function which is composition of isAnonymous & isRememberMe

[jzheaux]

Thanks, @karthikeyan-r! Sorry for the delay, I've left some feedback inline.

Will you please also adjust the commit message to be like the following:

Commit Title

Closes gh-11510

[jzheaux]

@karthikeyan-r, please also take a look at the ticket description to find the places to use this new method. Please change those places to use it.

[jzheaux]

Hi, @karthikeyan-r. Are you able to make the requested changes?

[karthikeyan-r]

Apologies for the delay @jzheaux, I was travelling & couldnt check and respond to your message. I will work on these comment & push code back in a day.

[karthikeyan-r]

Regarding changes in AuthenticatedAuthorizationManager#fullyAuthenticated as mentioned in this issue,

FullyAuthenticatedAuthorizationStrategy is using composition of isGranted and isRememberMe - not isAnonymous. Please clarify on this.

private static final class FullyAuthenticatedAuthorizationStrategy extends AuthenticatedAuthorizationStrategy {

		@Override
		boolean isGranted(Authentication authentication) {
			return super.isGranted(authentication) && !this.trustResolver.isRememberMe(authentication);
		}

	}

[jzheaux]

Now it's my turn to delay, @karthikeyan-r! Thanks for your question.

If you follow the method isGranted to its implementation, you'll see that it is called isAnonymous also.

I believe it is correct to change the implementation of FullyAuthenticatedAuthorizationStrategy to do:

return this.trustResolver.isFullyAuthenticated(authentication);

[karthikeyan-r]

Thanks for your suggestion @jzheaux, I have updated implementation

[jzheaux]

Thanks, @karthikeyan-r! It looks like there is a failing test. Can you check that out, please?

[karthikeyan-r]

@jzheaux resolved that test failure. Added null check for authentication to fix that.

[karthikeyan-r]

@jzheaux any comments on this fix ?

[jzheaux]

Thanks for the updates, @karthikeyan-r! I'll take things from here. We won't be able to merge it just yet since we are in the RC phase of the next release, but I'll merge it right after that.

[jzheaux]

Thanks, @karthikeyan-r! This is now merged into main with 5fcbb9f and will appear in the 6.1.0 release.